Skip to content

Commit

Permalink
model: Factor out new RESET_ACCOUNT_DATA action
Browse files Browse the repository at this point in the history
As Greg proposed in #4446, so the pattern of consistently clearing
data on all ways of leaving an account gets maintained naturally:

(except we chose the name RESET_ACCOUNT_DATA instead of
CLEAR_ACCOUNT_DATA)

> * I think it'd be a useful refactor to consolidate one action type
>   like `CLEAR_ACCOUNT_DATA`.
>   * The action creators for these three (in `accountActions.js`) can
>     dispatch that first, then also a `LOGOUT` etc.
>   * Almost all the reducers would respond only to the
>     `CLEAR_ACCOUNT_DATA`.
>   * There are a couple of exceptions, like `accountReducers` and
>     `sessionReducers`, that actually want to treat the three cases
>     differently. Those would be the only ones to continue responding
>     to `LOGOUT` etc.; and they'd stand out better as a result.
>   * Then there are further changes we might make to those, but
>     that'll be easier to see after that.

Fixes: #4446
  • Loading branch information
chrisbobbe committed Dec 14, 2022
1 parent 4736d8f commit b6218db
Show file tree
Hide file tree
Showing 44 changed files with 128 additions and 231 deletions.
3 changes: 3 additions & 0 deletions src/__tests__/lib/exampleData.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { CreateWebPublicStreamPolicy, EmailAddressVisibility } from '../../api/p
import type {
AccountSwitchAction,
LoginSuccessAction,
ResetAccountDataAction,
RegisterCompleteAction,
MessageFetchStartAction,
MessageFetchCompleteAction,
Expand All @@ -44,6 +45,7 @@ import { ZulipVersion } from '../../utils/zulipVersion';
import {
ACCOUNT_SWITCH,
LOGIN_SUCCESS,
RESET_ACCOUNT_DATA,
REGISTER_COMPLETE,
EVENT_NEW_MESSAGE,
MESSAGE_FETCH_START,
Expand Down Expand Up @@ -705,6 +707,7 @@ export const action = Object.freeze({
email: selfAccount.email,
apiKey: selfAccount.apiKey,
}): LoginSuccessAction),
reset_account_data: (deepFreeze({ type: RESET_ACCOUNT_DATA }): ResetAccountDataAction),

/**
* A minimal well-typed REGISTER_COMPLETE action from a recent server.
Expand Down
16 changes: 16 additions & 0 deletions src/account/accountActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { registerAndStartPolling } from '../events/eventActions';
import { resetToMainTabs } from '../nav/navActions';
import { sendOutbox } from '../outbox/outboxActions';
import { initNotifications } from '../notification/notifTokens';
import { resetAccountData } from './logoutActions';

export const dismissServerPushSetupNotice = (): PerAccountAction => ({
type: DISMISS_SERVER_PUSH_SETUP_NOTICE,
Expand All @@ -30,6 +31,13 @@ export const accountSwitch =
(index: number): GlobalThunkAction<Promise<void>> =>
async (dispatch, getState, { activeAccountDispatch }) => {
NavigationService.dispatch(resetToMainTabs());

// Clear out the space we use for the active account's server data, to
// make way for a new active account.
// TODO(#5006): When each account has its own space to hold server data,
// we won't have to do this.
activeAccountDispatch(resetAccountData());

dispatch(accountSwitchPlain(index));

// Now dispatch some actions on the new, post-switch active account.
Expand Down Expand Up @@ -60,6 +68,14 @@ export const loginSuccess =
(realm: URL, email: string, apiKey: string): GlobalThunkAction<Promise<void>> =>
async (dispatch, getState, { activeAccountDispatch }) => {
NavigationService.dispatch(resetToMainTabs());

// In case there's already an active account, clear out the space we use
// for the active account's server data, to make way for a new active
// account.
// TODO(#5006): When each account has its own space to hold server data,
// we won't have to do this.
activeAccountDispatch(resetAccountData());

dispatch(loginSuccessPlain(realm, email, apiKey));

// Now dispatch some actions on the new, post-login active account.
Expand Down
13 changes: 11 additions & 2 deletions src/account/logoutActions.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
/* @flow strict-local */
import * as NavigationService from '../nav/NavigationService';
import type { AllAccountsAction, ThunkAction } from '../types';
import { LOGOUT } from '../actionConstants';
import type { PerAccountAction, AllAccountsAction, ThunkAction } from '../types';
import { LOGOUT, RESET_ACCOUNT_DATA } from '../actionConstants';
import { resetToAccountPicker } from '../nav/navActions';

/**
* Reset per-account server data and some client-side data (drafts/outbox).
*/
// In this file just to prevent import cycles.
export const resetAccountData = (): PerAccountAction => ({
type: RESET_ACCOUNT_DATA,
});

const logoutPlain = (): AllAccountsAction => ({
type: LOGOUT,
});
Expand All @@ -14,5 +22,6 @@ const logoutPlain = (): AllAccountsAction => ({
// In its own file just to prevent import cycles.
export const logout = (): ThunkAction<Promise<void>> => async (dispatch, getState) => {
NavigationService.dispatch(resetToAccountPicker());
dispatch(resetAccountData());
dispatch(logoutPlain());
};
2 changes: 2 additions & 0 deletions src/actionConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ export const REGISTER_START: 'REGISTER_START' = 'REGISTER_START';
export const REGISTER_ABORT: 'REGISTER_ABORT' = 'REGISTER_ABORT';
export const REGISTER_COMPLETE: 'REGISTER_COMPLETE' = 'REGISTER_COMPLETE';

export const RESET_ACCOUNT_DATA: 'RESET_ACCOUNT_DATA' = 'RESET_ACCOUNT_DATA';

export const REFRESH_SERVER_EMOJI_DATA: 'REFRESH_SERVER_EMOJI_DATA' = 'REFRESH_SERVER_EMOJI_DATA';

export const DISMISS_SERVER_PUSH_SETUP_NOTICE: 'DISMISS_SERVER_PUSH_SETUP_NOTICE' =
Expand Down
7 changes: 7 additions & 0 deletions src/actionTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
ACCOUNT_REMOVE,
LOGIN_SUCCESS,
LOGOUT,
RESET_ACCOUNT_DATA,
DISMISS_SERVER_PUSH_SETUP_NOTICE,
GOT_PUSH_TOKEN,
UNACK_PUSH_TOKEN,
Expand Down Expand Up @@ -170,6 +171,10 @@ type LogoutAction = $ReadOnly<{|
type: typeof LOGOUT,
|}>;

export type ResetAccountDataAction = $ReadOnly<{|
type: typeof RESET_ACCOUNT_DATA,
|}>;

type DismissServerPushSetupNoticeAction = $ReadOnly<{|
type: typeof DISMISS_SERVER_PUSH_SETUP_NOTICE,
date: Date,
Expand Down Expand Up @@ -646,6 +651,7 @@ export type PerAccountAction =
// The grouping here is completely arbitrary; don't worry about it.
| EventAction
| LoadingAction
| ResetAccountDataAction
| RefreshServerEmojiDataAction
| MessageAction
| OutboxAction
Expand Down Expand Up @@ -739,6 +745,7 @@ export type DispatchableWithoutAccountAction =
/** True just if the action is a PerAccountApplicableAction. */
export function isPerAccountApplicableAction(action: Action): boolean {
switch (action.type) {
case RESET_ACCOUNT_DATA:
case EVENT:
case EVENT_ALERT_WORDS:
case EVENT_MESSAGE_DELETE:
Expand Down
12 changes: 2 additions & 10 deletions src/alertWords/alertWordsReducer.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
/* @flow strict-local */
import type { AlertWordsState, PerAccountApplicableAction } from '../types';
import {
REGISTER_COMPLETE,
EVENT_ALERT_WORDS,
ACCOUNT_SWITCH,
LOGOUT,
LOGIN_SUCCESS,
} from '../actionConstants';
import { REGISTER_COMPLETE, EVENT_ALERT_WORDS, RESET_ACCOUNT_DATA } from '../actionConstants';
import { NULL_ARRAY } from '../nullObjects';

const initialState = NULL_ARRAY;
Expand All @@ -16,9 +10,7 @@ export default (
action: PerAccountApplicableAction,
): AlertWordsState => {
switch (action.type) {
case LOGOUT:
case ACCOUNT_SWITCH:
case LOGIN_SUCCESS:
case RESET_ACCOUNT_DATA:
return initialState;

case REGISTER_COMPLETE:
Expand Down
8 changes: 2 additions & 6 deletions src/caughtup/caughtUpReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,12 @@
import type { CaughtUpState, PerAccountApplicableAction } from '../types';
import {
REGISTER_COMPLETE,
LOGOUT,
LOGIN_SUCCESS,
ACCOUNT_SWITCH,
MESSAGE_FETCH_START,
MESSAGE_FETCH_ERROR,
MESSAGE_FETCH_COMPLETE,
EVENT_UPDATE_MESSAGE,
EVENT_UPDATE_MESSAGE_FLAGS,
RESET_ACCOUNT_DATA,
} from '../actionConstants';
import { NULL_OBJECT } from '../nullObjects';
import { DEFAULT_CAUGHTUP } from './caughtUpSelectors';
Expand All @@ -35,9 +33,7 @@ export default (
): CaughtUpState => {
switch (action.type) {
case REGISTER_COMPLETE:
case LOGOUT:
case LOGIN_SUCCESS:
case ACCOUNT_SWITCH:
case RESET_ACCOUNT_DATA:
return initialState;

case MESSAGE_FETCH_START: {
Expand Down
6 changes: 3 additions & 3 deletions src/chat/__tests__/flagsReducer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import deepFreeze from 'deep-freeze';

import * as eg from '../../__tests__/lib/exampleData';
import flagsReducer from '../flagsReducer';
import { EVENT_UPDATE_MESSAGE_FLAGS, ACCOUNT_SWITCH } from '../../actionConstants';
import { EVENT_UPDATE_MESSAGE_FLAGS } from '../../actionConstants';

describe('flagsReducer', () => {
describe('MESSAGE_FETCH_COMPLETE', () => {
Expand Down Expand Up @@ -270,7 +270,7 @@ describe('flagsReducer', () => {
});
});

describe('ACCOUNT_SWITCH', () => {
describe('RESET_ACCOUNT_DATA', () => {
test('resets to initial state', () => {
const message = eg.streamMessage();

Expand All @@ -284,7 +284,7 @@ describe('flagsReducer', () => {
historical: { [message.id]: true },
});

expect(flagsReducer(prevState, deepFreeze({ type: ACCOUNT_SWITCH, index: 2 }))).toEqual(
expect(flagsReducer(prevState, eg.action.reset_account_data)).toEqual(
eg.baseReduxState.flags,
);
});
Expand Down
8 changes: 2 additions & 6 deletions src/chat/fetchingReducer.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
/* @flow strict-local */
import type { FetchingState, PerAccountApplicableAction } from '../types';
import {
LOGOUT,
LOGIN_SUCCESS,
ACCOUNT_SWITCH,
MESSAGE_FETCH_START,
MESSAGE_FETCH_ERROR,
MESSAGE_FETCH_COMPLETE,
RESET_ACCOUNT_DATA,
} from '../actionConstants';
import { NULL_OBJECT } from '../nullObjects';
import { DEFAULT_FETCHING } from './fetchingSelectors';
Expand Down Expand Up @@ -68,9 +66,7 @@ export default (
action: PerAccountApplicableAction,
): FetchingState => {
switch (action.type) {
case LOGOUT:
case LOGIN_SUCCESS:
case ACCOUNT_SWITCH:
case RESET_ACCOUNT_DATA:
return initialState;

case MESSAGE_FETCH_START:
Expand Down
8 changes: 2 additions & 6 deletions src/chat/flagsReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ import {
MESSAGE_FETCH_COMPLETE,
EVENT_NEW_MESSAGE,
EVENT_UPDATE_MESSAGE_FLAGS,
LOGOUT,
LOGIN_SUCCESS,
ACCOUNT_SWITCH,
RESET_ACCOUNT_DATA,
} from '../actionConstants';
import { deeperMerge } from '../utils/misc';
import type { UserMessageFlag } from '../api/modelTypes';
Expand Down Expand Up @@ -134,9 +132,7 @@ export default (
): FlagsState => {
switch (action.type) {
case REGISTER_COMPLETE:
case LOGOUT:
case LOGIN_SUCCESS:
case ACCOUNT_SWITCH:
case RESET_ACCOUNT_DATA:
return initialState;

case MESSAGE_FETCH_COMPLETE:
Expand Down
8 changes: 2 additions & 6 deletions src/chat/narrowsReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,14 @@ import type { NarrowsState, PerAccountApplicableAction } from '../types';
import { ensureUnreachable } from '../types';
import {
REGISTER_COMPLETE,
LOGOUT,
LOGIN_SUCCESS,
ACCOUNT_SWITCH,
MESSAGE_FETCH_START,
MESSAGE_FETCH_ERROR,
MESSAGE_FETCH_COMPLETE,
EVENT_NEW_MESSAGE,
EVENT_MESSAGE_DELETE,
EVENT_UPDATE_MESSAGE_FLAGS,
EVENT_UPDATE_MESSAGE,
RESET_ACCOUNT_DATA,
} from '../actionConstants';
import { LAST_MESSAGE_ANCHOR, FIRST_UNREAD_ANCHOR } from '../anchor';
import {
Expand Down Expand Up @@ -197,9 +195,7 @@ export default (
): NarrowsState => {
switch (action.type) {
case REGISTER_COMPLETE:
case LOGOUT:
case LOGIN_SUCCESS:
case ACCOUNT_SWITCH:
case RESET_ACCOUNT_DATA:
return initialState;

case MESSAGE_FETCH_START: {
Expand Down
6 changes: 2 additions & 4 deletions src/drafts/draftsReducer.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* @flow strict-local */
import type { DraftsState, PerAccountApplicableAction } from '../types';
import { DRAFT_UPDATE, LOGOUT, ACCOUNT_SWITCH, LOGIN_SUCCESS } from '../actionConstants';
import { DRAFT_UPDATE, RESET_ACCOUNT_DATA } from '../actionConstants';
import { NULL_OBJECT } from '../nullObjects';
import { keyFromNarrow } from '../utils/narrow';

Expand All @@ -27,9 +27,7 @@ export default (
action: PerAccountApplicableAction,
): DraftsState => {
switch (action.type) {
case LOGOUT:
case ACCOUNT_SWITCH:
case LOGIN_SUCCESS:
case RESET_ACCOUNT_DATA:
return initialState;

case DRAFT_UPDATE:
Expand Down
8 changes: 2 additions & 6 deletions src/message/messagesReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,14 @@ import type {
} from '../types';
import {
REGISTER_COMPLETE,
LOGOUT,
LOGIN_SUCCESS,
ACCOUNT_SWITCH,
MESSAGE_FETCH_COMPLETE,
EVENT_NEW_MESSAGE,
EVENT_SUBMESSAGE,
EVENT_MESSAGE_DELETE,
EVENT_REACTION_ADD,
EVENT_REACTION_REMOVE,
EVENT_UPDATE_MESSAGE,
RESET_ACCOUNT_DATA,
} from '../actionConstants';
import { getNarrowsForMessage } from '../utils/narrow';
import * as logging from '../utils/logging';
Expand Down Expand Up @@ -142,9 +140,7 @@ export default (
): MessagesState => {
switch (action.type) {
case REGISTER_COMPLETE:
case LOGOUT:
case LOGIN_SUCCESS:
case ACCOUNT_SWITCH:
case RESET_ACCOUNT_DATA:
return initialState;

case MESSAGE_FETCH_COMPLETE:
Expand Down
4 changes: 2 additions & 2 deletions src/mute/__tests__/muteModel-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ describe('reducer', () => {
});
});

describe('ACCOUNT_SWITCH', () => {
describe('RESET_ACCOUNT_DATA', () => {
test('resets state to initial state', () => {
const state = makeMuteState([[eg.stream, 'some_topic']]);
expect(reducer(state, eg.action.account_switch, eg.plusReduxState)).toEqual(initialState);
expect(reducer(state, eg.action.reset_account_data, eg.plusReduxState)).toEqual(initialState);
});
});

Expand Down
4 changes: 2 additions & 2 deletions src/mute/__tests__/mutedUsersReducer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ describe('mutedUsersReducer', () => {
});
});

describe('ACCOUNT_SWITCH', () => {
describe('RESET_ACCOUNT_DATA', () => {
test('resets state to initial state', () => {
expect(mutedUsersReducer(baseState, eg.action.account_switch)).toEqual(Immutable.Map());
expect(mutedUsersReducer(baseState, eg.action.reset_account_data)).toEqual(Immutable.Map());
});
});

Expand Down
12 changes: 2 additions & 10 deletions src/mute/muteModel.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,7 @@
/* @flow strict-local */

import type { MuteState, PerAccountApplicableAction, PerAccountState } from '../types';
import {
REGISTER_COMPLETE,
LOGOUT,
ACCOUNT_SWITCH,
EVENT_MUTED_TOPICS,
LOGIN_SUCCESS,
} from '../actionConstants';
import { REGISTER_COMPLETE, EVENT_MUTED_TOPICS, RESET_ACCOUNT_DATA } from '../actionConstants';
import { getStreamsByName } from '../subscriptions/subscriptionSelectors';
import * as logging from '../utils/logging';
import DefaultMap from '../utils/DefaultMap';
Expand Down Expand Up @@ -53,9 +47,7 @@ export const reducer = (
globalState: PerAccountState,
): MuteState => {
switch (action.type) {
case LOGOUT:
case ACCOUNT_SWITCH:
case LOGIN_SUCCESS:
case RESET_ACCOUNT_DATA:
return initialState;

case REGISTER_COMPLETE:
Expand Down
Loading

0 comments on commit b6218db

Please sign in to comment.