From 6938cabac6b7d177962b74bc84df75b57b16d233 Mon Sep 17 00:00:00 2001 From: "cursor[bot]" <206951365+cursor[bot]@users.noreply.github.com> Date: Tue, 18 Aug 2026 16:11:16 +0000 Subject: [PATCH] [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 * [MM-69646] Cover nil FeatureFlags guard in config validation test Co-authored-by: mattermost-code * 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 * [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 * [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 Co-authored-by: mattermost-code Co-authored-by: Mattermost Build Co-authored-by: Jesse Hallam --- e2e-tests/.ci/server.generate.sh | 1 - .../move_thread/move_thread_from_dm_spec.js | 4 ++- .../move_thread/move_thread_from_gm_spec.js | 4 ++- .../move_thread_from_private_channel_spec.js | 4 ++- .../move_thread_from_public_channel_spec.js | 4 ++- .../lib/src/containers/env_baseline.ts | 1 - .../edit_file_attachment.spec.ts | 32 +++++++++++++----- server/channels/api4/post_test.go | 4 +++ server/i18n/en.json | 4 +++ server/public/model/config.go | 6 ++++ server/public/model/config_test.go | 33 +++++++++++++++++++ server/public/model/feature_flags.go | 12 +++++++ 12 files changed, 94 insertions(+), 15 deletions(-) diff --git a/e2e-tests/.ci/server.generate.sh b/e2e-tests/.ci/server.generate.sh index a3ccd9bc92a..537616e2d04 100755 --- a/e2e-tests/.ci/server.generate.sh +++ b/e2e-tests/.ci/server.generate.sh @@ -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" diff --git a/e2e-tests/cypress/tests/integration/channels/move_thread/move_thread_from_dm_spec.js b/e2e-tests/cypress/tests/integration/channels/move_thread/move_thread_from_dm_spec.js index 592c5bfbbff..2d65a7df1b0 100644 --- a/e2e-tests/cypress/tests/integration/channels/move_thread/move_thread_from_dm_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/move_thread/move_thread_from_dm_spec.js @@ -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; diff --git a/e2e-tests/cypress/tests/integration/channels/move_thread/move_thread_from_gm_spec.js b/e2e-tests/cypress/tests/integration/channels/move_thread/move_thread_from_gm_spec.js index 906042fe16b..b761ea2e29c 100644 --- a/e2e-tests/cypress/tests/integration/channels/move_thread/move_thread_from_gm_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/move_thread/move_thread_from_gm_spec.js @@ -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; diff --git a/e2e-tests/cypress/tests/integration/channels/move_thread/move_thread_from_private_channel_spec.js b/e2e-tests/cypress/tests/integration/channels/move_thread/move_thread_from_private_channel_spec.js index 094783b0819..fbff6236150 100644 --- a/e2e-tests/cypress/tests/integration/channels/move_thread/move_thread_from_private_channel_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/move_thread/move_thread_from_private_channel_spec.js @@ -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; diff --git a/e2e-tests/cypress/tests/integration/channels/move_thread/move_thread_from_public_channel_spec.js b/e2e-tests/cypress/tests/integration/channels/move_thread/move_thread_from_public_channel_spec.js index 42a64a563be..50bf871cc91 100644 --- a/e2e-tests/cypress/tests/integration/channels/move_thread/move_thread_from_public_channel_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/move_thread/move_thread_from_public_channel_spec.js @@ -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; diff --git a/e2e-tests/playwright/lib/src/containers/env_baseline.ts b/e2e-tests/playwright/lib/src/containers/env_baseline.ts index 1419149a980..638fdad1c07 100644 --- a/e2e-tests/playwright/lib/src/containers/env_baseline.ts +++ b/e2e-tests/playwright/lib/src/containers/env_baseline.ts @@ -19,7 +19,6 @@ export const SERVER_ENV_BASELINE: Record = { // 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', diff --git a/e2e-tests/playwright/specs/functional/channels/file_attachments/edit_file_attachment.spec.ts b/e2e-tests/playwright/specs/functional/channels/file_attachments/edit_file_attachment.spec.ts index 670d8bd0d70..375709990f4 100644 --- a/e2e-tests/playwright/specs/functional/channels/file_attachments/edit_file_attachment.spec.ts +++ b/e2e-tests/playwright/specs/functional/channels/file_attachments/edit_file_attachment.spec.ts @@ -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); } diff --git a/server/channels/api4/post_test.go b/server/channels/api4/post_test.go index 385fb1dfdd7..abe98131c7d 100644 --- a/server/channels/api4/post_test.go +++ b/server/channels/api4/post_test.go @@ -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 diff --git a/server/i18n/en.json b/server/i18n/en.json index a0b96045a63..811692751b0 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -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'." diff --git a/server/public/model/config.go b/server/public/model/config.go index 0a001809fc6..62988357946 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -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 } diff --git a/server/public/model/config_test.go b/server/public/model/config_test.go index 263eb99957a..20c47ef673e 100644 --- a/server/public/model/config_test.go +++ b/server/public/model/config_test.go @@ -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 diff --git a/server/public/model/feature_flags.go b/server/public/model/feature_flags.go index 14894f3630c..94e849ef675 100644 --- a/server/public/model/feature_flags.go +++ b/server/public/model/feature_flags.go @@ -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