Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD #45408] Expense - Currently opened expense report is not displayed in LHN after returning from Search #46192

Closed
2 of 6 tasks
lanitochka17 opened this issue Jul 25, 2024 · 16 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Reviewing Has a PR in review Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Jul 25, 2024

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: 9.0.12-0
Reproducible in staging?: Y
Reproducible in production?: Y
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. Go to staging.new.expensify.com
  2. Go to FAB > Submit expense > Manual
  3. Submit a manual expense to user with no unsettled expense
  4. Go to Search
  5. Click on the expense submitted in Step 3
  6. Return to Inbox

Expected Result:

The currently opened expense thread will be displayed in LHN

Actual Result:

The currently opened expense thread is not displayed in LHN
This issue only happens when there is only one expense in the report

Workaround:

Unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6552332_1721892704989.bandicam_2024-07-25_15-28-17-666.mp4

View all open jobs on GitHub

@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jul 25, 2024
Copy link

melvin-bot bot commented Jul 25, 2024

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

@lanitochka17
Copy link
Author

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

@FitseTLT
Copy link
Contributor

FitseTLT commented Jul 25, 2024

Proposal

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

Expense - Currently opened expense report is not displayed in LHN after returning from Search

What is the root cause of that problem?

When we have an iou report with one transaction the item in search is a transaction type and for transaction case we are opening the transactionThreadReport here

let reportID = SearchUtils.isTransactionListItemType(item) ? item.transactionThreadReportID : item.reportID;

But there is a code here that returns falsey when the report is isOneTransactionThread to exclude it from lhn
if (ReportUtils.isOneTransactionThread(report.reportID, report.parentReportID ?? '0', parentReportAction)) {
return;
}

This is to avoid having two lhn items for the same transaction thread because in the case of one transaction in an iou pressing on the iou report opens the only transaction thread as it is the only one (isOneTransactionThread) in it. So we don't need an lhn for isOneTransactionThread case.

What alternative solutions did you explore? (Optional)

This issue doesn't occur when we open a report via report preview because in that case we open the iou report (with one transaction) not the transactionThreadReport therefore we should be consistent and follow the same logic in search page here too.
So, we should open the the report (iou report) instead of transaction thread here when the item is transaction type in here

let reportID = SearchUtils.isTransactionListItemType(item) ? item.transactionThreadReportID : item.reportID;

change it to

        let reportID = item.reportID;

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

@nkdengineer
Copy link
Contributor

Proposal

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

The currently opened expense thread is not displayed in LHN
This issue only happens when there is only one expense in the report

What is the root cause of that problem?

When we open the report from the search, we open the transaction thread report. Then after we return from Search, this report is opened but we hide this in LHN if the expense report is the combine report

if (ReportUtils.isOneTransactionThread(report.reportID, report.parentReportID ?? '0', parentReportAction)) {

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

We should remove this condition here

if (ReportUtils.isOneTransactionThread(report.reportID, report.parentReportID ?? '0', parentReportAction)) {

And in shouldReportBeInOptionList function we should move this condition below the condition here so the transaction thread report of the combine report will only display in LHN if it's opened.

What alternative solutions did you explore? (Optional)

For snapshot transaction item, BE should return an extra field isOneTransactionThread to know if it's one transaction thread or not, and then we will use this here to open the combined report in this case.

let reportID = SearchUtils.isTransactionListItemType(item) && !item.isOneTransactionThread ? item.transactionThreadReportID : item.reportID; 

let reportID = SearchUtils.isTransactionListItemType(item) ? item.transactionThreadReportID : item.reportID;

@FitseTLT
Copy link
Contributor

Updated only to add more explanation to my first solution.

@trjExpensify
Copy link
Contributor

@lanitochka17, I really don't understand this bug report as written in the OP. @luacmartins does this make sense to you? 🤔

@FitseTLT
Copy link
Contributor

FitseTLT commented Jul 25, 2024

@trjExpensify Whenever you open a report it is included in LHN (highlighted lhn item representing it) but in this case if you create an expense where it is the only expense in iou report then when you open it from search the report will not be any lhn item corresponding to it (which was supposed to be the highlighted one in normal cases) 👍

@luacmartins
Copy link
Contributor

Hmm I think this has the same root cause as #45408. Because we are opening the transaction thread in the RHP, we record the last visited report as the thread. Then when we navigate to the LHN we don't see it because we hide the thread if it's a one expense report.

@luacmartins
Copy link
Contributor

I think we can put this issue on hold for #45408.

@luacmartins luacmartins self-assigned this Jul 26, 2024
@luacmartins luacmartins changed the title Expense - Currently opened expense report is not displayed in LHN after returning from Search [HOLD #45408] Expense - Currently opened expense report is not displayed in LHN after returning from Search Jul 26, 2024
@luacmartins luacmartins added Weekly KSv2 and removed Daily KSv2 labels Jul 26, 2024
@trjExpensify
Copy link
Contributor

Makes sense!

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Jul 30, 2024
Copy link

melvin-bot bot commented Aug 16, 2024

⚠️ 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.

@melvin-bot melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels Aug 23, 2024
Copy link

melvin-bot bot commented Aug 23, 2024

This issue has not been updated in over 15 days. @trjExpensify, @luacmartins eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

@trjExpensify
Copy link
Contributor

This is on hold, Melv.

@trjExpensify trjExpensify added Weekly KSv2 and removed Monthly KSv2 labels Aug 23, 2024
@trjExpensify
Copy link
Contributor

Oh, why is there a Reviewing label on this @luacmartins? Aren't we held on #45408?

@luacmartins
Copy link
Contributor

luacmartins commented Aug 26, 2024

@trjExpensify I believe that I fixed #45408 and this issue with this PR. Can we retest to confirm that's indeed fixed and then close this issue? We'd handle payment via #45408.

@trjExpensify
Copy link
Contributor

Laveeeely! Looks fixed to me

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

5 participants