From eaa5cce3cebab4cdf07e13b09d38ccc2823fec4b Mon Sep 17 00:00:00 2001 From: Devin Binnie <52460000+devinbinnie@users.noreply.github.com> Date: Mon, 27 Nov 2023 09:11:04 -0500 Subject: [PATCH] [MM-55143] Disallow reacting with an emoji that does not exist, limit the total number of unique reactions per post (#25331) * [MM-55143] Disallow reacting with an emoji that does not exist * WIP for server limit on emoji reactions * WIP * Implement default limit of 25 unique emoji reactions * Add modal for reaction limit * Fix test * PR feedback * Fix i18n * Update admin string * Merge'd * Fixing some issues, check limits correctly based on other users reactions * Fix typos * Fix lint/test * Add tests, fix other tests --------- Co-authored-by: Mattermost Build --- .../support/server/default_config.ts | 1 + server/channels/api4/reaction_test.go | 6 +- server/channels/app/export_test.go | 10 +- server/channels/app/reaction.go | 24 +++++ server/channels/app/reaction_test.go | 95 +++++++++++++++++++ .../opentracinglayer/opentracinglayer.go | 36 +++++++ .../channels/store/retrylayer/retrylayer.go | 42 ++++++++ .../channels/store/sqlstore/reaction_store.go | 35 +++++++ server/channels/store/store.go | 2 + .../store/storetest/mocks/ReactionStore.go | 48 ++++++++++ .../store/storetest/reaction_store.go | 65 +++++++++++++ .../channels/store/timerlayer/timerlayer.go | 32 +++++++ server/config/client.go | 1 + server/i18n/en.json | 4 + server/public/model/config.go | 33 ++++--- .../channels/src/actions/post_actions.test.ts | 48 ++++++++-- webapp/channels/src/actions/post_actions.ts | 29 +++++- .../admin_console/admin_definition.tsx | 31 ++++++ .../reaction_limit_reached_modal.tsx | 63 ++++++++++++ webapp/channels/src/i18n/en.json | 8 ++ webapp/channels/src/utils/constants.tsx | 1 + webapp/channels/src/utils/post_utils.test.tsx | 40 ++++++++ webapp/channels/src/utils/post_utils.ts | 25 +++++ webapp/platform/types/src/config.ts | 2 + 24 files changed, 652 insertions(+), 29 deletions(-) create mode 100644 webapp/channels/src/components/reaction_limit_reached_modal.tsx diff --git a/e2e-tests/playwright/support/server/default_config.ts b/e2e-tests/playwright/support/server/default_config.ts index 035605cafa..1a1a0f0740 100644 --- a/e2e-tests/playwright/support/server/default_config.ts +++ b/e2e-tests/playwright/support/server/default_config.ts @@ -179,6 +179,7 @@ const defaultServerConfig: AdminConfig = { EnableCustomGroups: true, SelfHostedPurchase: true, AllowSyncedDrafts: true, + UniqueEmojiReactionLimitPerPost: 25, RefreshPostStatsRunTime: '00:00', }, TeamSettings: { diff --git a/server/channels/api4/reaction_test.go b/server/channels/api4/reaction_test.go index 03eb02d748..c0b08a17ad 100644 --- a/server/channels/api4/reaction_test.go +++ b/server/channels/api4/reaction_test.go @@ -55,7 +55,7 @@ func TestSaveReaction(t *testing.T) { }) t.Run("save-second-reaction", func(t *testing.T) { - reaction.EmojiName = "sad" + reaction.EmojiName = "cry" rr, _, err := client.SaveReaction(context.Background(), reaction) require.NoError(t, err) @@ -290,7 +290,7 @@ func TestDeleteReaction(t *testing.T) { r2 := &model.Reaction{ UserId: userId, PostId: postId, - EmojiName: "smile-", + EmojiName: "cry", } r3 := &model.Reaction{ @@ -302,7 +302,7 @@ func TestDeleteReaction(t *testing.T) { r4 := &model.Reaction{ UserId: user2Id, PostId: postId, - EmojiName: "smile_", + EmojiName: "grin", } // Check the appropriate permissions are enforced. diff --git a/server/channels/app/export_test.go b/server/channels/app/export_test.go index dd9190a6fb..3ca3d597bb 100644 --- a/server/channels/app/export_test.go +++ b/server/channels/app/export_test.go @@ -29,18 +29,20 @@ func TestReactionsOfPost(t *testing.T) { reactionObject := model.Reaction{ UserId: th.BasicUser.Id, PostId: post.Id, - EmojiName: "emoji", + EmojiName: "smile", CreateAt: model.GetMillis(), } reactionObjectDeleted := model.Reaction{ UserId: th.BasicUser2.Id, PostId: post.Id, - EmojiName: "emoji", + EmojiName: "smile", CreateAt: model.GetMillis(), } - th.App.SaveReactionForPost(th.Context, &reactionObject) - th.App.SaveReactionForPost(th.Context, &reactionObjectDeleted) + _, err := th.App.SaveReactionForPost(th.Context, &reactionObject) + require.Nil(t, err) + _, err = th.App.SaveReactionForPost(th.Context, &reactionObjectDeleted) + require.Nil(t, err) reactionsOfPost, err := th.App.BuildPostReactions(th.Context, post.Id) require.Nil(t, err) diff --git a/server/channels/app/reaction.go b/server/channels/app/reaction.go index 8adbe16784..449ed3aab8 100644 --- a/server/channels/app/reaction.go +++ b/server/channels/app/reaction.go @@ -20,6 +20,30 @@ func (a *App) SaveReactionForPost(c request.CTX, reaction *model.Reaction) (*mod return nil, err } + // Check whether this is a valid emoji + if _, ok := model.GetSystemEmojiId(reaction.EmojiName); !ok { + if _, emojiErr := a.GetEmojiByName(c, reaction.EmojiName); emojiErr != nil { + return nil, emojiErr + } + } + + existing, dErr := a.Srv().Store().Reaction().ExistsOnPost(reaction.PostId, reaction.EmojiName) + if dErr != nil { + return nil, model.NewAppError("SaveReactionForPost", "app.reaction.save.save.app_error", nil, "", http.StatusInternalServerError).Wrap(dErr) + } + + // If it exists already, we don't need to check for the limit + if !existing { + count, dErr := a.Srv().Store().Reaction().GetUniqueCountForPost(reaction.PostId) + if dErr != nil { + return nil, model.NewAppError("SaveReactionForPost", "app.reaction.save.save.app_error", nil, "", http.StatusInternalServerError).Wrap(dErr) + } + + if count >= *a.Config().ServiceSettings.UniqueEmojiReactionLimitPerPost { + return nil, model.NewAppError("SaveReactionForPost", "app.reaction.save.save.too_many_reactions", nil, "", http.StatusBadRequest) + } + } + channel, err := a.GetChannel(c, post.ChannelId) if err != nil { return nil, err diff --git a/server/channels/app/reaction_test.go b/server/channels/app/reaction_test.go index dd2f244a53..03ee890a5f 100644 --- a/server/channels/app/reaction_test.go +++ b/server/channels/app/reaction_test.go @@ -13,6 +13,89 @@ import ( "github.com/mattermost/mattermost/server/v8/channels/testlib" ) +func TestSaveReactionForPost(t *testing.T) { + th := Setup(t).InitBasic() + + post := th.CreatePost(th.BasicChannel) + reaction1, err := th.App.SaveReactionForPost(th.Context, &model.Reaction{ + UserId: th.BasicUser.Id, + PostId: post.Id, + EmojiName: "cry", + }) + require.NotNil(t, reaction1) + require.Nil(t, err) + reaction2, err := th.App.SaveReactionForPost(th.Context, &model.Reaction{ + UserId: th.BasicUser.Id, + PostId: post.Id, + EmojiName: "smile", + }) + require.NotNil(t, reaction2) + require.Nil(t, err) + reaction3, err := th.App.SaveReactionForPost(th.Context, &model.Reaction{ + UserId: th.BasicUser.Id, + PostId: post.Id, + EmojiName: "rofl", + }) + require.NotNil(t, reaction3) + require.Nil(t, err) + + t.Run("should not add reaction if it does not exist on the system", func(t *testing.T) { + reaction := &model.Reaction{ + UserId: th.BasicUser.Id, + PostId: th.BasicPost.Id, + EmojiName: "definitely-not-a-real-emoji", + } + + result, err := th.App.SaveReactionForPost(th.Context, reaction) + require.NotNil(t, err) + require.Nil(t, result) + }) + + t.Run("should not add reaction if we are over the limit", func(t *testing.T) { + var originalLimit *int + th.UpdateConfig(func(cfg *model.Config) { + originalLimit = cfg.ServiceSettings.UniqueEmojiReactionLimitPerPost + *cfg.ServiceSettings.UniqueEmojiReactionLimitPerPost = 3 + }) + defer th.UpdateConfig(func(cfg *model.Config) { + cfg.ServiceSettings.UniqueEmojiReactionLimitPerPost = originalLimit + }) + + reaction := &model.Reaction{ + UserId: th.BasicUser.Id, + PostId: post.Id, + EmojiName: "joy", + } + + result, err := th.App.SaveReactionForPost(th.Context, reaction) + require.NotNil(t, err) + require.Nil(t, result) + }) + + t.Run("should always add reaction if we are over the limit but the reaction is not unique", func(t *testing.T) { + user := th.CreateUser() + + var originalLimit *int + th.UpdateConfig(func(cfg *model.Config) { + originalLimit = cfg.ServiceSettings.UniqueEmojiReactionLimitPerPost + *cfg.ServiceSettings.UniqueEmojiReactionLimitPerPost = 3 + }) + defer th.UpdateConfig(func(cfg *model.Config) { + cfg.ServiceSettings.UniqueEmojiReactionLimitPerPost = originalLimit + }) + + reaction := &model.Reaction{ + UserId: user.Id, + PostId: post.Id, + EmojiName: "cry", + } + + result, err := th.App.SaveReactionForPost(th.Context, reaction) + require.Nil(t, err) + require.NotNil(t, result) + }) +} + func TestSharedChannelSyncForReactionActions(t *testing.T) { t.Run("adding a reaction in a shared channel performs a content sync when sync service is running on that node", func(t *testing.T) { th := Setup(t).InitBasic() @@ -84,3 +167,15 @@ func TestSharedChannelSyncForReactionActions(t *testing.T) { assert.Equal(t, channel.Id, sharedChannelService.channelNotifications[1]) }) } + +func (th *TestHelper) UpdateConfig(f func(*model.Config)) { + if th.ConfigStore.IsReadOnly() { + return + } + old := th.ConfigStore.Get() + updated := old.Clone() + f(updated) + if _, _, err := th.ConfigStore.Set(updated); err != nil { + panic(err) + } +} diff --git a/server/channels/store/opentracinglayer/opentracinglayer.go b/server/channels/store/opentracinglayer/opentracinglayer.go index dd171dab32..e434f399a5 100644 --- a/server/channels/store/opentracinglayer/opentracinglayer.go +++ b/server/channels/store/opentracinglayer/opentracinglayer.go @@ -7463,6 +7463,24 @@ func (s *OpenTracingLayerReactionStore) DeleteOrphanedRowsByIds(r *model.Retenti return err } +func (s *OpenTracingLayerReactionStore) ExistsOnPost(postId string, emojiName string) (bool, error) { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ReactionStore.ExistsOnPost") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + result, err := s.ReactionStore.ExistsOnPost(postId, emojiName) + if err != nil { + span.LogFields(spanlog.Error(err)) + ext.Error.Set(span, true) + } + + return result, err +} + func (s *OpenTracingLayerReactionStore) GetForPost(postID string, allowFromCache bool) ([]*model.Reaction, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ReactionStore.GetForPost") @@ -7499,6 +7517,24 @@ func (s *OpenTracingLayerReactionStore) GetForPostSince(postId string, since int return result, err } +func (s *OpenTracingLayerReactionStore) GetUniqueCountForPost(postId string) (int, error) { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ReactionStore.GetUniqueCountForPost") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + result, err := s.ReactionStore.GetUniqueCountForPost(postId) + if err != nil { + span.LogFields(spanlog.Error(err)) + ext.Error.Set(span, true) + } + + return result, err +} + func (s *OpenTracingLayerReactionStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ReactionStore.PermanentDeleteBatch") diff --git a/server/channels/store/retrylayer/retrylayer.go b/server/channels/store/retrylayer/retrylayer.go index 3c0103eb37..5ce7f03a1a 100644 --- a/server/channels/store/retrylayer/retrylayer.go +++ b/server/channels/store/retrylayer/retrylayer.go @@ -8474,6 +8474,27 @@ func (s *RetryLayerReactionStore) DeleteOrphanedRowsByIds(r *model.RetentionIdsF } +func (s *RetryLayerReactionStore) ExistsOnPost(postId string, emojiName string) (bool, error) { + + tries := 0 + for { + result, err := s.ReactionStore.ExistsOnPost(postId, emojiName) + 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 *RetryLayerReactionStore) GetForPost(postID string, allowFromCache bool) ([]*model.Reaction, error) { tries := 0 @@ -8516,6 +8537,27 @@ func (s *RetryLayerReactionStore) GetForPostSince(postId string, since int64, ex } +func (s *RetryLayerReactionStore) GetUniqueCountForPost(postId string) (int, error) { + + tries := 0 + for { + result, err := s.ReactionStore.GetUniqueCountForPost(postId) + 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 *RetryLayerReactionStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, error) { tries := 0 diff --git a/server/channels/store/sqlstore/reaction_store.go b/server/channels/store/sqlstore/reaction_store.go index 19bbeecc9a..95e2e96c05 100644 --- a/server/channels/store/sqlstore/reaction_store.go +++ b/server/channels/store/sqlstore/reaction_store.go @@ -4,6 +4,7 @@ package sqlstore import ( + "database/sql" "time" sq "github.com/mattermost/squirrel" @@ -107,6 +108,25 @@ func (s *SqlReactionStore) GetForPost(postId string, allowFromCache bool) ([]*mo return reactions, nil } +func (s *SqlReactionStore) ExistsOnPost(postId string, emojiName string) (bool, error) { + query := s.getQueryBuilder(). + Select("1"). + From("Reactions"). + Where(sq.Eq{"PostId": postId}). + Where(sq.Eq{"EmojiName": emojiName}). + Where(sq.Eq{"COALESCE(DeleteAt, 0)": 0}) + + var hasRows bool + if err := s.GetReplicaX().GetBuilder(&hasRows, query); err != nil { + if err == sql.ErrNoRows { + return false, nil + } + return false, errors.Wrap(err, "failed to check for existing reaction") + } + + return hasRows, nil +} + // GetForPostSince returns all reactions associated with `postId` updated after `since`. func (s *SqlReactionStore) GetForPostSince(postId string, since int64, excludeRemoteId string, inclDeleted bool) ([]*model.Reaction, error) { query := s.getQueryBuilder(). @@ -138,6 +158,21 @@ func (s *SqlReactionStore) GetForPostSince(postId string, since int64, excludeRe return reactions, nil } +func (s *SqlReactionStore) GetUniqueCountForPost(postId string) (int, error) { + query := s.getQueryBuilder(). + Select("COUNT(DISTINCT EmojiName)"). + From("Reactions"). + Where(sq.Eq{"PostId": postId}). + Where(sq.Eq{"DeleteAt": 0}) + + var count int64 + err := s.GetReplicaX().GetBuilder(&count, query) + if err != nil { + return 0, errors.Wrap(err, "failed to count Reactions") + } + return int(count), nil +} + func (s *SqlReactionStore) BulkGetForPosts(postIds []string) ([]*model.Reaction, error) { placeholder, values := constructArrayArgs(postIds) var reactions []*model.Reaction diff --git a/server/channels/store/store.go b/server/channels/store/store.go index 55ea29228d..b85d354efa 100644 --- a/server/channels/store/store.go +++ b/server/channels/store/store.go @@ -744,6 +744,8 @@ type ReactionStore interface { Delete(reaction *model.Reaction) (*model.Reaction, error) GetForPost(postID string, allowFromCache bool) ([]*model.Reaction, error) GetForPostSince(postId string, since int64, excludeRemoteId string, inclDeleted bool) ([]*model.Reaction, error) + GetUniqueCountForPost(postId string) (int, error) + ExistsOnPost(postId string, emojiName string) (bool, error) DeleteAllWithEmojiName(emojiName string) error BulkGetForPosts(postIds []string) ([]*model.Reaction, error) DeleteOrphanedRowsByIds(r *model.RetentionIdsForDeletion) error diff --git a/server/channels/store/storetest/mocks/ReactionStore.go b/server/channels/store/storetest/mocks/ReactionStore.go index 966010f668..aa023b3b29 100644 --- a/server/channels/store/storetest/mocks/ReactionStore.go +++ b/server/channels/store/storetest/mocks/ReactionStore.go @@ -94,6 +94,30 @@ func (_m *ReactionStore) DeleteOrphanedRowsByIds(r *model.RetentionIdsForDeletio return r0 } +// ExistsOnPost provides a mock function with given fields: postId, emojiName +func (_m *ReactionStore) ExistsOnPost(postId string, emojiName string) (bool, error) { + ret := _m.Called(postId, emojiName) + + var r0 bool + var r1 error + if rf, ok := ret.Get(0).(func(string, string) (bool, error)); ok { + return rf(postId, emojiName) + } + if rf, ok := ret.Get(0).(func(string, string) bool); ok { + r0 = rf(postId, emojiName) + } else { + r0 = ret.Get(0).(bool) + } + + if rf, ok := ret.Get(1).(func(string, string) error); ok { + r1 = rf(postId, emojiName) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetForPost provides a mock function with given fields: postID, allowFromCache func (_m *ReactionStore) GetForPost(postID string, allowFromCache bool) ([]*model.Reaction, error) { ret := _m.Called(postID, allowFromCache) @@ -146,6 +170,30 @@ func (_m *ReactionStore) GetForPostSince(postId string, since int64, excludeRemo return r0, r1 } +// GetUniqueCountForPost provides a mock function with given fields: postId +func (_m *ReactionStore) GetUniqueCountForPost(postId string) (int, error) { + ret := _m.Called(postId) + + var r0 int + var r1 error + if rf, ok := ret.Get(0).(func(string) (int, error)); ok { + return rf(postId) + } + if rf, ok := ret.Get(0).(func(string) int); ok { + r0 = rf(postId) + } else { + r0 = ret.Get(0).(int) + } + + if rf, ok := ret.Get(1).(func(string) error); ok { + r1 = rf(postId) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // PermanentDeleteBatch provides a mock function with given fields: endTime, limit func (_m *ReactionStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, error) { ret := _m.Called(endTime, limit) diff --git a/server/channels/store/storetest/reaction_store.go b/server/channels/store/storetest/reaction_store.go index f3ae24e12d..8350846ea6 100644 --- a/server/channels/store/storetest/reaction_store.go +++ b/server/channels/store/storetest/reaction_store.go @@ -29,6 +29,8 @@ func TestReactionStore(t *testing.T, rctx request.CTX, ss store.Store, s SqlStor t.Run("PermanentDeleteBatch", func(t *testing.T) { testReactionStorePermanentDeleteBatch(t, rctx, ss) }) t.Run("ReactionBulkGetForPosts", func(t *testing.T) { testReactionBulkGetForPosts(t, rctx, ss) }) t.Run("ReactionDeadlock", func(t *testing.T) { testReactionDeadlock(t, rctx, ss) }) + t.Run("ExistsOnPost", func(t *testing.T) { testExistsOnPost(t, rctx, ss) }) + t.Run("GetUniqueCountForPost", func(t *testing.T) { testGetUniqueCountForPost(t, rctx, ss) }) } func testReactionSave(t *testing.T, rctx request.CTX, ss store.Store) { @@ -873,3 +875,66 @@ func testReactionDeadlock(t *testing.T, rctx request.CTX, ss store.Store) { }() wg.Wait() } + +func testExistsOnPost(t *testing.T, rctx request.CTX, ss store.Store) { + post, _ := ss.Post().Save(&model.Post{ + ChannelId: model.NewId(), + UserId: model.NewId(), + }) + emojiName := model.NewId() + reaction := &model.Reaction{ + UserId: model.NewId(), + PostId: post.Id, + EmojiName: emojiName, + } + _, nErr := ss.Reaction().Save(reaction) + require.NoError(t, nErr) + exists, err := ss.Reaction().ExistsOnPost(post.Id, emojiName) + require.NoError(t, err) + require.True(t, exists) + exists, err = ss.Reaction().ExistsOnPost(post.Id, model.NewId()) + require.NoError(t, err) + require.False(t, exists) +} + +func testGetUniqueCountForPost(t *testing.T, rctx request.CTX, ss store.Store) { + post, _ := ss.Post().Save(&model.Post{ + ChannelId: model.NewId(), + UserId: model.NewId(), + }) + + userId := model.NewId() + emojiName := model.NewId() + + reaction := &model.Reaction{ + UserId: userId, + PostId: post.Id, + EmojiName: emojiName, + } + _, nErr := ss.Reaction().Save(reaction) + require.NoError(t, nErr) + + sameReaction := &model.Reaction{ + UserId: model.NewId(), + PostId: post.Id, + EmojiName: emojiName, + } + _, nErr = ss.Reaction().Save(sameReaction) + require.NoError(t, nErr) + + newReaction := &model.Reaction{ + UserId: userId, + PostId: post.Id, + EmojiName: model.NewId(), + } + _, nErr = ss.Reaction().Save(newReaction) + require.NoError(t, nErr) + + totalReactions, err := ss.Reaction().GetForPost(post.Id, false) + require.NoError(t, err) + require.Equal(t, 3, len(totalReactions)) + + count, err := ss.Reaction().GetUniqueCountForPost(post.Id) + require.NoError(t, err) + require.Equal(t, 2, count) +} diff --git a/server/channels/store/timerlayer/timerlayer.go b/server/channels/store/timerlayer/timerlayer.go index 93d0598942..b665304e6e 100644 --- a/server/channels/store/timerlayer/timerlayer.go +++ b/server/channels/store/timerlayer/timerlayer.go @@ -6745,6 +6745,22 @@ func (s *TimerLayerReactionStore) DeleteOrphanedRowsByIds(r *model.RetentionIdsF return err } +func (s *TimerLayerReactionStore) ExistsOnPost(postId string, emojiName string) (bool, error) { + start := time.Now() + + result, err := s.ReactionStore.ExistsOnPost(postId, emojiName) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("ReactionStore.ExistsOnPost", success, elapsed) + } + return result, err +} + func (s *TimerLayerReactionStore) GetForPost(postID string, allowFromCache bool) ([]*model.Reaction, error) { start := time.Now() @@ -6777,6 +6793,22 @@ func (s *TimerLayerReactionStore) GetForPostSince(postId string, since int64, ex return result, err } +func (s *TimerLayerReactionStore) GetUniqueCountForPost(postId string) (int, error) { + start := time.Now() + + result, err := s.ReactionStore.GetUniqueCountForPost(postId) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("ReactionStore.GetUniqueCountForPost", success, elapsed) + } + return result, err +} + func (s *TimerLayerReactionStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, error) { start := time.Now() diff --git a/server/config/client.go b/server/config/client.go index 278117430a..e7d47c8319 100644 --- a/server/config/client.go +++ b/server/config/client.go @@ -140,6 +140,7 @@ func GenerateClientConfig(c *model.Config, telemetryID string, license *model.Li props["PersistentNotificationMaxRecipients"] = strconv.FormatInt(int64(*c.ServiceSettings.PersistentNotificationMaxRecipients), 10) props["AllowSyncedDrafts"] = strconv.FormatBool(*c.ServiceSettings.AllowSyncedDrafts) props["DelayChannelAutocomplete"] = strconv.FormatBool(*c.ExperimentalSettings.DelayChannelAutocomplete) + props["UniqueEmojiReactionLimitPerPost"] = strconv.FormatInt(int64(*c.ServiceSettings.UniqueEmojiReactionLimitPerPost), 10) if license != nil { props["ExperimentalEnableAuthenticationTransfer"] = strconv.FormatBool(*c.ServiceSettings.ExperimentalEnableAuthenticationTransfer) diff --git a/server/i18n/en.json b/server/i18n/en.json index 02e38bd955..19beee848a 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -6526,6 +6526,10 @@ "id": "app.reaction.save.save.app_error", "translation": "Unable to save reaction." }, + { + "id": "app.reaction.save.save.too_many_reactions", + "translation": "Reaction limit has been reached for this post." + }, { "id": "app.recover.delete.app_error", "translation": "Unable to delete token." diff --git a/server/public/model/config.go b/server/public/model/config.go index b29f4c3737..030f939e5c 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -100,17 +100,19 @@ const ( SitenameMaxLength = 30 - ServiceSettingsDefaultSiteURL = "http://localhost:8065" - ServiceSettingsDefaultTLSCertFile = "" - ServiceSettingsDefaultTLSKeyFile = "" - ServiceSettingsDefaultReadTimeout = 300 - ServiceSettingsDefaultWriteTimeout = 300 - ServiceSettingsDefaultIdleTimeout = 60 - ServiceSettingsDefaultMaxLoginAttempts = 10 - ServiceSettingsDefaultAllowCorsFrom = "" - ServiceSettingsDefaultListenAndAddress = ":8065" - ServiceSettingsDefaultGiphySdkKeyTest = "s0glxvzVg9azvPipKxcPLpXV0q1x1fVP" - ServiceSettingsDefaultDeveloperFlags = "" + ServiceSettingsDefaultSiteURL = "http://localhost:8065" + ServiceSettingsDefaultTLSCertFile = "" + ServiceSettingsDefaultTLSKeyFile = "" + ServiceSettingsDefaultReadTimeout = 300 + ServiceSettingsDefaultWriteTimeout = 300 + ServiceSettingsDefaultIdleTimeout = 60 + ServiceSettingsDefaultMaxLoginAttempts = 10 + ServiceSettingsDefaultAllowCorsFrom = "" + ServiceSettingsDefaultListenAndAddress = ":8065" + ServiceSettingsDefaultGiphySdkKeyTest = "s0glxvzVg9azvPipKxcPLpXV0q1x1fVP" + ServiceSettingsDefaultDeveloperFlags = "" + ServiceSettingsDefaultUniqueReactionsPerPost = 50 + ServiceSettingsMaxUniqueReactionsPerPost = 500 TeamSettingsDefaultSiteName = "Mattermost" TeamSettingsDefaultMaxUsersPerTeam = 50 @@ -394,6 +396,7 @@ type ServiceSettings struct { EnableCustomGroups *bool `access:"site_users_and_teams"` SelfHostedPurchase *bool `access:"write_restrictable,cloud_restrictable"` AllowSyncedDrafts *bool `access:"site_posts"` + UniqueEmojiReactionLimitPerPost *int `access:"site_posts"` RefreshPostStatsRunTime *string `access:"site_users_and_teams"` } @@ -886,6 +889,14 @@ func (s *ServiceSettings) SetDefaults(isUpdate bool) { s.SelfHostedPurchase = NewBool(true) } + if s.UniqueEmojiReactionLimitPerPost == nil { + s.UniqueEmojiReactionLimitPerPost = NewInt(ServiceSettingsDefaultUniqueReactionsPerPost) + } + + if *s.UniqueEmojiReactionLimitPerPost > ServiceSettingsMaxUniqueReactionsPerPost { + s.UniqueEmojiReactionLimitPerPost = NewInt(ServiceSettingsMaxUniqueReactionsPerPost) + } + if s.RefreshPostStatsRunTime == nil { s.RefreshPostStatsRunTime = NewString("00:00") } diff --git a/webapp/channels/src/actions/post_actions.test.ts b/webapp/channels/src/actions/post_actions.test.ts index 452ecb2720..d5e4fae73f 100644 --- a/webapp/channels/src/actions/post_actions.test.ts +++ b/webapp/channels/src/actions/post_actions.test.ts @@ -50,7 +50,13 @@ jest.mock('utils/user_agent', () => ({ isDesktopApp: jest.fn().mockReturnValue(false), })); -const mockMakeGetIsReactionAlreadyAddedToPost = jest.spyOn(PostUtils, 'makeGetIsReactionAlreadyAddedToPost'); +jest.mock('utils/post_utils', () => ({ + makeGetUniqueEmojiNameReactionsForPost: jest.fn(), + makeGetIsReactionAlreadyAddedToPost: jest.fn(), +})); + +const mockMakeGetIsReactionAlreadyAddedToPost = PostUtils.makeGetIsReactionAlreadyAddedToPost as unknown as jest.Mock<() => boolean>; +const mockMakeGetUniqueEmojiNameReactionsForPost = PostUtils.makeGetUniqueEmojiNameReactionsForPost as unknown as jest.Mock<() => string[]>; const POST_CREATED_TIME = Date.now(); @@ -515,14 +521,40 @@ describe('Actions.Posts', () => { }); }); - test('addReaction', async () => { - const testStore = mockStore(initialState); + describe('addReaction', () => { + mockMakeGetUniqueEmojiNameReactionsForPost.mockReturnValue(() => []); - await testStore.dispatch(Actions.addReaction('post_id_1', 'emoji_name_1')); - expect(testStore.getActions()).toEqual([ - {args: ['post_id_1', 'emoji_name_1'], type: 'MOCK_ADD_REACTION'}, - {args: ['emoji_name_1'], type: 'MOCK_ADD_RECENT_EMOJI'}, - ]); + test('should add reaction', async () => { + const testStore = mockStore(initialState); + + await testStore.dispatch(Actions.addReaction('post_id_1', 'emoji_name_1')); + expect(testStore.getActions()).toEqual([ + {args: ['post_id_1', 'emoji_name_1'], type: 'MOCK_ADD_REACTION'}, + {args: ['emoji_name_1'], type: 'MOCK_ADD_RECENT_EMOJI'}, + ]); + }); + test('should not add reaction if we are over the limit', async () => { + mockMakeGetUniqueEmojiNameReactionsForPost.mockReturnValue(() => ['another_emoji']); + const testStore = mockStore({ + ...initialState, + entities: { + ...initialState.entities, + general: { + ...initialState.entities.general, + config: { + ...initialState.entities.general.config, + UniqueEmojiReactionLimitPerPost: '1', + }, + }, + }, + }); + + await testStore.dispatch(Actions.addReaction('post_id_1', 'emoji_name_1')); + expect(testStore.getActions()).not.toEqual([ + {args: ['post_id_1', 'emoji_name_1'], type: 'MOCK_ADD_REACTION'}, + {args: ['emoji_name_1'], type: 'MOCK_ADD_RECENT_EMOJI'}, + ]); + }); }); test('flagPost', async () => { diff --git a/webapp/channels/src/actions/post_actions.ts b/webapp/channels/src/actions/post_actions.ts index bd00ab49f7..a6e9ab38f0 100644 --- a/webapp/channels/src/actions/post_actions.ts +++ b/webapp/channels/src/actions/post_actions.ts @@ -10,10 +10,11 @@ import {getMyChannelMember} from 'mattermost-redux/actions/channels'; import * as PostActions from 'mattermost-redux/actions/posts'; import * as ThreadActions from 'mattermost-redux/actions/threads'; import {getChannel, getMyChannelMember as getMyChannelMemberSelector} from 'mattermost-redux/selectors/entities/channels'; +import {getConfig} from 'mattermost-redux/selectors/entities/general'; import * as PostSelectors from 'mattermost-redux/selectors/entities/posts'; import {isCollapsedThreadsEnabled} from 'mattermost-redux/selectors/entities/preferences'; import {getCurrentTeamId} from 'mattermost-redux/selectors/entities/teams'; -import {getCurrentUserId} from 'mattermost-redux/selectors/entities/users'; +import {getCurrentUserId, isCurrentUserSystemAdmin} from 'mattermost-redux/selectors/entities/users'; import type {DispatchFunc, GetStateFunc} from 'mattermost-redux/types/actions'; import {canEditPost, comparePosts} from 'mattermost-redux/utils/post_utils'; @@ -21,20 +22,24 @@ import {addRecentEmoji, addRecentEmojis} from 'actions/emoji_actions'; import * as StorageActions from 'actions/storage'; import {loadNewDMIfNeeded, loadNewGMIfNeeded} from 'actions/user_actions'; import {removeDraft} from 'actions/views/drafts'; +import {closeModal, openModal} from 'actions/views/modals'; import * as RhsActions from 'actions/views/rhs'; import {manuallyMarkThreadAsUnread} from 'actions/views/threads'; import {isEmbedVisible, isInlineImageVisible} from 'selectors/posts'; import {getSelectedPostId, getSelectedPostCardId, getRhsState} from 'selectors/rhs'; import {getGlobalItem} from 'selectors/storage'; +import ReactionLimitReachedModal from 'components/reaction_limit_reached_modal'; + import { ActionTypes, Constants, + ModalIdentifiers, RHSStates, StoragePrefixes, } from 'utils/constants'; import {matchEmoticons} from 'utils/emoticons'; -import {makeGetIsReactionAlreadyAddedToPost} from 'utils/post_utils'; +import {makeGetIsReactionAlreadyAddedToPost, makeGetUniqueEmojiNameReactionsForPost} from 'utils/post_utils'; import * as UserAgent from 'utils/user_agent'; import type {GlobalState} from 'types/store'; @@ -172,7 +177,25 @@ export function toggleReaction(postId: string, emojiName: string) { } export function addReaction(postId: string, emojiName: string) { - return (dispatch: DispatchFunc) => { + const getUniqueEmojiNameReactionsForPost = makeGetUniqueEmojiNameReactionsForPost(); + return (dispatch: DispatchFunc, getState: GetStateFunc) => { + const state = getState() as GlobalState; + const config = getConfig(state); + const uniqueEmojiNames = getUniqueEmojiNameReactionsForPost(state, postId) ?? []; + + // If we're adding a new reaction but we're already at or over the limit, stop + if (uniqueEmojiNames.length >= Number(config.UniqueEmojiReactionLimitPerPost) && !uniqueEmojiNames.some((name) => name === emojiName)) { + dispatch(openModal({ + modalId: ModalIdentifiers.REACTION_LIMIT_REACHED, + dialogType: ReactionLimitReachedModal, + dialogProps: { + isAdmin: isCurrentUserSystemAdmin(state), + onExited: () => closeModal(ModalIdentifiers.REACTION_LIMIT_REACHED), + }, + })); + return {data: false}; + } + dispatch(PostActions.addReaction(postId, emojiName)); dispatch(addRecentEmoji(emojiName)); return {data: true}; diff --git a/webapp/channels/src/components/admin_console/admin_definition.tsx b/webapp/channels/src/components/admin_console/admin_definition.tsx index 6daa8e5293..c0849c5077 100644 --- a/webapp/channels/src/components/admin_console/admin_definition.tsx +++ b/webapp/channels/src/components/admin_console/admin_definition.tsx @@ -236,6 +236,7 @@ export const it = { export const validators = { isRequired: (text: string, textDefault: string) => (value: string) => new ValidationResult(Boolean(value), text, textDefault), minValue: (min: number, text: string, textDefault: string) => (value: number) => new ValidationResult((value >= min), text, textDefault), + maxValue: (max: number, text: string, textDefault: string) => (value: number) => new ValidationResult((value <= max), text, textDefault), }; const usesLegacyOauth = (config: DeepPartial, state: any, license?: ClientLicense, enterpriseReady?: boolean, consoleAccess?: ConsoleAccess, cloud?: CloudState) => { @@ -3167,6 +3168,36 @@ const AdminDefinition: AdminDefinitionType = { 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, }, + { + type: 'number', + key: 'ServiceSettings.UniqueEmojiReactionLimitPerPost', + label: t('admin.customization.uniqueEmojiReactionLimitPerPost'), + label_default: 'Unique Emoji Reaction Limit:', + placeholder: t('admin.customization.uniqueEmojiReactionLimitPerPostPlaceholder'), + placeholder_default: 'E.g.: 25', + help_text: t('admin.customization.uniqueEmojiReactionLimitPerPostDesc'), + help_text_default: 'The number of unique emoji reactions that can be added to a post. Increasing this limit could lead to poor client performance. Maximum is 500.', + help_text_markdown: false, + validate: (value) => { + const maxResult = validators.maxValue( + 500, + t('admin.customization.uniqueEmojiReactionLimitPerPost.maxValue'), + 'Cannot increase the limit to a value above 500.', + )(value); + if (!maxResult.isValid()) { + return maxResult; + } + const minResult = validators.minValue(0, + t('admin.customization.uniqueEmojiReactionLimitPerPost.minValue'), + 'Cannot decrease the limit below 0.', + )(value); + if (!minResult.isValid()) { + return minResult; + } + + return new ValidationResult(true, '', ''); + }, + }, ], }, }, diff --git a/webapp/channels/src/components/reaction_limit_reached_modal.tsx b/webapp/channels/src/components/reaction_limit_reached_modal.tsx new file mode 100644 index 0000000000..39f8d8802f --- /dev/null +++ b/webapp/channels/src/components/reaction_limit_reached_modal.tsx @@ -0,0 +1,63 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React from 'react'; +import {FormattedMessage} from 'react-intl'; +import {Link} from 'react-router-dom'; + +import {GenericModal} from '@mattermost/components'; + +import ExternalLink from 'components/external_link'; + +export default function ReactionLimitReachedModal(props: {isAdmin: boolean; onExited: () => void}) { + const body = props.isAdmin ? ( + ( + + {msg} + + ), + linkAdmin: (msg: React.ReactNode) => ( + + {msg} + + ), + }} + /> + ) : ( + + ); + + return ( + + } + compassDesign={true} + confirmButtonText={ + + } + onExited={props.onExited} + handleConfirm={props.onExited} + > + {body} + + ); +} diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index 4578cefbbb..068892ee94 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -672,6 +672,11 @@ "admin.customization.restrictLinkPreviewsDesc": "Link previews and image link previews will not be shown for the above list of comma-separated domains.", "admin.customization.restrictLinkPreviewsExample": "E.g.: \"internal.mycompany.com, images.example.com\"", "admin.customization.restrictLinkPreviewsTitle": "Disable website link previews from these domains:", + "admin.customization.uniqueEmojiReactionLimitPerPost": "Unique Emoji Reaction Limit:", + "admin.customization.uniqueEmojiReactionLimitPerPost.maxValue": "Cannot increase the limit to a value above 500.", + "admin.customization.uniqueEmojiReactionLimitPerPost.minValue": "Cannot decrease the limit below 0.", + "admin.customization.uniqueEmojiReactionLimitPerPostDesc": "The number of unique emoji reactions that can be added to a post. Increasing this limit could lead to poor client performance. Maximum is 500.", + "admin.customization.uniqueEmojiReactionLimitPerPostPlaceholder": "E.g.: 25", "admin.data_grid.empty": "No items found", "admin.data_grid.loading": "Loading", "admin.data_grid.paginatorCount": "{startCount, number} - {endCount, number} of {total, number}", @@ -4567,6 +4572,9 @@ "quick_switch_modal.help_no_team": "Type to find a channel. Use **UP/DOWN** to browse, **ENTER** to select, **ESC** to dismiss.", "quick_switch_modal.input": "quick switch input", "quick_switch_modal.switchChannels": "Find Channels", + "reaction_limit_reached_modal.body": "Oops! It looks like we've hit a ceiling on emoji reactions for this message. Please contact your system administrator for any adjustments to this limit.", + "reaction_limit_reached_modal.body.admin": "Oops! It looks like we've hit a ceiling on emoji reactions for this message. We've set a limit to keep things running smoothly on your server. As a system administrator, you can adjust this limit from the system console.", + "reaction_limit_reached_modal.title": "You've reached the reaction limit", "reaction_list.addReactionTooltip": "Add a reaction", "reaction.add.ariaLabel": "Add a reaction", "reaction.clickToAdd": "(click to add)", diff --git a/webapp/channels/src/utils/constants.tsx b/webapp/channels/src/utils/constants.tsx index bce023ac39..6c70a57cb7 100644 --- a/webapp/channels/src/utils/constants.tsx +++ b/webapp/channels/src/utils/constants.tsx @@ -449,6 +449,7 @@ export const ModalIdentifiers = { IP_FILTERING_ADD_EDIT_MODAL: 'ip_filtering_add_edit_modal', IP_FILTERING_DELETE_CONFIRMATION_MODAL: 'ip_filtering_delete_confirmation_modal', IP_FILTERING_SAVE_CONFIRMATION_MODAL: 'ip_filtering_save_confirmation_modal', + REACTION_LIMIT_REACHED: 'reaction_limit_reached', }; export const UserStatuses = { diff --git a/webapp/channels/src/utils/post_utils.test.tsx b/webapp/channels/src/utils/post_utils.test.tsx index b2e893e696..af4d02cf42 100644 --- a/webapp/channels/src/utils/post_utils.test.tsx +++ b/webapp/channels/src/utils/post_utils.test.tsx @@ -1353,3 +1353,43 @@ describe('makeGetIsReactionAlreadyAddedToPost', () => { expect(getIsReactionAlreadyAddedToPost(baseState, 'post_id_1', 'smile')).toBeTruthy(); }); }); + +describe('makeGetUniqueEmojiNameReactionsForPost', () => { + const baseState = { + entities: { + posts: { + reactions: { + post_id_1: { + user_1_post_id_1_smile: { + emoji_name: 'smile', + post_id: 'post_id_1', + }, + user_2_post_id_1_smile: { + emoji_name: 'smile', + post_id: 'post_id_1', + }, + user_3_post_id_1_smile: { + emoji_name: 'smile', + post_id: 'post_id_1', + }, + user_1_post_id_1_cry: { + emoji_name: 'cry', + post_id: 'post_id_1', + }, + }, + + }, + }, + general: { + config: {}, + }, + emojis: {}, + }, + } as unknown as GlobalState; + + test('should only return names of unique reactions', () => { + const getUniqueEmojiNameReactionsForPost = PostUtils.makeGetUniqueEmojiNameReactionsForPost(); + + expect(getUniqueEmojiNameReactionsForPost(baseState, 'post_id_1')).toEqual(['smile', 'cry']); + }); +}); diff --git a/webapp/channels/src/utils/post_utils.ts b/webapp/channels/src/utils/post_utils.ts index 9a60d3cbab..289275006c 100644 --- a/webapp/channels/src/utils/post_utils.ts +++ b/webapp/channels/src/utils/post_utils.ts @@ -710,6 +710,31 @@ export function makeGetUniqueReactionsToPost(): (state: GlobalState, postId: Pos ); } +export function makeGetUniqueEmojiNameReactionsForPost(): (state: GlobalState, postId: Post['id']) => string[] | undefined | null { + const getReactionsForPost = makeGetReactionsForPost(); + + return createSelector( + 'makeGetUniqueEmojiReactionsForPost', + (state: GlobalState, postId: string) => getReactionsForPost(state, postId), + getEmojiMap, + (reactions, emojiMap) => { + if (!reactions) { + return null; + } + + const emojiNames: string[] = []; + + Object.values(reactions).forEach((reaction) => { + if (emojiMap.get(reaction.emoji_name) && !emojiNames.includes(reaction.emoji_name)) { + emojiNames.push(reaction.emoji_name); + } + }); + + return emojiNames; + }, + ); +} + export function makeGetIsReactionAlreadyAddedToPost(): (state: GlobalState, postId: Post['id'], emojiName: string) => boolean { const getUniqueReactionsToPost = makeGetUniqueReactionsToPost(); diff --git a/webapp/platform/types/src/config.ts b/webapp/platform/types/src/config.ts index f0e66db4b0..83fb2923b0 100644 --- a/webapp/platform/types/src/config.ts +++ b/webapp/platform/types/src/config.ts @@ -200,6 +200,7 @@ export type ClientConfig = { AllowPersistentNotificationsForGuests: string; DelayChannelAutocomplete: 'true' | 'false'; ServiceEnvironment: string; + UniqueEmojiReactionLimitPerPost: string; }; export type License = { @@ -378,6 +379,7 @@ export type ServiceSettings = { PersistentNotificationIntervalMinutes: number; PersistentNotificationMaxCount: number; PersistentNotificationMaxRecipients: number; + UniqueEmojiReactionLimitPerPost: number; RefreshPostStatsRunTime: string; };