[HOLD for payment 2024-04-15] [$500] Start chat - App shows data from User A after logging out of User A and logging in with User B

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.4.60-8
Reproducible in staging?: y
Reproducible in production?: n
If this was caught during regression testing, add the test name, ID and link from TestRail: Exploratory around https://expensify.testrail.io/index.php?/tests/view/4476067
Issue reported by: Applause - Internal Team

Action Performed:

Precondition:

• Account A and B are under the same private domain, and they share the same list of contacts.
• Account A and B have different set of Recent contacts in Start chat list.

1. Close and open incognito mode again for a fresh session.
2. Go to staging.new.expensify.com
3. Log in as Account A.
4. Go to FAB > Start chat > Chat.
5. Note that list of five contacts in Recent section.
6. Log out and log in as Account B.
7. Go to FAB > Start chat > Chat.
8. Note that Account B has the exact same list of five contacts in Recent section as Account A.
9. Close and open incognito mode.
10. Repeat Step 6 and 7 for Account B.

Expected Result:

When user logs out and logs in with another account, the Recent section list will follow the data from the logged-in account.

Actual Result:

When user logs out from Account A and logs in with Account B, the Recent section list still shows the data from Account A.

Workaround:

n/a

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6439021_1712310651825.20240405_174248.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e820a2af95c4423e
• Upwork Job ID: 1776276873995067392
• Last Price Increase: 2024-04-05
• Automatic offers:
• paultsimura | Reviewer | 0
• bernhardoj | Contributor | 0

Issue Owner

Current Issue Owner: @puneetlath

[melvin-bot]

