[HOLD for payment 2024-06-20] [Search v1] Opening transactions on search page can lead to not found page

[luacmartins]

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:
Reproducible in staging?:
Reproducible in production?:
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
Expensify/Expensify Issue URL:
Issue reported by:
Slack conversation:

Action Performed:

1. Open the search page, Profile > Troubleshoot > New Search Page
2. Click on a transaction
3. Verify that you see a not found page

Note: this seems to only happen for older transactions that don't have a transaction thread created since the value for transactionThreadReportID is 0.

Expected Result:

We should create the transaction thread and let users open the transaction

Actual Result:

Not found page is shown

Workaround:

N/A

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

https://github.com/Expensify/App/assets/22219519/ed64830c-ca4d-4ba7-b95e-c066fe5d6084

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01b78bbfebf5ef557c
• Upwork Job ID: 1786400627125800960
• Last Price Increase: 2024-05-03

Issue Owner

Current Issue Owner: @jliexpensify

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to Contributor Plus for review of internal employee PR - @Santhosh-Sellavel (Internal)

[grgia]

What we chatted about before:
Create a command - OpenTransactionThread (maybe OpenSearchItemPreview if we later need to add support for previewing reports? )
That calls auth.transaction.createTransactionThread if it doesn't already exist

App/src/components/Search.tsx

Line 68 in b475b28

openReport(item.transactionThreadReportID);

Use the loading indicator on the page

[jliexpensify]

Not overdue. Should we switch to Weekly to stop the pings?

[melvin-bot]

@luacmartins, @jliexpensify, @Santhosh-Sellavel Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[luacmartins]

I think we might need a new command as @grgia described above, since calling openReport wouldn't be enough because we might not have a lot of the data needed, e.g. reportActions, parentReportActions, participant list, etc

[luacmartins]

@jliexpensify I think we should leave this as daily, since it's high priority

[Kicu]

@luacmartins @grgia I did some testing and in my experience the problem is almost exclusively that a lot of items simply have reportID set to 0 and so we try to open route with 0 inside and that gets us the "Not Found" page - which is correct behavior.

A lot of other items that looked legit opened correctly:

rec-web-openreport.mp4

Sorry if I'm misunderstanding something - can't this be fixed in api by simply always returning some existing reportID?

[luacmartins]

You understood it right. This happens because in the past we didn't create these threads for all reports, so they might not exist and we end up returning 0. The solution is to create a new command that will create the thread if it doesn't exist.

[luacmartins]

Still looking for volunteers. @grgia are you interested in working on this one?

[lakchote]

You understood it right. This happens because in the past we didn't create these threads for all reports, so they might not exist and we end up returning 0. The solution is to create a new command that will create the thread if it doesn't exist.

It should be fixed by https://github.com/Expensify/Auth/pull/10456. We should create threads automatically from now on, even for old preexisting transactions.

[trjExpensify]

Nice, @lakchote! That's on prod, so we can retest then, yeah? If so, @grgia @Kicu did you have some transactions in your account that definitely weren't working before to validate @lakchote's theory?

[Kicu]

@trjExpensify @lakchote

For me on dev every row that "looks" normally is correctly opening a report/IOU in RHP.
However every row with __fake__ field has no transactionID and opens error page. I have no idea what these rows are and where do they come from.

Seems there is an issue for this: #41985

// edit
btw Im using staging api if that changes anything

sample object with fake

{
  accountID: 0,
  action: "view",
  amount: 55500,
  canDelete: false,
  category: "",
  comment: {
    comment: "",
  },
  created: "2024-04-29 10:13:59",
  currency: "PLN",
  hasEReceipt: false,
  managerID: 0,
  merchant: "(none)",
  modifiedAmount: 0,
  modifiedCreated: "",
  modifiedCurrency: "",
  modifiedMerchant: "",
  parentTransactionID: "",
  policyID: "",
  reportID: "-2",
  reportType: "",
  tag: "",
  transactionID: "5321901916525874903",
  transactionThreadReportID: "0",
  type: "cash",
  from: {
    accountID: 0,
    avatar:
      "https://d2k5nsl2zxldvw.cloudfront.net/images/avatars/default-avatar_1.png",
    displayName: "__fake__",
    firstName: "",
    lastName: "",
    login: "__fake__",
    payPalMeAddress: "",
    phoneNumber: "",
    pronouns: "",
    timezone: {
      automatic: true,
      selected: "America/Los_Angeles",
    },
    validated: false,
  },
  to: {
    accountID: 0,
    avatar:
      "https://d2k5nsl2zxldvw.cloudfront.net/images/avatars/default-avatar_1.png",
    displayName: "__fake__",
    firstName: "",
    lastName: "",
    login: "__fake__",
    payPalMeAddress: "",
    phoneNumber: "",
    pronouns: "",
    timezone: {
      automatic: true,
      selected: "America/Los_Angeles",
    },
    validated: false,
  },
  shouldShowMerchant: true,
  shouldShowCategory: false,
  shouldShowTag: false,
  shouldShowTax: false,
  keyForList: "5321901916525874903",
}

[grgia]

@trjExpensify we haven't implemented the fix for this one yet

[grgia]

I missed this comment #41578 (comment) let's see

[grgia]

@lakchote the chronos expenses are making this hard to test since those won't open / take up all of my results

Screen.Recording.2024-05-15.at.1.27.15.PM.mov

[luacmartins]

I'll work on creating the thread on Monday.

[melvin-bot]

@luacmartins, @jliexpensify, @Santhosh-Sellavel Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.81-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Create transaction thread if needed #43070

If no regressions arise, payment will be issued on 2024-06-18. 🎊

For reference, here are some details about the assignees on this issue:

• @Santhosh-Sellavel requires payment through NewDot Manual Requests

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@luacmartins] The PR that introduced the bug has been identified. Link to the PR:
• [@luacmartins] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@luacmartins] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@Santhosh-Sellavel] Determine if we should create a regression test for this bug.
• [@Santhosh-Sellavel] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@jliexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[jliexpensify]

Payment Summary

• C+: @Santhosh-Sellavel $250 (ND payment)

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.82-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Create transaction thread if needed #43070

If no regressions arise, payment will be issued on 2024-06-20. 🎊

For reference, here are some details about the assignees on this issue:

• @Santhosh-Sellavel requires payment through NewDot Manual Requests

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@luacmartins] The PR that introduced the bug has been identified. Link to the PR:
• [@luacmartins] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@luacmartins] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@Santhosh-Sellavel] Determine if we should create a regression test for this bug.
• [@Santhosh-Sellavel] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@jliexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[jliexpensify]

Is a checklist needed here @Santhosh-Sellavel ?

[luacmartins]

No checklist needed. This is a new feature and we'll add tests as part of the project wrap up

[jliexpensify]

Awesome, ty!

Here's the ND payment summary for JMills

[melvin-bot]

Payment Summary

Upwork Job

• Reviewer: @Santhosh-Sellavel owed $250 via NewDot

BugZero Checklist (@jliexpensify)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1786400627125800960/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct