[$1000] Web - A non-stoping spinner is displayed when opening an image previewer carousal with a deep link

[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 a chat with a random user
2. Send a picture
3. Open the sent picture
4. Copy link form the address bar
5. Sign out
6. Paste the copied link to the address bar
7. Press enter
8. Enter the same email address you have used to send the picture
9. Click continue > enter magic code

Expected Result:

Picture should finish loading & should be displayed

Actual Result:

Spinner keeps spinning without stop

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.47.3
Reproducible in staging?: y
Reproducible in production?: y
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

Screencast.from.2023-07-29.14-09-00.mp4

Recording.4018.mp4

Expensify/Expensify Issue URL:
Issue reported by: @Natnael-Guchima
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1690629431697969

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e0613ad8be2f9b94
• Upwork Job ID: 1686036587549114368
• Last Price Increase: 2023-07-31
• Automatic offers:
• huzaifa-99 | Contributor | 25986445
• Natnael-guchima | Reporter | 25986447

[melvin-bot]

Triggered auto assignment to @flaviadefaria (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

[tienifr]

Proposal

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

Web - A non-stoping spinner is displayed when opening an image previewer carousal with a deep link

What is the root cause of that problem?

When users login successfully, we're showing the AttachmentView because the report isn't loaded at that time, and we don't listen the report change.

App/src/components/AttachmentModal.js

Lines 331 to 337 in 024d210

	<AttachmentView
	containerStyles={[styles.mh5]}
	source={sourceForAttachmentView}
	isAuthTokenRequired={isAuthTokenRequired}
	file={file}
	onToggleKeyboard={updateConfirmButtonVisibility}
	/>

In

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

Lines 28 to 38 in 024d210

	<AttachmentModal
	allowDownload
	defaultOpen
	report={report}
	source={source}
	onModalHide={() => Navigation.dismissModal(reportID)}
	onCarouselAttachmentChange={(attachment) => {
	const route = ROUTES.getReportAttachmentRoute(reportID, attachment.source);
	Navigation.navigate(route);
	}}
	/>

We don't pass isAuthTokenRequired so isAuthTokenRequired is false by default, but the report attachment must have isAuthTokenRequired is true to add encryptedAuthToken => We can't load the image

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

In

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

Lines 28 to 38 in 024d210

	<AttachmentModal
	allowDownload
	defaultOpen
	report={report}
	source={source}
	onModalHide={() => Navigation.dismissModal(reportID)}
	onCarouselAttachmentChange={(attachment) => {
	const route = ROUTES.getReportAttachmentRoute(reportID, attachment.source);
	Navigation.navigate(route);
	}}
	/>

we should enable isAuthTokenRequired because it comes from report.

What alternative solutions did you explore? (Optional)

If we don't want to enable isAuthTokenRequired in ReportAttachments because of some reasons, we should hide the attachment modal if we can't load the image

Result

Screen.Recording.2023-07-31.at.15.39.18.mp4

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[huzaifa-99]

Proposal

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

We want the image preview to show when an attachment is opened with a deep link

What is the root cause of that problem?

I have a similar RCA as @tienifr but the solution is different.

The issue here is that when we open the report attachment via Deeplink the report is empty here and because of this we show the <AttachmentView/> here which displays a loading indicator until the image is loaded

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

I think we should not directly pass isAuthTokenRequired: true, that doesn't feel safe and it also wont show the carousel when there are multiple attachments

The proper way I think would be to listen for report changes in <ReportAttachments/> instead of only getting the report once here by subscribing to the onyx key.

When we listen to the report changes and when the report loads, we will be displaying <AttachmentsCarousel/> here which would be the proper way.

// just some pseudocode for <ReportAttachments/>

// pass report from onyx
<AttachmentModal
    allowDownload
    defaultOpen
    report={props.report}

// subscribe to report onyx key
withOnyx({
    report: {
        key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${getReportID(route)}`,
    },
}),

Now with the above changes, we are updating the attachment modal when the report is loaded via deep link. Since we are using deep link, its obvious that report actions are also not loaded (this is valid if we logout, then visit deeplink, then login and when deeplink is processed -- basically the reproduction steps in OP). We need them to be loaded since we use them in createInitialState.js which is used in <AttachmentsCarousel/> here. So we would have to wait for them to be also loaded as well.

This can be done by listening for report action onyx changes in <AttachmentModal/> and only render the <AttachmentCarousel/> when both report and report actions are loaded here

{!_.isEmpty(props.report) && !_.isEmpty(props.reportActions) ? (
    <AttachmentCarousel
        // ....

I added the !_.isEmpty(props.reportActions) here because it would show the loading indicator until reportActions are also loaded to be consistent with the loading process. However, this condition can be added in other places as well based on how we prefer to show the loader when reportActions are loading, like we can return early if(_.isEmpty(props.reportActions) return here as well and show loading indicator in <AttachmentCarousel/>, -- again based on when we prefer.

What alternative solutions did you explore? (Optional)

N/A

[tienifr]

listening for report actions onyx changes in is not the good idea, since it's not just used in ReportAttachment. And in AttachmentCarousel we're already listening report actions.

In small screen we don't call api openReport when access attachment preview from deeplink that why if we wait for the actions to be loaded, it can cause the infinity loading.

I believe showing the only one attachment preview when access from deeplink is good in this case (users just need to access this attachment and other attachments are not loaded)