Skip to content

Commit

Permalink
unreads: Don't add messages with "read" flag to unreads.
Browse files Browse the repository at this point in the history
Previously, the main (only?) case where we wouldn't add a message to
unreads was if we were the one who sent it. This check was not correct -
instead, we want to check to see if the message has the "read" flag,
which is set by the server.

In the past, these checks were very close to the same (with the
exception of "Notification Bot" announcing a stream that you created,
which would be "read" for you), but with the addition of muted users,
the check is now incorrect.

This commit removes the logic to not mark a message as read when we are
the one who sent it, and instead just looks at the server flag, since
that should be the canonical place for that information to be.
  • Loading branch information
WesleyAC committed May 1, 2021
1 parent 3682ee4 commit 5076aae
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 10 deletions.
5 changes: 3 additions & 2 deletions src/unread/__tests__/unreadHuddlesReducer-test.js
Expand Up @@ -118,16 +118,17 @@ describe('unreadHuddlesReducer', () => {
expect(actualState).toBe(initialState);
});

test('if message is sent by self, do not mutate state', () => {
test('if message has "read" flag, do not mutate state', () => {
const selfUser = { ...eg.selfUser, user_id: makeUserId(1) };
const user2 = { ...eg.otherUser, user_id: makeUserId(2) };
const user3 = { ...eg.thirdUser, user_id: makeUserId(3) };

const initialState = deepFreeze([]);

const message2 = eg.pmMessage({
sender: selfUser,
sender: user2,
recipients: [selfUser, user2, user3],
flags: ['read'],
});

const action = deepFreeze({
Expand Down
16 changes: 16 additions & 0 deletions src/unread/__tests__/unreadMentionsReducer-test.js
Expand Up @@ -67,6 +67,22 @@ describe('unreadMentionsReducer', () => {
expect(actualState).toBe(initialState);
});

test('if message has "read" flag, do not mutate state', () => {
const initialState = deepFreeze([]);

const action = deepFreeze({
type: EVENT_NEW_MESSAGE,
message: {
id: 2,
flags: ['mentioned', 'read'],
},
});

const actualState = unreadMentionsReducer(initialState, action);

expect(actualState).toBe(initialState);
});

test('if message id already exists, do not mutate state', () => {
const initialState = deepFreeze([1, 2]);

Expand Down
4 changes: 2 additions & 2 deletions src/unread/__tests__/unreadModel-test.js
Expand Up @@ -84,10 +84,10 @@ describe('stream substate', () => {
expect(state.streams).toBe(baseState.streams);
});

test('if message is sent by self, do not mutate state', () => {
test('if message has "read" flag, do not mutate state', () => {
const state = reducer(
baseState,
action(eg.streamMessage({ sender: eg.selfUser })),
action(eg.streamMessage({ sender: eg.selfUser, flags: ['read'] })),
eg.plusReduxState,
);
expect(state).toBe(baseState);
Expand Down
5 changes: 3 additions & 2 deletions src/unread/__tests__/unreadPmsReducer-test.js
Expand Up @@ -111,11 +111,12 @@ describe('unreadPmsReducer', () => {
expect(actualState).toBe(initialState);
});

test('if message is sent by self, do not mutate state', () => {
test('if message is marked read, do not mutate state', () => {
const initialState = deepFreeze([]);
const message1 = eg.pmMessage({
sender: eg.selfUser,
sender: eg.otherUser,
recipients: [eg.otherUser, eg.selfUser],
flags: ['read'],
});

const action = deepFreeze({
Expand Down
2 changes: 1 addition & 1 deletion src/unread/unreadHuddlesReducer.js
Expand Up @@ -24,7 +24,7 @@ const eventNewMessage = (state, action) => {
return state;
}

if (action.ownUserId === action.message.sender_id) {
if (action.message.flags?.includes('read')) {
return state;
}

Expand Down
3 changes: 3 additions & 0 deletions src/unread/unreadMentionsReducer.js
Expand Up @@ -46,6 +46,9 @@ export default (state: UnreadMentionsState = initialState, action: Action): Unre
if (!flags) {
throw new Error('action.message.flags should be defined.');
}
if (flags.includes('read')) {
return state;
}
return (flags.includes('mentioned') || flags.includes('wildcard_mentioned'))
&& !state.includes(action.message.id)
? addItemsToArray(state, [action.message.id])
Expand Down
3 changes: 1 addition & 2 deletions src/unread/unreadModel.js
Expand Up @@ -22,7 +22,6 @@ import {
MESSAGE_FETCH_COMPLETE,
REALM_INIT,
} from '../actionConstants';
import { getOwnUserId } from '../users/userSelectors';

//
//
Expand Down Expand Up @@ -146,7 +145,7 @@ function streamsReducer(
return state;
}

if (message.sender_id === getOwnUserId(globalState)) {
if (message.flags?.includes('read')) {
return state;
}

Expand Down
2 changes: 1 addition & 1 deletion src/unread/unreadPmsReducer.js
Expand Up @@ -24,7 +24,7 @@ const eventNewMessage = (state, action) => {
return state;
}

if (action.ownUserId === action.message.sender_id) {
if (action.message.flags?.includes('read')) {
return state;
}

Expand Down

0 comments on commit 5076aae

Please sign in to comment.