Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.56k stars 2.9k forks source link

Android- expense - In camera screen receipt header is missing #49881

Closed lanitochka17 closed 1 month ago

lanitochka17 commented 1 month ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 9.0.41-1 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 Issue reported by: Applause - Internal Team

Action Performed:

  1. Launch app
  2. Tap on a workspace chat
  3. Create a expense
  4. Open the expense and tap plus receipt placeholder and try to upload a image using camera

Expected Result:

In camera screen receipt header must not be missing

Actual Result:

In camera screen receipt header is missing Reproduction on: Redmi note 10s android 13

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/15f63d78-51c8-45fb-9492-c1c0e9037c22

View all open jobs on GitHub

melvin-bot[bot] commented 1 month ago

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

github-actions[bot] commented 1 month ago

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

  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.
lanitochka17 commented 1 month ago

Production:

huult commented 1 month ago

Edited by proposal-police: This proposal was edited at 2024-09-29 07:56:39 UTC.

Proposal

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

Android- expense - In camera screen receipt header is missing

What is the root cause of that problem?

The backTo is undefined, which prevents the wrapper from showing and causes this issue.

https://github.com/Expensify/App/blob/3047c1baa21f47825f5c1ffd652184341dd22256/src/pages/iou/request/step/IOURequestStepScan/index.native.tsx#L479

backTo is undefined because Navigation.getReportRHPActiveRoute() returns undefined when clicked from the money request view. https://github.com/Expensify/App/blob/3047c1baa21f47825f5c1ffd652184341dd22256/src/components/ReportActionItem/MoneyRequestView.tsx#L538

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

We should check if getReportRHPActiveRoute returns undefined, and if so, use getActiveRouteWithoutParams.

// src/components/ReportActionItem/MoneyRequestView.tsx#L538
{shouldShowReceiptEmptyState && (
                    <ReceiptEmptyState
                        hasError={hasErrors}
                        disabled={!canEditReceipt}
                        onPress={() =>
                            Navigation.navigate(
                                ROUTES.MONEY_REQUEST_STEP_SCAN.getRoute(
                                    CONST.IOU.ACTION.EDIT,
                                    iouType,
                                    transaction?.transactionID ?? '-1',
                                    report?.reportID ?? '-1',
-                                    Navigation.getReportRHPActiveRoute(),
+                                    Navigation.getReportRHPActiveRoute() || Navigation.getActiveRouteWithoutParams(),
                                ),
                            )
                        }
                    />
                )}

What alternative solutions did you explore? (Optional)

We used getActiveRouteWithoutParams instead of getReportRHPActiveRoute.

// src/components/ReportActionItem/MoneyRequestView.tsx#L538
{shouldShowReceiptEmptyState && (
                    <ReceiptEmptyState
                        hasError={hasErrors}
                        disabled={!canEditReceipt}
                        onPress={() =>
                            Navigation.navigate(
                                ROUTES.MONEY_REQUEST_STEP_SCAN.getRoute(
                                    CONST.IOU.ACTION.EDIT,
                                    iouType,
                                    transaction?.transactionID ?? '-1',
                                    report?.reportID ?? '-1',
-                                    Navigation.getReportRHPActiveRoute(),
+                                    Navigation.getActiveRouteWithoutParams(),
                                ),
                            )
                        }
                    />
                )}
POC https://github.com/user-attachments/assets/7982ca0b-18b3-4c66-87eb-14eb4e83d009
Nodebrute commented 1 month ago

Edited by proposal-police: This proposal was edited at 2024-09-29 20:35:32 UTC.

Proposal

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

Unable to drag and drop receipt to Upload receipt RHP

What is the root cause of that problem?

We recently started using Navigation.getReportRHPActiveRoute() everywhere, but this issue only happens on Scan page https://github.com/Expensify/App/blob/3047c1baa21f47825f5c1ffd652184341dd22256/src/pages/iou/request/step/IOURequestStepScan/index.tsx#L698 It's because shouldShowWrapper will be false and the top part will be cut off https://github.com/Expensify/App/blob/3047c1baa21f47825f5c1ffd652184341dd22256/src/pages/iou/request/step/IOURequestStepScan/index.tsx#L698

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

We can create a new variable const isEditing = action === CONST.IOU.ACTION.EDIT and we can pass it here https://github.com/Expensify/App/blob/3047c1baa21f47825f5c1ffd652184341dd22256/src/pages/iou/request/step/IOURequestStepScan/index.tsx#L698

            shouldShowWrapper={isEditing}

We should also fix this in IOURequestStepScan/index.native.tsx file We should check for other pages too where we have this issue and fix it

What alternative solutions did you explore? (Optional)

Or we can do something like this here

         shouldShowWrapper={!!backTo || isEditing}

Alternative 2 Pass shouldShowWrapper here

 shouldShowWrapper

https://github.com/Expensify/App/blob/3047c1baa21f47825f5c1ffd652184341dd22256/src/pages/iou/request/step/IOURequestStepScan/index.tsx#L698

melvin-bot[bot] commented 1 month ago

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

huult commented 1 month ago

Proposal Update

NikkiWines commented 1 month ago

Looks like this is already being resolved here - asked for a status update there.

rayane-djouah commented 1 month ago

The linked PR fixes this blocker and is ready for merging and cherry-picking to staging

luacmartins commented 1 month ago

Thank you @rayane-djouah!

luacmartins commented 1 month ago

PR merged and being CPed

luacmartins commented 1 month ago

Confirmed fix on staging.