[HOLD #11768] [$250] Report detail page is opened and closed immediately after login while trying to open that deeplink url reported by @0xmiroslav

[kavimuru]

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. Login to any account
2. Go to report detail page
3. Copy url link
4. Logout
5. Visit copied url link
6. Login to same account

Expected Result:

Report detail page should keep open

Actual Result:

Report detail page is opened and closed immediately

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web

Version Number: 1.2.23-7 Reproducible in staging?: y Reproducible in production?: y Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos:

https://user-images.githubusercontent.com/43996225/199752503-924df49e-ab59-4455-93d8-cb7c1c3fdb05.mov

https://user-images.githubusercontent.com/43996225/199752547-f1fa3c65-230a-4337-a5ff-3ecd3d5100ad.mp4

Expensify/Expensify Issue URL: Issue reported by: @0xmiroslav Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1667406681481269

View all open jobs on GitHub

[melvin-bot[bot]]

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

[kevinksullivan]

Hm I'm not sure entering the URL should work when completely signed out. If you sent me that link, for example, I shouldn't be able to login/view the details without first entering your username / password.

[melvin-bot[bot]]

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

[kevinksullivan]

Closing unless someone can help me understand this issue better.

[0xmiros]

@kevinksullivan that's called deep link. The issue description and video clearly explains how to reproduce. @kavimuru was also able to reproduce the issue as well and attached video. https://github.com/Expensify/App/issues/11982: this is the same step to follow but the difference is that one crashes while this one closes page immediately. If you're not familiar with deep link, it would better to add Engineering label to assign engineer than just closing it.

[kevinksullivan]

@0xmiroslav I see what you're saying now. I misread your reproduction steps in slack, as if they implied you should be able to access the page in (5) before logging in.

Thanks for explaining further, that's all I was asking for. FYI everyone on our team knows what a deeplink is 😉 .

[kevinksullivan]

https://www.upwork.com/jobs/~0157c3b18be3398b9d

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

Triggered auto assignment to @NikkiWines (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[puneetlath]

For when it comes time to do the RCA checklist, @aimane-chnaif brought up that this was likely caused by this PR.

[yrshva]

it hits Navigation.dismissModal() in withReportOrNavigateHome, which does navigateBackToRootDrawer and doesn't enter any ifs, need to fix dismissModal() function + make sure the drawer is ready once it opens

https://user-images.githubusercontent.com/58461507/200952901-b5c8d9bb-cced-4c4f-93d9-128d8971fb93.mov

[melvin-bot[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.

[mollfpr]

Proposal

RCA This issue happens because in withReportOrNavigateHome we navigate to home if the report data is not available. In this case, the report data is not available because we visit the page directly and there's no initiation to fetch the report data.

While we should fetch the data when accessing ReportDetailsPage, ReportParticipantsPage, or ReportSettingsPage.

Solution We could fetch the report data optimistically like we do in ReportScreen and will navigate to home if there report data is not available from the API response.

diff --git a/src/pages/home/report/withReportOrNavigateHome.js b/src/pages/home/report/withReportOrNavigateHome.js
index 5e74f65a0..86cf797b9 100644
--- a/src/pages/home/report/withReportOrNavigateHome.js
+++ b/src/pages/home/report/withReportOrNavigateHome.js
@@ -1,7 +1,9 @@
 import PropTypes from 'prop-types';
 import React, {Component} from 'react';
 import {withOnyx} from 'react-native-onyx';
+import lodashGet from 'lodash/get';
 import _ from 'underscore';
+import * as Report from '../../../libs/actions/Report';
 import getComponentDisplayName from '../../../libs/getComponentDisplayName';
 import Navigation from '../../../libs/Navigation/Navigation';
 import ONYXKEYS from '../../../ONYXKEYS';
@@ -15,6 +17,15 @@ export default function (WrappedComponent) {

         /** The report currently being looked at */
         report: reportPropTypes,
+
+        /** Navigation route context info provided by react navigation */
+        route: PropTypes.shape({
+            /** Route specific parameters used on this screen */
+            params: PropTypes.shape({
+                /** The ID of the report this screen should display */
+                reportID: PropTypes.string,
+            }).isRequired,
+        }).isRequired,
     };

     const defaultProps = {
@@ -24,15 +35,23 @@ export default function (WrappedComponent) {

     class WithReportOrNavigateHome extends Component {
         componentDidMount() {
-            if (!_.isEmpty(this.props.report)) {
+            const reportIDFromPath = this.props.route.params.reportID;
+
+            if (this.props.report.reportID) {
                 return;
             }
-            Navigation.dismissModal();
+
+            Report.openReport(reportIDFromPath);
         }

         render() {
             const rest = _.omit(this.props, ['forwardedRef']);

+            // Navigate to home if the optimistic report data return errors
+            if (!_.isEmpty(lodashGet(this.props.report, 'errorFields.createChat'))) {
+                Navigation.dismissModal();
+            }
+
             return (
                 <WrappedComponent
                     // eslint-disable-next-line react/jsx-props-no-spreading

We also need to update the menu items on ReportDetailsPage should update when the report data is ready.

diff --git a/src/pages/ReportDetailsPage.js b/src/pages/ReportDetailsPage.js
index 6efd4bf72..2881a44d1 100644
--- a/src/pages/ReportDetailsPage.js
+++ b/src/pages/ReportDetailsPage.js
@@ -57,31 +57,48 @@ class ReportDetailsPage extends Component {
     constructor(props) {
         super(props);

+        this.prepareMenuItems = this.prepareMenuItems.bind(this);
+
         this.menuItems = [];
+
+        this.prepareMenuItems();
+    }
+
+    componentDidUpdate(prevProps) {
+        if (prevProps.report === this.props.report) {
+            return;
+        }
+
+        this.prepareMenuItems();
+    }
+
+    prepareMenuItems() {
         if (ReportUtils.isArchivedRoom(this.props.report)) {
             return;
         }

+        const menuItems = [];
+
         // All nonarchived chats should let you see their members
-        this.menuItems.push({
+        menuItems.push({
             key: CONST.REPORT_DETAILS_MENU_ITEM.MEMBERS,
             translationKey: 'common.members',
             icon: Expensicons.Users,
-            subtitle: lodashGet(props.report, 'participants', []).length,
-            action: () => { Navigation.navigate(ROUTES.getReportParticipantsRoute(props.report.reportID)); },
+            subtitle: lodashGet(this.props.report, 'participants', []).length,
+            action: () => { Navigation.navigate(ROUTES.getReportParticipantsRoute(this.props.report.reportID)); },
         });

         if (ReportUtils.isPolicyExpenseChat(this.props.report) || ReportUtils.isChatRoom(this.props.report)) {
-            this.menuItems.push({
+            menuItems.push({
                 key: CONST.REPORT_DETAILS_MENU_ITEM.SETTINGS,
                 translationKey: 'common.settings',
                 icon: Expensicons.Gear,
-                action: () => { Navigation.navigate(ROUTES.getReportSettingsRoute(props.report.reportID)); },
+                action: () => { Navigation.navigate(ROUTES.getReportSettingsRoute(this.props.report.reportID)); },
             });
         }

         if (ReportUtils.isUserCreatedPolicyRoom(this.props.report)) {
-            this.menuItems.push({
+            menuItems.push({
                 key: CONST.REPORT_DETAILS_MENU_ITEM.INVITE,
                 translationKey: 'common.invite',
                 icon: Expensicons.Plus,
@@ -94,6 +111,8 @@ class ReportDetailsPage extends Component {
                 action: () => { /* Placeholder for when leaving rooms is built in */ },
             });
         }
+
+        this.menuItems = menuItems;
     }

     render() {
@@ -129,7 +148,7 @@ class ReportDetailsPage extends Component {
                                     <DisplayNames
                                         fullTitle={ReportUtils.getReportName(this.props.report, this.props.policies)}
                                         displayNamesWithTooltips={displayNamesWithTooltips}
-                                        tooltipEnabled
+                                        tooltipEnabled={false}
                                         numberOfLines={1}
                                         textStyles={[styles.headerText, styles.mb2, styles.textAlignCenter]}
                                         shouldUseFullTitle={isChatRoom || isPolicyExpenseChat}

Result This case when using deep link

https://user-images.githubusercontent.com/25520267/201155214-45cea68e-b05f-4200-bcee-27ae9433b9fe.mov

This case when the report data is when the account doesn't have access

https://user-images.githubusercontent.com/25520267/201155723-30c3a675-0862-442c-afd0-3a16b05dd316.mov

[parasharrajat]

I will review the proposal in few hours.

[parasharrajat]

Reviewing now.

[puneetlath]

@NikkiWines this is one of the oldest issues in the /App repo. To help us clear out the large backlog of bugs, can you:

• Decide whether any proposals currently meet our guidelines and can be approved as-is
• For any that can't, please take this issue internal and treat it as one of your highest priorities
• If you have any questions, don't hesitate to start a discussion in #bug-zero

[NikkiWines]

@mollfpr, thanks for your proposal. It looks pretty good for the deep linking flow, but there're some anomalies in the UI as it loads the view. It looks like it cycles through a few options for the name of the workspace and the name of the room before loading correctly.

For the account doesn't have access flow, it seems like we still end up showing the ReportSettingsPage very briefly.

I'll look into this issue further tomorrow.

[NikkiWines]

Chatted with @puneetlath, since this issue is only a week only we're going to continue moving forward through the normal external process. @mollfpr please see my comments in this PR on your current proposal 🙇

[mollfpr]

It looks like it cycles through a few options for the name of the workspace and the name of the room before loading correctly.

Could be related to https://github.com/Expensify/App/issues/12629

For the account doesn't have access flow, it seems like we still end up showing the ReportSettingsPage very briefly.

Any suggestion for this part?

[parasharrajat]

I am trying to figure out why the settings page is opened after login. It should take the user directly to the home screen.

[parasharrajat]

@aimane-chnaif Interested to take this on. I am not getting time for this. Unassigning...

[mountiny]

Just noting here, this might be similar cause in terms of react-navigation state so we might want to consider holding this for react-navigation project and apply workarounds if this persists. Similarly to this https://github.com/Expensify/App/issues/12717

[thienlnam]

Added this issue to the main tracking issue https://github.com/Expensify/App/issues/11768

[marcaaron]

I think we should revert this PR instead of build on it further. I've explained in detail here why the HOC solution is most likely not the one we want and have concerns that we are just adding complexity to a design that should be investigated and potentially rolled back.

[kevinksullivan]

Hm, ok Melv. So is the approach @marcaaron revert the PR, re-test to see if this issue exists, and if not close it out?

[JmillsExpensify]

Assigning myself to this issue as part of the larger navigation reboot and also putting this issue on hold.

[melvin-bot[bot]]

@JmillsExpensify, @NikkiWines, @kevinksullivan Whoops! This issue is 2 days overdue. Let's get this updated quick!

[kevinksullivan]

Just double checking on this @marcaaron . So we need to revert https://github.com/Expensify/App/pull/12089 (@mollfpr ), then see if this issue still exists?

[marcaaron]

@kevinksullivan That's my understanding, yes.

@Beamanator Are you able to follow up on the plan we discussed here?

[Beamanator]

Yep yep didn't have time the last few days but planning on that follow-up for early next week 👍

[JmillsExpensify]

Still working on the revert of #12089 as well as the larger react navigation initiative.

[JmillsExpensify]

No longer on hold for #12089 but still on hold for navigation re-write.

[JmillsExpensify]

Still on hold for the navigation revamp.

[JmillsExpensify]

Switched out weekly for monthly given that the navigation project is large in scope and has no clear ETA.

[JmillsExpensify]

Still on hold for navigation. We're going to be stuck here for some time to come.

[JmillsExpensify]

Same same.

[NikkiWines]

Still on hold I believe!

[JmillsExpensify]

Still on hold.

[0xmiros]

This was fixed in #15499 and no longer reproducible

Can I get reporting bonus and then close the issue?

cc: @JmillsExpensify @kevinksullivan

[kevinksullivan]

offer sent @0xmiroslav

https://www.upwork.com/jobs/~01f027dd8b9450b032

[kevinksullivan]

FYI I am going OOO a few days so this payment may be delayed.

[Naveed273]

Hi, I am here to help you, this issue a deeplinking issue. And I think some props are missing to pass and receive which will need to be fixed. I am also going to submit proposal on upwork for this job. If you are satisfy with my answer, then please submit my proposal, and we will sort it out within 3 business days

[melvin-bot[bot]]

📣 @Naveed273! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Format:

   Contributor details
   Your Expensify account email: <REPLACE EMAIL HERE>
   Upwork Profile Link: <REPLACE LINK HERE>

[Naveed273]

Contributor details Your Expensify account email: softengr273@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~019c8de8fba62a664d

[melvin-bot[bot]]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[0xmiros]

offer sent @0xmiroslav

@kevinksullivan accepted, thanks.

FYI I am going OOO a few days so this payment may be delayed.

no problem, take your time

[kevinksullivan]

Got around to the payment! Should be all set here.