Skip to content

Commit 23e3b4c

Browse files
sharabaisharabai
committed
Address PR review comments
1 parent adaae03 commit 23e3b4c

3 files changed

Lines changed: 47 additions & 27 deletions

File tree

‎src/pages/NewChatPage/index.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ import type {SelectedParticipant} from '@src/types/onyx/NewGroupChatDraft';
4545
import getEmptyArray from '@src/types/utils/getEmptyArray';
4646
import KeyboardUtils from '@src/utils/keyboard';
4747
import type SelectedOption from './types';
48-
import useGroupDraftRestore from './useGroupDraftRestore';
48+
import useGroupChatDraftParticipantSync from './useGroupDraftRestore';
4949

5050
const excludedGroupEmails = new Set<string>(CONST.EXPENSIFY_EMAILS.filter((value) => value !== CONST.EMAIL.CONCIERGE));
5151

@@ -88,7 +88,7 @@ function useOptions(reportAttributesDerived: ReportAttributesDerivedValue['repor
8888

8989
const reports = listOptions?.reports ?? [];
9090
const personalDetails = listOptions?.personalDetails ?? [];
91-
useGroupDraftRestore(personalDetails, !isLoading, allPersonalDetails, loginList, currentUserEmail, currentUserAccountID, selectedOptions, setSelectedOptions);
91+
useGroupChatDraftParticipantSync(personalDetails, !isLoading, allPersonalDetails, loginList, currentUserEmail, currentUserAccountID, selectedOptions, setSelectedOptions);
9292

9393
const defaultOptions = getValidOptions(
9494
{

‎src/pages/NewChatPage/useGroupDraftRestore.ts‎

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,16 @@ import type NewGroupChatDraft from '@src/types/onyx/NewGroupChatDraft';
1010
import isLoadingOnyxValue from '@src/types/utils/isLoadingOnyxValue';
1111
import type SelectedOption from './types';
1212

13-
function useGroupDraftRestore(
13+
/**
14+
* Keeps the NewChatPage's `selectedOptions` state aligned with the `NEW_GROUP_CHAT_DRAFT` Onyx draft.
15+
*
16+
* - On mount / reload, restores the draft participants into `selectedOptions` once so an
17+
* in-progress group chat survives refreshes.
18+
* - While the screen is backgrounded (e.g. the user navigated to NewChatConfirmPage), mirrors
19+
* participant removals made against the draft back into `selectedOptions` so the two stay
20+
* consistent when the user returns.
21+
*/
22+
function useGroupChatDraftParticipantSync(
1423
allPersonalDetailOptions: Array<SearchOption<PersonalDetails>>,
1524
areAllPersonalDetailOptionsLoaded: boolean,
1625
allPersonalDetails: OnyxEntry<PersonalDetailsList>,
@@ -25,21 +34,18 @@ function useGroupDraftRestore(
2534

2635
const draftParticipantsSelector = (draft: NewGroupChatDraft | undefined) => {
2736
const isSubscriptionActive = shouldRestoreSelectedOptionsRef.current || isScreenInBackgroundRef.current;
28-
if (!isSubscriptionActive) {
29-
return undefined;
30-
}
31-
return draft?.participants;
37+
return isSubscriptionActive ? draft?.participants : undefined;
3238
};
3339

3440
const [draftParticipants, draftParticipantsMetadata] = useOnyx(ONYXKEYS.NEW_GROUP_CHAT_DRAFT, {
3541
selector: draftParticipantsSelector,
3642
});
3743

38-
const restoreFromDraft = useEffectEvent(() => {
44+
const restoreParticipantsFromDraft = useEffectEvent(() => {
3945
// Flip the ref first so the useOnyx selector disables the subscription
4046
shouldRestoreSelectedOptionsRef.current = false;
4147

42-
const restored = (draftParticipants ?? []).reduce<SelectedOption[]>((result, participant) => {
48+
const restoredOptionsFromDraft = (draftParticipants ?? []).reduce<SelectedOption[]>((result, participant) => {
4349
if (participant.accountID === currentUserAccountID) {
4450
return result;
4551
}
@@ -58,18 +64,21 @@ function useGroupDraftRestore(
5864
}, []);
5965

6066
// No draft or only original creator in draft
61-
if (!restored.length) {
67+
if (!restoredOptionsFromDraft.length) {
6268
return;
6369
}
6470

65-
setSelectedOptions(restored);
71+
setSelectedOptions(restoredOptionsFromDraft);
6672
});
6773

68-
const syncDraftRemovals = useEffectEvent(() => {
74+
// NewChatConfirmPage can only deselect participants,
75+
// so we don't need the complex logic from the restoreParticipantsFromDraft.
76+
// Simple filtering out of deselected participants is enough here
77+
const syncSelectedOptionsWithDraft = useEffectEvent(() => {
6978
const draftLogins = new Set((draftParticipants ?? []).map((participant) => participant.login));
70-
const synced = selectedOptions.filter((option) => draftLogins.has(option.login));
79+
const filteredSelectionOptions = selectedOptions.filter((option) => draftLogins.has(option.login));
7180

72-
setSelectedOptions(synced);
81+
setSelectedOptions(filteredSelectionOptions);
7382
});
7483

7584
useFocusEffect(
@@ -82,23 +91,23 @@ function useGroupDraftRestore(
8291
}, []),
8392
);
8493

85-
// handle removing participants on other pages (e.g. NewChatConfirmPage)
94+
// Handle removing participants on other pages (e.g. NewChatConfirmPage)
8695
useEffect(() => {
8796
if (!isScreenInBackgroundRef.current) {
8897
return;
8998
}
90-
syncDraftRemovals();
99+
syncSelectedOptionsWithDraft();
91100
}, [draftParticipants]);
92101

93102
const areRestoreInputsReady = areAllPersonalDetailOptionsLoaded && !isLoadingOnyxValue(draftParticipantsMetadata);
94103

95-
// handle reload with existing draft participants
104+
// Handle reload with existing draft participants
96105
useEffect(() => {
97106
if (!shouldRestoreSelectedOptionsRef.current || !areRestoreInputsReady) {
98107
return;
99108
}
100-
restoreFromDraft();
109+
restoreParticipantsFromDraft();
101110
}, [draftParticipants, areRestoreInputsReady]);
102111
}
103112

104-
export default useGroupDraftRestore;
113+
export default useGroupChatDraftParticipantSync;

‎tests/unit/useGroupDraftRestoreTest.ts‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import type {SearchOption} from '@libs/OptionsListUtils';
77
import type {OptionData} from '@libs/ReportUtils';
88
import type {PersonalDetails} from '@src/types/onyx';
99
import type {SelectedParticipant} from '@src/types/onyx/NewGroupChatDraft';
10-
import useGroupDraftRestore from '../../src/pages/NewChatPage/useGroupDraftRestore';
10+
import useGroupChatDraftParticipantSync from '../../src/pages/NewChatPage/useGroupDraftRestore';
1111

1212
const mockUseOnyx = useOnyx as jest.MockedFunction<typeof useOnyx>;
1313
const mockGetUserToInviteOption = OptionsListUtilsModule.getUserToInviteOption as jest.MockedFunction<typeof OptionsListUtilsModule.getUserToInviteOption>;
@@ -110,7 +110,16 @@ describe('useGroupDraftRestore', () => {
110110
setupUseOnyx(draftParticipants, draftStatus);
111111

112112
const {rerender} = renderHook(() =>
113-
useGroupDraftRestore(allPersonalDetailOptions, areAllPersonalDetailOptionsLoaded, {}, {}, CURRENT_USER_EMAIL, CURRENT_USER_ACCOUNT_ID, selectedOptions, setSelectedOptions),
113+
useGroupChatDraftParticipantSync(
114+
allPersonalDetailOptions,
115+
areAllPersonalDetailOptionsLoaded,
116+
{},
117+
{},
118+
CURRENT_USER_EMAIL,
119+
CURRENT_USER_ACCOUNT_ID,
120+
selectedOptions,
121+
setSelectedOptions,
122+
),
114123
);
115124

116125
return {setSelectedOptions, rerender};
@@ -159,7 +168,9 @@ describe('useGroupDraftRestore', () => {
159168
setupUseOnyx(draftParticipants);
160169

161170
const setSelectedOptions = jest.fn();
162-
const {rerender} = renderHook(() => useGroupDraftRestore(ALL_PERSONAL_DETAIL_OPTIONS, true, {}, {}, CURRENT_USER_EMAIL, CURRENT_USER_ACCOUNT_ID, [], setSelectedOptions));
171+
const {rerender} = renderHook(() =>
172+
useGroupChatDraftParticipantSync(ALL_PERSONAL_DETAIL_OPTIONS, true, {}, {}, CURRENT_USER_EMAIL, CURRENT_USER_ACCOUNT_ID, [], setSelectedOptions),
173+
);
163174

164175
expect(setSelectedOptions).toHaveBeenCalledTimes(1);
165176

@@ -192,7 +203,7 @@ describe('useGroupDraftRestore', () => {
192203
const {rerender} = renderHook(
193204
({draftParticipants}) => {
194205
setupUseOnyx(draftParticipants);
195-
return useGroupDraftRestore(ALL_PERSONAL_DETAIL_OPTIONS, true, {}, {}, CURRENT_USER_EMAIL, CURRENT_USER_ACCOUNT_ID, selectedAfterRestore, setSelectedOptions);
206+
return useGroupChatDraftParticipantSync(ALL_PERSONAL_DETAIL_OPTIONS, true, {}, {}, CURRENT_USER_EMAIL, CURRENT_USER_ACCOUNT_ID, selectedAfterRestore, setSelectedOptions);
196207
},
197208
{initialProps: {draftParticipants: initialDraftParticipants}},
198209
);
@@ -236,7 +247,7 @@ describe('useGroupDraftRestore', () => {
236247
const {rerender} = renderHook(
237248
({draftParticipants}) => {
238249
setupUseOnyx(draftParticipants);
239-
return useGroupDraftRestore(ALL_PERSONAL_DETAIL_OPTIONS, true, {}, {}, CURRENT_USER_EMAIL, CURRENT_USER_ACCOUNT_ID, selectedAfterRestore, setSelectedOptions);
250+
return useGroupChatDraftParticipantSync(ALL_PERSONAL_DETAIL_OPTIONS, true, {}, {}, CURRENT_USER_EMAIL, CURRENT_USER_ACCOUNT_ID, selectedAfterRestore, setSelectedOptions);
240251
},
241252
{initialProps: {draftParticipants: initialDraftParticipants}},
242253
);
@@ -269,7 +280,7 @@ describe('useGroupDraftRestore', () => {
269280
const {rerender} = renderHook(
270281
({draft}) => {
271282
setupUseOnyx(draft);
272-
return useGroupDraftRestore(ALL_PERSONAL_DETAIL_OPTIONS, true, {}, {}, CURRENT_USER_EMAIL, CURRENT_USER_ACCOUNT_ID, selectedAfterRestore, setSelectedOptions);
283+
return useGroupChatDraftParticipantSync(ALL_PERSONAL_DETAIL_OPTIONS, true, {}, {}, CURRENT_USER_EMAIL, CURRENT_USER_ACCOUNT_ID, selectedAfterRestore, setSelectedOptions);
273284
},
274285
{initialProps: {draft: draftParticipants}},
275286
);
@@ -297,7 +308,7 @@ describe('useGroupDraftRestore', () => {
297308
const {rerender} = renderHook(
298309
({draft}) => {
299310
setupUseOnyx(draft);
300-
return useGroupDraftRestore(ALL_PERSONAL_DETAIL_OPTIONS, true, {}, {}, CURRENT_USER_EMAIL, CURRENT_USER_ACCOUNT_ID, [], setSelectedOptions);
311+
return useGroupChatDraftParticipantSync(ALL_PERSONAL_DETAIL_OPTIONS, true, {}, {}, CURRENT_USER_EMAIL, CURRENT_USER_ACCOUNT_ID, [], setSelectedOptions);
301312
},
302313
{initialProps: {draft: draftParticipants}},
303314
);
@@ -345,7 +356,7 @@ describe('useGroupDraftRestore', () => {
345356
setupUseOnyx(draftParticipants);
346357

347358
const {rerender} = renderHook(
348-
({areLoaded}) => useGroupDraftRestore(ALL_PERSONAL_DETAIL_OPTIONS, areLoaded, {}, {}, CURRENT_USER_EMAIL, CURRENT_USER_ACCOUNT_ID, [], setSelectedOptions),
359+
({areLoaded}) => useGroupChatDraftParticipantSync(ALL_PERSONAL_DETAIL_OPTIONS, areLoaded, {}, {}, CURRENT_USER_EMAIL, CURRENT_USER_ACCOUNT_ID, [], setSelectedOptions),
349360
{initialProps: {areLoaded: false}},
350361
);
351362

0 commit comments

Comments
 (0)