From 5779bd49d5efc328a5b9de12bc36800d5fc9e0bb Mon Sep 17 00:00:00 2001 From: Maria A Nunez Date: Wed, 5 Jul 2023 12:18:30 -0400 Subject: [PATCH] Remove Global Drafts Feature Flag (#23767) * Remove global draft feature flag * More removal - in progress * Removed the rest in webapp * Removed the rest in webapp * Fix tests * Update feature_flags.go --------- Co-authored-by: Mattermost Build --- .../support/server/default_config.ts | 1 - server/channels/api4/drafts_test.go | 4 ---- server/channels/app/draft.go | 8 ++++---- server/channels/app/draft_test.go | 18 ------------------ server/public/model/feature_flags.go | 3 --- .../channels/src/actions/views/drafts.test.ts | 1 - .../admin_console/admin_definition.jsx | 6 ------ .../drafts/__snapshots__/drafts.test.tsx.snap | 2 -- .../src/components/drafts/drafts.test.tsx | 2 -- .../channels/src/components/drafts/drafts.tsx | 6 ------ .../drafts/drafts_link/drafts_link.tsx | 5 ++--- webapp/channels/src/components/drafts/index.ts | 3 +-- .../src/selectors/entities/preferences.ts | 12 ++---------- webapp/channels/src/selectors/lhs.test.ts | 6 ------ webapp/channels/src/selectors/lhs.ts | 14 +++++--------- 15 files changed, 14 insertions(+), 77 deletions(-) diff --git a/e2e-tests/playwright/support/server/default_config.ts b/e2e-tests/playwright/support/server/default_config.ts index d260375ca0..eec3d2d068 100644 --- a/e2e-tests/playwright/support/server/default_config.ts +++ b/e2e-tests/playwright/support/server/default_config.ts @@ -677,7 +677,6 @@ const defaultServerConfig: AdminConfig = { PeopleProduct: false, ReduceOnBoardingTaskList: false, ThreadsEverywhere: false, - GlobalDrafts: true, OnboardingTourTips: true, DeprecateCloudFree: false, CloudReverseTrial: false, diff --git a/server/channels/api4/drafts_test.go b/server/channels/api4/drafts_test.go index 605de27898..87dbfef8bb 100644 --- a/server/channels/api4/drafts_test.go +++ b/server/channels/api4/drafts_test.go @@ -25,7 +25,6 @@ func TestUpsertDraft(t *testing.T) { defer th.TearDown() // set config - th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.GlobalDrafts = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowSyncedDrafts = true }) client := th.Client @@ -95,7 +94,6 @@ func TestGetDrafts(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.GlobalDrafts = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowSyncedDrafts = true }) client := th.Client @@ -120,7 +118,6 @@ func TestGetDrafts(t *testing.T) { Message: "draft2", } - th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.GlobalDrafts = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowSyncedDrafts = true }) // upsert draft1 @@ -169,7 +166,6 @@ func TestDeleteDraft(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.GlobalDrafts = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowSyncedDrafts = true }) client := th.Client diff --git a/server/channels/app/draft.go b/server/channels/app/draft.go index 3debfb6c69..14f5f19df8 100644 --- a/server/channels/app/draft.go +++ b/server/channels/app/draft.go @@ -16,7 +16,7 @@ import ( ) func (a *App) GetDraft(userID, channelID, rootID string) (*model.Draft, *model.AppError) { - if !a.Config().FeatureFlags.GlobalDrafts || !*a.Config().ServiceSettings.AllowSyncedDrafts { + if !*a.Config().ServiceSettings.AllowSyncedDrafts { return nil, model.NewAppError("GetDraft", "app.draft.feature_disabled", nil, "", http.StatusNotImplemented) } @@ -35,7 +35,7 @@ func (a *App) GetDraft(userID, channelID, rootID string) (*model.Draft, *model.A } func (a *App) UpsertDraft(c *request.Context, draft *model.Draft, connectionID string) (*model.Draft, *model.AppError) { - if !a.Config().FeatureFlags.GlobalDrafts || !*a.Config().ServiceSettings.AllowSyncedDrafts { + if !*a.Config().ServiceSettings.AllowSyncedDrafts { return nil, model.NewAppError("CreateDraft", "app.draft.feature_disabled", nil, "", http.StatusNotImplemented) } @@ -75,7 +75,7 @@ func (a *App) UpsertDraft(c *request.Context, draft *model.Draft, connectionID s } func (a *App) GetDraftsForUser(userID, teamID string) ([]*model.Draft, *model.AppError) { - if !a.Config().FeatureFlags.GlobalDrafts || !*a.Config().ServiceSettings.AllowSyncedDrafts { + if !*a.Config().ServiceSettings.AllowSyncedDrafts { return nil, model.NewAppError("GetDraftsForUser", "app.draft.feature_disabled", nil, "", http.StatusNotImplemented) } @@ -118,7 +118,7 @@ func (a *App) getFileInfosForDraft(draft *model.Draft) ([]*model.FileInfo, *mode } func (a *App) DeleteDraft(userID, channelID, rootID, connectionID string) (*model.Draft, *model.AppError) { - if !a.Config().FeatureFlags.GlobalDrafts || !*a.Config().ServiceSettings.AllowSyncedDrafts { + if !*a.Config().ServiceSettings.AllowSyncedDrafts { return nil, model.NewAppError("DeleteDraft", "app.draft.feature_disabled", nil, "", http.StatusNotImplemented) } diff --git a/server/channels/app/draft_test.go b/server/channels/app/draft_test.go index 7c35dc214f..dc8893e7f3 100644 --- a/server/channels/app/draft_test.go +++ b/server/channels/app/draft_test.go @@ -26,7 +26,6 @@ func TestGetDraft(t *testing.T) { th.Server.platform.SetConfigReadOnlyFF(false) defer th.Server.platform.SetConfigReadOnlyFF(true) - th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.GlobalDrafts = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowSyncedDrafts = true }) user := th.BasicUser @@ -57,10 +56,7 @@ func TestGetDraft(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_ALLOWSYNCEDDRAFTS", "false") defer os.Unsetenv("MM_SERVICESETTINGS_ALLOWSYNCEDDRAFTS") - th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.GlobalDrafts = false }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowSyncedDrafts = false }) - - defer th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.GlobalDrafts = true }) defer th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowSyncedDrafts = true }) _, err := th.App.GetDraft(user.Id, channel.Id, "") @@ -75,7 +71,6 @@ func TestUpsertDraft(t *testing.T) { th.Server.platform.SetConfigReadOnlyFF(false) defer th.Server.platform.SetConfigReadOnlyFF(true) - th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.GlobalDrafts = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowSyncedDrafts = true }) user := th.BasicUser @@ -124,10 +119,7 @@ func TestUpsertDraft(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_ALLOWSYNCEDDRAFTS", "false") defer os.Unsetenv("MM_SERVICESETTINGS_ALLOWSYNCEDDRAFTS") - th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.GlobalDrafts = false }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowSyncedDrafts = false }) - - defer th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.GlobalDrafts = true }) defer th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowSyncedDrafts = true }) _, err := th.App.UpsertDraft(th.Context, draft, "") @@ -142,7 +134,6 @@ func TestCreateDraft(t *testing.T) { th.Server.platform.SetConfigReadOnlyFF(false) defer th.Server.platform.SetConfigReadOnlyFF(true) - th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.GlobalDrafts = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowSyncedDrafts = true }) user := th.BasicUser @@ -201,7 +192,6 @@ func TestUpdateDraft(t *testing.T) { th.Server.platform.SetConfigReadOnlyFF(false) defer th.Server.platform.SetConfigReadOnlyFF(true) - th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.GlobalDrafts = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowSyncedDrafts = true }) user := th.BasicUser @@ -247,7 +237,6 @@ func TestGetDraftsForUser(t *testing.T) { th.Server.platform.SetConfigReadOnlyFF(false) defer th.Server.platform.SetConfigReadOnlyFF(true) - th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.GlobalDrafts = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowSyncedDrafts = true }) user := th.BasicUser @@ -325,10 +314,7 @@ func TestGetDraftsForUser(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_ALLOWSYNCEDDRAFTS", "false") defer os.Unsetenv("MM_SERVICESETTINGS_ALLOWSYNCEDDRAFTS") - th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.GlobalDrafts = false }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowSyncedDrafts = false }) - - defer th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.GlobalDrafts = true }) defer th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowSyncedDrafts = true }) _, err := th.App.GetDraftsForUser(user.Id, th.BasicTeam.Id) @@ -343,7 +329,6 @@ func TestDeleteDraft(t *testing.T) { th.Server.platform.SetConfigReadOnlyFF(false) defer th.Server.platform.SetConfigReadOnlyFF(true) - th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.GlobalDrafts = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowSyncedDrafts = true }) user := th.BasicUser @@ -377,10 +362,7 @@ func TestDeleteDraft(t *testing.T) { os.Setenv("MM_SERVICESETTINGS_ALLOWSYNCEDDRAFTS", "false") defer os.Unsetenv("MM_SERVICESETTINGS_ALLOWSYNCEDDRAFTS") - th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.GlobalDrafts = false }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowSyncedDrafts = false }) - - defer th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.GlobalDrafts = true }) defer th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowSyncedDrafts = true }) _, err := th.App.DeleteDraft(user.Id, channel.Id, "", "") diff --git a/server/public/model/feature_flags.go b/server/public/model/feature_flags.go index 9db09788d9..ed1f9994a7 100644 --- a/server/public/model/feature_flags.go +++ b/server/public/model/feature_flags.go @@ -55,8 +55,6 @@ type FeatureFlags struct { PeopleProduct bool - GlobalDrafts bool - OnboardingTourTips bool DeprecateCloudFree bool @@ -82,7 +80,6 @@ func (f *FeatureFlags) SetDefaults() { f.CallsEnabled = true f.PostPriority = true f.PeopleProduct = false - f.GlobalDrafts = true f.DeprecateCloudFree = false f.WysiwygEditor = false f.OnboardingTourTips = true diff --git a/webapp/channels/src/actions/views/drafts.test.ts b/webapp/channels/src/actions/views/drafts.test.ts index 7a95882237..eee7e8ccfb 100644 --- a/webapp/channels/src/actions/views/drafts.test.ts +++ b/webapp/channels/src/actions/views/drafts.test.ts @@ -102,7 +102,6 @@ describe('draft actions', () => { general: { config: { EnableCustomEmoji: 'true', - FeatureFlagGlobalDrafts: 'true', AllowSyncedDrafts: 'true', }, }, diff --git a/webapp/channels/src/components/admin_console/admin_definition.jsx b/webapp/channels/src/components/admin_console/admin_definition.jsx index cfd7756ae7..645ef34322 100644 --- a/webapp/channels/src/components/admin_console/admin_definition.jsx +++ b/webapp/channels/src/components/admin_console/admin_definition.jsx @@ -3068,12 +3068,6 @@ const AdminDefinition = { help_text: t('admin.customization.allowSyncedDraftsDesc'), help_text_default: 'When enabled, users message drafts will sync with the server so they can be accessed from any device. Users may opt out of this behaviour in Account settings.', help_text_markdown: false, - isHidden: it.any( - it.configIsFalse('FeatureFlags', 'GlobalDrafts'), - ), - isDisabled: it.any( - it.configIsFalse('FeatureFlags', 'GlobalDrafts'), - ), }, ], }, diff --git a/webapp/channels/src/components/drafts/__snapshots__/drafts.test.tsx.snap b/webapp/channels/src/components/drafts/__snapshots__/drafts.test.tsx.snap index c95826930d..f07a9a7c1d 100644 --- a/webapp/channels/src/components/drafts/__snapshots__/drafts.test.tsx.snap +++ b/webapp/channels/src/components/drafts/__snapshots__/drafts.test.tsx.snap @@ -36,7 +36,6 @@ exports[`components/drafts/drafts should match snapshot 1`] = ` displayName="display_name" draftRemotes={Object {}} drafts={Array []} - localDraftsAreEnabled={true} status={Object {}} user={Object {}} /> @@ -79,7 +78,6 @@ exports[`components/drafts/drafts should match snapshot for local drafts disable displayName="display_name" draftRemotes={Object {}} drafts={Array []} - localDraftsAreEnabled={false} status={Object {}} user={Object {}} /> diff --git a/webapp/channels/src/components/drafts/drafts.test.tsx b/webapp/channels/src/components/drafts/drafts.test.tsx index 3453780f41..35cc49804e 100644 --- a/webapp/channels/src/components/drafts/drafts.test.tsx +++ b/webapp/channels/src/components/drafts/drafts.test.tsx @@ -19,7 +19,6 @@ describe('components/drafts/drafts', () => { user: {} as UserProfile, displayName: 'display_name', status: {} as UserStatus['status'], - localDraftsAreEnabled: true, draftRemotes: {}, }; @@ -41,7 +40,6 @@ describe('components/drafts/drafts', () => { const props = { ...baseProps, - localDraftsAreEnabled: false, }; const wrapper = shallow( diff --git a/webapp/channels/src/components/drafts/drafts.tsx b/webapp/channels/src/components/drafts/drafts.tsx index 11cf376e2d..20fc3f8e30 100644 --- a/webapp/channels/src/components/drafts/drafts.tsx +++ b/webapp/channels/src/components/drafts/drafts.tsx @@ -26,7 +26,6 @@ type Props = { user: UserProfile; displayName: string; status: UserStatus['status']; - localDraftsAreEnabled: boolean; draftRemotes: Record; } @@ -36,7 +35,6 @@ function Drafts({ draftRemotes, status, user, - localDraftsAreEnabled, }: Props) { const dispatch = useDispatch(); const {formatMessage} = useIntl(); @@ -50,10 +48,6 @@ function Drafts({ }; }, []); - if (!localDraftsAreEnabled) { - return null; - } - return (
({ jest.mock('mattermost-redux/selectors/entities/preferences', () => ({ insightsAreEnabled: jest.fn(), isCollapsedThreadsEnabled: jest.fn(), - localDraftsAreEnabled: jest.fn(), })); beforeEach(() => { @@ -58,7 +57,6 @@ describe('Selectors.Lhs', () => { it('handles nothing enabled', () => { jest.spyOn(PreferencesSelectors, 'insightsAreEnabled').mockImplementationOnce(() => false); jest.spyOn(PreferencesSelectors, 'isCollapsedThreadsEnabled').mockImplementationOnce(() => false); - jest.spyOn(PreferencesSelectors, 'localDraftsAreEnabled').mockImplementationOnce(() => false); jest.spyOn(Lhs, 'getDraftsCount').mockImplementationOnce(() => 0); const items = Lhs.getVisibleStaticPages(state as GlobalState); expect(items).toEqual([]); @@ -67,7 +65,6 @@ describe('Selectors.Lhs', () => { it('handles insights', () => { jest.spyOn(PreferencesSelectors, 'insightsAreEnabled').mockImplementation(() => true); jest.spyOn(PreferencesSelectors, 'isCollapsedThreadsEnabled').mockImplementation(() => false); - jest.spyOn(PreferencesSelectors, 'localDraftsAreEnabled').mockImplementation(() => false); jest.spyOn(Lhs, 'getDraftsCount').mockImplementationOnce(() => 0); const items = Lhs.getVisibleStaticPages(state as GlobalState); expect(items).toEqual([ @@ -81,7 +78,6 @@ describe('Selectors.Lhs', () => { it('handles threads - default off', () => { jest.spyOn(PreferencesSelectors, 'insightsAreEnabled').mockImplementation(() => false); jest.spyOn(PreferencesSelectors, 'isCollapsedThreadsEnabled').mockImplementation(() => true); - jest.spyOn(PreferencesSelectors, 'localDraftsAreEnabled').mockImplementation(() => false); jest.spyOn(Lhs, 'getDraftsCount').mockImplementationOnce(() => 0); const items = Lhs.getVisibleStaticPages(state as GlobalState); expect(items).toEqual([ @@ -95,7 +91,6 @@ describe('Selectors.Lhs', () => { it('should not return drafts when empty', () => { jest.spyOn(PreferencesSelectors, 'insightsAreEnabled').mockImplementation(() => false); jest.spyOn(PreferencesSelectors, 'isCollapsedThreadsEnabled').mockImplementation(() => false); - jest.spyOn(PreferencesSelectors, 'localDraftsAreEnabled').mockImplementation(() => true); jest.spyOn(Lhs, 'getDraftsCount').mockImplementationOnce(() => 0); const items = Lhs.getVisibleStaticPages(state as GlobalState); expect(items).toEqual([]); @@ -104,7 +99,6 @@ describe('Selectors.Lhs', () => { it('should return drafts when there are available', () => { jest.spyOn(PreferencesSelectors, 'insightsAreEnabled').mockImplementation(() => false); jest.spyOn(PreferencesSelectors, 'isCollapsedThreadsEnabled').mockImplementation(() => false); - jest.spyOn(PreferencesSelectors, 'localDraftsAreEnabled').mockImplementation(() => true); jest.spyOn(Lhs, 'getDraftsCount').mockImplementationOnce(() => 1); const items = Lhs.getVisibleStaticPages(state as GlobalState); expect(items).toEqual([ diff --git a/webapp/channels/src/selectors/lhs.ts b/webapp/channels/src/selectors/lhs.ts index 308fefaf6e..ce2ea89698 100644 --- a/webapp/channels/src/selectors/lhs.ts +++ b/webapp/channels/src/selectors/lhs.ts @@ -9,7 +9,6 @@ import {makeGetDraftsCount} from 'selectors/drafts'; import { insightsAreEnabled, isCollapsedThreadsEnabled, - localDraftsAreEnabled, } from 'mattermost-redux/selectors/entities/preferences'; export function getIsLhsOpen(state: GlobalState): boolean { @@ -26,9 +25,8 @@ export const getVisibleStaticPages = createSelector( 'getVisibleSidebarStaticPages', insightsAreEnabled, isCollapsedThreadsEnabled, - localDraftsAreEnabled, getDraftsCount, - (insightsEnabled, collapsedThreadsEnabled, localDraftsEnabled, draftsCount) => { + (insightsEnabled, collapsedThreadsEnabled, draftsCount) => { const staticPages: StaticPage[] = []; if (insightsEnabled) { @@ -45,12 +43,10 @@ export const getVisibleStaticPages = createSelector( }); } - if (localDraftsEnabled) { - staticPages.push({ - id: 'drafts', - isVisible: draftsCount > 0, - }); - } + staticPages.push({ + id: 'drafts', + isVisible: draftsCount > 0, + }); return staticPages.filter((item) => item.isVisible); },