Skip to content

[Fix] Workspace - Inconsistent workspace subtitle when approver is changed in offline mode #56770

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
grgia merged 3 commits into from
Feb 27, 2025
Merged

[Fix] Workspace - Inconsistent workspace subtitle when approver is changed in offline mode #56770

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
007a94a
initialize options on MoneyRequestParticipantsSelector render
JKobrynski Feb 12, 2025
a4f6257
revert subtitle logic in getReportOption
JKobrynski Feb 13, 2025
ea22c28
Merge remote-tracking branch 'upstream/main' into JKobrynski/fix/5650…
JKobrynski Feb 24, 2025
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
4 changes: 0 additions & 4 deletions src/components/OptionListContextProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,6 @@ function OptionsListContextProvider({children}: OptionsListProviderProps) {
}, [personalDetails, reports]);

const initializeOptions = useCallback(() => {
if (areOptionsInitialized.current) {

@mananjadhav mananjadhav Feb 12, 2025

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 this cause any performance impact?

@JKobrynski JKobrynski Feb 13, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good question! The way I see it is the following - this functionality was missing, and it needed to be added, so the app can function properly, it's not like we are compounding the performance of an existing functionality, we are adding one.

Previously, these options would only be initialized on the very first render of the MoneyRequestParticipantsList and stay that way until you relog/restart, ignoring changes made to the reports/policies made by you and others.

@mananjadhav mananjadhav Feb 13, 2025

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.

Yeah I don't have the best context on this one, so hence I am checking. May be @grgia could take a look as well.

@grgia grgia Feb 27, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't have the full context on why we needed this initially

return;
}

loadOptions();
areOptionsInitialized.current = true;
}, [loadOptions]);
Expand Down
3 changes: 2 additions & 1 deletion src/libs/ReportUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4599,7 +4599,8 @@ function getChatRoomSubtitle(report: OnyxEntry<Report>, config: GetChatRoomSubti
return report?.reportName?.substring(1) ?? '';
}
if ((isPolicyExpenseChat(report) && !!report?.isOwnPolicyExpenseChat) || isExpenseReport(report)) {
const submitToAccountID = getSubmitToAccountID(getPolicy(report?.policyID), report);
const policy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`];
const submitToAccountID = getSubmitToAccountID(policy, report);
const submitsToAccountDetails = allPersonalDetails?.[submitToAccountID];
const subtitle = submitsToAccountDetails?.displayName ?? submitsToAccountDetails?.login;

Expand Down
8 changes: 7 additions & 1 deletion src/pages/iou/request/MoneyRequestParticipantsSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF
const policy = usePolicy(activePolicyID);
const [isSearchingForReports] = useOnyx(ONYXKEYS.IS_SEARCHING_FOR_REPORTS, {initWithStoredValues: false});
const [currentUserLogin] = useOnyx(ONYXKEYS.SESSION, {selector: (session) => session?.email});
const {options, areOptionsInitialized} = useOptionsList({
const {options, areOptionsInitialized, initializeOptions} = useOptionsList({
shouldInitialize: didScreenTransitionEnd,
});
const cleanSearchTerm = useMemo(() => debouncedSearchTerm.trim().toLowerCase(), [debouncedSearchTerm]);
Expand All @@ -92,6 +92,12 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF
searchInServer(debouncedSearchTerm.trim());
}, [debouncedSearchTerm]);

useEffect(() => {

@mananjadhav mananjadhav Feb 12, 2025

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.

does it cause any extra rerender? Do we have a way to check this?

@JKobrynski JKobrynski Feb 13, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Based on my testing, this just adds a function call

// This is necessary to ensure the options list is always up to date
// e.g. if the approver was changed in the policy, we need to update the options list
initializeOptions();
}, [initializeOptions]);

const defaultOptions = useMemo(() => {
if (!areOptionsInitialized || !didScreenTransitionEnd) {
return {
Expand Down