TRY 2: MM-56201, MM-56280: Suppress typing and emoji events (#26282)

* Revert "Revert "MM-56201, MM-56280: Suppress typing and emoji events (#25794)…"

This reverts commit 6808a1c733.

* Remove bad ff merge

```release-note
NONE
```
This commit is contained in:
Agniva De Sarker 2024-02-22 21:25:34 +05:30 committed by GitHub
parent 6808a1c733
commit 64504b80e6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
23 changed files with 469 additions and 188 deletions

View File

@ -25,6 +25,7 @@ import (
"github.com/mattermost/mattermost/server/public/shared/i18n"
"github.com/mattermost/mattermost/server/public/shared/mlog"
"github.com/mattermost/mattermost/server/public/shared/request"
"github.com/mattermost/mattermost/server/public/utils"
)
const (
@ -48,6 +49,10 @@ const (
const websocketMessagePluginPrefix = "custom_"
// UnsetPresenceIndicator is the value that gets set initially for active channel/
// thread/team. This is done to differentiate it from an explicitly set empty value.
const UnsetPresenceIndicator = "<>"
type pluginWSPostedHook struct {
connectionID string
userID string
@ -109,10 +114,12 @@ type WebConn struct {
sessionToken atomic.Value
session atomic.Pointer[model.Session]
connectionID atomic.Value
activeChannelID atomic.Value
activeTeamID atomic.Value
activeRHSThreadChannelID atomic.Value
activeThreadViewThreadChannelID atomic.Value
endWritePump chan struct{}
pumpFinished chan struct{}
pluginPosted chan pluginWSPostedHook
@ -236,10 +243,12 @@ func (ps *PlatformService) NewWebConn(cfg *WebConnConfig, suite SuiteIFace, runn
wc.SetSessionToken(cfg.Session.Token)
wc.SetSessionExpiresAt(cfg.Session.ExpiresAt)
wc.SetConnectionID(cfg.ConnectionID)
wc.SetActiveChannelID("")
wc.SetActiveTeamID("")
wc.SetActiveRHSThreadChannelID("")
wc.SetActiveThreadViewThreadChannelID("")
// <> means unset. This is to differentiate from empty value.
// Because we need to support mobile clients where the value might be unset.
wc.SetActiveChannelID(UnsetPresenceIndicator)
wc.SetActiveTeamID(UnsetPresenceIndicator)
wc.SetActiveRHSThreadChannelID(UnsetPresenceIndicator)
wc.SetActiveThreadViewThreadChannelID(UnsetPresenceIndicator)
ps.Go(func() {
runner.RunMultiHook(func(hooks plugin.Hooks) bool {
@ -305,6 +314,9 @@ func (wc *WebConn) SetActiveChannelID(id string) {
// GetActiveChannelID returns the active channel id of the connection.
func (wc *WebConn) GetActiveChannelID() string {
if wc.activeChannelID.Load() == nil {
return UnsetPresenceIndicator
}
return wc.activeChannelID.Load().(string)
}
@ -315,11 +327,17 @@ func (wc *WebConn) SetActiveTeamID(id string) {
// GetActiveTeamID returns the active team id of the connection.
func (wc *WebConn) GetActiveTeamID() string {
if wc.activeTeamID.Load() == nil {
return UnsetPresenceIndicator
}
return wc.activeTeamID.Load().(string)
}
// GetActiveRHSThreadChannelID returns the channel id of the active thread of the connection.
func (wc *WebConn) GetActiveRHSThreadChannelID() string {
if wc.activeRHSThreadChannelID.Load() == nil {
return UnsetPresenceIndicator
}
return wc.activeRHSThreadChannelID.Load().(string)
}
@ -330,6 +348,9 @@ func (wc *WebConn) SetActiveRHSThreadChannelID(id string) {
// GetActiveThreadViewThreadChannelID returns the channel id of the active thread of the connection.
func (wc *WebConn) GetActiveThreadViewThreadChannelID() string {
if wc.activeThreadViewThreadChannelID.Load() == nil {
return UnsetPresenceIndicator
}
return wc.activeThreadViewThreadChannelID.Load().(string)
}
@ -338,6 +359,11 @@ func (wc *WebConn) SetActiveThreadViewThreadChannelID(id string) {
wc.activeThreadViewThreadChannelID.Store(id)
}
// isSet is a helper to check if a value is unset or not.
func (wc *WebConn) isSet(val string) bool {
return val != UnsetPresenceIndicator
}
// areAllInactive returns whether all of the connections
// are inactive or not.
func areAllInactive(conns []*WebConn) bool {
@ -847,7 +873,18 @@ func (wc *WebConn) ShouldSendEvent(msg *model.WebSocketEvent) bool {
}
// Only report events to users who are in the channel for the event
if msg.GetBroadcast().ChannelId != "" {
if chID := msg.GetBroadcast().ChannelId; chID != "" {
// For typing events, we don't send them to users who don't have
// that channel or thread opened.
if wc.Platform.Config().FeatureFlags.WebSocketEventScope &&
utils.Contains([]model.WebsocketEventType{
model.WebsocketEventTyping,
model.WebsocketEventReactionAdded,
model.WebsocketEventReactionRemoved,
}, msg.EventType()) && wc.notInChannel(chID) && wc.notInThread(chID) {
return false
}
if model.GetMillis()-wc.lastAllChannelMembersTime > webConnMemberCacheTime {
wc.allChannelMembers = nil
wc.lastAllChannelMembersTime = 0
@ -863,7 +900,7 @@ func (wc *WebConn) ShouldSendEvent(msg *model.WebSocketEvent) bool {
wc.lastAllChannelMembersTime = model.GetMillis()
}
if _, ok := wc.allChannelMembers[msg.GetBroadcast().ChannelId]; ok {
if _, ok := wc.allChannelMembers[chID]; ok {
return true
}
return false
@ -881,6 +918,15 @@ func (wc *WebConn) ShouldSendEvent(msg *model.WebSocketEvent) bool {
return true
}
func (wc *WebConn) notInChannel(val string) bool {
return (wc.isSet(wc.GetActiveChannelID()) && val != wc.GetActiveChannelID())
}
func (wc *WebConn) notInThread(val string) bool {
return (wc.isSet(wc.GetActiveRHSThreadChannelID()) && val != wc.GetActiveRHSThreadChannelID()) &&
(wc.isSet(wc.GetActiveThreadViewThreadChannelID()) && val != wc.GetActiveThreadViewThreadChannelID())
}
// IsMemberOfTeam returns whether the user of the WebConn
// is a member of the given teamID or not.
func (wc *WebConn) isMemberOfTeam(teamID string) bool {

View File

@ -4,6 +4,7 @@
package app
import (
"os"
"testing"
"github.com/stretchr/testify/assert"
@ -15,6 +16,8 @@ import (
)
func TestWebConnShouldSendEvent(t *testing.T) {
os.Setenv("MM_FEATUREFLAGS_WEBSOCKETEVENTSCOPE", "true")
defer os.Unsetenv("MM_FEATUREFLAGS_WEBSOCKETEVENTSCOPE")
th := Setup(t).InitBasic()
defer th.TearDown()
session, err := th.App.CreateSession(th.Context, &model.Session{UserId: th.BasicUser.Id, Roles: th.BasicUser.GetRawRoles(), TeamMembers: []*model.TeamMember{
@ -162,6 +165,63 @@ func TestWebConnShouldSendEvent(t *testing.T) {
assert.False(t, adminUserWc.ShouldSendEvent(event), "did not expect admin")
})
t.Run("should not send typing event unless in scope", func(t *testing.T) {
event2 := model.NewWebSocketEvent(model.WebsocketEventTyping, "", th.BasicChannel.Id, "", nil, "")
// Basic, unset case
basicUserWc.SetActiveChannelID(platform.UnsetPresenceIndicator)
basicUserWc.SetActiveRHSThreadChannelID(platform.UnsetPresenceIndicator)
basicUserWc.SetActiveThreadViewThreadChannelID(platform.UnsetPresenceIndicator)
assert.True(t, basicUserWc.ShouldSendEvent(event2))
// Active channel is set to something else, thread unset
basicUserWc.SetActiveChannelID("ch1")
basicUserWc.SetActiveRHSThreadChannelID(platform.UnsetPresenceIndicator)
basicUserWc.SetActiveThreadViewThreadChannelID(platform.UnsetPresenceIndicator)
assert.True(t, basicUserWc.ShouldSendEvent(event2))
// Active channel is unset, thread set
basicUserWc.SetActiveChannelID(platform.UnsetPresenceIndicator)
basicUserWc.SetActiveRHSThreadChannelID("ch1")
basicUserWc.SetActiveThreadViewThreadChannelID("ch2")
assert.True(t, basicUserWc.ShouldSendEvent(event2))
// both are set to correct channel
basicUserWc.SetActiveChannelID(th.BasicChannel.Id)
basicUserWc.SetActiveRHSThreadChannelID(th.BasicChannel.Id)
basicUserWc.SetActiveThreadViewThreadChannelID(th.BasicChannel.Id)
assert.True(t, basicUserWc.ShouldSendEvent(event2))
// channel is correct, thread is something else.
basicUserWc.SetActiveChannelID(th.BasicChannel.Id)
basicUserWc.SetActiveRHSThreadChannelID("ch1")
basicUserWc.SetActiveThreadViewThreadChannelID("ch2")
assert.True(t, basicUserWc.ShouldSendEvent(event2))
// channel is wrong, thread is correct.
basicUserWc.SetActiveChannelID("ch1")
basicUserWc.SetActiveRHSThreadChannelID(th.BasicChannel.Id)
basicUserWc.SetActiveThreadViewThreadChannelID(th.BasicChannel.Id)
assert.True(t, basicUserWc.ShouldSendEvent(event2))
// FINALLY, both are set to something else.
basicUserWc.SetActiveChannelID("ch1")
basicUserWc.SetActiveRHSThreadChannelID("ch1")
basicUserWc.SetActiveThreadViewThreadChannelID("ch2")
assert.False(t, basicUserWc.ShouldSendEvent(event2))
// Different threads and channel
basicUserWc.SetActiveChannelID("ch1")
basicUserWc.SetActiveRHSThreadChannelID("ch2")
basicUserWc.SetActiveThreadViewThreadChannelID("ch3")
assert.False(t, basicUserWc.ShouldSendEvent(event2))
// Other channel. Thread unset explicitly.
basicUserWc.SetActiveChannelID("ch1")
basicUserWc.SetActiveRHSThreadChannelID("")
basicUserWc.SetActiveThreadViewThreadChannelID("")
assert.False(t, basicUserWc.ShouldSendEvent(event2))
})
t.Run("should send to basic user and admin in channel2", func(t *testing.T) {
event = event.SetBroadcast(&model.WebsocketBroadcast{ChannelId: channel2.Id})

View File

@ -50,6 +50,7 @@ type FeatureFlags struct {
ConsumePostHook bool
CloudAnnualRenewals bool
WebSocketEventScope bool
}
func (f *FeatureFlags) SetDefaults() {
@ -69,6 +70,7 @@ func (f *FeatureFlags) SetDefaults() {
f.CloudIPFiltering = false
f.ConsumePostHook = false
f.CloudAnnualRenewals = false
f.WebSocketEventScope = false
}
// ToMap returns the feature flags as a map[string]string

View File

@ -11,7 +11,6 @@ import type {UserProfile} from '@mattermost/types/users';
import type {IDMappedObjects} from '@mattermost/types/utilities';
import {SearchTypes} from 'mattermost-redux/action_types';
import * as PostActions from 'mattermost-redux/actions/posts';
import * as SearchActions from 'mattermost-redux/actions/search';
import {trackEvent} from 'actions/telemetry_actions.jsx';
@ -171,15 +170,6 @@ describe('rhs view actions', () => {
root_id: 'root123',
} as Post;
test('it dispatches PostActions.getPostThread correctly', () => {
store.dispatch(selectPostFromRightHandSideSearch(post));
const compareStore = mockStore(initialState);
compareStore.dispatch(PostActions.getPostThread(post.root_id));
expect(store.getActions()[0]).toEqual(compareStore.getActions()[0]);
});
describe(`it dispatches ${ActionTypes.SELECT_POST} correctly`, () => {
it('with mocked date', async () => {
store = mockStore({
@ -202,7 +192,7 @@ describe('rhs view actions', () => {
timestamp: POST_CREATED_TIME,
};
expect(store.getActions()[1]).toEqual(action);
expect(store.getActions()[0]).toEqual(action);
});
});
});
@ -792,7 +782,6 @@ describe('rhs view actions', () => {
await store.dispatch(openAtPrevious({selectedPostId: previousSelectedPost.id, previousRhsState: previousState}));
const compareStore = mockStore(initialState);
compareStore.dispatch(PostActions.getPostThread(previousSelectedPost.root_id));
compareStore.dispatch({
type: ActionTypes.SELECT_POST,
postId: previousSelectedPost.root_id,

View File

@ -9,7 +9,6 @@ import type {Post} from '@mattermost/types/posts';
import {SearchTypes} from 'mattermost-redux/action_types';
import {getChannel} from 'mattermost-redux/actions/channels';
import * as PostActions from 'mattermost-redux/actions/posts';
import {getPostsByIds, getPost as fetchPost} from 'mattermost-redux/actions/posts';
import {
clearSearch,
@ -40,7 +39,6 @@ import type {RhsState} from 'types/store/rhs';
function selectPostFromRightHandSideSearchWithPreviousState(post: Post, previousRhsState?: RhsState): ActionFuncAsync<boolean, GlobalState> {
return async (dispatch, getState) => {
const postRootId = post.root_id || post.id;
await dispatch(PostActions.getPostThread(postRootId));
const state = getState();
dispatch({

View File

@ -13,6 +13,7 @@ exports[`components/channel_view Should match snapshot if channel is archived 1`
channelIsArchived={true}
deactivatedChannel={false}
enableOnboardingFlow={true}
enableWebSocketEventScope={false}
goToLastViewedChannel={[MockFunction]}
history={Object {}}
isCloud={false}
@ -69,6 +70,7 @@ exports[`components/channel_view Should match snapshot if channel is deactivated
channelIsArchived={false}
deactivatedChannel={true}
enableOnboardingFlow={true}
enableWebSocketEventScope={false}
goToLastViewedChannel={[MockFunction]}
history={Object {}}
isCloud={false}
@ -124,6 +126,7 @@ exports[`components/channel_view Should match snapshot with base props 1`] = `
channelIsArchived={false}
deactivatedChannel={false}
enableOnboardingFlow={true}
enableWebSocketEventScope={false}
goToLastViewedChannel={[MockFunction]}
history={Object {}}
isCloud={false}

View File

@ -24,6 +24,7 @@ describe('components/channel_view', () => {
isCloud: false,
goToLastViewedChannel: jest.fn(),
isFirstAdmin: false,
enableWebSocketEventScope: false,
};
it('Should match snapshot with base props', () => {

View File

@ -89,7 +89,7 @@ export default class ChannelView extends React.PureComponent<Props, State> {
componentDidUpdate(prevProps: Props) {
// TODO: debounce
if (prevProps.channelId !== this.props.channelId) {
if (prevProps.channelId !== this.props.channelId && this.props.enableWebSocketEventScope) {
WebSocketClient.updateActiveChannel(this.props.channelId);
}
if (prevProps.channelId !== this.props.channelId || prevProps.channelIsArchived !== this.props.channelIsArchived) {

View File

@ -29,6 +29,7 @@ function mapStateToProps(state: GlobalState) {
const viewArchivedChannels = config.ExperimentalViewArchivedChannels === 'true';
const enableOnboardingFlow = config.EnableOnboardingFlow === 'true';
const enableWebSocketEventScope = config.FeatureFlagWebSocketEventScope === 'true';
return {
channelId: channel ? channel.id : '',
@ -39,6 +40,7 @@ function mapStateToProps(state: GlobalState) {
isCloud: getLicense(state).Cloud === 'true',
teamUrl: getCurrentRelativeTeamUrl(state),
isFirstAdmin: isFirstAdmin(state),
enableWebSocketEventScope,
};
}

View File

@ -38,6 +38,7 @@ function mapStateToProps(state: GlobalState) {
const products = state.plugins.components.Product || [];
const [unreadTeamsSet, mentionsInTeamMap, teamHasUrgentMap] = getTeamsUnreadStatuses(state);
const enableWebSocketEventScope = config.FeatureFlagWebSocketEventScope === 'true';
return {
currentTeamId: getCurrentTeamId(state),
@ -51,6 +52,7 @@ function mapStateToProps(state: GlobalState) {
unreadTeamsSet,
mentionsInTeamMap,
teamHasUrgentMap,
enableWebSocketEventScope,
};
}

View File

@ -149,7 +149,7 @@ export default class TeamSidebar extends React.PureComponent<Props, State> {
componentDidUpdate(prevProps: Props) {
// TODO: debounce
if (prevProps.currentTeamId !== this.props.currentTeamId) {
if (prevProps.currentTeamId !== this.props.currentTeamId && this.props.enableWebSocketEventScope) {
WebSocketClient.updateActiveTeam(this.props.currentTeamId);
}
}

View File

@ -6,6 +6,7 @@ import {bindActionCreators} from 'redux';
import type {Dispatch} from 'redux';
import type {Channel} from '@mattermost/types/channels';
import type {ClientConfig} from '@mattermost/types/config';
import type {UserThread} from '@mattermost/types/threads';
import {fetchRHSAppsBindings} from 'mattermost-redux/actions/apps';
@ -13,6 +14,7 @@ import {getNewestPostThread, getPostThread} from 'mattermost-redux/actions/posts
import {getThread as fetchThread, updateThreadRead} from 'mattermost-redux/actions/threads';
import {appsEnabled} from 'mattermost-redux/selectors/entities/apps';
import {makeGetChannel} from 'mattermost-redux/selectors/entities/channels';
import {getConfig} from 'mattermost-redux/selectors/entities/general';
import {getPost, makeGetPostIdsForThread} from 'mattermost-redux/selectors/entities/posts';
import {isCollapsedThreadsEnabled} from 'mattermost-redux/selectors/entities/preferences';
import {getCurrentTeamId} from 'mattermost-redux/selectors/entities/teams';
@ -43,6 +45,8 @@ function makeMapStateToProps() {
const socketStatus = getSocketStatus(state);
const highlightedPostId = getHighlightedPostId(state);
const selectedPostFocusedAt = getSelectedPostFocussedAt(state);
const config: Partial<ClientConfig> = getConfig(state);
const enableWebSocketEventScope = config.FeatureFlagWebSocketEventScope === 'true';
let postIds: string[] = [];
let userThread: UserThread | null = null;
@ -66,6 +70,7 @@ function makeMapStateToProps() {
channel,
highlightedPostId,
selectedPostFocusedAt,
enableWebSocketEventScope,
};
};
}

View File

@ -68,6 +68,7 @@ describe('components/threading/ThreadViewer', () => {
appsEnabled: true,
rootPostId: post.id,
isThreadView: true,
enableWebSocketEventScope: false,
};
test('should match snapshot', async () => {

View File

@ -53,6 +53,7 @@ export type Props = Attrs & {
inputPlaceholder?: string;
rootPostId: string;
fromSuppressed?: boolean;
enableWebSocketEventScope: boolean;
};
type State = {
@ -81,8 +82,10 @@ export default class ThreadViewer extends React.PureComponent<Props, State> {
}
public componentWillUnmount() {
if (this.props.enableWebSocketEventScope) {
WebSocketClient.updateActiveThread(this.props.isThreadView, '');
}
}
public componentDidUpdate(prevProps: Props) {
const reconnected = this.props.socketConnectionStatus && !prevProps.socketConnectionStatus;
@ -170,11 +173,7 @@ export default class ThreadViewer extends React.PureComponent<Props, State> {
// scrolls to either bottom or new messages line
private onInit = async (reconnected = false): Promise<void> => {
this.setState({isLoading: !reconnected});
if (reconnected || this.morePostsToFetch()) {
await this.props.actions.getPostThread(this.props.selected?.id || this.props.rootPostId, !reconnected);
} else {
await this.props.actions.getNewestPostThread(this.props.selected?.id || this.props.rootPostId);
}
if (
this.props.isCollapsedThreadsEnabled &&
@ -183,7 +182,7 @@ export default class ThreadViewer extends React.PureComponent<Props, State> {
await this.fetchThread();
}
if (this.props.channel) {
if (this.props.channel && this.props.enableWebSocketEventScope) {
WebSocketClient.updateActiveThread(this.props.isThreadView, this.props.channel?.id);
}
this.setState({isLoading: false});

View File

@ -41,6 +41,8 @@ export default keyMirror({
POST_DELETED: null,
POST_REMOVED: null,
POST_PINNED_CHANGED: null,
RECEIVED_FOCUSED_POST: null,
RECEIVED_EDIT_POST: null,
RECEIVED_REACTION: null,

View File

@ -1131,87 +1131,115 @@ describe('Actions.Posts', () => {
expect(state.entities.teams.myMembers[teamId].mention_count).toBe(1);
});
it('pinPost', async () => {
const {dispatch, getState} = store;
describe('pinPost', () => {
test('should update post and channel stats', async () => {
nock(Client4.getBaseRoute()).
get(`/channels/${TestHelper.basicChannel!.id}/stats?exclude_files_count=true`).
reply(200, {channel_id: TestHelper.basicChannel!.id, member_count: 1, pinnedpost_count: 0});
await store.dispatch(getChannelStats(TestHelper.basicChannel!.id));
await dispatch(getChannelStats(TestHelper.basicChannel!.id));
const post = TestHelper.fakePostWithId(TestHelper.basicChannel!.id);
store.dispatch(Actions.receivedPost(post));
nock(Client4.getBaseRoute()).
post('/posts').
reply(201, TestHelper.fakePostWithId(TestHelper.basicChannel!.id));
const post1 = await Client4.createPost(
TestHelper.fakePost(TestHelper.basicChannel!.id),
);
const postList = {order: [post1.id], posts: {}} as PostList;
postList.posts[post1.id] = post1;
nock(Client4.getBaseRoute()).
get(`/posts/${post1.id}/thread?skipFetchThreads=false&collapsedThreads=true&collapsedThreadsExtended=false&direction=down&perPage=60`).
reply(200, postList);
await dispatch(Actions.getPostThread(post1.id));
nock(Client4.getBaseRoute()).
post(`/posts/${post1.id}/pin`).
post(`/posts/${post.id}/pin`).
reply(200, OK_RESPONSE);
await dispatch(Actions.pinPost(post1.id));
const state = getState();
const {stats} = state.entities.channels;
const post = state.entities.posts.posts[post1.id];
const pinnedPostCount = stats[TestHelper.basicChannel!.id].pinnedpost_count;
const result = await store.dispatch(Actions.pinPost(post.id));
expect(result.error).toBeUndefined();
expect(post).toBeTruthy();
expect(post.is_pinned === true).toBeTruthy();
expect(pinnedPostCount === 1).toBeTruthy();
const state = store.getState();
expect(state.entities.posts.posts[post.id].is_pinned).toBe(true);
expect(state.entities.channels.stats[TestHelper.basicChannel!.id].pinnedpost_count).toBe(1);
});
it('unpinPost', async () => {
const {dispatch, getState} = store;
test('MM-14115 should not clobber reactions on pinned post', async () => {
const post = TestHelper.getPostMock({
id: TestHelper.generateId(),
metadata: {
embeds: [],
emojis: [],
files: [],
images: {},
reactions: [
TestHelper.getReactionMock({emoji_name: 'test'}),
],
},
});
store.dispatch(Actions.receivedPost(post));
let state = store.getState();
expect(state.entities.posts.posts[post.id].is_pinned).toBe(false);
expect(Object.keys(state.entities.posts.reactions[post.id])).toHaveLength(1);
nock(Client4.getBaseRoute()).
post(`/posts/${post.id}/pin`).
reply(200, OK_RESPONSE);
const result = await store.dispatch(Actions.pinPost(post.id));
expect(result.error).toBeUndefined();
state = store.getState();
expect(state.entities.posts.posts[post.id].is_pinned).toBe(true);
expect(Object.keys(state.entities.posts.reactions[post.id])).toHaveLength(1);
});
});
describe('unpinPost', () => {
test('should update post and channel stats', async () => {
nock(Client4.getBaseRoute()).
get(`/channels/${TestHelper.basicChannel!.id}/stats?exclude_files_count=true`).
reply(200, {channel_id: TestHelper.basicChannel!.id, member_count: 1, pinnedpost_count: 0});
reply(200, {channel_id: TestHelper.basicChannel!.id, member_count: 1, pinnedpost_count: 1});
await store.dispatch(getChannelStats(TestHelper.basicChannel!.id));
await dispatch(getChannelStats(TestHelper.basicChannel!.id));
const post = TestHelper.fakePostWithId(TestHelper.basicChannel!.id);
store.dispatch(Actions.receivedPost(post));
nock(Client4.getBaseRoute()).
post('/posts').
reply(201, TestHelper.fakePostWithId(TestHelper.basicChannel!.id));
const post1 = await Client4.createPost(
TestHelper.fakePost(TestHelper.basicChannel!.id),
);
const postList = {order: [post1.id], posts: {}} as PostList;
postList.posts[post1.id] = post1;
nock(Client4.getBaseRoute()).
get(`/posts/${post1.id}/thread?skipFetchThreads=false&collapsedThreads=true&collapsedThreadsExtended=false&direction=down&perPage=60`).
reply(200, postList);
await dispatch(Actions.getPostThread(post1.id));
nock(Client4.getBaseRoute()).
post(`/posts/${post1.id}/pin`).
post(`/posts/${post.id}/unpin`).
reply(200, OK_RESPONSE);
await dispatch(Actions.pinPost(post1.id));
const result = await store.dispatch(Actions.unpinPost(post.id));
expect(result.error).toBeUndefined();
const state = store.getState();
expect(state.entities.posts.posts[post.id].is_pinned).toBe(false);
expect(state.entities.channels.stats[TestHelper.basicChannel!.id].pinnedpost_count).toBe(0);
});
test('MM-14115 should not clobber reactions on pinned post', async () => {
const post = TestHelper.getPostMock({
id: TestHelper.generateId(),
is_pinned: true,
metadata: {
embeds: [],
emojis: [],
files: [],
images: {},
reactions: [
TestHelper.getReactionMock({emoji_name: 'test'}),
],
},
});
store.dispatch(Actions.receivedPost(post));
let state = store.getState();
expect(state.entities.posts.posts[post.id].is_pinned).toBe(true);
expect(Object.keys(state.entities.posts.reactions[post.id])).toHaveLength(1);
nock(Client4.getBaseRoute()).
post(`/posts/${post1.id}/unpin`).
post(`/posts/${post.id}/unpin`).
reply(200, OK_RESPONSE);
await dispatch(Actions.unpinPost(post1.id));
const state = getState();
const {stats} = state.entities.channels;
const post = state.entities.posts.posts[post1.id];
const pinnedPostCount = stats[TestHelper.basicChannel!.id].pinnedpost_count;
const result = await store.dispatch(Actions.unpinPost(post.id));
expect(result.error).toBeUndefined();
expect(post).toBeTruthy();
expect(post.is_pinned === false).toBeTruthy();
expect(pinnedPostCount === 0).toBeTruthy();
state = store.getState();
expect(state.entities.posts.posts[post.id].is_pinned).toBe(false);
expect(Object.keys(state.entities.posts.reactions[post.id])).toHaveLength(1);
});
});
it('addReaction', async () => {

View File

@ -137,6 +137,15 @@ export function postRemoved(post: Post) {
};
}
export function postPinnedChanged(postId: string, isPinned: boolean, updateAt = Date.now()) {
return {
type: PostTypes.POST_PINNED_CHANGED,
postId,
isPinned,
updateAt,
};
}
export function getPost(postId: string): ActionFuncAsync<Post> {
return async (dispatch, getState) => {
let post;
@ -525,11 +534,7 @@ export function pinPost(postId: string): ActionFuncAsync {
const post = PostSelectors.getPost(state, postId);
if (post) {
actions.push(
receivedPost({
...post,
is_pinned: true,
update_at: Date.now(),
}, isCollapsedThreadsEnabled(state)),
postPinnedChanged(postId, true, Date.now()),
{
type: ChannelTypes.INCREMENT_PINNED_POST_COUNT,
id: post.channel_id,
@ -577,11 +582,7 @@ export function unpinPost(postId: string): ActionFuncAsync {
const post = PostSelectors.getPost(state, postId);
if (post) {
actions.push(
receivedPost({
...post,
is_pinned: false,
update_at: Date.now(),
}, isCollapsedThreadsEnabled(state)),
postPinnedChanged(postId, false, Date.now()),
decrementPinnedPostCount(post.channel_id),
);
}

View File

@ -3615,79 +3615,116 @@ describe('reactions', () => {
PostTypes.RECEIVED_POST,
]) {
describe(`single post received (${actionType})`, () => {
it('no post metadata', () => {
it('should not store anything for a post first received without metadata', () => {
// This shouldn't occur based on our type definitions, but it is possible
const post = TestHelper.getPostMock({
id: 'post',
});
(post as any).metadata = undefined;
const state = deepFreeze({});
const action = {
type: actionType,
data: {
id: 'post',
},
data: post,
};
const nextState = reducers.reactions(state, action);
expect(nextState).toEqual(state);
expect(nextState).toBe(state);
});
it('no reactions in post metadata', () => {
const state = deepFreeze({});
it('should not change stored state for a post received without metadata', () => {
// This shouldn't occur based on our type definitions, but it is possible
const post = TestHelper.getPostMock({
id: 'post',
});
(post as any).metadata = undefined;
const state = deepFreeze({
post: {
'user-taco': TestHelper.getReactionMock({user_id: 'user', emoji_name: 'taco'}),
},
});
const action = {
type: actionType,
data: {
id: 'post',
metadata: {reactions: []},
},
data: post,
};
const nextState = reducers.reactions(state, action);
expect(nextState).not.toEqual(state);
expect(nextState).toBe(state);
});
it('should store when a post is first received without reactions', () => {
const post = TestHelper.getPostMock({
id: 'post',
});
post.metadata.reactions = undefined;
const state = deepFreeze({});
const action = {
type: actionType,
data: post,
};
const nextState = reducers.reactions(state, action);
expect(nextState).not.toBe(state);
expect(nextState).toEqual({
post: {},
});
});
it('should not clobber reactions when metadata empty', () => {
const state = deepFreeze({post: {name: 'smiley', post_id: 'post'}});
it('should remove existing reactions when a post is received without reactions', () => {
const post = TestHelper.getPostMock({
id: 'post',
});
post.metadata.reactions = undefined;
const state = deepFreeze({
post: {
'user-taco': TestHelper.getReactionMock({user_id: 'abcd', emoji_name: '+1'}),
},
});
const action = {
type: actionType,
data: {
id: 'post',
metadata: {},
},
data: post,
};
const nextState = reducers.reactions(state, action);
expect(nextState).not.toBe(state);
expect(nextState).toEqual({
post: {name: 'smiley', post_id: 'post'},
post: {},
});
});
it('should save reactions', () => {
const reactions = [
TestHelper.getReactionMock({user_id: 'abcd', emoji_name: '+1'}),
TestHelper.getReactionMock({user_id: 'efgh', emoji_name: '+1'}),
TestHelper.getReactionMock({user_id: 'abcd', emoji_name: '-1'}),
];
const state = deepFreeze({});
const action = {
type: actionType,
data: {
data: TestHelper.getPostMock({
id: 'post',
metadata: {
reactions: [
{user_id: 'abcd', emoji_name: '+1'},
{user_id: 'efgh', emoji_name: '+1'},
{user_id: 'abcd', emoji_name: '-1'},
],
},
reactions,
},
}),
};
const nextState = reducers.reactions(state, action);
expect(nextState).not.toEqual(state);
expect(nextState).not.toBe(state);
expect(nextState).toEqual({
post: {
'abcd-+1': {user_id: 'abcd', emoji_name: '+1'},
'efgh-+1': {user_id: 'efgh', emoji_name: '+1'},
'abcd--1': {user_id: 'abcd', emoji_name: '-1'},
'abcd-+1': reactions[0],
'efgh-+1': reactions[1],
'abcd--1': reactions[2],
},
});
});
@ -3696,10 +3733,10 @@ describe('reactions', () => {
const state = deepFreeze({});
const action = {
type: actionType,
data: {
data: TestHelper.getPostMock({
id: 'post',
delete_at: '1571366424287',
},
delete_at: 1571366424287,
}),
};
const nextState = reducers.reactions(state, action);
@ -3710,150 +3747,218 @@ describe('reactions', () => {
}
describe('receiving multiple posts', () => {
it('no post metadata', () => {
it('should not store anything for a post first received without metadata', () => {
// This shouldn't occur based on our type definitions, but it is possible
const post = TestHelper.getPostMock({
id: 'post',
});
(post as any).metadata = undefined;
const state = deepFreeze({});
const action = {
type: PostTypes.RECEIVED_POSTS,
data: {
posts: {
post: {
id: 'post',
},
post,
},
},
};
const nextState = reducers.reactions(state, action);
expect(nextState).toEqual(state);
expect(state).toBe(nextState);
});
it('no reactions in post metadata', () => {
const state = deepFreeze({});
it('should not change stored state for a post received without metadata', () => {
// This shouldn't occur based on our type definitions, but it is possible
const post = TestHelper.getPostMock({
id: 'post',
});
(post as any).metadata = undefined;
const state = deepFreeze({
post: {
'user-taco': TestHelper.getReactionMock({user_id: 'abcd', emoji_name: '+1'}),
},
});
const action = {
type: PostTypes.RECEIVED_POSTS,
data: {
posts: {
post: {
id: 'post',
metadata: {reactions: []},
},
post,
},
},
};
const nextState = reducers.reactions(state, action);
expect(nextState).not.toEqual(state);
expect(state).toBe(nextState);
});
it('should store when a post is first received without reactions', () => {
const post = TestHelper.getPostMock({
id: 'post',
});
post.metadata.reactions = undefined;
const state = deepFreeze({});
const action = {
type: PostTypes.RECEIVED_POSTS,
data: {
posts: {
post,
},
},
};
const nextState = reducers.reactions(state, action);
expect(nextState).not.toBe(state);
expect(nextState).toEqual({
post: {},
});
});
it('should remove existing reactions when a post is received without reactions', () => {
const post = TestHelper.getPostMock({
id: 'post',
});
post.metadata.reactions = undefined;
const state = deepFreeze({
post: {
'user-taco': TestHelper.getReactionMock({user_id: 'abcd', emoji_name: '+1'}),
},
});
const action = {
type: PostTypes.RECEIVED_POSTS,
data: {
posts: {
post,
},
},
};
const nextState = reducers.reactions(state, action);
expect(nextState).not.toBe(state);
expect(nextState).toEqual({
post: {},
});
});
it('should save reactions', () => {
const reactions = [
TestHelper.getReactionMock({user_id: 'abcd', emoji_name: '+1'}),
TestHelper.getReactionMock({user_id: 'efgh', emoji_name: '+1'}),
TestHelper.getReactionMock({user_id: 'abcd', emoji_name: '-1'}),
];
const state = deepFreeze({});
const action = {
type: PostTypes.RECEIVED_POSTS,
data: {
posts: {
post: {
post: TestHelper.getPostMock({
id: 'post',
metadata: {
reactions: [
{user_id: 'abcd', emoji_name: '+1'},
{user_id: 'efgh', emoji_name: '+1'},
{user_id: 'abcd', emoji_name: '-1'},
],
},
reactions,
},
}),
},
},
};
const nextState = reducers.reactions(state, action);
expect(nextState).not.toEqual(state);
expect(nextState).not.toBe(state);
expect(nextState).toEqual({
post: {
'abcd-+1': {user_id: 'abcd', emoji_name: '+1'},
'efgh-+1': {user_id: 'efgh', emoji_name: '+1'},
'abcd--1': {user_id: 'abcd', emoji_name: '-1'},
'abcd-+1': reactions[0],
'efgh-+1': reactions[1],
'abcd--1': reactions[2],
},
});
});
it('should save reactions for multiple posts', () => {
const reaction1 = TestHelper.getReactionMock({user_id: 'abcd', emoji_name: '+1'});
const reaction2 = TestHelper.getReactionMock({user_id: 'abcd', emoji_name: '-1'});
const state = deepFreeze({});
const action = {
type: PostTypes.RECEIVED_POSTS,
data: {
posts: {
post1: {
post1: TestHelper.getPostMock({
id: 'post1',
metadata: {
reactions: [
{user_id: 'abcd', emoji_name: '+1'},
reaction1,
],
},
},
post2: {
}),
post2: TestHelper.getPostMock({
id: 'post2',
metadata: {
reactions: [
{user_id: 'abcd', emoji_name: '-1'},
reaction2,
],
},
},
}),
},
},
};
const nextState = reducers.reactions(state, action);
expect(nextState).not.toEqual(state);
expect(nextState).not.toBe(state);
expect(nextState).toEqual({
post1: {
'abcd-+1': {user_id: 'abcd', emoji_name: '+1'},
'abcd-+1': reaction1,
},
post2: {
'abcd--1': {user_id: 'abcd', emoji_name: '-1'},
'abcd--1': reaction2,
},
});
});
it('should save reactions for multiple posts except deleted posts', () => {
const reaction1 = TestHelper.getReactionMock({user_id: 'abcd', emoji_name: '+1'});
const reaction2 = TestHelper.getReactionMock({user_id: 'abcd', emoji_name: '-1'});
const state = deepFreeze({});
const action = {
type: PostTypes.RECEIVED_POSTS,
data: {
posts: {
post1: {
post1: TestHelper.getPostMock({
id: 'post1',
metadata: {
reactions: [
{user_id: 'abcd', emoji_name: '+1'},
reaction1,
],
},
},
post2: {
}),
post2: TestHelper.getPostMock({
id: 'post2',
delete_at: '1571366424287',
delete_at: 1571366424287,
metadata: {
reactions: [
{user_id: 'abcd', emoji_name: '-1'},
reaction2,
],
},
},
}),
},
},
};
const nextState = reducers.reactions(state, action);
expect(nextState).not.toEqual(state);
expect(nextState).not.toBe(state);
expect(nextState).toEqual({
post1: {
'abcd-+1': {user_id: 'abcd', emoji_name: '+1'},
'abcd-+1': reaction1,
},
});
});

View File

@ -158,7 +158,7 @@ export function nextPostsReplies(state: {[x in Post['id']]: number} = {}, action
}
}
export function handlePosts(state: RelationOneToOne<Post, Post> = {}, action: AnyAction) {
export function handlePosts(state: IDMappedObjects<Post> = {}, action: AnyAction) {
switch (action.type) {
case PostTypes.RECEIVED_POST:
case PostTypes.RECEIVED_NEW_POST: {
@ -263,6 +263,23 @@ export function handlePosts(state: RelationOneToOne<Post, Post> = {}, action: An
return nextState;
}
case PostTypes.POST_PINNED_CHANGED: {
const {postId, isPinned, updateAt} = action;
if (!state[postId]) {
return state;
}
return {
...state,
[postId]: {
...state[postId],
is_pinned: isPinned,
last_update_at: updateAt,
},
};
}
case ChannelTypes.RECEIVED_CHANNEL_DELETED:
case ChannelTypes.DELETE_CHANNEL_SUCCESS:
case ChannelTypes.LEAVE_CHANNEL: {
@ -1290,8 +1307,8 @@ export function acknowledgements(state: RelationOneToOne<Post, Record<UserProfil
}
}
function storeReactionsForPost(state: any, post: Post) {
if (!post.metadata || !post.metadata.reactions || post.delete_at > 0) {
function storeReactionsForPost(state: RelationOneToOne<Post, Record<string, Reaction>>, post: Post) {
if (!post.metadata || post.delete_at > 0) {
return state;
}

View File

@ -12,6 +12,7 @@ import type {FileInfo} from '@mattermost/types/files';
import type {Group} from '@mattermost/types/groups';
import type {Command, DialogElement, OAuthApp} from '@mattermost/types/integrations';
import type {Post, PostMetadata} from '@mattermost/types/posts';
import type {Reaction} from '@mattermost/types/reactions';
import type {Role} from '@mattermost/types/roles';
import type {Scheme} from '@mattermost/types/schemes';
import type {Team, TeamMembership} from '@mattermost/types/teams';
@ -723,6 +724,16 @@ class TestHelper {
};
}
getReactionMock(override: Partial<Reaction> = {}): Reaction {
return {
user_id: '',
post_id: '',
emoji_name: '',
create_at: 0,
...override,
};
}
mockLogin = () => {
const clientBaseRoute = this.basicClient4!.getBaseRoute();
nock(clientBaseRoute).

View File

@ -319,7 +319,7 @@ export class TestHelper {
return Object.assign({}, defaultOutgoingWebhook, override);
}
public static getPostMock(override: Partial<Post> = {}): Post {
public static getPostMock(override: Omit<Partial<Post>, 'metadata'> & {metadata?: Partial<Post['metadata']>} = {}): Post {
const defaultPost: Post = {
edit_at: 0,
original_id: '',
@ -345,7 +345,15 @@ export class TestHelper {
update_at: 0,
user_id: 'user_id',
};
return Object.assign({}, defaultPost, override);
return {
...defaultPost,
...override,
metadata: {
...defaultPost.metadata,
...override.metadata,
},
};
}
public static getFileInfoMock(override: Partial<FileInfo> = {}): FileInfo {

View File

@ -121,6 +121,7 @@ export type ClientConfig = {
FileLevel: string;
FeatureFlagAppsEnabled: string;
FeatureFlagCallsEnabled: string;
FeatureFlagWebSocketEventScope: string;
ForgotPasswordLink: string;
GiphySdkKey: string;
GoogleDeveloperKey: string;

View File

@ -66,7 +66,7 @@ export type PostMetadata = {
emojis: CustomEmoji[];
files: FileInfo[];
images: Record<string, PostImage>;
reactions: Reaction[];
reactions?: Reaction[];
priority?: PostPriorityMetadata;
acknowledgements?: PostAcknowledgement[];
};