Triggered auto assignment to @puneetlath (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[kbecciv]

We think that this bug might be related to #wave-collect - Release 1

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[mountiny]

I think this does not have to be a blocker

@kbecciv @kavimuru does the account B not know the recents accounts it shows?

[kavimuru]

@mountiny Logged in with old account with recent contacts, then logged out and sign up with a completely new account. The contact list with old account still shows up for new account.

bandicam.2024-04-05.22-12-52-747.mp4

[mountiny]

@TMisiukiewicz Could it be that we do not clear the cache for the search options in this case?

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01e820a2af95c4423e

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @paultsimura (External)

[mountiny]

Referring to this PR #38207

[melvin-bot]

Upwork job price has been updated to $500

[mountiny]

Increasing to $500 since its a blocker, author of the PR is out for the day and its quite annoying bug as you could potentially see someone else's contacts cached

[bernhardoj]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The report data is from another user is still cached even after logging out which makes it possible for other user that log in will see it.

What is the root cause of that problem?

We have OptionsListContextProvider that will load the report list (and update it when it's updated) and cache it.

App/src/components/OptionListContextProvider.tsx

Lines 39 to 112 in b10893a

	function OptionsListContextProvider({reports, children}: OptionsListProviderProps) {
	const areOptionsInitialized = useRef(false);
	const [options, setOptions] = useState<OptionList>({
	reports: [],
	personalDetails: [],
	});
	const personalDetails = usePersonalDetails();
	useEffect(() => {
	// there is no need to update the options if the options are not initialized
	if (!areOptionsInitialized.current) {
	return;
	}
	const lastUpdatedReport = ReportUtils.getLastUpdatedReport();
	if (!lastUpdatedReport) {
	return;
	}
	const newOption = OptionsListUtils.createOptionFromReport(lastUpdatedReport, personalDetails);
	const replaceIndex = options.reports.findIndex((option) => option.reportID === lastUpdatedReport.reportID);
	if (replaceIndex === -1) {
	return;
	}
	setOptions((prevOptions) => {
	const newOptions = {...prevOptions};
	newOptions.reports[replaceIndex] = newOption;
	return newOptions;
	});
	// eslint-disable-next-line react-hooks/exhaustive-deps
	}, [reports]);
	useEffect(() => {
	// there is no need to update the options if the options are not initialized
	if (!areOptionsInitialized.current) {
	return;
	}
	// since personal details are not a collection, we need to recreate the whole list from scratch
	const newPersonalDetailsOptions = OptionsListUtils.createOptionList(personalDetails).personalDetails;
	setOptions((prevOptions) => {
	const newOptions = {...prevOptions};
	newOptions.personalDetails = newPersonalDetailsOptions;
	return newOptions;
	});
	}, [personalDetails]);
	const loadOptions = useCallback(() => {
	const optionLists = OptionsListUtils.createOptionList(personalDetails, reports);
	setOptions({
	reports: optionLists.reports,
	personalDetails: optionLists.personalDetails,
	});
	}, [personalDetails, reports]);
	const initializeOptions = useCallback(() => {
	if (areOptionsInitialized.current) {
	return;
	}
	loadOptions();
	areOptionsInitialized.current = true;
	}, [loadOptions]);
	return (
	<OptionsListContext.Provider value={useMemo(() => ({options, initializeOptions, areOptionsInitialized: areOptionsInitialized.current}), [options, initializeOptions])}>
	{children}
	</OptionsListContext.Provider>
	);
	}

The provider is put in App.tsx which means the state (cache) is persisted even after we log out.

App/src/App.tsx

Line 86 in b10893a

OptionsListContextProvider,

What changes do you think we should make in order to solve the problem?

Move the provider OptionsListContextProvider from App.tsx to AuthScreens. This way, when the user is logged out, the context provider is destroyed.

What alternative solutions did you explore? (Optional)

Use useSession hook and clear both reports and personalDetails and also set areOptionsInitialized to false whenever the user logged out by checking the session auth token.

useEffect(() => {
    if (!session.authToken) {
        setOptions({reports: [], personalDetails: []});
        areOptionsInitialized.current = false;
    }
}, [session.authToken]);

[jasperhuangg]

@bernhardoj your proposal makes sense to me, let's try to implement it quickly!

[melvin-bot]

📣 @paultsimura 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @bernhardoj 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[bernhardoj]

@jasperhuangg can you confirm if we are going with my main solution? (moving the provider)

[kavimuru]

@mountiny Bug is fixed in all the platforms but Desktop. Desktop does not have the correct build version.

https://platform.applause.com/services/links/v1/external/4f3f83c90fa54dbe7ff4691984edde71edefd6da9e238ea3414d49eab3cf4534
https://platform.applause.com/services/links/v1/external/a08ef70d6c97c6d745314b5b2ef9786c0b9fb23b6be3c2c2bb255007e15b446d

https://platform.applause.com/services/links/v1/external/72ebf3a8b4239a38df20870a8e42f0e536209ed2d8b8831a2c2edb630a6fcfcd
https://platform.applause.com/services/links/v1/external/0bd5c0575b1498f4e4996700b6114144d631289b290a5ff431c4054cbeb4ef00

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.60-13 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [CP Staging] Fix report and personal detail options are persisted after switching account #39753

If no regressions arise, payment will be issued on 2024-04-15. 🎊

For reference, here are some details about the assignees on this issue:

• @paultsimura requires payment automatic offer (Reviewer)
• @bernhardoj requires payment automatic offer (Contributor)

[Julesssss]

FYI this appeared on today's checklist as we forgot to remove the blocker label (I think I forgot to do that too when I marked it off this morning)

[melvin-bot]

Current assignee @puneetlath is eligible for the Bug assigner, not assigning anyone new.

[mountiny]

Thanks @kbecciv it was repro because the desktop build failed before

@puneetlath this will be ready for payment $500 to @bernhardoj and to @paultsimura

[puneetlath]

Sorry, to clarify, you're saying I should pay them now? Or on April 15th?

[melvin-bot]

@puneetlath, @paultsimura, @bernhardoj Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[paultsimura]

Sorry, to clarify, you're saying I should pay them now? Or on April 15th?

On April 15th.

Not overdue, Melv.

[paultsimura]

• The PR that introduced the bug has been identified. Link to the PR: perf: Cache search options #38207
• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/App/pull/38207/files#r1563935428
• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
• Determine if we should create a regression test for this bug: Yes
• If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

Precondition:

• Accounts A and B share the same list of contacts
• Accounts A and B have different sets of "Recent" contacts in the "Start chat" list.

Test:

1. Log in as Account A
2. Go to FAB > Start chat > Chat
3. Note the list of five contacts in the "Recent" section
4. Log out and log in as Account B
5. Go to FAB > Start chat > Chat
6. Verify the "Recent" lists are different (unless they were indeed interacted with in the same order by both accounts A and B).

Do we agree 👍 or 👎

[puneetlath]

All paid. Thanks everyone!