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.35k stars 2.77k forks source link

[$250] Distance - LHN shows "Hidden" report with RBR after dismissing invalid waypoint error #47976

Open IuliiaHerets opened 4 weeks ago

IuliiaHerets commented 4 weeks 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: v9.0.24-1 Reproducible in staging?: Y Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: Exp https://expensify.testrail.io/index.php?/tests/view/4885243 Email or phone of affected tester (no customers): applausetester+kh050806@applause.expensifail.com Issue reported by: Applause Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to workspace chat.
  3. Submit an expense (any type).
  4. Go offline.
  5. Submit a distance expense with invalid waypoints (any random letters can be invalid waypoint).
  6. Go to expense report.
  7. Go online.
  8. Dismiss the "Please select a valid waypoint" error on the expense report (not transaction thread).
  9. Note that LHN shows a "Hidden" report with RBR after dismissing the error on the expense report.

Expected Result:

LHN will not show a "Hidden" report with RBR after dismissing the error on the expense report.

Actual Result:

LHN shows a "Hidden" report with RBR after dismissing the invalid waypoint error on the expense report.

Workaround:

Unknown

Platforms:

Screenshots/Videos

https://github.com/user-attachments/assets/61619e5c-f84f-4d5a-9e8a-0622507f092f

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0134c247d0315476d6
  • Upwork Job ID: 1828546129529773284
  • Last Price Increase: 2024-09-03
  • Automatic offers:
    • nkdengineer | Contributor | 103911159
melvin-bot[bot] commented 4 weeks ago

