Web - New Group - Aviators icon is missing when create a new group

[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. Go to https://staging.new.expensify.com/
2. Log in with any account
3. Click on Fub menu - New Group
   4 Select any 4 - 5 user and click Create

Expected Result:

Aviators icon is not missing when create a new group

Actual Result:

Aviators icon is missing when create a new group

Workaround:

Unknown

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.1.89.0

Reproducible in staging?: Yes

Reproducible in production?: No

Email or phone of affected tester (no customers): any

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

Notes/Photos/Videos: Any additional supporting documentation

Recording.1002.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

[OSBotify]

👋 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.

[melvin-bot]

Triggered auto assignment to @TomatoToaster (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[tgolen]

I'll take this one

[JmillsExpensify]

Depending on how we get on, I'd be fine deploying with this shortcoming and we can address tomorrow. It's largely cosmetic.

[tgolen]

@marcaaron Maybe you can help me out with this... I found the problem, but I don't know the cause.

It looks like the route.match.params that is being passed to ReportScreen is not getting the right reportID. Because of this, the reportActions end up being empty, and that's why the ReportWelcomeText isn't shown. Once you refresh the page, then everything works properly.

The redirect happens here:

App/src/libs/actions/Report.js

Line 618 in 599ca8b

Navigation.navigate(ROUTES.getReportRoute(data.reportID));

but I can't find any recent changes to anything anywhere that are suspect.

[tgolen]

I'm also noticing a double-navigation event happening, which I can't make sense of.

I don't know if that's part of the same problem or not.

[marcaaron]

Which one of those routes is from data.reportID ?

[Gonals]

Discussing on this thread, but TLDR, we think we are not calling fetchInitialActions at some point where we should

[tgolen]

@marcaaron the second one.

[tgolen]

I verified the double-navigation also happens on production, so that could be something completely unrelated and existing.

[marcaaron]

Leaving a summary of the problem here:

In this commit of this PR we changed a behavior of the ReportActionsView which is to fetch the report object if we attempt to render the component with a reportID prop (that come from url params) but have no report object (which comes from Onyx). The resulting code doesn't quite make sense:

App/src/pages/home/report/ReportActionsView.js

Lines 118 to 122 in 2d7de46

	// If the reportID is not found then we have either not loaded this chat or the user is unable to access it.
	// We will attempt to fetch it and redirect if still not accessible.
	if (!this.props.report.reportID) {
	Report.fetchChatReportsByIDs([this.props.report.reportID], true);
	}

The reportID is undefined which means this "report fetch" will fail and the report never gets loaded. That's why we get a blank screen and why none of the other API calls succeed in loading report actions data.

This is relatively easy to fix by just moving the broken code into the ReportScreen and showing the loading indicator if the report object has not loaded yet instead of the ReportActionsView. This also sets up the ReportActionsView for further improvements later since it doesn't have to worry about a state where the report object doesn't exist and only has one reportID source to worry about instead of two different sources (url and report object).

The action of navigating to a report page e.g. via an internal link, manual redirect, or url all might need to trigger a fetch of the report summary in the rare cases where it's not yet available. And as per the new API design would happen via a call to a SwitchChat command (or something similar) instead of Report.fetchChatReportByIDs().

Looks like @chiragsalian opened an issue to explore improvements here. So I think we should do the easy solution for now to fix the regression and next think about how to remove the 7 or so remaining usages of Report.fetchChatReportsByIDs().