[HOLD for payment 2023-08-08] [$1000] Reload on custom room details page after adding member in workspace opens admins room of workspace in background

[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!

---

Action Performed:

1. Open the app
2. Create new room by: click on plus->New room-> name anything, select workspace to which you are admin and select any visibility->click on create room
3. Open settings
4. Open workspaces
5. Open workspace used in step 2
6. Click on members
7. Invite any member
8. Close the RHN to navigate back to the room created in step 2, then click on room name to open room details page
9. Reload and observe that app open #admin room in background instead of keeping the room chat opened in step 2

Expected Result:

App should keep the same room chat open in background when we reload on the details page of any room

Actual Result:

App opens #admins room chat in background when we reload on any of the workspace room details page after adding new member in that workspace

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.27-6

Reproducible in staging?: yes

Reproducible in production?: n/a

If this was caught during regression testing, add the test name, ID and link from TestRail:

Email or phone of affected tester (no customers):

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Recording.720.mp4

reload.on.room.opens.admin.room.1.mp4

Expensify/Expensify Issue URL:

Issue reported by: @dhanashree-sawant

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1686213190480009

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~012b84a8d6c15fa6d2
• Upwork Job ID: 1669387246687850496
2023-07-06
• Automatic offers:
• abdulrahuman5196 | Reviewer | 25761235
• bernhardoj | Contributor | 25761237
• dhanashree-sawant | Reporter | 25761242

[melvin-bot]

Triggered auto assignment to @sakluger (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[bernhardoj]

Proposal

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

Wrong chat is shown in the background when we refresh the page while a report detail is open. From my testing, I can't reproduce it with the provided steps, but I can easily reproduce it only in some of my account by simply doing this:

1. Open the web, it will show report/chat A
2. Open any other chat (B)
3. Press the chat header to open the details
4. Refresh
5. It will show chat A in the background

What is the root cause of that problem?

When the report screen is mounted, it will check if report ID is in the params. If not, we will show the last accessed report.

App/src/libs/Navigation/AppNavigator/ReportScreenWrapper.js

Lines 79 to 92 in 4280edd

	// If there is no reportID in route, try to find last accessed and use it for setParams
	if (!lodashGet(this.props.route, 'params.reportID', null)) {
	const reportID = getLastAccessedReportID(
	this.props.reports,
	!Permissions.canUseDefaultRooms(this.props.betas),
	this.props.policies,
	this.props.isFirstTimeNewExpensifyUser,
	this.props.route.params.openOnAdminRoom,
	);
	// It's possible that props.reports aren't fully loaded yet
	// in that case the reportID is undefined
	if (reportID) {
	this.props.navigation.setParams({reportID: String(reportID)});

To get the last accessed report, the report list is sorted by 'lastReadTime', then the last item is retrieved.

App/src/libs/ReportUtils.js

Lines 388 to 422 in 4280edd

	function findLastAccessedReport(reports, ignoreDomainRooms, policies, isFirstTimeNewExpensifyUser, openOnAdminRoom = false) {
	// If it's the user's first time using New Expensify, then they could either have:
	// - just a Concierge report, if so we'll return that
	// - their Concierge report, and a separate report that must have deeplinked them to the app before they created their account.
	// If it's the latter, we'll use the deeplinked report over the Concierge report,
	// since the Concierge report would be incorrectly selected over the deep-linked report in the logic below.
	let sortedReports = sortReportsByLastRead(reports);
	if (isFirstTimeNewExpensifyUser) {
	if (sortedReports.length === 1) {
	return sortedReports[0];
	}
	return _.find(sortedReports, (report) => !isConciergeChatReport(report));
	}
	if (ignoreDomainRooms) {
	// We allow public announce rooms, admins, and announce rooms through since we bypass the default rooms beta for them.
	// Check where ReportUtils.findLastAccessedReport is called in MainDrawerNavigator.js for more context.
	// Domain rooms are now the only type of default room that are on the defaultRooms beta.
	sortedReports = _.filter(
	sortedReports,
	(report) => !isDomainRoom(report) || getPolicyType(report, policies) === CONST.POLICY.TYPE.FREE || hasExpensifyGuidesEmails(lodashGet(report, ['participants'], [])),
	);
	}
	let adminReport;
	if (openOnAdminRoom) {
	adminReport = _.find(sortedReports, (report) => {
	const chatType = getChatType(report);
	return chatType === CONST.REPORT.CHAT_TYPE.POLICY_ADMINS;
	});
	}
	return adminReport || _.last(sortedReports);
	}

App/src/libs/ReportUtils.js

Lines 166 to 172 in 4280edd

	function sortReportsByLastRead(reports) {
	return _.chain(reports)
	.toArray()
	.filter((report) => report && report.reportID && !isIOUReport(report))
	.sortBy('lastReadTime')
	.value();
	}

There are 3 problems:

1. The report is not sorted correctly. Currently, we are doing the sorting with underscore. I tested by using lodash instead and the sorting works correctly (except that it will put null value higher than any value). From my further investigation, underscore sort will fail if the array contains a null value. A report will have null value if it's a new report that we never open/read it yet. For example:
   a. User B assigns a Task to A
   b. In user A, you will receive a Pusher update of the new task report. At this point, the task report will have undefined lastReadTime.
   c. User A log out and log in back
   d. The task report now has null lastReadTime
   We can repeat these steps with other types of report, such as group, money request, etc.

So, one of the requirements to reproduce this is to have a null lastReadTime report. Looks like underscore doing the sort by dividing the array into sub-array based on null value and sorting the sub-array. For example, if we have this array:
[3, 2, 5, null, 1, 2, null, 5]
underscore will sort [3, 2, 5] -> [2, 3, 5]
[1, 2] -> [1, 2]
[5] -> [5]
then get combined into [2, 3, 5, null, 1, 2, null, 5]

I don't know how can we reproduce this reliably as we need to at least have the last items of the array has such values below:
[{lastReadTime: july 8, id: 1},{lastReadTime: null}, {lastReadTime: july 6, id: 2}]
If we have the report Onyx looks like above (which some of my accounts had), we can open the first chat (id: 1), open settings, then refresh, and the second chat (id: 2) will be the one that shows instead of first chat. So, if you want to reproduce it in your account, check the report Onyx first.

(idk how QA will test this)

2. sortReportsByLastRead filtering out iou report. It is done to fix this issue [HOLD for payment 2022-12-05] Empty chat screen appears with welcome message after login #13000. I believe the reason we did that before is because iou report previously is not a chat, they don't have any report action. But now, we can chat in an iou report.

3. When we just log in, money request report will have undefined value. This means, if we read a money request report, the lastReadTime is only saved locally. Relogin will fetch the data from the server and the server doesn't include the lastReadTime for every money request report.

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

1. Filter out null/undefined lastReadTime.
2. Remove filtering out iou report.
3. Fix in BE.

view diff

diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js
index e4a6b801db..390426b9d0 100644
--- a/src/libs/ReportUtils.js
+++ b/src/libs/ReportUtils.js
@@ -202,7 +202,7 @@ function isMoneyRequestReport(reportOrID) {
 function sortReportsByLastRead(reports) {
     return _.chain(reports)
         .toArray()
-        .filter((report) => report && report.reportID && !isIOUReport(report))
+        .filter((report) => report && report.reportID && report.lastReadTime)
         .sortBy('lastReadTime')
         .value();
 }

[melvin-bot]

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

[sakluger]

I was able to reproduce. I changed the wording of the steps slightly to make it more clear that you can't navigate away from the created room before refreshing.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~012b84a8d6c15fa6d2

[melvin-bot]

Current assignee @sakluger is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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