Triggered auto assignment to @johncschuster (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

IuliiaHerets commented 4 weeks ago

We think that this bug might be related to #wave-collect - Release 1

IuliiaHerets commented 4 weeks ago

@johncschuster 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

NJ-2020 commented 4 weeks ago

Edited by proposal-police: This proposal was edited at 2024-08-25 07:41:13 UTC.

Proposal

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

Distance - LHN shows "Hidden" report with RBR after dismissing invalid waypoint error

What is the root cause of that problem?

When we dismiss the error we run the below code: https://github.com/Expensify/App/blob/0c8455280c738a5db596f34409a0a3177e682e7f/src/pages/home/report/ReportActionItem.tsx#L939-L943 We invoke Transaction.clearError and ReportActions.clearAllRelatedReportActionErrors function, but inside the ReportActions.clearAllRelatedReportActionErrors function we invoke the clearReportActionErrors function which will delete the optimistic data if the pending action is ADD or the data is from optimistic mean from Onyx https://github.com/Expensify/App/blob/0c8455280c738a5db596f34409a0a3177e682e7f/src/libs/actions/ReportActions.ts#L29-L32 And delete the linkedTransaction if exist https://github.com/Expensify/App/blob/0c8455280c738a5db596f34409a0a3177e682e7f/src/libs/actions/ReportActions.ts#L37 So when we try to get the title inside the LHN, the parentReportAction become undefined because inside the clearReportActionErrors function will delete the optimistic data if the pending action is ADD or the data is from optimistic mean from Onyx https://github.com/Expensify/App/blob/071f11ce1012db7d07eb750d74f58b552eba6144/src/libs/actions/ReportActions.ts#L28-L32

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

We can remove the report chat from the LHN because when we clear cache and restart, it will disappear from the LHN as it is not exist in the BE

Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${reportAction.childReportID}`, null)

Result

https://github.com/user-attachments/assets/eafbbb72-8eb0-4b43-8e27-8363441fcc0c

What alternative solutions did you explore? (Optional)

dominictb commented 4 weeks ago

Proposal

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

LHN shows a "Hidden" report with RBR after dismissing the invalid waypoint error on the expense report.

What is the root cause of that problem?

When users remove error in expense report, we also delete reportActions related to this

https://github.com/Expensify/App/blob/f58439cfb116abd338dae880088a26725794108e/src/libs/actions/ReportActions.ts#L90-L97

In this case we delete all reportActions in report, so the parentReportActions is undefined

https://github.com/Expensify/App/blob/0c8455280c738a5db596f34409a0a3177e682e7f/src/libs/ReportUtils.ts#L3678

-> The LHN show Hidden

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

If the CREATED action is deleted, we should clear the report too

    if (reportAction.childReportID && ignore !== 'child') {
        const childActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportAction.childReportID}`] ?? {};
        let shouldDeleteReport = false
        Object.values(childActions).forEach((action) => {
            const childErrorKeys = Object.keys(action.errors ?? {}).filter((err) => errorKeys.includes(err));
            if(!shouldDeleteReport && childErrorKeys.length > 0 && action.actionName==='CREATED'){
                shouldDeleteReport = true
            }
            clearAllRelatedReportActionErrors(reportAction.childReportID ?? '-1', action, 'parent', childErrorKeys);
        });
        if(shouldDeleteReport){
            Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${reportAction.childReportID}`, null);
        }
    }

What alternative solutions did you explore? (Optional)

nkdengineer commented 4 weeks ago

Proposal

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

LHN shows a "Hidden" report with RBR after dismissing the invalid waypoint error on the expense report.

What is the root cause of that problem?

When we clear the report action error, we call clearAllRelatedReportActionErrors

https://github.com/Expensify/App/blob/f58439cfb116abd338dae880088a26725794108e/src/pages/home/report/ReportActionItem.tsx#L940

Which will clear the report action and the linked transaction if the action has pendingAction is add https://github.com/Expensify/App/blob/f58439cfb116abd338dae880088a26725794108e/src/libs/actions/ReportActions.ts#L28-L40

Then Hidden appears for the transaction thread report in LHN because the parent report action and the linked transaction are cleared

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

We also need to clear the transaction thread report when we clear the linked transaction here

if (linkedTransactionID) {
    Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${linkedTransactionID}`, null);
    Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${reportAction.childReportID}`, null);
}

https://github.com/Expensify/App/blob/f58439cfb116abd338dae880088a26725794108e/src/libs/actions/ReportActions.ts#L36-L40

What alternative solutions did you explore? (Optional)

melvin-bot[bot] commented 3 weeks ago

Job added to Upwork: https://www.upwork.com/jobs/~0134c247d0315476d6

melvin-bot[bot] commented 3 weeks ago

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

NJ-2020 commented 3 weeks ago

Proposal

Updated

melvin-bot[bot] commented 2 weeks ago

@johncschuster, @getusha Huh... This is 4 days overdue. Who can take care of this?

melvin-bot[bot] commented 2 weeks ago

πŸ“£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πŸ’Έ

johncschuster commented 2 weeks ago

Bumped in Slack

getusha commented 2 weeks ago

~~@NJ-2020's proposal looks good to me, as it was the first to identify the root cause and provide a working solution. πŸŽ€ πŸ‘€ πŸŽ€ C+ Reviewed.~~

melvin-bot[bot] commented 2 weeks ago

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

nkdengineer commented 2 weeks ago

We can keep the current code, but we can also remove the report chat from LHN because when we clear cache and restart, it will disappear from the LHN as it is not exist in the BE

@getusha I think this solution doesn't identify what we should do or where we should do it. And this proposal is only added the code change without anything after I posted my proposal.

getusha commented 2 weeks ago

I thought it was edited before other proposals, this is what the most recent edit i saw.

Screenshot 2024-09-04 at 2 17 03 in the afternoon

Thanks for flagging @nkdengineer i will re-check

Gonals commented 2 weeks ago

@getusha, let me know what you end up deciding!

melvin-bot[bot] commented 2 weeks ago

@johncschuster @Gonals @getusha this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

johncschuster commented 1 week ago

@getusha bump on the above!

getusha commented 1 week ago

@nkdengineer's proposal looks good to me, has a clear RCA and the solution makes the most sense. πŸŽ€ πŸ‘€ πŸŽ€ C+ Reviewed

melvin-bot[bot] commented 1 week ago

Current assignee @Gonals is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

melvin-bot[bot] commented 1 week ago

πŸ“£ @nkdengineer πŸŽ‰ An offer has been automatically sent to your Upwork account for the Contributor role πŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review πŸ§‘β€πŸ’» Keep in mind: Code of Conduct | Contributing πŸ“–

johncschuster commented 1 week ago

Not overdue. @nkdengineer is working on a PR