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

Distance - IOU report for changed details, title is not shown in LHN when user re-login #32111

Closed
6 tasks done
kbecciv opened this issue Nov 28, 2023 · 17 comments
Closed
6 tasks done
Assignees

Comments

@kbecciv
Copy link

kbecciv commented Nov 28, 2023

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.4.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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

  1. Go to https://staging.new.expensify.com/
  2. Tap on Fab
  3. Tap request money - Distance
  4. Enter start and finish waypoints
  5. Tap next
  6. Select a Workspace
  7. Tap distance
  8. Tap on distance request created to open distance request detail page
  9. Tap description
  10. Enter an description and save
  11. Navigate to LHN and note IOU report created for changes made by "set description"
  12. Tap profile icon
  13. Tap sign out
  14. Login as same user
  15. Note the IOU report created for changes made by "set description"

Expected Result:

When user log out and re-login again, the IOU report created for changes made by "set description" must be displayed with "title" in LHN.

Actual Result:

The IOU report created for changes made by "set description" is displayed with "title" in LHN but when user log out and re-login again, the IOU report is displayed "without title" in LHN.

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

Bug6293007_1701170082729.use_this.mp4

View all open jobs on GitHub

@kbecciv kbecciv added the DeployBlockerCash This issue or pull request should block deployment label Nov 28, 2023
Copy link
Contributor

👋 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.

Copy link

melvin-bot bot commented Nov 28, 2023

Triggered auto assignment to @danieldoglas (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@danieldoglas
Copy link
Contributor

@kbecciv , can you confirm this is not happening in production?

@s-alves10
Copy link
Contributor

s-alves10 commented Nov 28, 2023

This looks like a backend issue
When a user signs in, openApp API is called, but this API doesn't return transactions data. Transaction data is returned when openReport API is called and so some gap exists between them

As you can see below, we get report name of the transaction thread from the transaction
https://github.com/Expensify/App/blob/c03a541c909f3619390d6ead73c8df23cd964e62/src/libs/ReportUtils.ts#L1859C5-L1859C5

That's why the transaction thread title appears a bit later
I think we need to return transactions data from openApp call in order to solve this issue

@danieldoglas
Copy link
Contributor

danieldoglas commented Nov 28, 2023

I agree it sounds like a backend issue. I think adding transactions to the OpenApp call is in progress in the backend right now, so I don't think this needs to be a deploy blocker for the app.

@danieldoglas
Copy link
Contributor

but my question remains, if this is not happening in production, it might mean that we had a change in our staging backend that caused this

@kbecciv
Copy link
Author

kbecciv commented Nov 28, 2023

@danieldoglas Issue is not reproduced on production

20231128_184214.mp4

@danieldoglas
Copy link
Contributor

Hm, this is very intersting. I couldn't find anything that was deployed in staging that could cause this.

@shubham1206agra
Copy link
Contributor

@danieldoglas This problem is already known and will be fixed with #31302.
You can see this problem now because we make the title blank instead of a placeholder when the transaction is null or empty.

@danieldoglas
Copy link
Contributor

@shubham1206agra I think those are different issues. Looks like this one specifically was caused by the TS migration that was deployed to staging yesterday with #29012.

I'm removing the deploy blocker on this for now

CC @kubabutkiewicz

@danieldoglas danieldoglas added Daily KSv2 Hourly KSv2 and removed Hourly KSv2 DeployBlockerCash This issue or pull request should block deployment Daily KSv2 labels Nov 28, 2023
@danieldoglas danieldoglas added Daily KSv2 and removed Hourly KSv2 labels Nov 28, 2023
@kubabutkiewicz
Copy link
Contributor

@danieldoglas Checking

@kubabutkiewicz
Copy link
Contributor

@danieldoglas Didn't find the root cause yet, but have some observations.
Firstly I think on dev there is some bug (?) where I cannot find any users to send distance money request

search-bug.mp4

Secondly I think my PR just show hidden bug by adding this if statement which I think is related to bug reported here

App/src/libs/ReportUtils.ts

Lines 1858 to 1863 in 2f9ae2d

const transaction = TransactionUtils.getLinkedTransaction(reportAction);
if (!isNotEmptyObject(transaction)) {
return '';
}
if (TransactionUtils.hasReceipt(transaction) && TransactionUtils.isReceiptBeingScanned(transaction)) {

Here are two videos , one for current main and second from before merging ReportUtils migration

1.mp4
2.mp4

My first guess is that at the begining before opening Report we dont have a data for transactions in Onyx

Onyx.connect({
key: ONYXKEYS.COLLECTION.TRANSACTION,
waitForCollectionCallback: true,
callback: (value) => {
if (!value) {
return;
}
allTransactions = Object.fromEntries(Object.entries(value).filter(([, transaction]) => !!transaction));
},
});

And before opening Report allTransactions are null and from here
https://github.com/Expensify/App/blob/2f9ae2d37354ce3335d16805d05c313ef3d25fa9/src/libs/TransactionUtils.ts#L389-L397C2

we will always get {} as a return value
so here
https://github.com/Expensify/App/blob/0a4985466fce930193290b38684f3c721db199be/src/libs/ReportUtils.ts#L1860C1-L1876
currently we will get '' and previously we will get some not wanted value like 0$ request.

I can revert my change for returning '' but there will still be a bug. And I think @shubham1206agra was mentioning it earlier here

@danieldoglas
Copy link
Contributor

I understand the origin of the bug with the new check, and I think I agree that the real solution is to return the transaction.

We have a PR for that coming up next week, I think we can wait for it.

@melvin-bot melvin-bot bot added the Overdue label Dec 4, 2023
@danieldoglas
Copy link
Contributor

Moving this one to weekly while we wait for the backend pull request

@melvin-bot melvin-bot bot removed the Overdue label Dec 4, 2023
@danieldoglas danieldoglas added Weekly KSv2 and removed Daily KSv2 labels Dec 4, 2023
@melvin-bot melvin-bot bot added the Overdue label Dec 12, 2023
@danieldoglas
Copy link
Contributor

I think this is now in staging. @kbecciv can you please test this?

@melvin-bot melvin-bot bot removed the Overdue label Dec 13, 2023
@kbecciv
Copy link
Author

kbecciv commented Dec 18, 2023

Issue is not reproducible

hi.mp4

@danieldoglas
Copy link
Contributor

Great, closing this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants