mirror of
https://github.com/mattermost/mattermost.git
synced 2026-08-27 05:37:15 -05:00
[MM-69646] Disallow MoveThreadsEnabled feature flag (fail server startup) (#37966)
* [MM-69646] Disallow MoveThreadsEnabled feature flag Reject the MoveThreadsEnabled feature flag during config validation so the server fails to start while it is enabled. The feature is being retired in favor of Wrangler and will be removed later. Co-authored-by: mattermost-code <matty-code@mattermost.com> * [MM-69646] Cover nil FeatureFlags guard in config validation test Co-authored-by: mattermost-code <matty-code@mattermost.com> * Move MoveThreadsEnabled comment into isValid method body Keep isValid's doc comment generic since it will validate more flag combinations in the future, and place the MoveThreadsEnabled-specific rationale next to the actual flag check. * [MM-69646] Update TestMoveThread for retired MoveThreadsEnabled flag Config.IsValid now rejects enabling MoveThreadsEnabled, so the move-thread API stays disabled. Replace the enabled-path suite with assertions that the flag cannot be turned on and MoveThread returns 501. Co-authored-by: mattermost-code <matty-code@mattermost.com> * [MM-69646] Stop forcing MoveThreadsEnabled in e2e environments E2E was setting MM_FEATUREFLAGS_MOVETHREADSENABLED=true, which now fails Config.IsValid and prevents the test server from starting. Remove the override and skip Cypress move-thread specs that require the retired flag. Co-authored-by: mattermost-code <matty-code@mattermost.com> * [MM-69646] Skip TestMoveThread instead of asserting disabled flag Mirror the E2E describe.skip approach: retain the original TestMoveThread body and skip it at the top, since MoveThreadsEnabled is retired and rejected by Config.IsValid. * [MM-69646] Park cursor away from post dot menu in edit_file_attachment specs --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: mattermost-code <matty-code@mattermost.com> Co-authored-by: Mattermost Build <build@mattermost.com> Co-authored-by: Jesse Hallam <jesse@mattermost.com>
This commit is contained in:
co-authored by
Cursor Agent
mattermost-code
Mattermost Build
Jesse Hallam
parent
eb3966e30b
commit
6938cabac6
@@ -64,7 +64,6 @@ services:
|
||||
MM_EMAILSETTINGS_SMTPSERVER: "localhost"
|
||||
MM_CLUSTERSETTINGS_READONLYCONFIG: "false"
|
||||
MM_SERVICEENVIRONMENT: "test"
|
||||
MM_FEATUREFLAGS_MOVETHREADSENABLED: "true"
|
||||
MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES: "true"
|
||||
MM_FEATUREFLAGS_PERMISSIONPOLICIES: "true"
|
||||
MM_FEATUREFLAGS_TEAMMEMBERSHIPACCESSCONTROL: "true"
|
||||
|
||||
+3
-1
@@ -12,7 +12,9 @@
|
||||
|
||||
import * as TIMEOUTS from '@/fixtures/timeouts';
|
||||
|
||||
describe('Move Thread', () => {
|
||||
// Skipped: MoveThreadsEnabled is retired and rejected by Config.IsValid (MM-69646).
|
||||
// These specs require the flag and cannot run while the server refuses to enable it.
|
||||
describe.skip('Move Thread', () => {
|
||||
let user1;
|
||||
let user2;
|
||||
let testTeam;
|
||||
|
||||
+3
-1
@@ -12,7 +12,9 @@
|
||||
|
||||
import * as TIMEOUTS from '@/fixtures/timeouts';
|
||||
|
||||
describe('Move thread', () => {
|
||||
// Skipped: MoveThreadsEnabled is retired and rejected by Config.IsValid (MM-69646).
|
||||
// These specs require the flag and cannot run while the server refuses to enable it.
|
||||
describe.skip('Move thread', () => {
|
||||
let user1;
|
||||
let user2;
|
||||
let user3;
|
||||
|
||||
+3
-1
@@ -12,7 +12,9 @@
|
||||
|
||||
import * as TIMEOUTS from '@/fixtures/timeouts';
|
||||
|
||||
describe('Move thread', () => {
|
||||
// Skipped: MoveThreadsEnabled is retired and rejected by Config.IsValid (MM-69646).
|
||||
// These specs require the flag and cannot run while the server refuses to enable it.
|
||||
describe.skip('Move thread', () => {
|
||||
let user1;
|
||||
let testTeam;
|
||||
let privateChannel;
|
||||
|
||||
+3
-1
@@ -10,7 +10,9 @@
|
||||
// Stage: @prod
|
||||
// Group: @channels @enterprise @messaging
|
||||
|
||||
describe('Move Thread', () => {
|
||||
// Skipped: MoveThreadsEnabled is retired and rejected by Config.IsValid (MM-69646).
|
||||
// These specs require the flag and cannot run while the server refuses to enable it.
|
||||
describe.skip('Move Thread', () => {
|
||||
let user1;
|
||||
let user2;
|
||||
let user3;
|
||||
|
||||
@@ -19,7 +19,6 @@ export const SERVER_ENV_BASELINE: Record<string, string> = {
|
||||
// Feature flags this test suite needs on, off by default in the server
|
||||
MM_FEATUREFLAGS_ATTRIBUTEVALUEMASKING: 'true',
|
||||
MM_FEATUREFLAGS_ENABLEREMOTECLUSTERSERVICE: 'true',
|
||||
MM_FEATUREFLAGS_MOVETHREADSENABLED: 'true',
|
||||
MM_FEATUREFLAGS_PERMISSIONPOLICIES: 'true',
|
||||
MM_FEATUREFLAGS_PROPERTYFIELDRANK: 'true',
|
||||
MM_FEATUREFLAGS_RECURRINGSCHEDULEDPOSTS: 'true',
|
||||
|
||||
+23
-9
@@ -13,7 +13,7 @@ test('MM-T5654_1 should be able to add attachments while editing a post', {tag:
|
||||
|
||||
// # Initialize user and login
|
||||
const {user} = await pw.initSetup();
|
||||
const {channelsPage} = await pw.testBrowser.login(user);
|
||||
const {channelsPage, page} = await pw.testBrowser.login(user);
|
||||
|
||||
// # Navigate to channels page and post a message
|
||||
await channelsPage.goto();
|
||||
@@ -28,6 +28,7 @@ test('MM-T5654_1 should be able to add attachments while editing a post', {tag:
|
||||
|
||||
// # Open the dot menu and click edit
|
||||
await post.postMenu.dotMenuButton.click();
|
||||
await moveMouseAway(page);
|
||||
await channelsPage.postDotMenu.toBeVisible();
|
||||
await channelsPage.postDotMenu.editMenuItem.click();
|
||||
|
||||
@@ -59,6 +60,7 @@ test('MM-T5654_2 should be able to add attachments while editing a threaded post
|
||||
|
||||
// open the dot menu
|
||||
await post.postMenu.dotMenuButton.click();
|
||||
await moveMouseAway(page);
|
||||
await channelsPage.postDotMenu.toBeVisible();
|
||||
await channelsPage.postDotMenu.replyMenuItem.click();
|
||||
await channelsPage.sidebarRight.toBeVisible();
|
||||
@@ -71,6 +73,7 @@ test('MM-T5654_2 should be able to add attachments while editing a threaded post
|
||||
await replyPost.hover();
|
||||
await replyPost.postMenu.toBeVisible();
|
||||
await replyPost.postMenu.dotMenuButton.click();
|
||||
await moveMouseAway(page);
|
||||
await channelsPage.postDotMenu.toBeVisible();
|
||||
await channelsPage.postDotMenu.editMenuItem.click();
|
||||
await channelsPage.sidebarRight.postEdit.toBeVisible();
|
||||
@@ -85,6 +88,7 @@ test('MM-T5654_2 should be able to add attachments while editing a threaded post
|
||||
await updatedReplyPost.hover();
|
||||
await updatedReplyPost.postMenu.toBeVisible();
|
||||
await updatedReplyPost.postMenu.dotMenuButton.click();
|
||||
await moveMouseAway(page);
|
||||
await channelsPage.postDotMenu.toBeVisible();
|
||||
await channelsPage.postDotMenu.editMenuItem.click();
|
||||
await channelsPage.sidebarRight.postEdit.toBeVisible();
|
||||
@@ -103,7 +107,7 @@ test('MM-T5654_2 should be able to add attachments while editing a threaded post
|
||||
await updatedReplyPost.hover();
|
||||
await updatedReplyPost.postMenu.toBeVisible();
|
||||
await updatedReplyPost.postMenu.clickOnDotMenu();
|
||||
await moveMouseToCenter(page);
|
||||
await moveMouseAway(page);
|
||||
await channelsPage.postDotMenu.toBeVisible();
|
||||
await channelsPage.postDotMenu.editMenuItem.click();
|
||||
await channelsPage.sidebarRight.postEdit.toBeVisible();
|
||||
@@ -124,7 +128,7 @@ test('MM-T5654_3 should be able to edit post message originally containing files
|
||||
const originalMessage = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit';
|
||||
|
||||
const {user} = await pw.initSetup();
|
||||
const {channelsPage} = await pw.testBrowser.login(user);
|
||||
const {channelsPage, page} = await pw.testBrowser.login(user);
|
||||
|
||||
await channelsPage.goto();
|
||||
await channelsPage.toBeVisible();
|
||||
@@ -137,6 +141,7 @@ test('MM-T5654_3 should be able to edit post message originally containing files
|
||||
|
||||
// open the dot menu
|
||||
await post.postMenu.dotMenuButton.click();
|
||||
await moveMouseAway(page);
|
||||
await channelsPage.postDotMenu.toBeVisible();
|
||||
await channelsPage.postDotMenu.editMenuItem.click();
|
||||
await channelsPage.centerView.postEdit.toBeVisible();
|
||||
@@ -152,7 +157,7 @@ test('MM-T5654_4 should be able to add files when editing a post', async ({pw})
|
||||
const originalMessage = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit';
|
||||
|
||||
const {user} = await pw.initSetup();
|
||||
const {channelsPage} = await pw.testBrowser.login(user);
|
||||
const {channelsPage, page} = await pw.testBrowser.login(user);
|
||||
|
||||
await channelsPage.goto();
|
||||
await channelsPage.toBeVisible();
|
||||
@@ -165,6 +170,7 @@ test('MM-T5654_4 should be able to add files when editing a post', async ({pw})
|
||||
|
||||
// open the dot menu
|
||||
await post.postMenu.dotMenuButton.click();
|
||||
await moveMouseAway(page);
|
||||
await channelsPage.postDotMenu.toBeVisible();
|
||||
await channelsPage.postDotMenu.editMenuItem.click();
|
||||
await channelsPage.centerView.postEdit.toBeVisible();
|
||||
@@ -179,6 +185,7 @@ test('MM-T5654_4 should be able to add files when editing a post', async ({pw})
|
||||
|
||||
// now we'll add multiple files
|
||||
await post.postMenu.dotMenuButton.click();
|
||||
await moveMouseAway(page);
|
||||
await channelsPage.postDotMenu.toBeVisible();
|
||||
await channelsPage.postDotMenu.editMenuItem.click();
|
||||
await channelsPage.centerView.postEdit.toBeVisible();
|
||||
@@ -213,7 +220,7 @@ test('MM-5654_5 should be able to remove attachments while editing a post', asyn
|
||||
await post.hover();
|
||||
await post.postMenu.toBeVisible();
|
||||
await post.postMenu.clickOnDotMenu();
|
||||
await moveMouseToCenter(page);
|
||||
await moveMouseAway(page);
|
||||
|
||||
await channelsPage.postDotMenu.toBeVisible();
|
||||
await channelsPage.postDotMenu.editMenuItem.click();
|
||||
@@ -233,7 +240,7 @@ test('MM-T5655_1 removing message content and files should delete the post', asy
|
||||
const originalMessage = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit';
|
||||
|
||||
const {user} = await pw.initSetup();
|
||||
const {channelsPage} = await pw.testBrowser.login(user);
|
||||
const {channelsPage, page} = await pw.testBrowser.login(user);
|
||||
|
||||
await channelsPage.goto();
|
||||
await channelsPage.toBeVisible();
|
||||
@@ -247,6 +254,7 @@ test('MM-T5655_1 removing message content and files should delete the post', asy
|
||||
await post.hover();
|
||||
await post.postMenu.toBeVisible();
|
||||
await post.postMenu.dotMenuButton.click();
|
||||
await moveMouseAway(page);
|
||||
|
||||
await channelsPage.postDotMenu.toBeVisible();
|
||||
await channelsPage.postDotMenu.editMenuItem.click();
|
||||
@@ -283,7 +291,7 @@ test('MM-T5655_2 should be able to remove all files when editing a post', async
|
||||
await post.hover();
|
||||
await post.postMenu.toBeVisible();
|
||||
await post.postMenu.clickOnDotMenu();
|
||||
await moveMouseToCenter(page);
|
||||
await moveMouseAway(page);
|
||||
|
||||
await channelsPage.postDotMenu.toBeVisible();
|
||||
await channelsPage.postDotMenu.editMenuItem.click();
|
||||
@@ -306,7 +314,7 @@ test('MM-T5656_1 should be able to restore previously edited post version that c
|
||||
const newMessage = 'New Message';
|
||||
|
||||
const {user} = await pw.initSetup();
|
||||
const {channelsPage} = await pw.testBrowser.login(user);
|
||||
const {channelsPage, page} = await pw.testBrowser.login(user);
|
||||
|
||||
await channelsPage.goto();
|
||||
await channelsPage.toBeVisible();
|
||||
@@ -320,6 +328,7 @@ test('MM-T5656_1 should be able to restore previously edited post version that c
|
||||
await post.hover();
|
||||
await post.postMenu.toBeVisible();
|
||||
await post.postMenu.dotMenuButton.click();
|
||||
await moveMouseAway(page);
|
||||
|
||||
await channelsPage.postDotMenu.toBeVisible();
|
||||
await channelsPage.postDotMenu.editMenuItem.click();
|
||||
@@ -350,6 +359,11 @@ test('MM-T5656_1 should be able to restore previously edited post version that c
|
||||
await restoredPost.toContainText('sample_text_file.txt');
|
||||
});
|
||||
|
||||
async function moveMouseToCenter(page: Page) {
|
||||
/**
|
||||
* Parks the cursor away from the post so it does not hover whatever menu item happens to render
|
||||
* under it. A hovered submenu item (e.g. "Remind") opens a nested MUI popover, which marks the
|
||||
* parent menu aria-hidden and makes getByRole('menu') unresolvable.
|
||||
*/
|
||||
async function moveMouseAway(page: Page) {
|
||||
await page.mouse.move(0, 0);
|
||||
}
|
||||
|
||||
@@ -996,6 +996,10 @@ func TestCreatePostWithOutgoingHook_no_content_type(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestMoveThread(t *testing.T) {
|
||||
// Skipped: MoveThreadsEnabled is retired and rejected by Config.IsValid (MM-69646).
|
||||
// This test requires the flag and cannot run while the server refuses to enable it.
|
||||
t.Skip("MoveThreadsEnabled feature flag is retired (MM-69646)")
|
||||
|
||||
th := SetupEnterprise(t).InitBasic(t)
|
||||
|
||||
// Enable MoveThreads feature flag
|
||||
|
||||
@@ -11962,6 +11962,10 @@
|
||||
"id": "model.config.is_valid.extract_content_timeout.app_error",
|
||||
"translation": "Invalid content extraction timeout for file settings. Must be a whole number of seconds greater than or equal to zero."
|
||||
},
|
||||
{
|
||||
"id": "model.config.is_valid.feature_flags.move_threads_enabled.app_error",
|
||||
"translation": "The MoveThreadsEnabled feature flag is no longer supported and must be disabled."
|
||||
},
|
||||
{
|
||||
"id": "model.config.is_valid.file_driver.app_error",
|
||||
"translation": "Invalid driver name for file settings. Must be 'local', 'amazons3', or 'azureblob'."
|
||||
|
||||
@@ -4519,6 +4519,12 @@ func (o *Config) IsValid() *AppError {
|
||||
return appErr
|
||||
}
|
||||
|
||||
if o.FeatureFlags != nil {
|
||||
if appErr := o.FeatureFlags.isValid(); appErr != nil {
|
||||
return appErr
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@@ -116,6 +116,39 @@ func TestConfigIsValid(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
func TestFeatureFlagsIsValid(t *testing.T) {
|
||||
t.Run("defaults are valid", func(t *testing.T) {
|
||||
f := &FeatureFlags{}
|
||||
f.SetDefaults()
|
||||
require.Nil(t, f.isValid())
|
||||
})
|
||||
|
||||
t.Run("MoveThreadsEnabled is rejected", func(t *testing.T) {
|
||||
f := &FeatureFlags{}
|
||||
f.SetDefaults()
|
||||
f.MoveThreadsEnabled = true
|
||||
|
||||
appErr := f.isValid()
|
||||
require.NotNil(t, appErr)
|
||||
require.Equal(t, "model.config.is_valid.feature_flags.move_threads_enabled.app_error", appErr.Id)
|
||||
})
|
||||
}
|
||||
|
||||
func TestConfigIsValidMoveThreadsEnabled(t *testing.T) {
|
||||
c := Config{}
|
||||
c.SetDefaults()
|
||||
require.Nil(t, c.IsValid())
|
||||
|
||||
c.FeatureFlags.MoveThreadsEnabled = true
|
||||
appErr := c.IsValid()
|
||||
require.NotNil(t, appErr)
|
||||
require.Equal(t, "model.config.is_valid.feature_flags.move_threads_enabled.app_error", appErr.Id)
|
||||
|
||||
// A nil FeatureFlags must not panic the validation chain.
|
||||
c.FeatureFlags = nil
|
||||
require.Nil(t, c.IsValid())
|
||||
}
|
||||
|
||||
func TestAccessControlSettingsIsValid(t *testing.T) {
|
||||
for name, test := range map[string]struct {
|
||||
AccessControlSettings AccessControlSettings
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
package model
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"reflect"
|
||||
"strconv"
|
||||
)
|
||||
@@ -219,6 +220,17 @@ func (f *FeatureFlags) SetDefaults() {
|
||||
f.RecurringScheduledPosts = false
|
||||
}
|
||||
|
||||
// isValid rejects feature flag combinations that are no longer supported.
|
||||
func (f *FeatureFlags) isValid() *AppError {
|
||||
// MoveThreadsEnabled is being retired in favor of Wrangler, so the server
|
||||
// refuses to start while it is enabled.
|
||||
if f.MoveThreadsEnabled {
|
||||
return NewAppError("FeatureFlags.IsValid", "model.config.is_valid.feature_flags.move_threads_enabled.app_error", nil, "", http.StatusBadRequest)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// IsChannelPermissionPoliciesEnabled reports whether channel-scope
|
||||
// policies may carry permission-rule actions (file upload/download)
|
||||
// and whether the Channel Settings → Permissions Policy tab should
|
||||
|
||||
Reference in New Issue
Block a user