App [$500] Private notes - Hmm its not here page displayed when clicking on thumbnail

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

Version Number: 1.4.56-0 Reproducible in staging?: Y Reproducible in production?: N If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Email or phone of affected tester (no customers): [email protected] Issue reported by: Applause - Internal Team

Issue found when executing PR https://github.com/Expensify/App/pull/37566

Action Performed:

Navigate to any report
Click on header > Private notes
Add markdown image to private notes: holla
Click on 'Private notes'
Click on the image

Expected Result:

Image should be opened in the carousel
Or, image shouldn’t be rendered in private notes

Actual Result:

Hmm its not here page is displayed.
And markdown images are rendered in Private notes

Workaround:

Unknown

Platforms:

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

[ ] Android: Native
[ ] Android: mWeb Chrome
[ ] iOS: Native
[ ] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/babb6b39-c2f7-4739-a88c-dd90e35844e2

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01c6ef0c33505c63ef
Upwork Job ID: 1771446597185703936
Last Price Increase: 2024-03-23
Automatic offers:

shubham1206agra | Reviewer | 0

Mar 22 '24 20:03 lanitochka17

:wave: 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:

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

Mar 22 '24 20:03 github-actions[bot]

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

Mar 22 '24 20:03 melvin-bot[bot]

@chiragsalian FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

Mar 22 '24 20:03 lanitochka17

Don't think this is blocker since markdown image is new feature and still WIP of polish

cc: @kidroca

Mar 22 '24 20:03 situchan

Yup, I agree too. This does not feel blocker-worthy since it's a new feature. A simple resolution would be to revert the feature here - https://github.com/Expensify/App/pull/37566 but I would prefer if move forward and tidy up this bug.

Mar 22 '24 21:03 chiragsalian

edge case that I wouldn't consider much of a blocker, but what I would recommend is that it opens an attachment carousel that only includes attachments in the private notes message.

Mar 23 '24 00:03 roryabraham

in my opinion the value of inline images way outweighs the downside of this bug, so I wouldn't want to see us revert that because of this

Mar 23 '24 00:03 roryabraham

Proposal

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

Private notes - Hmm its not here page displayed when clicking on thumbnail

What is the root cause of that problem?

We use ImageRenderer to display image
When clicking on an image to open the attachment modal, onPress is called, it works well with report attachments. But in case of private note, the not here page is displayed because we do not have route for this attachment

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

We need to create a new attachment page to display image for other images that do not belong to report.

Create a CommonAttachments page:

function CommonAttachments({route}: ReportAttachmentsProps) {
    const source = Number(route.params.source) || route.params.source;
    const imageSource = tryResolveUrlFromApiRoot(source);
    return (
        <AttachmentModal
            allowDownload
            defaultOpen
            originalFileName='test-image.jpg'
            source={imageSource}
            onModalClose={() => {
                Navigation.goBack();
            }}
        />
    );
}

In here, add:

    ATTACHMENTS: 'attachments',

In here, add:

    ATTACHMENTS: {
        route: '/attachment',
        getRoute: (source: string) => `attachment?source=${encodeURIComponent(source)}` as const
    }

In here, add:

                 <RootStack.Screen
                    name={SCREENS.ATTACHMENTS}
                    options={{
                        headerShown: false,
                        presentation: 'transparentModal',
                    }}
                    getComponent={loadCommonAttachments}
                    listeners={modalScreenListeners}
                />

In here, add:

        [SCREENS.ATTACHMENTS]: ROUTES.ATTACHMENTS.route,

Update this to:

                    onPress={() => {
                       if(report?.reportID){
                        const route = ROUTES.REPORT_ATTACHMENTS.getRoute(report?.reportID ?? '', source);
                        Navigation.navigate(route);
                       }
                       else {
                        const route = ROUTES.ATTACHMENTS.getRoute(source)
                        Navigation.navigate(route)
                       }
                    }}

What alternative solutions did you explore? (Optional)

NA

Result

https://github.com/Expensify/App/assets/161821005/cafc5bf4-ee35-4fb8-9d76-135581dabfda