Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ module.exports = {

// Finding and transforming source code.

testPathIgnorePatterns: ['/node_modules/', '/src/__tests__/lib/'],
testPathIgnorePatterns: ['/node_modules/', '/src/__tests__/lib/', '-testlib.js$'],

// When some source file foo.js says `import 'bar'`, Jest looks in the
// directories above foo.js for a directory like `node_modules` to find
Expand Down
16 changes: 4 additions & 12 deletions src/boot/reducers.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/* @flow strict-local */
import { combineReducers } from 'redux';
import type { CombinedReducer, Reducer } from 'redux';
import type { CombinedReducer } from 'redux';
import { enableBatching } from 'redux-batched-actions';

import config from '../config';
import { logSlowReducers } from '../utils/redux';
import { NULL_OBJECT } from '../nullObjects';
import type { Action, GlobalState, MigrationsState, UnreadState } from '../types';
import type { Action, GlobalState, MigrationsState } from '../types';

import accounts from '../account/accountsReducer';
import alertWords from '../alertWords/alertWordsReducer';
Expand All @@ -27,10 +27,7 @@ import streams from '../streams/streamsReducer';
import subscriptions from '../subscriptions/subscriptionsReducer';
import topics from '../topics/topicsReducer';
import typing from '../typing/typingReducer';
import unreadHuddles from '../unread/unreadHuddlesReducer';
import unreadMentions from '../unread/unreadMentionsReducer';
import unreadPms from '../unread/unreadPmsReducer';
import unreadStreams from '../unread/unreadStreamsReducer';
import { reducer as unread } from '../unread/unreadModel';
import userGroups from '../user-groups/userGroupsReducer';
import userStatus from '../user-status/userStatusReducer';
import users from '../users/usersReducer';
Expand All @@ -56,12 +53,7 @@ const reducers = {
subscriptions,
topics,
typing,
unread: (combineReducers({
streams: unreadStreams,
pms: unreadPms,
huddles: unreadHuddles,
mentions: unreadMentions,
}): Reducer<UnreadState, Action>),
unread,
userGroups,
userStatus,
users,
Expand Down
12 changes: 0 additions & 12 deletions src/directSelectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@ import type {
RealmEmojiById,
RealmState,
SettingsState,
StreamUnreadItem,
TypingState,
UnreadHuddlesState,
UnreadMentionsState,
UnreadPmsState,
Account,
Debug,
Subscription,
Expand Down Expand Up @@ -88,14 +84,6 @@ export const getPresence = (state: GlobalState): PresenceState => state.presence

export const getOutbox = (state: GlobalState): Outbox[] => state.outbox;

export const getUnreadStreams = (state: GlobalState): StreamUnreadItem[] => state.unread.streams;

export const getUnreadPms = (state: GlobalState): UnreadPmsState => state.unread.pms;

export const getUnreadHuddles = (state: GlobalState): UnreadHuddlesState => state.unread.huddles;

export const getUnreadMentions = (state: GlobalState): UnreadMentionsState => state.unread.mentions;

export const getRealm = (state: GlobalState): RealmState => state.realm;

export const getCrossRealmBots = (state: GlobalState): CrossRealmBot[] =>
Expand Down
24 changes: 1 addition & 23 deletions src/reduxTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,12 @@ import type { Account, Outbox } from './types';
import type { Action } from './actionTypes';
import type {
Topic,
HuddlesUnreadItem,
Message,
MuteTuple,
PmsUnreadItem,
CrossRealmBot,
RealmEmojiById,
RealmFilter,
Stream,
StreamUnreadItem,
Subscription,
User,
UserGroup,
Expand All @@ -32,6 +29,7 @@ import type {
import type { Narrow } from './utils/narrow';
import type { SessionState } from './session/sessionReducer';
import type { PmConversationsState } from './pm-conversations/pmConversationsModel';
import type { UnreadState } from './unread/unreadModelTypes';

export type * from './actionTypes';

Expand Down Expand Up @@ -301,26 +299,6 @@ export type TypingState = {
},
};

// These four are fragments of UnreadState; see below.
export type UnreadStreamsState = StreamUnreadItem[];
export type UnreadHuddlesState = HuddlesUnreadItem[];
export type UnreadPmsState = PmsUnreadItem[];
export type UnreadMentionsState = number[];

/**
* A summary of (almost) all unread messages, even those we don't have.
*
* The initial version the server gives us for this data is `unread_msgs` in
* the `/register` initial state, and we largely follow the structure of
* that. See there (in `src/api/initialDataTypes.js`) for details.
*/
export type UnreadState = {|
streams: UnreadStreamsState,
huddles: UnreadHuddlesState,
pms: UnreadPmsState,
mentions: UnreadMentionsState,
|};

export type UserGroupsState = UserGroup[];

export type UserStatusState = UserStatusMapObject;
Expand Down
35 changes: 12 additions & 23 deletions src/topics/__tests__/topicsSelectors-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import Immutable from 'immutable';

import { getTopicsForNarrow, getLastMessageTopic, getTopicsForStream } from '../topicSelectors';
import { HOME_NARROW, streamNarrow, keyFromNarrow } from '../../utils/narrow';
import { reducer as unreadReducer } from '../../unread/unreadModel';
import * as eg from '../../__tests__/lib/exampleData';
import { mkMessageAction } from '../../unread/__tests__/unread-testlib';

describe('getTopicsForNarrow', () => {
test('when no topics return an empty list', () => {
Expand Down Expand Up @@ -67,10 +69,6 @@ describe('getTopicsForStream', () => {
streams: [],
topics: {},
mute: [],
unread: {
...eg.baseReduxState.unread,
streams: [],
},
});

const topics = getTopicsForStream(state, 123);
Expand All @@ -86,10 +84,6 @@ describe('getTopicsForStream', () => {
[stream.stream_id]: [{ name: 'topic', max_id: 456 }],
},
mute: [],
unread: {
...eg.baseReduxState.unread,
streams: [],
},
});

const topics = getTopicsForStream(state, stream.stream_id);
Expand All @@ -112,21 +106,16 @@ describe('getTopicsForStream', () => {
],
},
mute: [['stream 1', 'topic 1'], ['stream 1', 'topic 3'], ['stream 2', 'topic 2']],
unread: {
...eg.baseReduxState.unread,
streams: [
{
stream_id: 1,
topic: 'topic 2',
unread_message_ids: [1, 5, 6],
},
{
stream_id: 1,
topic: 'topic 4',
unread_message_ids: [7, 8],
},
],
},
unread: [
eg.streamMessage({ stream_id: 1, subject: 'topic 2', id: 1 }),
eg.streamMessage({ stream_id: 1, subject: 'topic 2', id: 5 }),
eg.streamMessage({ stream_id: 1, subject: 'topic 2', id: 6 }),
eg.streamMessage({ stream_id: 1, subject: 'topic 4', id: 7 }),
eg.streamMessage({ stream_id: 1, subject: 'topic 4', id: 8 }),
].reduce(
(st, message) => unreadReducer(st, mkMessageAction(message)),
eg.baseReduxState.unread,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would the initialState export from unread-testlib.js do just as well as eg.baseReduxState.unread, or should we stick with the latter?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes. I think it was as I was fixing up this test file (after writing most of the other changes) that I realized that eg.baseReduxState.unread should be exactly equivalent to that initialState export -- after all, the point of the way we construct that initialState, with { type: eg.randString() }, is that it exercises exactly the same path through the reducer that Redux uses to initialize the state. (And eg.baseReduxState comes via Redux's createStore.)

I'm not entirely sure what the most helpful style is for referring to that state.

  • Here, eg.baseReduxState.unread feels natural because it fits in with the rest of this global state we're assembling.
  • In the unread-model tests (including unreadSelectors-test.js), initialState or initialUnreadState feels natural because it makes it clear that it's local -- these are the tests of this very code, so they shouldn't need to appeal to some central library to get an instance of this state.

I'm inclined to leave the style different between these places at least for now, and perhaps in the future we'll settle on one of them.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm inclined to leave the style different between these places at least for now, and perhaps in the future we'll settle on one of them.

Sounds good; I don't have strong feelings either way.

),
});
const expected = [
{ name: 'topic 1', max_id: 5, isMuted: true, unreadCount: 0 },
Expand Down
14 changes: 3 additions & 11 deletions src/topics/topicSelectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,15 @@
import { createSelector } from 'reselect';

import type {
MuteState,
Narrow,
GlobalState,
Selector,
Stream,
StreamsState,
StreamUnreadItem,
Topic,
TopicExtended,
TopicsState,
} from '../types';
import { getMute, getStreams, getTopics, getUnreadStreams } from '../directSelectors';
import { getMute, getStreams, getTopics } from '../directSelectors';
import { getUnreadStreams } from '../unread/unreadModel';
import { getShownMessagesForNarrow } from '../chat/narrowsSelectors';
import { getStreamsById } from '../subscriptions/subscriptionSelectors';
import { NULL_ARRAY } from '../nullObjects';
Expand Down Expand Up @@ -48,12 +45,7 @@ export const getTopicsForStream: Selector<?(TopicExtended[]), number> = createSe
state => getMute(state),
(state, streamId) => getStreamsById(state).get(streamId),
state => getUnreadStreams(state),
(
topicList: Topic[],
mute: MuteState,
stream: Stream | void,
unreadStreams: StreamUnreadItem[],
) => {
(topicList, mute, stream, unreadStreams) => {
if (!topicList || !stream) {
return undefined;
}
Expand Down
46 changes: 46 additions & 0 deletions src/unread/__tests__/unread-testlib.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/* @flow strict-local */

import type { Message } from '../../types';
import { reducer } from '../unreadModel';
import * as eg from '../../__tests__/lib/exampleData';

export const initialState = reducer(undefined, ({ type: eg.randString() }: $FlowFixMe));

export const mkMessageAction = (message: Message) => ({
...eg.eventNewMessageActionBase,
message: { ...message, flags: message.flags ?? [] },
});

export const stream0 = { ...eg.makeStream({ name: 'stream 0' }), stream_id: 0 };
export const stream2 = { ...eg.makeStream({ name: 'stream 2' }), stream_id: 2 };

const [user0, user1, user2, user3, user4, user5] = [0, 1, 2, 3, 4, 5].map(user_id =>
eg.makeUser({ user_id }),
);

export const selectorBaseState = (() => {
let state = initialState;
for (const message of [
eg.streamMessage({ stream_id: 0, subject: 'a topic', id: 1, flags: ['mentioned'] }),
eg.streamMessage({ stream_id: 0, subject: 'a topic', id: 2, flags: ['mentioned'] }),
eg.streamMessage({ stream_id: 0, subject: 'a topic', id: 3, flags: ['mentioned'] }),
eg.streamMessage({ stream_id: 0, subject: 'another topic', id: 4 }),
eg.streamMessage({ stream_id: 0, subject: 'another topic', id: 5 }),
eg.streamMessage({ stream_id: 2, subject: 'some other topic', id: 6 }),
eg.streamMessage({ stream_id: 2, subject: 'some other topic', id: 7 }),
// We take user1 to be self.
eg.pmMessageFromTo(user0, [user1], { id: 11 }),
eg.pmMessageFromTo(user0, [user1], { id: 12 }),
eg.pmMessageFromTo(user2, [user1], { id: 13 }),
eg.pmMessageFromTo(user2, [user1], { id: 14 }),
eg.pmMessageFromTo(user2, [user1], { id: 15 }),
eg.pmMessageFromTo(user2, [user1, user3], { id: 21 }),
eg.pmMessageFromTo(user2, [user1, user3], { id: 22 }),
eg.pmMessageFromTo(user4, [user1, user5], { id: 23 }),
eg.pmMessageFromTo(user4, [user1, user5], { id: 24 }),
eg.pmMessageFromTo(user4, [user1, user5], { id: 25 }),
]) {
state = reducer(state, mkMessageAction(message));
}
return state;
})();
Loading