From 19e7a2be283415958757eca10d65825d9e1f099f Mon Sep 17 00:00:00 2001 From: "cursor[bot]" <206951365+cursor[bot]@users.noreply.github.com> Date: Tue, 2 Jun 2026 14:11:15 +0530 Subject: [PATCH 1/4] Fix flaky TestCheckUsersEmojiIntegrity (#36756) * Fix flaky TestCheckUsersEmojiIntegrity Integrity checks scan the full database for orphaned emoji rows, so parallel sqlstore tests and leftover rows from sibling tests can inflate global record counts and flip index-based assertions. Reset tables at test start and scope the one-record assertion to the emoji child ID created in that subtest. Tests-only change. Verified compilation locally; full test loop requires PostgreSQL (CI). Co-authored-by: mattermost-code * ci: nudge CodeRabbit after all checks green Co-authored-by: mattermost-code * Address PR feedback: 1 items resolved, 0 declined * Use t.Cleanup with require.NoError for emoji test fixture cleanup Replace silent defer dbmap.Exec cleanup calls with t.Cleanup handlers that assert on errors, addressing CodeRabbit review feedback. Co-authored-by: mattermost-code --------- Co-authored-by: Cursor Agent Co-authored-by: mattermost-code --- .../channels/store/sqlstore/integrity_test.go | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/server/channels/store/sqlstore/integrity_test.go b/server/channels/store/sqlstore/integrity_test.go index 9734f9fba23..7c1078aebfa 100644 --- a/server/channels/store/sqlstore/integrity_test.go +++ b/server/channels/store/sqlstore/integrity_test.go @@ -111,7 +111,7 @@ func createCompliance(ss store.Store, userId string) *model.Compliance { func createEmoji(ss store.Store, userId string) *model.Emoji { m := model.Emoji{} m.CreatorId = userId - m.Name = "emoji" + m.Name = "emoji" + model.NewId() emoji, _ := ss.Emoji().Save(&m) return emoji } @@ -1214,26 +1214,47 @@ func TestCheckUsersEmojiIntegrity(t *testing.T) { dbmap := store.GetMaster() t.Run("should generate a report with no records", func(t *testing.T) { + user := createUser(rctx, ss) + emoji := createEmoji(ss, user.Id) + t.Cleanup(func() { + _, err := dbmap.Exec(`DELETE FROM Emoji WHERE Id=?`, emoji.Id) + require.NoError(t, err) + }) + t.Cleanup(func() { + _, err := dbmap.Exec(`DELETE FROM Users WHERE Id=?`, user.Id) + require.NoError(t, err) + }) + result := checkUsersEmojiIntegrity(store) require.NoError(t, result.Err) data := result.Data.(model.RelationalIntegrityCheckData) - require.Empty(t, data.Records) + records := orphanedRecordsWithChildIDs(data.Records, emoji.Id) + require.Empty(t, records) }) t.Run("should generate a report with one record", func(t *testing.T) { user := createUser(rctx, ss) userId := user.Id emoji := createEmoji(ss, userId) + t.Cleanup(func() { + _, err := dbmap.Exec(`DELETE FROM Emoji WHERE Id=?`, emoji.Id) + require.NoError(t, err) + }) + t.Cleanup(func() { + _, err := dbmap.Exec(`DELETE FROM Users WHERE Id=?`, user.Id) + require.NoError(t, err) + }) + dbmap.Exec(`DELETE FROM Users WHERE Id=?`, user.Id) result := checkUsersEmojiIntegrity(store) require.NoError(t, result.Err) data := result.Data.(model.RelationalIntegrityCheckData) - require.Len(t, data.Records, 1) + records := orphanedRecordsWithChildIDs(data.Records, emoji.Id) + require.Len(t, records, 1) require.Equal(t, model.OrphanedRecord{ ParentId: &userId, ChildId: &emoji.Id, - }, data.Records[0]) - dbmap.Exec(`DELETE FROM Emoji WHERE Id=?`, emoji.Id) + }, records[0]) }) }) } From 6c401066f73462d445d4f6e3571646320debb8a5 Mon Sep 17 00:00:00 2001 From: Harshil Sharma <18575143+harshilsharma63@users.noreply.github.com> Date: Tue, 2 Jun 2026 14:18:25 +0530 Subject: [PATCH 2/4] Deleted removed post from content flagging redux store (#36803) * DEleted removed post from content flagging redux store * Create a separate action --- .../lib/src/ui/pages/content_review_dm.ts | 12 +++ .../reviewer-actions/reviewer-actions.spec.ts | 9 ++ ...lagged_message_confirmation_modal.test.tsx | 97 +++++++++++++++++++ ...ove_flagged_message_confirmation_modal.tsx | 8 +- .../src/action_types/content_flagging.ts | 1 + .../src/actions/content_flagging.ts | 13 ++- .../entities/content_flagging.test.ts | 88 +++++++++++++++++ .../src/reducers/entities/content_flagging.ts | 18 ++++ 8 files changed, 244 insertions(+), 2 deletions(-) diff --git a/e2e-tests/playwright/lib/src/ui/pages/content_review_dm.ts b/e2e-tests/playwright/lib/src/ui/pages/content_review_dm.ts index 4f090f1760e..3a70a26c323 100644 --- a/e2e-tests/playwright/lib/src/ui/pages/content_review_dm.ts +++ b/e2e-tests/playwright/lib/src/ui/pages/content_review_dm.ts @@ -166,6 +166,18 @@ export default class ContentReviewPage { await expect(this.reportCard!.locator('.row:has-text("Message") .post-message__text')).toHaveText(expected); } + async verifyFlaggedPostMessageInRHS(expected: string) { + await expect(this.rhsCard.locator('.row:has-text("Message") .post-message__text')).toHaveText(expected); + } + + async verifyFlaggedPostMessageInCenter(postID: string, expected: string) { + const centerCard = this.page + .getByTestId('channel_view') + .locator('div.DataSpillageReport') + .filter({has: this.page.locator(`#postMessageText_${postID}`)}); + await expect(centerCard.locator('.row:has-text("Message") .post-message__text')).toHaveText(expected); + } + async clickKeepMessage() { await this.keepMessageButton.scrollIntoViewIfNeeded(); await this.keepMessageButton.click(); diff --git a/e2e-tests/playwright/specs/functional/channels/content_flagging/reviewer-actions/reviewer-actions.spec.ts b/e2e-tests/playwright/specs/functional/channels/content_flagging/reviewer-actions/reviewer-actions.spec.ts index 232412e4ebd..1764d62d9cb 100644 --- a/e2e-tests/playwright/specs/functional/channels/content_flagging/reviewer-actions/reviewer-actions.spec.ts +++ b/e2e-tests/playwright/specs/functional/channels/content_flagging/reviewer-actions/reviewer-actions.spec.ts @@ -78,6 +78,15 @@ test('Verify Removed Flagged posts show appropriate status and do not show the p await secondContentReviewPage.confirmRemovePermanently(); await setupContentFlagging(adminClient, [adminUser.id, secondUserID, thirdUserID]); + // After the remove action succeeds, the reviewer's own view of the flagged + // post should be replaced with the moderation placeholder rather than the + // original message that is now cleared from the redux store (MM-69043). + // The placeholder appears both in the RHS detail view and in the report + // card shown in the @content-review center channel — assert each scope + // separately so a regression in either location is caught. + await secondContentReviewPage.verifyFlaggedPostMessageInRHS(contentModerationMessage); + await secondContentReviewPage.verifyFlaggedPostMessageInCenter(post.id, contentModerationMessage); + const {channelsPage: channelsPageThird, contentReviewPage: contentReviewPageThird} = await pw.testBrowser.login(thirdUser); await verifyAuthorNotification( diff --git a/webapp/channels/src/components/remove_flagged_message_confirmation_modal/remove_flagged_message_confirmation_modal.test.tsx b/webapp/channels/src/components/remove_flagged_message_confirmation_modal/remove_flagged_message_confirmation_modal.test.tsx index 485c3a2407a..4f59b5a481e 100644 --- a/webapp/channels/src/components/remove_flagged_message_confirmation_modal/remove_flagged_message_confirmation_modal.test.tsx +++ b/webapp/channels/src/components/remove_flagged_message_confirmation_modal/remove_flagged_message_confirmation_modal.test.tsx @@ -4,7 +4,9 @@ import {screen, waitFor} from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; +import type {MockStoreEnhanced} from 'redux-mock-store'; +import {ContentFlaggingTypes} from 'mattermost-redux/action_types'; import {Client4} from 'mattermost-redux/client'; import {renderWithContext} from 'tests/react_testing_utils'; @@ -517,6 +519,101 @@ describe('KeepRemoveFlaggedMessageConfirmationModal', () => { }); }); + describe('store cleanup after remove', () => { + test('dispatches FLAGGED_POST_REMOVED after a successful removeFlaggedPost call', async () => { + const {store} = renderWithContext( + , + {}, + {useMockedStore: true}, + ); + + await userEvent.click(screen.getByRole('button', {name: 'Continue'})); + + await waitFor(() => { + expect(screen.getByTestId('generated-section')).toBeVisible(); + }); + + await userEvent.click(screen.getByRole('button', {name: 'Remove permanently'})); + + await waitFor(() => { + expect(Client4.removeFlaggedPost).toHaveBeenCalledWith(flaggedPost.id, ''); + }); + + await waitFor(() => { + expect((store as unknown as MockStoreEnhanced).getActions()).toContainEqual({ + type: ContentFlaggingTypes.FLAGGED_POST_REMOVED, + data: {postId: flaggedPost.id}, + }); + }); + }); + + test('does not dispatch FLAGGED_POST_REMOVED when removeFlaggedPost fails', async () => { + Client4.removeFlaggedPost = jest.fn().mockRejectedValue({message: 'boom'}); + + const {store} = renderWithContext( + , + {}, + {useMockedStore: true}, + ); + + await userEvent.click(screen.getByTestId('download-report-checkbox')); + await userEvent.click(screen.getByRole('button', {name: 'Remove message'})); + + await waitFor(() => { + expect(screen.getByTestId('skip-confirm-body')).toBeVisible(); + }); + + await userEvent.click(screen.getByRole('button', {name: 'Remove without report'})); + + await waitFor(() => { + expect(Client4.removeFlaggedPost).toHaveBeenCalledWith(flaggedPost.id, ''); + }); + + expect((store as unknown as MockStoreEnhanced).getActions()).not.toContainEqual( + expect.objectContaining({type: ContentFlaggingTypes.FLAGGED_POST_REMOVED}), + ); + }); + + test('does not dispatch FLAGGED_POST_REMOVED for keep action', async () => { + const {store} = renderWithContext( + , + {}, + {useMockedStore: true}, + ); + + await userEvent.click(screen.getByRole('button', {name: 'Continue'})); + + await waitFor(() => { + expect(screen.getByTestId('generated-section')).toBeVisible(); + }); + + await userEvent.click(screen.getByRole('button', {name: 'Keep permanently'})); + + await waitFor(() => { + expect(Client4.keepFlaggedPost).toHaveBeenCalledWith(flaggedPost.id, ''); + }); + + expect((store as unknown as MockStoreEnhanced).getActions()).not.toContainEqual( + expect.objectContaining({type: ContentFlaggingTypes.FLAGGED_POST_REMOVED}), + ); + }); + }); + describe('error handling', () => { test('should show request error when API call fails', async () => { const errorMessage = 'Failed to remove flagged post'; diff --git a/webapp/channels/src/components/remove_flagged_message_confirmation_modal/remove_flagged_message_confirmation_modal.tsx b/webapp/channels/src/components/remove_flagged_message_confirmation_modal/remove_flagged_message_confirmation_modal.tsx index e746f2fb1aa..895d2ddce0b 100644 --- a/webapp/channels/src/components/remove_flagged_message_confirmation_modal/remove_flagged_message_confirmation_modal.tsx +++ b/webapp/channels/src/components/remove_flagged_message_confirmation_modal/remove_flagged_message_confirmation_modal.tsx @@ -3,12 +3,14 @@ import React, {useCallback, useEffect, useRef, useState} from 'react'; import {useIntl} from 'react-intl'; +import {useDispatch} from 'react-redux'; import {GenericModal} from '@mattermost/components'; import type {ServerError} from '@mattermost/types/errors'; import type {Post} from '@mattermost/types/posts'; import type {UserProfile} from '@mattermost/types/users'; +import {removeContentFlaggingPost} from 'mattermost-redux/actions/content_flagging'; import {Client4} from 'mattermost-redux/client'; import {useChannel} from 'components/common/hooks/useChannel'; @@ -39,6 +41,7 @@ type Step = 'form' | 'skip_confirm' | 'generating' | 'generated' | 'error'; export default function KeepRemoveFlaggedMessageConfirmationModal({action, onExited, flaggedPost, reportingUser}: Props) { const {formatMessage} = useIntl(); + const dispatch = useDispatch(); const flaggedPostChannel = useChannel(flaggedPost.channel_id); const contentFlaggingConfig = useContentFlaggingConfig(flaggedPostChannel?.team_id || ''); @@ -91,6 +94,9 @@ export default function KeepRemoveFlaggedMessageConfirmationModal({action, onExi setSubmitting(true); setRequestError(''); await actionFunc(flaggedPost.id, comment); + if (action === 'remove') { + dispatch(removeContentFlaggingPost(flaggedPost.id)); + } handleClose(); } catch (error) { // eslint-disable-next-line no-console @@ -99,7 +105,7 @@ export default function KeepRemoveFlaggedMessageConfirmationModal({action, onExi } finally { setSubmitting(false); } - }, [action, comment, flaggedPost.id, handleClose]); + }, [action, comment, dispatch, flaggedPost.id, handleClose]); const handleFormPrimary = useCallback(() => { if (validateForm()) { diff --git a/webapp/channels/src/packages/mattermost-redux/src/action_types/content_flagging.ts b/webapp/channels/src/packages/mattermost-redux/src/action_types/content_flagging.ts index 405fa9ccfb3..d2139d32cc9 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/action_types/content_flagging.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/action_types/content_flagging.ts @@ -11,4 +11,5 @@ export default keyMirror({ RECEIVED_FLAGGED_POST: null, RECEIVED_CONTENT_FLAGGING_CHANNEL: null, RECEIVED_CONTENT_FLAGGING_TEAM: null, + FLAGGED_POST_REMOVED: null, }); diff --git a/webapp/channels/src/packages/mattermost-redux/src/actions/content_flagging.ts b/webapp/channels/src/packages/mattermost-redux/src/actions/content_flagging.ts index 04ba86d3407..4e4728ef1a8 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/actions/content_flagging.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/actions/content_flagging.ts @@ -11,7 +11,7 @@ import {TeamTypes, ContentFlaggingTypes} from 'mattermost-redux/action_types'; import {logError} from 'mattermost-redux/actions/errors'; import {forceLogoutIfNecessary} from 'mattermost-redux/actions/helpers'; import {Client4} from 'mattermost-redux/client'; -import type {ActionFuncAsync} from 'mattermost-redux/types/actions'; +import type {ActionFuncAsync, ActionFunc} from 'mattermost-redux/types/actions'; import {DelayedDataLoader} from 'mattermost-redux/utils/data_loader'; export type ContentFlaggingChannelRequestIdentifier = { @@ -264,3 +264,14 @@ export function getPostContentFlaggingValues(postId: string): ActionFuncAsync { + return (dispatch) => { + dispatch({ + type: ContentFlaggingTypes.FLAGGED_POST_REMOVED, + data: {postId}, + }); + + return {}; + }; +} diff --git a/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/content_flagging.test.ts b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/content_flagging.test.ts index af535ad4a30..bef85decc10 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/content_flagging.test.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/content_flagging.test.ts @@ -183,4 +183,92 @@ describe('Reducers.ContentFlagging', () => { expect(state.postValues['post-1']).toEqual([updatedValue]); }); + + describe('FLAGGED_POST_REMOVED', () => { + test('clears the flagged post and its property values for the given post id', () => { + const value = makeValue({field_id: 'review_status', value: 'pending'}); + const otherValue = makeValue({id: 'value-2', target_id: 'post-2', field_id: 'review_status', value: 'pending'}); + + const flaggedPost = {id: 'post-1'} as any; + const otherFlaggedPost = {id: 'post-2'} as any; + + const stateWithValues = contentFlaggingReducer(undefined, { + type: ContentFlaggingTypes.RECEIVED_POST_CONTENT_FLAGGING_VALUES, + data: { + postId: 'post-1', + values: [value], + }, + }); + const stateWithOtherValues = contentFlaggingReducer(stateWithValues, { + type: ContentFlaggingTypes.RECEIVED_POST_CONTENT_FLAGGING_VALUES, + data: { + postId: 'post-2', + values: [otherValue], + }, + }); + const stateWithFlaggedPost = contentFlaggingReducer(stateWithOtherValues, { + type: ContentFlaggingTypes.RECEIVED_FLAGGED_POST, + data: flaggedPost, + }); + const stateWithBothFlaggedPosts = contentFlaggingReducer(stateWithFlaggedPost, { + type: ContentFlaggingTypes.RECEIVED_FLAGGED_POST, + data: otherFlaggedPost, + }); + + const state = contentFlaggingReducer(stateWithBothFlaggedPosts, { + type: ContentFlaggingTypes.FLAGGED_POST_REMOVED, + data: {postId: 'post-1'}, + }); + + expect(state.postValues['post-1']).toBeUndefined(); + expect(state.flaggedPosts['post-1']).toBeUndefined(); + expect(state.postValues['post-2']).toEqual([otherValue]); + expect(state.flaggedPosts['post-2']).toEqual(otherFlaggedPost); + }); + + test('returns the same state reference when the post id is not present', () => { + const value = makeValue({field_id: 'review_status', value: 'pending'}); + + const stateWithValues = contentFlaggingReducer(undefined, { + type: ContentFlaggingTypes.RECEIVED_POST_CONTENT_FLAGGING_VALUES, + data: { + postId: 'post-1', + values: [value], + }, + }); + + const state = contentFlaggingReducer(stateWithValues, { + type: ContentFlaggingTypes.FLAGGED_POST_REMOVED, + data: {postId: 'unknown-post'}, + }); + + expect(state.postValues).toBe(stateWithValues.postValues); + expect(state.flaggedPosts).toBe(stateWithValues.flaggedPosts); + }); + + test('is a no-op when postId is missing from the action data', () => { + const value = makeValue({field_id: 'review_status', value: 'pending'}); + const flaggedPost = {id: 'post-1'} as any; + + const stateWithValues = contentFlaggingReducer(undefined, { + type: ContentFlaggingTypes.RECEIVED_POST_CONTENT_FLAGGING_VALUES, + data: { + postId: 'post-1', + values: [value], + }, + }); + const stateWithFlaggedPost = contentFlaggingReducer(stateWithValues, { + type: ContentFlaggingTypes.RECEIVED_FLAGGED_POST, + data: flaggedPost, + }); + + const state = contentFlaggingReducer(stateWithFlaggedPost, { + type: ContentFlaggingTypes.FLAGGED_POST_REMOVED, + data: {}, + }); + + expect(state.postValues['post-1']).toEqual([value]); + expect(state.flaggedPosts['post-1']).toEqual(flaggedPost); + }); + }); }); diff --git a/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/content_flagging.ts b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/content_flagging.ts index 531dd6e2096..d747967d1e1 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/content_flagging.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/content_flagging.ts @@ -83,6 +83,15 @@ function postValues(state: ContentFlaggingState['postValues'] = {}, action: MMRe [postId]: Object.values(valuesByFieldId), }; } + case ContentFlaggingTypes.FLAGGED_POST_REMOVED: { + const postId = action.data?.postId as string | undefined; + if (!postId || !(postId in state)) { + return state; + } + const nextState = {...state}; + Reflect.deleteProperty(nextState, postId); + return nextState; + } case UserTypes.LOGOUT_SUCCESS: return {}; default: @@ -98,6 +107,15 @@ function flaggedPosts(state: ContentFlaggingState['flaggedPosts'] = {}, action: [action.data.id]: action.data, }; } + case ContentFlaggingTypes.FLAGGED_POST_REMOVED: { + const postId = action.data?.postId as string | undefined; + if (!postId || !(postId in state)) { + return state; + } + const nextState = {...state}; + Reflect.deleteProperty(nextState, postId); + return nextState; + } case UserTypes.LOGOUT_SUCCESS: return {}; default: From 1b3dc637845e65adf2ae0ea45212526d4914e58c Mon Sep 17 00:00:00 2001 From: Ibrahim Serdar Acikgoz Date: Tue, 2 Jun 2026 12:16:35 +0200 Subject: [PATCH 3/4] [MM-69078] Surface plugin upload rejections as a toast (parity with download rejections) (#36838) * Surface plugin upload rejections as a toast (parity with download rejections) * add some tests --- server/channels/app/file.go | 19 ++++ server/channels/app/file_test.go | 50 +++++++++++ server/channels/app/upload.go | 1 + server/public/model/websocket_message.go | 1 + .../channels/src/actions/file_actions.test.ts | 89 +++++++++++++++++++ webapp/channels/src/actions/file_actions.ts | 9 +- .../src/actions/websocket_actions.test.jsx | 51 ++++++++++- .../channels/src/actions/websocket_actions.ts | 30 +++++++ webapp/channels/src/i18n/en.json | 1 + .../platform/client/src/websocket_events.ts | 1 + .../platform/client/src/websocket_message.ts | 1 + .../platform/client/src/websocket_messages.ts | 6 ++ 12 files changed, 257 insertions(+), 2 deletions(-) create mode 100644 webapp/channels/src/actions/file_actions.test.ts diff --git a/server/channels/app/file.go b/server/channels/app/file.go index 7608c627bec..d0fd0338491 100644 --- a/server/channels/app/file.go +++ b/server/channels/app/file.go @@ -2018,6 +2018,25 @@ func (a *App) sendFileDownloadRejectedEvent(info *model.FileInfo, userID string, a.Publish(message) } +// sendFileUploadRejectedEvent sends a websocket event to notify the user that their file upload was +// rejected by a plugin. It mirrors sendFileDownloadRejectedEvent so the webapp can surface the +// rejection as a toast instead of an inline composer error. When connectionID is provided, the event +// is only sent to that specific connection. +func (a *App) sendFileUploadRejectedEvent(info *model.FileInfo, userID string, connectionID string, rejectionReason string) { + if userID == "" { + return + } + + message := model.NewWebSocketEvent(model.WebsocketEventFileUploadRejected, "", info.ChannelId, userID, nil, "") + if connectionID != "" { + message.GetBroadcast().ConnectionId = connectionID + } + message.Add("file_name", info.Name) + message.Add("rejection_reason", rejectionReason) + message.Add("channel_id", info.ChannelId) + a.Publish(message) +} + // RunFileWillBeDownloadedHook executes the FileWillBeDownloaded hook with a timeout. // Returns empty string to allow download, or a rejection reason to block it. func (a *App) RunFileWillBeDownloadedHook(rctx request.CTX, fileInfo *model.FileInfo, userID string, connectionID string, downloadType model.FileDownloadType) string { diff --git a/server/channels/app/file_test.go b/server/channels/app/file_test.go index d30bda3e40c..d035f17b869 100644 --- a/server/channels/app/file_test.go +++ b/server/channels/app/file_test.go @@ -1207,3 +1207,53 @@ func TestFilterFilesByChannelPermissions_ABAC(t *testing.T) { mockACS.AssertNotCalled(t, "AccessEvaluation") }) } + +func TestSendFileUploadRejectedEvent(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + info := &model.FileInfo{ + Id: model.NewId(), + Name: "secret.tdf", + ChannelId: th.BasicChannel.Id, + CreatorId: th.BasicUser.Id, + } + + t.Run("publishes the event with populated fields", func(t *testing.T) { + wsMessages, closeWS := connectFakeWebSocket(t, th, th.BasicUser.Id, "", []model.WebsocketEventType{model.WebsocketEventFileUploadRejected}) + defer closeWS() + + th.App.sendFileUploadRejectedEvent(info, th.BasicUser.Id, "", "blocked by policy") + + var received *model.WebSocketEvent + require.Eventually(t, func() bool { + select { + case received = <-wsMessages: + return true + default: + return false + } + }, 5*time.Second, 100*time.Millisecond, "did not receive file_upload_rejected event in time") + + data := received.GetData() + assert.Equal(t, info.Name, data["file_name"]) + assert.Equal(t, "blocked by policy", data["rejection_reason"]) + assert.Equal(t, info.ChannelId, data["channel_id"]) + }) + + t.Run("does not publish when userID is empty", func(t *testing.T) { + wsMessages, closeWS := connectFakeWebSocket(t, th, th.BasicUser.Id, "", []model.WebsocketEventType{model.WebsocketEventFileUploadRejected}) + defer closeWS() + + th.App.sendFileUploadRejectedEvent(info, "", "", "blocked by policy") + + require.Never(t, func() bool { + select { + case <-wsMessages: + return true + default: + return false + } + }, 1*time.Second, 100*time.Millisecond, "should not publish file_upload_rejected event when userID is empty") + }) +} diff --git a/server/channels/app/upload.go b/server/channels/app/upload.go index 6f695a0424d..232ecf044c7 100644 --- a/server/channels/app/upload.go +++ b/server/channels/app/upload.go @@ -75,6 +75,7 @@ func (a *App) runPluginsHook(rctx request.CTX, info *model.FileInfo, file io.Rea if rejStr != "" { rejErr = model.NewAppError("runPluginsHook", "app.upload.run_plugins_hook.rejected", map[string]any{"Filename": info.Name, "Reason": rejStr}, "", http.StatusBadRequest) + a.sendFileUploadRejectedEvent(info, info.CreatorId, rctx.ConnectionId(), rejStr) return false } if newInfo != nil { diff --git a/server/public/model/websocket_message.go b/server/public/model/websocket_message.go index 87f9ead3544..6f5ddd4c128 100644 --- a/server/public/model/websocket_message.go +++ b/server/public/model/websocket_message.go @@ -115,6 +115,7 @@ const ( WebsocketEventPropertyFieldDeleted WebsocketEventType = "property_field_deleted" WebsocketEventPropertyValuesUpdated WebsocketEventType = "property_values_updated" WebsocketEventFileDownloadRejected WebsocketEventType = "file_download_rejected" + WebsocketEventFileUploadRejected WebsocketEventType = "file_upload_rejected" WebsocketEventShowToast WebsocketEventType = "show_toast" WebsocketEventSharedChannelRemoteUpdated WebsocketEventType = "shared_channel_remote_updated" WebsocketEventChannelJoinRequestCreated WebsocketEventType = "channel_join_request_created" diff --git a/webapp/channels/src/actions/file_actions.test.ts b/webapp/channels/src/actions/file_actions.test.ts new file mode 100644 index 00000000000..c6d360adb8a --- /dev/null +++ b/webapp/channels/src/actions/file_actions.test.ts @@ -0,0 +1,89 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {uploadFile} from 'actions/file_actions'; + +import testConfigureStore from 'tests/test_store'; + +jest.mock('selectors/general', () => ({ + ...jest.requireActual('selectors/general'), + getConnectionId: jest.fn(() => ''), +})); + +jest.mock('utils/utils', () => ({ + ...jest.requireActual('utils/utils'), + localizeMessage: jest.fn((descriptor: {id: string; defaultMessage: string}) => descriptor.defaultMessage || descriptor.id), +})); + +class MockXMLHttpRequest { + status = 0; + readyState = 0; + response = ''; + upload: {onprogress?: (event: ProgressEvent) => void} = {}; + onload: (() => void) | null = null; + onerror: (() => void) | null = null; + open = jest.fn(); + setRequestHeader = jest.fn(); + send = jest.fn(); +} + +describe('actions/file_actions', () => { + describe('uploadFile', () => { + const originalXHR = window.XMLHttpRequest; + let mockXhr: MockXMLHttpRequest; + + beforeEach(() => { + mockXhr = new MockXMLHttpRequest(); + window.XMLHttpRequest = jest.fn(() => mockXhr) as unknown as typeof XMLHttpRequest; + }); + + afterEach(() => { + window.XMLHttpRequest = originalXHR; + }); + + function startUpload(onError: jest.Mock) { + const store = testConfigureStore(); + store.dispatch(uploadFile({ + file: new File(['data'], 'secret.tdf'), + name: 'secret.tdf', + type: 'application/octet-stream', + rootId: 'root1', + channelId: 'channel1', + clientId: 'client1', + onProgress: jest.fn(), + onSuccess: jest.fn(), + onError, + })); + } + + test('suppresses the inline error on plugin rejection by passing an empty message', () => { + const onError = jest.fn(); + startUpload(onError); + + mockXhr.status = 400; + mockXhr.readyState = 4; + mockXhr.response = JSON.stringify({ + id: 'app.upload.run_plugins_hook.rejected', + message: 'Unable to upload the file. File rejected by plugin. blocked by policy', + }); + mockXhr.onload!(); + + expect(onError).toHaveBeenCalledWith('', 'client1', 'channel1', 'root1'); + }); + + test('passes the original error message for non-plugin upload failures', () => { + const onError = jest.fn(); + startUpload(onError); + + mockXhr.status = 400; + mockXhr.readyState = 4; + mockXhr.response = JSON.stringify({ + id: 'api.file.upload_file.too_large.app_error', + message: 'File is too large', + }); + mockXhr.onload!(); + + expect(onError).toHaveBeenCalledWith('File is too large', 'client1', 'channel1', 'root1'); + }); + }); +}); diff --git a/webapp/channels/src/actions/file_actions.ts b/webapp/channels/src/actions/file_actions.ts index 85f1b8aa1a8..928ce9ac4b9 100644 --- a/webapp/channels/src/actions/file_actions.ts +++ b/webapp/channels/src/actions/file_actions.ts @@ -103,8 +103,13 @@ export function uploadFile({file, name, type, rootId, channelId, clientId, onPro onSuccess(response, channelId, rootId); } else if (xhr.status >= 400 && xhr.readyState === 4) { let errorMessage = ''; + let pluginRejected = false; try { const errorResponse = JSON.parse(xhr.response); + + // Plugin upload rejections are surfaced as a toast via the + // file_upload_rejected websocket event, so suppress the inline composer error. + pluginRejected = errorResponse?.id === 'app.upload.run_plugins_hook.rejected'; errorMessage = (errorResponse?.id && errorResponse?.message) ? localizeMessage({id: errorResponse.id, defaultMessage: errorResponse.message}) : localizeMessage({id: 'file_upload.generic_error', defaultMessage: 'There was a problem uploading your files.'}); } catch (e) { @@ -118,7 +123,9 @@ export function uploadFile({file, name, type, rootId, channelId, clientId, onPro rootId, }); - onError?.(errorMessage, clientId, channelId, rootId); + // Pass an empty message on plugin rejection so the in-progress upload is cleared + // without showing the inline error; the websocket-driven toast explains the rejection. + onError?.(pluginRejected ? '' : errorMessage, clientId, channelId, rootId); } }; } diff --git a/webapp/channels/src/actions/websocket_actions.test.jsx b/webapp/channels/src/actions/websocket_actions.test.jsx index 2a981e429f8..bca502e1e2a 100644 --- a/webapp/channels/src/actions/websocket_actions.test.jsx +++ b/webapp/channels/src/actions/websocket_actions.test.jsx @@ -30,14 +30,17 @@ import store from 'stores/redux_store'; import {invalidateAccessControlAttributesCache} from 'components/common/hooks/useAccessControlAttributes'; import mergeObjects from 'packages/mattermost-redux/test/merge_objects'; +import {defaultIntl} from 'tests/helpers/intl-test-helper'; import configureStore from 'tests/test_store'; import {getHistory} from 'utils/browser_history'; -import Constants, {ActionTypes, UserStatuses} from 'utils/constants'; +import Constants, {ActionTypes, ModalIdentifiers, UserStatuses} from 'utils/constants'; +import {setIntl} from 'utils/i18n'; import { handleChannelUpdatedEvent, handleChannelAccessControlUpdatedEvent, handleEvent, + handleFileUploadRejected, handleNewPostEvent, handleNewPostEvents, handlePluginEnabled, @@ -2002,3 +2005,49 @@ describe('handleSharedChannelRemoteUpdatedEvent', () => { expect(fetchChannelRemotes).not.toHaveBeenCalled(); }); }); + +describe('handleFileUploadRejected', () => { + beforeAll(() => { + setIntl(defaultIntl); + }); + + afterAll(() => { + setIntl(null); + }); + + const msg = { + event: WebSocketEvents.FileUploadRejected, + data: { + file_name: 'secret.tdf', + rejection_reason: 'blocked by policy', + channel_id: 'channel1', + }, + broadcast: {}, + }; + + test('opens an info toast with the rejection reason', () => { + const testStore = configureStore(); + + testStore.dispatch(handleFileUploadRejected(msg)); + + const openModalAction = testStore.getActions().find((action) => action.type === ActionTypes.MODAL_OPEN); + expect(openModalAction).toBeDefined(); + expect(openModalAction.modalId).toBe(ModalIdentifiers.INFO_TOAST); + expect(openModalAction.dialogProps.position).toBe('bottom-center'); + expect(openModalAction.dialogProps.content.message).toContain('blocked by policy'); + expect(typeof openModalAction.dialogProps.onExited).toBe('function'); + }); + + test('onExited closes the info toast', () => { + const testStore = configureStore(); + + testStore.dispatch(handleFileUploadRejected(msg)); + + const openModalAction = testStore.getActions().find((action) => action.type === ActionTypes.MODAL_OPEN); + openModalAction.dialogProps.onExited(); + + const closeModalAction = testStore.getActions().find((action) => action.type === ActionTypes.MODAL_CLOSE); + expect(closeModalAction).toBeDefined(); + expect(closeModalAction.modalId).toBe(ModalIdentifiers.INFO_TOAST); + }); +}); diff --git a/webapp/channels/src/actions/websocket_actions.ts b/webapp/channels/src/actions/websocket_actions.ts index d201a1bfa03..4321ce59f08 100644 --- a/webapp/channels/src/actions/websocket_actions.ts +++ b/webapp/channels/src/actions/websocket_actions.ts @@ -763,6 +763,9 @@ export function handleEvent(msg: WebSocketMessage) { case WebSocketEvents.FileDownloadRejected: dispatch(handleFileDownloadRejected(msg)); break; + case WebSocketEvents.FileUploadRejected: + dispatch(handleFileUploadRejected(msg)); + break; case WebSocketEvents.ShowToast: dispatch(handleShowToast(msg)); break; @@ -2367,6 +2370,33 @@ export function handleFileDownloadRejected(msg: WebSocketMessages.FileDownloadRe }; } +export function handleFileUploadRejected(msg: WebSocketMessages.FileUploadRejected): ThunkActionFunc { + return (dispatch) => { + const {rejection_reason: rejectionReason} = msg.data; + + const intl = getIntl(); + const displayMessage = intl.formatMessage( + {id: 'file_upload.rejected.file', defaultMessage: 'File upload blocked: {reason}'}, + {reason: rejectionReason}, + ); + + dispatch(openModal({ + modalId: ModalIdentifiers.INFO_TOAST, + dialogType: InfoToast, + dialogProps: { + content: { + icon: React.createElement(AlertCircleOutlineIcon, {size: 18}), + message: displayMessage, + }, + position: 'bottom-center', + onExited: () => { + dispatch(closeModal(ModalIdentifiers.INFO_TOAST)); + }, + }, + })); + }; +} + function handleShowToast(msg: WebSocketMessages.ShowToast): ThunkActionFunc { return (doDispatch) => { const {message, position} = msg.data; diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index 934167970e7..2aaaeb52645 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -4978,6 +4978,7 @@ "file_upload.limited": "Uploads limited to {count, number} files maximum. Please use additional posts for more files.", "file_upload.menuAriaLabel": "Upload type selector", "file_upload.pasted": "Image Pasted at ", + "file_upload.rejected.file": "File upload blocked: {reason}", "file_upload.upload_files": "Upload files", "file_upload.zeroBytesFile": "You are uploading an empty file: {filename}", "file_upload.zeroBytesFiles": "You are uploading empty files: {filenames}", diff --git a/webapp/platform/client/src/websocket_events.ts b/webapp/platform/client/src/websocket_events.ts index 56bf0179e8a..49d5890fc95 100644 --- a/webapp/platform/client/src/websocket_events.ts +++ b/webapp/platform/client/src/websocket_events.ts @@ -97,6 +97,7 @@ export const enum WebSocketEvents { RecapUpdated = 'recap_updated', PostTranslationUpdated = 'post_translation_updated', FileDownloadRejected = 'file_download_rejected', + FileUploadRejected = 'file_upload_rejected', ShowToast = 'show_toast', SharedChannelRemoteUpdated = 'shared_channel_remote_updated', ChannelJoinRequestCreated = 'channel_join_request_created', diff --git a/webapp/platform/client/src/websocket_message.ts b/webapp/platform/client/src/websocket_message.ts index bdb3af3826a..a45ceff973a 100644 --- a/webapp/platform/client/src/websocket_message.ts +++ b/webapp/platform/client/src/websocket_message.ts @@ -100,6 +100,7 @@ export type WebSocketMessage = ( Messages.RecapUpdated | Messages.FileDownloadRejected | + Messages.FileUploadRejected | Messages.ShowToast | Messages.Plugin | diff --git a/webapp/platform/client/src/websocket_messages.ts b/webapp/platform/client/src/websocket_messages.ts index b5d4ddc3999..a0583248599 100644 --- a/webapp/platform/client/src/websocket_messages.ts +++ b/webapp/platform/client/src/websocket_messages.ts @@ -504,6 +504,12 @@ export type FileDownloadRejected = BaseWebSocketMessage; +export type FileUploadRejected = BaseWebSocketMessage; + export type ShowToast = BaseWebSocketMessage Date: Tue, 2 Jun 2026 16:11:59 +0300 Subject: [PATCH 4/4] Add user setting to disable auto-follow on channel-wide mentions (#36068) * Add server config to disable auto-follow on channel-wide mentions Adds `ServiceSettings.ChannelMentionAutoFollowThreads` (default: true) which, when disabled, prevents @channel/@here/@all mentions in thread replies from automatically adding users as thread followers. Users still receive mention notifications; only the thread membership is skipped. * Refactor: move channel-mention auto-follow to per-user notification setting Replaces the server-level ServiceSettings.ChannelMentionAutoFollowThreads config with a per-user notification preference channel_mention_auto_follow_threads (default: true). Users can now opt out individually via Notification Settings -> "Auto-follow threads on channel-wide mentions" (placed above "Keywords that trigger notifications"), without requiring admin intervention. Behavior is unchanged for users who have not modified the setting. * Add additional test case * linter fixes and webapp snapshot update * update user setting description * em dash removed in description * Update E2E tests * prettier:fix --------- Co-authored-by: gtsaturyan Co-authored-by: Harrison Healey --- .../accessibility_account_settings_spec.js | 1 + .../user_management_admin_control_spec.js | 4 +- .../settings/notifications_settings.ts | 4 + .../settings_dialog/notifications.spec.ts | 7 + server/channels/app/notification.go | 14 +- server/channels/app/notification_test.go | 104 +++++++++ server/public/model/user.go | 52 +++-- .../user_settings_notifications.test.tsx.snap | 216 ++++++++++++++++++ .../user_settings_notifications.tsx | 103 +++++++++ webapp/channels/src/i18n/en.json | 4 + webapp/channels/src/utils/constants.tsx | 1 + webapp/platform/types/src/users.ts | 1 + 12 files changed, 482 insertions(+), 29 deletions(-) diff --git a/e2e-tests/cypress/tests/integration/channels/accessibility/accessibility_account_settings_spec.js b/e2e-tests/cypress/tests/integration/channels/accessibility/accessibility_account_settings_spec.js index 3d30c658009..68b81b13df1 100644 --- a/e2e-tests/cypress/tests/integration/channels/accessibility/accessibility_account_settings_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/accessibility/accessibility_account_settings_spec.js @@ -34,6 +34,7 @@ describe('Verify Accessibility Support in different sections in Settings and Pro {key: 'desktopAndMobile', label: 'Desktop and mobile notifications', type: 'radio'}, {key: 'desktopNotificationSound', label: 'Desktop notification sounds', type: 'radio'}, {key: 'email', label: 'Email notifications', type: 'radio'}, + {key: 'channelMentionAutoFollow', label: 'Auto-follow threads on channel-wide mentions', type: 'radio'}, {key: 'keywordsAndMentions', label: 'Keywords that trigger notifications', type: 'checkbox'}, {key: 'keywordsAndHighlight', label: 'Keywords that get highlighted (without notifications)', type: 'checkbox'}, {key: 'replyNotifications', label: 'Reply notifications', type: 'radio'}, diff --git a/e2e-tests/cypress/tests/integration/channels/enterprise/system_console/user_management/user_management_admin_control_spec.js b/e2e-tests/cypress/tests/integration/channels/enterprise/system_console/user_management/user_management_admin_control_spec.js index b0ed8a6d9a8..0bb6590d8c8 100644 --- a/e2e-tests/cypress/tests/integration/channels/enterprise/system_console/user_management/user_management_admin_control_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/enterprise/system_console/user_management/user_management_admin_control_spec.js @@ -46,7 +46,7 @@ describe('User Management', () => { verifyManageUserSettingModal(testUser, true); - cy.get('#replyNotificationsTitle').should('be.visible').should('have.text', 'Reply notifications').click(); + cy.findByRole('heading', {name: 'Reply notifications'}).scrollIntoView().click(); cy.get('#notificationCommentsNever').should('be.checked'); cy.get('#notificationCommentsAny').check(); cy.get('button#saveSetting').last().scrollIntoView().click(); @@ -56,7 +56,7 @@ describe('User Management', () => { cy.visit(`/${testTeam.name}/channels/${testChannel.name}`); cy.get('[aria-label="Settings"]').click(); - cy.get('#replyNotificationsTitle').should('be.visible').should('have.text', 'Reply notifications').click(); + cy.findByRole('heading', {name: 'Reply notifications'}).scrollIntoView().click(); cy.get('#notificationCommentsAny').should('be.checked'); cy.apiLogout(); }); diff --git a/e2e-tests/playwright/lib/src/ui/components/channels/settings/notifications_settings.ts b/e2e-tests/playwright/lib/src/ui/components/channels/settings/notifications_settings.ts index a10c45b4429..47bd7eb5798 100644 --- a/e2e-tests/playwright/lib/src/ui/components/channels/settings/notifications_settings.ts +++ b/e2e-tests/playwright/lib/src/ui/components/channels/settings/notifications_settings.ts @@ -17,6 +17,7 @@ export default class NotificationsSettings { readonly desktopAndMobileEditButton; readonly desktopNotificationSoundEditButton; readonly emailEditButton; + readonly channelMentionAutoFollowEditButton; readonly keywordsTriggerNotificationsEditButton; readonly keywordsGetHighlightedEditButton; @@ -35,6 +36,9 @@ export default class NotificationsSettings { this.desktopAndMobileEditButton = container.locator('#desktopAndMobileEdit'); this.desktopNotificationSoundEditButton = container.locator('#desktopNotificationSoundEdit'); this.emailEditButton = container.locator('#emailEdit'); + this.channelMentionAutoFollowEditButton = container.getByRole('button', { + name: 'Auto-follow threads on channel-wide mentions Edit', + }); this.keywordsTriggerNotificationsEditButton = container.locator('#keywordsAndMentionsEdit'); this.keywordsGetHighlightedEditButton = container.locator('#keywordsAndHighlightEdit'); diff --git a/e2e-tests/playwright/specs/accessibility/channels/settings_dialog/notifications.spec.ts b/e2e-tests/playwright/specs/accessibility/channels/settings_dialog/notifications.spec.ts index aa251a3cbd6..955afac8f0e 100644 --- a/e2e-tests/playwright/specs/accessibility/channels/settings_dialog/notifications.spec.ts +++ b/e2e-tests/playwright/specs/accessibility/channels/settings_dialog/notifications.spec.ts @@ -52,6 +52,10 @@ test( await page.keyboard.press('Tab'); await pw.toBeFocusedWithFocusVisible(notificationsSettings.emailEditButton); + // # Press Tab to move focus to Auto-follow threads on channel-wide mentions button + await page.keyboard.press('Tab'); + await pw.toBeFocusedWithFocusVisible(notificationsSettings.channelMentionAutoFollowEditButton); + // # Press Tab to move focus to Keywords that trigger notifications button await page.keyboard.press('Tab'); await pw.toBeFocusedWithFocusVisible(notificationsSettings.keywordsTriggerNotificationsEditButton); @@ -110,6 +114,9 @@ test( - text: "\\"Bing\\" for messages" - heading "Email notifications" [level=4] - button "Email notifications Edit" + - heading "Auto-follow threads on channel-wide mentions" [level=4] + - button "Auto-follow threads on channel-wide mentions Edit" + - text: "On" - heading "Keywords that trigger notifications" [level=4] - button "Keywords that trigger notifications Edit" - text: "\\"@${user.username}\\", \\"@channel\\", \\"@all\\", \\"@here\\"" diff --git a/server/channels/app/notification.go b/server/channels/app/notification.go index f194b20ab6e..c4792a2dabd 100644 --- a/server/channels/app/notification.go +++ b/server/channels/app/notification.go @@ -252,12 +252,22 @@ func (a *App) SendNotifications(rctx request.CTX, post *model.Post, team *model. } if channel.Type != model.ChannelTypeDirect { rootMentions = getExplicitMentions(rootPost, keywords) - for id := range rootMentions.Mentions { + for id, mentionType := range rootMentions.Mentions { + if mentionType == ChannelMention { + if profile, ok := profileMap[id]; ok && profile.NotifyProps[model.ChannelMentionAutoFollowThreadsProp] == "false" { + continue + } + } threadParticipants[id] = true } } } - for id := range mentions.Mentions { + for id, mentionType := range mentions.Mentions { + if mentionType == ChannelMention { + if profile, ok := profileMap[id]; ok && profile.NotifyProps[model.ChannelMentionAutoFollowThreadsProp] == "false" { + continue + } + } threadParticipants[id] = true } diff --git a/server/channels/app/notification_test.go b/server/channels/app/notification_test.go index 4c7d91af8a1..80e84e7a82b 100644 --- a/server/channels/app/notification_test.go +++ b/server/channels/app/notification_test.go @@ -3022,6 +3022,110 @@ func TestChannelAutoFollowThreads(t *testing.T) { assert.False(t, threadMembership.Following) } +func TestChannelMentionAutoFollowThreads(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + u1 := th.BasicUser + u2 := th.BasicUser2 + u3 := th.CreateUser(t) + th.LinkUserToTeam(t, u3, th.BasicTeam) + c1 := th.BasicChannel + th.AddUserToChannel(t, u2, c1) + th.AddUserToChannel(t, u3, c1) + + rootPost := &model.Post{ + ChannelId: c1.Id, + Message: "root post by user3", + UserId: u3.Id, + } + rpost, _, appErr := th.App.CreatePost(th.Context, rootPost, c1, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, appErr) + + t.Run("channel mention auto-follow enabled (default)", func(t *testing.T) { + // u2 has default notify props (channel_mention_auto_follow_threads = "true") + require.Equal(t, "true", u2.NotifyProps[model.ChannelMentionAutoFollowThreadsProp]) + + replyPost := &model.Post{ + ChannelId: c1.Id, + Message: "@channel reply by user1", + UserId: u1.Id, + RootId: rpost.Id, + } + _, _, appErr = th.App.CreatePost(th.Context, replyPost, c1, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, appErr) + + // u2 should be auto-following because channel_mention_auto_follow_threads is enabled + threadMembership, getThreadErr := th.App.GetThreadMembershipForUser(u2.Id, rpost.Id) + require.Nil(t, getThreadErr) + require.NotNil(t, threadMembership) + assert.True(t, threadMembership.Following) + }) + + t.Run("channel mention auto-follow disabled for user", func(t *testing.T) { + // Disable auto-follow for u2 + u2.NotifyProps[model.ChannelMentionAutoFollowThreadsProp] = "false" + u2, appErr = th.App.UpdateUser(th.Context, u2, false) + require.Nil(t, appErr) + require.Equal(t, "false", u2.NotifyProps[model.ChannelMentionAutoFollowThreadsProp]) + + // Reset u2 membership so the prior sub-test doesn't interfere + _, err := th.App.Srv().Store().Thread().MaintainMembership(u2.Id, rpost.Id, store.ThreadMembershipOpts{ + Following: false, + UpdateFollowing: true, + }) + require.NoError(t, err) + + replyPost := &model.Post{ + ChannelId: c1.Id, + Message: "@channel reply by user1 again", + UserId: u1.Id, + RootId: rpost.Id, + } + _, _, appErr = th.App.CreatePost(th.Context, replyPost, c1, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, appErr) + + // u2 should NOT be auto-following because they opted out + threadMembership, getThreadErr := th.App.GetThreadMembershipForUser(u2.Id, rpost.Id) + require.Nil(t, getThreadErr) + if threadMembership != nil { + assert.False(t, threadMembership.Following) + } + }) + + t.Run("channel mention auto-follow undefined (old default)", func(t *testing.T) { + // Remove the auto-follow setting for u2 to mimic a user created before this setting was added + delete(u2.NotifyProps, model.ChannelMentionAutoFollowThreadsProp) + u2, appErr = th.App.UpdateUser(th.Context, u2, false) + require.Nil(t, appErr) + + _, ok := u2.NotifyProps[model.ChannelMentionAutoFollowThreadsProp] + require.False(t, ok) + + // Reset u2 membership so the prior sub-test doesn't interfere + _, err := th.App.Srv().Store().Thread().MaintainMembership(u2.Id, rpost.Id, store.ThreadMembershipOpts{ + Following: false, + UpdateFollowing: true, + }) + require.NoError(t, err) + + replyPost := &model.Post{ + ChannelId: c1.Id, + Message: "@channel reply by user1", + UserId: u1.Id, + RootId: rpost.Id, + } + _, _, appErr = th.App.CreatePost(th.Context, replyPost, c1, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, appErr) + + // u2 should be auto-following because channel_mention_auto_follow_threads isn't defined + threadMembership, getThreadErr := th.App.GetThreadMembershipForUser(u2.Id, rpost.Id) + require.Nil(t, getThreadErr) + require.NotNil(t, threadMembership) + assert.True(t, threadMembership.Following) + }) +} + func TestRemoveNotifications(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic(t) diff --git a/server/public/model/user.go b/server/public/model/user.go index d80deea3c9b..828f5597c43 100644 --- a/server/public/model/user.go +++ b/server/public/model/user.go @@ -23,31 +23,32 @@ import ( ) const ( - Me = "me" - UserNotifyAll = "all" - UserNotifyHere = "here" - UserNotifyMention = "mention" - UserNotifyNone = "none" - DesktopNotifyProp = "desktop" - DesktopSoundNotifyProp = "desktop_sound" - MarkUnreadNotifyProp = "mark_unread" - PushNotifyProp = "push" - PushStatusNotifyProp = "push_status" - EmailNotifyProp = "email" - ChannelMentionsNotifyProp = "channel" - CommentsNotifyProp = "comments" - MentionKeysNotifyProp = "mention_keys" - HighlightsNotifyProp = "highlight_keys" - CommentsNotifyNever = "never" - CommentsNotifyRoot = "root" - CommentsNotifyAny = "any" - CommentsNotifyCRT = "crt" - FirstNameNotifyProp = "first_name" - AutoResponderActiveNotifyProp = "auto_responder_active" - AutoResponderMessageNotifyProp = "auto_responder_message" - DesktopThreadsNotifyProp = "desktop_threads" - PushThreadsNotifyProp = "push_threads" - EmailThreadsNotifyProp = "email_threads" + Me = "me" + UserNotifyAll = "all" + UserNotifyHere = "here" + UserNotifyMention = "mention" + UserNotifyNone = "none" + DesktopNotifyProp = "desktop" + DesktopSoundNotifyProp = "desktop_sound" + MarkUnreadNotifyProp = "mark_unread" + PushNotifyProp = "push" + PushStatusNotifyProp = "push_status" + EmailNotifyProp = "email" + ChannelMentionsNotifyProp = "channel" + CommentsNotifyProp = "comments" + MentionKeysNotifyProp = "mention_keys" + HighlightsNotifyProp = "highlight_keys" + CommentsNotifyNever = "never" + CommentsNotifyRoot = "root" + CommentsNotifyAny = "any" + CommentsNotifyCRT = "crt" + FirstNameNotifyProp = "first_name" + AutoResponderActiveNotifyProp = "auto_responder_active" + AutoResponderMessageNotifyProp = "auto_responder_message" + DesktopThreadsNotifyProp = "desktop_threads" + PushThreadsNotifyProp = "push_threads" + EmailThreadsNotifyProp = "email_threads" + ChannelMentionAutoFollowThreadsProp = "channel_mention_auto_follow_threads" DefaultLocale = "en" UserAuthServiceEmail = "email" @@ -607,6 +608,7 @@ func (u *User) SetDefaultNotifications() { u.NotifyProps[DesktopThreadsNotifyProp] = UserNotifyAll u.NotifyProps[EmailThreadsNotifyProp] = UserNotifyAll u.NotifyProps[PushThreadsNotifyProp] = UserNotifyAll + u.NotifyProps[ChannelMentionAutoFollowThreadsProp] = "true" } func (u *User) UpdateMentionKeysFromUsername(oldUsername string) { diff --git a/webapp/channels/src/components/user_settings/notifications/__snapshots__/user_settings_notifications.test.tsx.snap b/webapp/channels/src/components/user_settings/notifications/__snapshots__/user_settings_notifications.test.tsx.snap index 631cf002b25..17c2f4dc73b 100644 --- a/webapp/channels/src/components/user_settings/notifications/__snapshots__/user_settings_notifications.test.tsx.snap +++ b/webapp/channels/src/components/user_settings/notifications/__snapshots__/user_settings_notifications.test.tsx.snap @@ -192,6 +192,42 @@ Object {
+
+
+

+ Auto-follow threads on channel-wide mentions +

+