Closed m-natarajan closed 3 months ago
Triggered auto assignment to @carlosmiceli (DeployBlockerCash
), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.
Triggered auto assignment to @garrettmknight (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.
: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:
@garrettmknight 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
We think that this bug might be related to #wave-collect - Release 1
Scanning expenses display 0.00 in the Total column, and the Merchant column is blank.
transactionItem.formattedMerchant
and transactionItem.formattedMerchant
are displayed without checking if a scan request state is "SCANNING". There is no state
property on transactionItem.receipt
for TotalCell and MerchantCell. Therefore we can't determine if the scan transaction state is "SCANNING" following the same pattern used on MoneyRequestView. This is because BE doesn't return transaction.receipt.state
value on a transaction object for Search
query, but it does for RequestMoney
query.
Search query BE response
RequestMoney query BE response
transactionItem.receipt.state
value for Search
query transactionItem.formattedTotal
and transactionItem.formattedMerchant
, if scan transaction stat is 'SACNNING' or display 'Scanning...' if the sate is not 'SCANNING' N/A
Looking at the checklist, I don't think this is a deploy blocker, but it does look like a bug that could be External.
Unable to auto-create job on Upwork. The BZ team member should create it manually for this issue.
Triggered auto assignment to Contributor-plus team member for initial proposal review - @mollfpr (External
)
Checking 👀
@garrettmknight question, why does it say "Unable to auto-create job on Upwork. The BZ team member should create it manually for this issue." when adding the External label?
@JmillsExpensify @trjExpensify what's the expected behavior here?
Scanning expense displays 0.00 in Total column and the Merchant column is blank
The root cause of the problem is that the MerchantCell
, TotalCell
, and the TaxCell
in the TransactionListItemRow
does not check if the receipt is being scanned.
Therefore, the TransactionListItemRow
will show the transactionItem.formattedMerchant
which is an empty string.
Also, both the TotalCell
and the TaxCell
rows will show zero.
Hence, we should check if the transaction has a receipt and is being scanned so we should show scanning
in those cells.
We check with the following condition illustrated below.
transactionItem.hasEReceipt && TransactionUtils.isReceiptBeingScanned(transactionItem)
For the MerchantCell
, we can change the text prop to something like the one below.
text={transactionItem.shouldShowMerchant ? (transactionItem.hasEReceipt && TransactionUtils.isReceiptBeingScanned(transactionItem) ? transactionItem.formattedMerchant : translate('iou.receiptStatusTitle')) : description}
For the TotalCell
and the TaxCell
, we can change the text prop to something like the one below.
text={transactionItem.hasEReceipt && TransactionUtils.isReceiptBeingScanned(transactionItem) ? translate('iou.receiptStatusTitle') : CurrencyUtils.convertToDisplayString(transactionItem.formattedTotal, currency)}
We can change the TransactionUtils.getDescription
function to return scanning
if the receipt is scanning and transactionItem.shouldShowMerchant
is false
.
@JmillsExpensify @trjExpensify what's the expected behavior here?
Great question! I don't think we really considered this initially. We could go with the Scanning...
in the merchant
and amount
fields, or we could do something more visual on the row to signal that it's in the scanning state perhaps?
CC: @Expensify/design for thoughts!
I am down with that as an easy place to start.
I am down with that as an easy place to start.
Agree. This seems like the most straight forward approach to fix this quickly. I'm not against trying to improve it, but I like starting here.
Updated my proposal expounding on my alternative solution and discarding my main solution.
@garrettmknight question, why does it say "Unable to auto-create job on Upwork. The BZ team member should create it manually for this issue." when adding the External label?
@carlosmiceli sometimes the automation fails so the BZ team member has to create the Upwork job manually. Typically, I just create a separate issue like this and add External
to run the automation again.
@Tony-MK The merchant field shows Scanning even though the expense is scanned.
The proposal from @etCoderDysto looks good to me!
🎀 👀 🎀 C+ reviewed!
Current assignees @luacmartins and @carlosmiceli are eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.
Thank you! I will raise a pr ASAP.
It looks like the selected proposal relies on backend changes. @carlosmiceli are you available to work on that?
Yes! I'll wait for that PR and then do the BE part.
Hmm I think the App PR depends on the backend providing this data, no? So shouldn't we start with the BE PR?
Ah, good point, I skimmed it quickly and didn't think of the best order. I'll work on it as soon as I can 🙏
I will raise a pr once the BE pr is merged.
Hi @mollfpr this is my first PR, and I want to check if it is alright to apply on other issues until BE pr is merged?
New contributors are limited to working on one job at a time
I have seen the above guidline but I wanted to make sure if it is alright to apply on other issues until the BE pr is merged.
Hey @etCoderDysto - since this one is held on raising the BE, you can apply to other issues. Feel free to link this comment as proof of approval.
Hey @etCoderDysto - since this one is held on raising the BE, you can apply to other issues. Feel free to link this comment as proof of approval.
Thank you, Garrett🙇♂️
@etCoderDysto I think you could get started on your PR too and assume that the API response would include the receipt state. We'd have to wait on the backend changes to test it properly though.
@etCoderDysto I think you could get started on your PR too and assume that the API response would include the receipt state. We'd have to wait on the backend changes to test it properly though.
Great! I will do that.
Not overdue Melv!
@carlosmiceli, @garrettmknight, @luacmartins, @mollfpr, @etCoderDysto Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
Not overdue, just waiting on BE progress so we can unlock everything.
Came back today from OOO, trying to catch up some things and ideally I should be able to start this tomorrow.
@luacmartins added you as reviewer for the Auth fix.
Approved & merged
@mollfpr PR is ready for review.
⚠️ 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.
Reviewing
label has been removed, please complete the "BugZero Checklist".
The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.3-7 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2024-07-10. :confetti_ball:
For reference, here are some details about the assignees on this issue:
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:
[Upwork Job]()
Payment Summary:
@etCoderDysto offer out to you in Upwork
I have accepted the offer. Thank you!
Contributor paid, dropping to weekly for reveiwer to request.
[@mollfpr] The PR that introduced the bug has been identified. Link to the PR: [@mollfpr] 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:
No offending PR was found.
[@mollfpr] 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:
The regression step should be enough.
[@mollfpr] Determine if we should create a regression test for this bug. [@mollfpr] 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.
$250 approved for @mollfpr
We'll add tests as part of the project wrap up, so no need to do that now.
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: 1.4.81-8 Reproducible in staging?: y Reproducible in production?: new feature 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:
Expected Result:
The scanning expense will display "Scanning" in Merchant and Total column, similar to transaction thread.
Actual Result:
The scanning expense displays 0.00 in Total column, and the Merchant column is blank.
Workaround:
unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
https://github.com/Expensify/App/assets/38435837/60340ad0-ad1b-42c9-8dd3-7736fc948361
View all open jobs on GitHub
Issue Owner
Current Issue Owner: @garrettmknight