From b244bb621d4d6b635fa636e5fc5feb0d9d5395c4 Mon Sep 17 00:00:00 2001 From: Ben Cooke Date: Thu, 25 Jul 2024 15:57:23 -0400 Subject: [PATCH] [MM-56904] Reduce the number of api requests made to fetch user information for GMs on page load (#27149) * use new endpoint to fetch group members --- server/channels/api4/preference_test.go | 86 +++++++++++++++++++ server/channels/app/migrations.go | 22 +++++ server/channels/app/server.go | 6 ++ .../delete_dms_preferences_migration.go | 43 ++++++++++ .../delete_dms_preferences_migration_test.go | 57 ++++++++++++ .../opentracinglayer/opentracinglayer.go | 18 ++++ .../channels/store/retrylayer/retrylayer.go | 21 +++++ .../store/sqlstore/preference_store.go | 51 +++++++++++ .../store/sqlstore/preference_store_test.go | 2 +- server/channels/store/store.go | 1 + .../store/storetest/mocks/PreferenceStore.go | 28 ++++++ .../store/storetest/preference_store.go | 78 ++++++++++++++++- .../channels/store/timerlayer/timerlayer.go | 16 ++++ server/channels/testlib/store.go | 1 + server/i18n/en.json | 4 + server/public/model/job.go | 63 +++++++------- server/public/model/migration.go | 1 + server/public/model/preference.go | 12 ++- server/public/model/preference_test.go | 28 ++++++ .../channels/src/actions/user_actions.test.ts | 12 ++- webapp/channels/src/actions/user_actions.ts | 15 ++-- ...idebar_category_sorting_menu.test.tsx.snap | 11 --- .../sidebar_category_sorting_menu.tsx | 11 --- .../limit_visible_gms_dms.tsx | 3 - webapp/channels/src/i18n/en.json | 2 - 25 files changed, 516 insertions(+), 76 deletions(-) create mode 100644 server/channels/jobs/delete_dms_preferences_migration/delete_dms_preferences_migration.go create mode 100644 server/channels/jobs/delete_dms_preferences_migration/delete_dms_preferences_migration_test.go diff --git a/server/channels/api4/preference_test.go b/server/channels/api4/preference_test.go index 3c8ab60ad2..63cdbfc08c 100644 --- a/server/channels/api4/preference_test.go +++ b/server/channels/api4/preference_test.go @@ -933,3 +933,89 @@ func TestDeleteSidebarPreferences(t *testing.T) { assert.NotContains(t, categories.Categories[1].Channels, channel.Id) }) } + +func TestUpdateLimitVisibleDMsGMs(t *testing.T) { + t.Run("Update limit_visible_dms_gms to a valid value", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + client := th.Client + + th.LoginBasic() + user := th.BasicUser + + _, err := client.UpdatePreferences(context.Background(), user.Id, model.Preferences{ + { + UserId: user.Id, + Category: model.PreferenceCategorySidebarSettings, + Name: model.PreferenceLimitVisibleDmsGms, + Value: "40", + }, + }) + require.NoError(t, err) + + pref, _, err := client.GetPreferenceByCategoryAndName(context.Background(), user.Id, model.PreferenceCategorySidebarSettings, model.PreferenceLimitVisibleDmsGms) + require.NoError(t, err) + + require.Equal(t, "40", pref.Value, "Value was not updated") + }) + + t.Run("Update limit_visible_dms_gms to a value greater PreferenceMaxLimitVisibleDmsGmsValue", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + client := th.Client + + th.LoginBasic() + user := th.BasicUser + + resp, err := client.UpdatePreferences(context.Background(), user.Id, model.Preferences{ + { + UserId: user.Id, + Category: model.PreferenceCategorySidebarSettings, + Name: model.PreferenceLimitVisibleDmsGms, + Value: "10000", + }, + }) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + }) + + t.Run("Update limit_visible_dms_gms to an invalid value", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + client := th.Client + + th.LoginBasic() + user := th.BasicUser + + resp, err := client.UpdatePreferences(context.Background(), user.Id, model.Preferences{ + { + UserId: user.Id, + Category: model.PreferenceCategorySidebarSettings, + Name: model.PreferenceLimitVisibleDmsGms, + Value: "one thousand", + }, + }) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + }) + + t.Run("Update limit_visible_dms_gms to a negative number", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + client := th.Client + + th.LoginBasic() + user := th.BasicUser + + resp, err := client.UpdatePreferences(context.Background(), user.Id, model.Preferences{ + { + UserId: user.Id, + Category: model.PreferenceCategorySidebarSettings, + Name: model.PreferenceLimitVisibleDmsGms, + Value: "-20", + }, + }) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + }) +} diff --git a/server/channels/app/migrations.go b/server/channels/app/migrations.go index 20aa6b3129..5f4b272721 100644 --- a/server/channels/app/migrations.go +++ b/server/channels/app/migrations.go @@ -646,6 +646,27 @@ func (s *Server) doDeleteOrphanDraftsMigration(c request.CTX) error { return nil } +func (s *Server) doDeleteDmsPreferencesMigration(c request.CTX) error { + // If the migration is already marked as completed, don't do it again. + if _, err := s.Store().System().GetByName(model.MigrationKeyDeleteDmsPreferences); err == nil { + return nil + } + + jobs, err := s.Store().Job().GetAllByTypeAndStatus(c, model.JobTypeDeleteDmsPreferencesMigration, model.JobStatusPending) + if err != nil { + return fmt.Errorf("failed to get jobs by type and status: %w", err) + } + if len(jobs) > 0 { + return nil + } + + if _, appErr := s.Jobs.CreateJobOnce(c, model.JobTypeDeleteDmsPreferencesMigration, nil); appErr != nil { + return fmt.Errorf("failed to start job for deleting dm preferences: %w", appErr) + } + + return nil +} + func (a *App) DoAppMigrations() { a.Srv().doAppMigrations() } @@ -688,6 +709,7 @@ func (s *Server) doAppMigrations() { {"Encode S3 Image Paths Migration", s.doCloudS3PathMigrations}, {"Delete Empty Drafts Migration", s.doDeleteEmptyDraftsMigration}, {"Delete Orphan Drafts Migration", s.doDeleteOrphanDraftsMigration}, + {"Delete Invalid Dms Preferences Migration", s.doDeleteDmsPreferencesMigration}, } c := request.EmptyContext(s.Log()) diff --git a/server/channels/app/server.go b/server/channels/app/server.go index 30685411c0..93ebb04f59 100644 --- a/server/channels/app/server.go +++ b/server/channels/app/server.go @@ -41,6 +41,7 @@ import ( "github.com/mattermost/mattermost/server/v8/channels/jobs" "github.com/mattermost/mattermost/server/v8/channels/jobs/active_users" "github.com/mattermost/mattermost/server/v8/channels/jobs/cleanup_desktop_tokens" + "github.com/mattermost/mattermost/server/v8/channels/jobs/delete_dms_preferences_migration" "github.com/mattermost/mattermost/server/v8/channels/jobs/delete_empty_drafts_migration" "github.com/mattermost/mattermost/server/v8/channels/jobs/delete_orphan_drafts_migration" "github.com/mattermost/mattermost/server/v8/channels/jobs/expirynotify" @@ -1603,6 +1604,11 @@ func (s *Server) initJobs() { nil, ) + s.Jobs.RegisterJobType( + model.JobTypeDeleteDmsPreferencesMigration, + delete_dms_preferences_migration.MakeWorker(s.Jobs, s.Store(), New(ServerConnector(s.Channels()))), + nil) + s.platform.Jobs = s.Jobs } diff --git a/server/channels/jobs/delete_dms_preferences_migration/delete_dms_preferences_migration.go b/server/channels/jobs/delete_dms_preferences_migration/delete_dms_preferences_migration.go new file mode 100644 index 0000000000..1f60c4570b --- /dev/null +++ b/server/channels/jobs/delete_dms_preferences_migration/delete_dms_preferences_migration.go @@ -0,0 +1,43 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package delete_dms_preferences_migration + +import ( + "time" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/v8/channels/jobs" + "github.com/mattermost/mattermost/server/v8/channels/store" + "github.com/pkg/errors" +) + +const ( + timeBetweenBatches = 1 * time.Second +) + +// MakeWorker creates a batch migration worker to delete empty drafts. +func MakeWorker(jobServer *jobs.JobServer, store store.Store, app jobs.BatchMigrationWorkerAppIFace) model.Worker { + return jobs.MakeBatchMigrationWorker( + jobServer, + store, + app, + model.MigrationKeyDeleteDmsPreferences, + timeBetweenBatches, + doDeleteDmsPreferencesMigrationBatch, + ) +} + +// doDeleteDmsPreferencesMigrationBatch deletes any limit_visible_dms_gms preferences with a value > 40 and less than 1. +func doDeleteDmsPreferencesMigrationBatch(data model.StringMap, store store.Store) (model.StringMap, bool, error) { + rowAffected, err := store.Preference().DeleteInvalidVisibleDmsGms() + if err != nil { + return nil, false, errors.Wrapf(err, "failed to delete invalid limit_visible_dms_gms") + } + + if rowAffected == 0 { + return nil, true, nil + } + + return nil, false, nil +} diff --git a/server/channels/jobs/delete_dms_preferences_migration/delete_dms_preferences_migration_test.go b/server/channels/jobs/delete_dms_preferences_migration/delete_dms_preferences_migration_test.go new file mode 100644 index 0000000000..a7a9f2e5a1 --- /dev/null +++ b/server/channels/jobs/delete_dms_preferences_migration/delete_dms_preferences_migration_test.go @@ -0,0 +1,57 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package delete_dms_preferences_migration + +import ( + "errors" + "testing" + + "github.com/mattermost/mattermost/server/v8/channels/store/storetest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDoDeleteDMsPreferencesMigrationBatch(t *testing.T) { + t.Run("failure deleting batch", func(t *testing.T) { + mockStore := &storetest.Store{} + t.Cleanup(func() { + mockStore.AssertExpectations(t) + }) + + mockStore.PreferenceStore.On("DeleteInvalidVisibleDmsGms").Return(int64(0), errors.New("failure")) + + data, done, err := doDeleteDmsPreferencesMigrationBatch(nil, mockStore) + require.EqualError(t, err, "failed to delete invalid limit_visible_dms_gms: failure") + assert.False(t, done) + assert.Nil(t, data) + }) + + t.Run("do batches", func(t *testing.T) { + mockStore := &storetest.Store{} + t.Cleanup(func() { + mockStore.AssertExpectations(t) + }) + + mockStore.PreferenceStore.On("DeleteInvalidVisibleDmsGms").Return(int64(10), nil) + + data, done, err := doDeleteDmsPreferencesMigrationBatch(nil, mockStore) + require.NoError(t, err) + assert.False(t, done) + assert.Nil(t, data) + }) + + t.Run("done batches", func(t *testing.T) { + mockStore := &storetest.Store{} + t.Cleanup(func() { + mockStore.AssertExpectations(t) + }) + + mockStore.PreferenceStore.On("DeleteInvalidVisibleDmsGms").Return(int64(0), nil) + + data, done, err := doDeleteDmsPreferencesMigrationBatch(nil, mockStore) + require.NoError(t, err) + assert.True(t, done) + assert.Nil(t, data) + }) +} diff --git a/server/channels/store/opentracinglayer/opentracinglayer.go b/server/channels/store/opentracinglayer/opentracinglayer.go index 8a5f871426..6df7e1d4a9 100644 --- a/server/channels/store/opentracinglayer/opentracinglayer.go +++ b/server/channels/store/opentracinglayer/opentracinglayer.go @@ -7378,6 +7378,24 @@ func (s *OpenTracingLayerPreferenceStore) DeleteCategoryAndName(category string, return err } +func (s *OpenTracingLayerPreferenceStore) DeleteInvalidVisibleDmsGms() (int64, error) { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "PreferenceStore.DeleteInvalidVisibleDmsGms") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + result, err := s.PreferenceStore.DeleteInvalidVisibleDmsGms() + if err != nil { + span.LogFields(spanlog.Error(err)) + ext.Error.Set(span, true) + } + + return result, err +} + func (s *OpenTracingLayerPreferenceStore) DeleteOrphanedRows(limit int) (int64, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "PreferenceStore.DeleteOrphanedRows") diff --git a/server/channels/store/retrylayer/retrylayer.go b/server/channels/store/retrylayer/retrylayer.go index d30b9eee18..cfbb2ccb55 100644 --- a/server/channels/store/retrylayer/retrylayer.go +++ b/server/channels/store/retrylayer/retrylayer.go @@ -8384,6 +8384,27 @@ func (s *RetryLayerPreferenceStore) DeleteCategoryAndName(category string, name } +func (s *RetryLayerPreferenceStore) DeleteInvalidVisibleDmsGms() (int64, error) { + + tries := 0 + for { + result, err := s.PreferenceStore.DeleteInvalidVisibleDmsGms() + if err == nil { + return result, nil + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + func (s *RetryLayerPreferenceStore) DeleteOrphanedRows(limit int) (int64, error) { tries := 0 diff --git a/server/channels/store/sqlstore/preference_store.go b/server/channels/store/sqlstore/preference_store.go index b2b5384a14..d78a95847a 100644 --- a/server/channels/store/sqlstore/preference_store.go +++ b/server/channels/store/sqlstore/preference_store.go @@ -315,3 +315,54 @@ func (s SqlPreferenceStore) CleanupFlagsBatch(limit int64) (int64, error) { return rowsAffected, nil } + +// Delete preference for limit_visible_dms_gms where their value is greater than "40" or less than "1" +func (s SqlPreferenceStore) DeleteInvalidVisibleDmsGms() (int64, error) { + var queryString string + var args []interface{} + var err error + + // We need to pad the value field with zeros when doing comparison's because the value is stored as a string. + // Having them the same length allows Postgres/MySQL to compare them correctly. + whereClause := sq.And{ + sq.Eq{"Category": model.PreferenceCategorySidebarSettings}, + sq.Eq{"Name": model.PreferenceLimitVisibleDmsGms}, + sq.Or{ + sq.Gt{"SUBSTRING(CONCAT('000000000000000', Value), LENGTH(Value) + 1, 15)": "000000000000040"}, + sq.Lt{"SUBSTRING(CONCAT('000000000000000', Value), LENGTH(Value) + 1, 15)": "000000000000001"}, + }, + } + if s.DriverName() == "postgres" { + subQuery := s.getQueryBuilder(). + Select("UserId, Category, Name"). + From("Preferences"). + Where(whereClause). + Limit(100) + queryString, args, err = s.getQueryBuilder(). + Delete("Preferences"). + Where(sq.Expr("(userid, category, name) IN (?)", subQuery)). + ToSql() + if err != nil { + return int64(0), errors.Wrap(err, "could not build sql query to delete preference") + } + } else { + queryString, args, err = s.getQueryBuilder(). + Delete("Preferences"). + Where(whereClause). + Limit(100). + ToSql() + if err != nil { + return int64(0), errors.Wrap(err, "could not build sql query to delete preference") + } + } + + result, err := s.GetMasterX().Exec(queryString, args...) + if err != nil { + return 0, errors.Wrap(err, "failed to delete Preference") + } + rowsAffected, err := result.RowsAffected() + if err != nil { + return 0, errors.Wrap(err, "unable to get rows affected") + } + return rowsAffected, nil +} diff --git a/server/channels/store/sqlstore/preference_store_test.go b/server/channels/store/sqlstore/preference_store_test.go index 8e04794af4..e4559a9f46 100644 --- a/server/channels/store/sqlstore/preference_store_test.go +++ b/server/channels/store/sqlstore/preference_store_test.go @@ -15,7 +15,7 @@ import ( ) func TestPreferenceStore(t *testing.T) { - StoreTest(t, storetest.TestPreferenceStore) + StoreTestWithSqlStore(t, storetest.TestPreferenceStore) } func TestDeleteUnusedFeatures(t *testing.T) { diff --git a/server/channels/store/store.go b/server/channels/store/store.go index df3a04227b..2a2910826c 100644 --- a/server/channels/store/store.go +++ b/server/channels/store/store.go @@ -652,6 +652,7 @@ type PreferenceStore interface { PermanentDeleteByUser(userID string) error DeleteOrphanedRows(limit int) (deleted int64, err error) CleanupFlagsBatch(limit int64) (int64, error) + DeleteInvalidVisibleDmsGms() (int64, error) } type LicenseStore interface { diff --git a/server/channels/store/storetest/mocks/PreferenceStore.go b/server/channels/store/storetest/mocks/PreferenceStore.go index 9f0a1f9eaa..719860e852 100644 --- a/server/channels/store/storetest/mocks/PreferenceStore.go +++ b/server/channels/store/storetest/mocks/PreferenceStore.go @@ -96,6 +96,34 @@ func (_m *PreferenceStore) DeleteCategoryAndName(category string, name string) e return r0 } +// DeleteInvalidVisibleDmsGms provides a mock function with given fields: +func (_m *PreferenceStore) DeleteInvalidVisibleDmsGms() (int64, error) { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for DeleteInvalidVisibleDmsGms") + } + + var r0 int64 + var r1 error + if rf, ok := ret.Get(0).(func() (int64, error)); ok { + return rf() + } + if rf, ok := ret.Get(0).(func() int64); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(int64) + } + + if rf, ok := ret.Get(1).(func() error); ok { + r1 = rf() + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // DeleteOrphanedRows provides a mock function with given fields: limit func (_m *PreferenceStore) DeleteOrphanedRows(limit int) (int64, error) { ret := _m.Called(limit) diff --git a/server/channels/store/storetest/preference_store.go b/server/channels/store/storetest/preference_store.go index 82946b97c9..0a89b5bc1b 100644 --- a/server/channels/store/storetest/preference_store.go +++ b/server/channels/store/storetest/preference_store.go @@ -14,7 +14,7 @@ import ( "github.com/mattermost/mattermost/server/v8/channels/store" ) -func TestPreferenceStore(t *testing.T, rctx request.CTX, ss store.Store) { +func TestPreferenceStore(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) { t.Run("PreferenceSave", func(t *testing.T) { testPreferenceSave(t, rctx, ss) }) t.Run("PreferenceGet", func(t *testing.T) { testPreferenceGet(t, rctx, ss) }) t.Run("PreferenceGetCategory", func(t *testing.T) { testPreferenceGetCategory(t, rctx, ss) }) @@ -24,6 +24,7 @@ func TestPreferenceStore(t *testing.T, rctx request.CTX, ss store.Store) { t.Run("PreferenceDeleteCategory", func(t *testing.T) { testPreferenceDeleteCategory(t, rctx, ss) }) t.Run("PreferenceDeleteCategoryAndName", func(t *testing.T) { testPreferenceDeleteCategoryAndName(t, rctx, ss) }) t.Run("PreferenceDeleteOrphanedRows", func(t *testing.T) { testPreferenceDeleteOrphanedRows(t, rctx, ss) }) + t.Run("PreferenceDeleteInvalidVisibleDmsGms", func(t *testing.T) { testDeleteInvalidVisibleDmsGms(t, rctx, ss, s) }) } func testPreferenceSave(t *testing.T, rctx request.CTX, ss store.Store) { @@ -401,3 +402,78 @@ func testPreferenceDeleteOrphanedRows(t *testing.T, rctx request.CTX, ss store.S _, nErr = ss.Preference().Get(userId, category, preference2.Name) assert.NoError(t, nErr, "newer preference should not have been deleted") } + +func testDeleteInvalidVisibleDmsGms(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) { + userId1 := model.NewId() + userId2 := model.NewId() + userId3 := model.NewId() + userId4 := model.NewId() + category := model.PreferenceCategorySidebarSettings + name := model.PreferenceLimitVisibleDmsGms + + preferences := model.Preferences{ + { + UserId: userId1, + Category: category, + Name: name, + Value: "10000", + }, + { + UserId: userId2, + Category: category, + Name: name, + Value: "40", + }, + { + UserId: userId3, + Category: category, + Name: name, + Value: "invalid", + }, + { + UserId: model.NewId(), + Category: category, + Name: name, + Value: "-10", + }, + { + UserId: model.NewId(), + Category: category, + Name: name, + Value: "0", + }, + { + UserId: model.NewId(), + Category: category, + Name: name, + Value: "00000", + }, + { + UserId: userId4, + Category: category, + Name: name, + Value: "20", + }, + } + + // Can't insert with Save methods because the values are invalid + _, execerr := s.GetMasterX().NamedExec(` + INSERT INTO + Preferences(UserId, Category, Name, Value) + VALUES + (:UserId, :Category, :Name, :Value); + `, preferences) + require.NoError(t, execerr) + + count, err := ss.Preference().DeleteInvalidVisibleDmsGms() + require.NoError(t, err) + assert.Equal(t, int64(5), count) + + preference, err := ss.Preference().Get(userId2, category, name) + require.NoError(t, err) + require.Equal(t, &preferences[1], preference) + + preference, err = ss.Preference().Get(userId4, category, name) + require.NoError(t, err) + require.Equal(t, &preferences[6], preference) +} diff --git a/server/channels/store/timerlayer/timerlayer.go b/server/channels/store/timerlayer/timerlayer.go index ff6f294573..6e4cd98b34 100644 --- a/server/channels/store/timerlayer/timerlayer.go +++ b/server/channels/store/timerlayer/timerlayer.go @@ -6665,6 +6665,22 @@ func (s *TimerLayerPreferenceStore) DeleteCategoryAndName(category string, name return err } +func (s *TimerLayerPreferenceStore) DeleteInvalidVisibleDmsGms() (int64, error) { + start := time.Now() + + result, err := s.PreferenceStore.DeleteInvalidVisibleDmsGms() + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("PreferenceStore.DeleteInvalidVisibleDmsGms", success, elapsed) + } + return result, err +} + func (s *TimerLayerPreferenceStore) DeleteOrphanedRows(limit int) (int64, error) { start := time.Now() diff --git a/server/channels/testlib/store.go b/server/channels/testlib/store.go index 630c9434e9..9fca69b6cb 100644 --- a/server/channels/testlib/store.go +++ b/server/channels/testlib/store.go @@ -76,6 +76,7 @@ func GetMockStoreForSetupFunctions() *mocks.Store { systemStore.On("GetByName", model.MigrationKeyAddIPFilteringPermissions).Return(&model.System{Name: model.MigrationKeyAddIPFilteringPermissions, Value: "true"}, nil) systemStore.On("GetByName", model.MigrationKeyAddOutgoingOAuthConnectionsPermissions).Return(&model.System{Name: model.MigrationKeyAddOutgoingOAuthConnectionsPermissions, Value: "true"}, nil) systemStore.On("GetByName", model.MigrationKeyAddChannelBookmarksPermissions).Return(&model.System{Name: model.MigrationKeyAddChannelBookmarksPermissions, Value: "true"}, nil) + systemStore.On("GetByName", model.MigrationKeyDeleteDmsPreferences).Return(&model.System{Name: model.MigrationKeyDeleteDmsPreferences, Value: "true"}, nil) systemStore.On("GetByName", model.MigrationKeyAddManageJobAncillaryPermissions).Return(&model.System{Name: model.MigrationKeyAddManageJobAncillaryPermissions, Value: "true"}, nil) systemStore.On("GetByName", "CustomGroupAdminRoleCreationMigrationComplete").Return(&model.System{Name: model.MigrationKeyAddPlayboosksManageRolesPermissions, Value: "true"}, nil) systemStore.On("GetByName", "products_boards").Return(&model.System{Name: "products_boards", Value: "true"}, nil) diff --git a/server/i18n/en.json b/server/i18n/en.json index 7d88590fd1..d9b99e7ab8 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -9486,6 +9486,10 @@ "id": "model.preference.is_valid.id.app_error", "translation": "Invalid user id." }, + { + "id": "model.preference.is_valid.limit_visible_dms_gms.app_error", + "translation": "Invalid value for limit_visible_dms_gms." + }, { "id": "model.preference.is_valid.name.app_error", "translation": "Invalid name." diff --git a/server/public/model/job.go b/server/public/model/job.go index 14f00f3594..7db6860da5 100644 --- a/server/public/model/job.go +++ b/server/public/model/job.go @@ -8,37 +8,38 @@ import ( ) const ( - JobTypeDataRetention = "data_retention" - JobTypeMessageExport = "message_export" - JobTypeElasticsearchPostIndexing = "elasticsearch_post_indexing" - JobTypeElasticsearchPostAggregation = "elasticsearch_post_aggregation" - JobTypeBlevePostIndexing = "bleve_post_indexing" - JobTypeLdapSync = "ldap_sync" - JobTypeMigrations = "migrations" - JobTypePlugins = "plugins" - JobTypeExpiryNotify = "expiry_notify" - JobTypeProductNotices = "product_notices" - JobTypeActiveUsers = "active_users" - JobTypeImportProcess = "import_process" - JobTypeImportDelete = "import_delete" - JobTypeExportProcess = "export_process" - JobTypeExportDelete = "export_delete" - JobTypeCloud = "cloud" - JobTypeResendInvitationEmail = "resend_invitation_email" - JobTypeExtractContent = "extract_content" - JobTypeLastAccessiblePost = "last_accessible_post" - JobTypeLastAccessibleFile = "last_accessible_file" - JobTypeUpgradeNotifyAdmin = "upgrade_notify_admin" - JobTypeTrialNotifyAdmin = "trial_notify_admin" - JobTypePostPersistentNotifications = "post_persistent_notifications" - JobTypeInstallPluginNotifyAdmin = "install_plugin_notify_admin" - JobTypeHostedPurchaseScreening = "hosted_purchase_screening" - JobTypeS3PathMigration = "s3_path_migration" - JobTypeCleanupDesktopTokens = "cleanup_desktop_tokens" - JobTypeDeleteEmptyDraftsMigration = "delete_empty_drafts_migration" - JobTypeRefreshPostStats = "refresh_post_stats" - JobTypeDeleteOrphanDraftsMigration = "delete_orphan_drafts_migration" - JobTypeExportUsersToCSV = "export_users_to_csv" + JobTypeDataRetention = "data_retention" + JobTypeMessageExport = "message_export" + JobTypeElasticsearchPostIndexing = "elasticsearch_post_indexing" + JobTypeElasticsearchPostAggregation = "elasticsearch_post_aggregation" + JobTypeBlevePostIndexing = "bleve_post_indexing" + JobTypeLdapSync = "ldap_sync" + JobTypeMigrations = "migrations" + JobTypePlugins = "plugins" + JobTypeExpiryNotify = "expiry_notify" + JobTypeProductNotices = "product_notices" + JobTypeActiveUsers = "active_users" + JobTypeImportProcess = "import_process" + JobTypeImportDelete = "import_delete" + JobTypeExportProcess = "export_process" + JobTypeExportDelete = "export_delete" + JobTypeCloud = "cloud" + JobTypeResendInvitationEmail = "resend_invitation_email" + JobTypeExtractContent = "extract_content" + JobTypeLastAccessiblePost = "last_accessible_post" + JobTypeLastAccessibleFile = "last_accessible_file" + JobTypeUpgradeNotifyAdmin = "upgrade_notify_admin" + JobTypeTrialNotifyAdmin = "trial_notify_admin" + JobTypePostPersistentNotifications = "post_persistent_notifications" + JobTypeInstallPluginNotifyAdmin = "install_plugin_notify_admin" + JobTypeHostedPurchaseScreening = "hosted_purchase_screening" + JobTypeS3PathMigration = "s3_path_migration" + JobTypeCleanupDesktopTokens = "cleanup_desktop_tokens" + JobTypeDeleteEmptyDraftsMigration = "delete_empty_drafts_migration" + JobTypeRefreshPostStats = "refresh_post_stats" + JobTypeDeleteOrphanDraftsMigration = "delete_orphan_drafts_migration" + JobTypeExportUsersToCSV = "export_users_to_csv" + JobTypeDeleteDmsPreferencesMigration = "delete_dms_preferences_migration" JobStatusPending = "pending" JobStatusInProgress = "in_progress" diff --git a/server/public/model/migration.go b/server/public/model/migration.go index bdd0531bd6..7ac3c7e4ed 100644 --- a/server/public/model/migration.go +++ b/server/public/model/migration.go @@ -47,5 +47,6 @@ const ( MigrationKeyAddIPFilteringPermissions = "add_ip_filtering_permissions" MigrationKeyAddOutgoingOAuthConnectionsPermissions = "add_outgoing_oauth_connections_permissions" MigrationKeyAddChannelBookmarksPermissions = "add_channel_bookmarks_permissions" + MigrationKeyDeleteDmsPreferences = "delete_dms_preferences_migration" MigrationKeyAddManageJobAncillaryPermissions = "add_manage_jobs_ancillary_permissions" ) diff --git a/server/public/model/preference.go b/server/public/model/preference.go index 921e3611d2..7b98a1cb4a 100644 --- a/server/public/model/preference.go +++ b/server/public/model/preference.go @@ -7,6 +7,7 @@ import ( "encoding/json" "net/http" "regexp" + "strconv" "strings" "unicode/utf8" ) @@ -61,7 +62,9 @@ const ( PreferenceEmailIntervalHourAsSeconds = "3600" PreferenceCloudUserEphemeralInfo = "cloud_user_ephemeral_info" - MaxPreferenceValueLength = 20000 + PreferenceLimitVisibleDmsGms = "limit_visible_dms_gms" + PreferenceMaxLimitVisibleDmsGmsValue = 40 + MaxPreferenceValueLength = 20000 ) type Preference struct { @@ -97,6 +100,13 @@ func (o *Preference) IsValid() *AppError { } } + if o.Category == PreferenceCategorySidebarSettings && o.Name == PreferenceLimitVisibleDmsGms { + visibleDmsGmsValue, convErr := strconv.Atoi(o.Value) + if convErr != nil || visibleDmsGmsValue < 1 || visibleDmsGmsValue > PreferenceMaxLimitVisibleDmsGmsValue { + return NewAppError("Preference.IsValid", "model.preference.is_valid.limit_visible_dms_gms.app_error", nil, "value="+o.Value, http.StatusBadRequest) + } + } + return nil } diff --git a/server/public/model/preference_test.go b/server/public/model/preference_test.go index 231130cf98..7d28bde648 100644 --- a/server/public/model/preference_test.go +++ b/server/public/model/preference_test.go @@ -70,6 +70,34 @@ func TestPreferenceIsValid(t *testing.T) { require.Nil(t, preference.IsValid()) }) + + t.Run("limit_visible_dms_gms has a valid value", func(t *testing.T) { + preference.Category = PreferenceCategorySidebarSettings + preference.Name = PreferenceLimitVisibleDmsGms + preference.Value = "40" + require.Nil(t, preference.IsValid()) + }) + + t.Run("limit_visible_dms_gms has a value greater than PreferenceMaxLimitVisibleDmsGmsValue", func(t *testing.T) { + preference.Category = PreferenceCategorySidebarSettings + preference.Name = PreferenceLimitVisibleDmsGms + preference.Value = "10000" + require.NotNil(t, preference.IsValid()) + }) + + t.Run("limit_visible_dms_gms has an invalid value", func(t *testing.T) { + preference.Category = PreferenceCategorySidebarSettings + preference.Name = PreferenceLimitVisibleDmsGms + preference.Value = "one thousand" + require.NotNil(t, preference.IsValid()) + }) + + t.Run("limit_visible_dms_gms has a negative number", func(t *testing.T) { + preference.Category = PreferenceCategorySidebarSettings + preference.Name = PreferenceLimitVisibleDmsGms + preference.Value = "-10" + require.NotNil(t, preference.IsValid()) + }) } func TestPreferencePreUpdate(t *testing.T) { diff --git a/webapp/channels/src/actions/user_actions.test.ts b/webapp/channels/src/actions/user_actions.test.ts index 9188d0e377..8f62fd1238 100644 --- a/webapp/channels/src/actions/user_actions.test.ts +++ b/webapp/channels/src/actions/user_actions.test.ts @@ -520,11 +520,8 @@ describe('Actions.User', () => { }); }); - test('Should call p-queue APIs on loadProfilesForGM', async () => { + test('Should call getProfilesInGroupChannels on loadProfilesForGM', async () => { const gmChannel = {id: 'gmChannel', type: General.GM_CHANNEL, team_id: '', delete_at: 0}; - UserActions.queue.add = jest.fn().mockReturnValue(jest.fn()); - UserActions.queue.onEmpty = jest.fn(); - const user = TestHelper.fakeUser(); const profiles = { @@ -555,7 +552,7 @@ describe('Actions.User', () => { profiles, statuses: {}, profilesInChannel: { - [gmChannel.id]: new Set(['current_user_id']), + [gmChannel.id]: new Set([]), }, }, teams: { @@ -605,9 +602,10 @@ describe('Actions.User', () => { const testStore = mockStore(state); store.getState.mockImplementation(testStore.getState); + store.dispatch.mockImplementation(testStore.dispatch); + const actions = testStore.getActions(); await UserActions.loadProfilesForGM(); - expect(UserActions.queue.onEmpty).toHaveBeenCalled(); - expect(UserActions.queue.add).toHaveBeenCalled(); + expect(actions).toEqual([{args: [['gmChannel']], type: 'MOCK_GET_PROFILES_IN_GROUP_CHANNELS'}]); }); }); diff --git a/webapp/channels/src/actions/user_actions.ts b/webapp/channels/src/actions/user_actions.ts index afcb9ba377..faad7e32a5 100644 --- a/webapp/channels/src/actions/user_actions.ts +++ b/webapp/channels/src/actions/user_actions.ts @@ -1,8 +1,6 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import PQueue from 'p-queue'; - import type {UserAutocomplete} from '@mattermost/types/autocomplete'; import type {Channel} from '@mattermost/types/channels'; import type {UserProfile, UserStatus} from '@mattermost/types/users'; @@ -37,7 +35,6 @@ import * as Utils from 'utils/utils'; import type {GlobalState} from 'types/store'; -export const queue = new PQueue({concurrency: 4}); const dispatch = store.dispatch; const getState = store.getState; @@ -312,9 +309,9 @@ export async function loadProfilesForGM() { const collapsedThreads = isCollapsedThreadsEnabled(state); const userIdsForLoadingCustomEmojis = new Set(); + const channelUsersToLoad: string[] = []; for (const channel of getGMsForLoading(state)) { const userIds = userIdsInChannels[channel.id] || new Set(); - userIds.forEach((userId) => userIdsForLoadingCustomEmojis.add(userId)); if (userIds.size >= Constants.MIN_USERS_IN_GM) { @@ -340,12 +337,14 @@ export async function loadProfilesForGM() { value: 'true', }); } - - const getProfilesAction = UserActions.getProfilesInChannel(channel.id, 0, Constants.MAX_USERS_IN_GM); - queue.add(() => dispatch(getProfilesAction)); + if (userIds.size === 0) { + channelUsersToLoad.push(channel.id); + } } - await queue.onEmpty(); + if (channelUsersToLoad.length > 0) { + await dispatch(UserActions.getProfilesInGroupChannels(channelUsersToLoad)); + } if (userIdsForLoadingCustomEmojis.size > 0) { dispatch(loadCustomEmojisForCustomStatusesByUserIds(userIdsForLoadingCustomEmojis)); diff --git a/webapp/channels/src/components/sidebar/sidebar_category/__snapshots__/sidebar_category_sorting_menu.test.tsx.snap b/webapp/channels/src/components/sidebar/sidebar_category/__snapshots__/sidebar_category_sorting_menu.test.tsx.snap index aef32d6964..192edeb8a6 100644 --- a/webapp/channels/src/components/sidebar/sidebar_category/__snapshots__/sidebar_category_sorting_menu.test.tsx.snap +++ b/webapp/channels/src/components/sidebar/sidebar_category/__snapshots__/sidebar_category_sorting_menu.test.tsx.snap @@ -102,17 +102,6 @@ exports[`components/sidebar/sidebar_category/sidebar_category_sorting_menu shoul } > - - } - onClick={[Function]} - /> - - - )} - onClick={() => handlelimitVisibleDMsGMs(Constants.HIGHEST_DM_SHOW_COUNT)} - /> - {Constants.DM_AND_GM_SHOW_COUNTS.map((dmGmShowCount) => (