[HOLD for payment 2024-07-02] [$125] Expense - "Receipt" label does not appear above receipt placeholder for p2p expense

m-natarajan 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: 1.4.79-3 Reproducible in staging?: y Reproducible in production?: n If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4574596 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:

Go to staging.new.expensify.com
Go to workspace chat.
Submit an expense without receipt.
Go to transaction thread.
Note that "Receipt" label appears above receipt placeholder.
Submit an expense to any user.
Go to transaction thread.

Expected Result:

"Receipt" label will appear above receipt placeholder for p2p expense.

Actual Result:

"Receipt" label does not appear above receipt placeholder for p2p expense.

Workaround:

unknown

Platforms:

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

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

Screenshots/Videos

https://github.com/Expensify/App/assets/38435837/3900f88a-26f0-4cb7-bd48-f40e1520e29f

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01ed4993b4406f58d2
Upwork Job ID: 1798815121621554421
Last Price Increase: 2024-06-06
Automatic offers:

getusha | Reviewer | 102644040
neonbhai | Contributor | 102644041

Issue Owner

Current Issue Owner: @CortneyOfstad

melvin-bot[bot] commented 4 weeks ago

Triggered auto assignment to @luacmartins (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

melvin-bot[bot] commented 4 weeks ago

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

github-actions[bot] commented 4 weeks ago

: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:

Identify the pull request that introduced this issue and revert it.
Find someone who can quickly fix the issue.
Fix the issue yourself.

m-natarajan commented 4 weeks ago

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

m-natarajan commented 4 weeks ago

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

neonbhai commented 4 weeks ago

Proposal

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

"Receipt" label does not appear above receipt placeholder for p2p expense

What is the root cause of that problem?

We check for a paid group policy when showing receipt header https://github.com/Expensify/App/blob/594ae488b4b933d52307caa82da8f6e385e65723/src/components/ReportActionItem/MoneyRequestView.tsx#L333

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

We should remove the paid group policy check here

const shouldShowReceiptHeader = isReceiptAllowed && (shouldShowReceiptEmptyState || shouldShowMapOrReceipt) && canUseViolations;

luacmartins commented 3 weeks ago

I don't think we need to block the deploy on this and it seems to be a regression from https://github.com/Expensify/App/issues/41489. @Krishna2323 @sobitneupane could you please take a look?

Krishna2323 commented 3 weeks ago

I believe we have decided to hide header and receipt notice violations for unpaid policy groups.

cc: @cead22 @sobitneupane

sobitneupane commented 3 weeks ago

Receipt Header was added for Notice Violations which is present only in paid policy. So, it is not being displayed for p2p expenses. @cead22 Can you please confirm the expected behavior here? Would we want to show receipt header for all the cases where either Receipt or Receipt Place Holder is present?

cead22 commented 3 weeks ago

I think it makes sense to show the receipt header in all cases, since all the other fields have headers as well. What do you think @JmillsExpensify ?

melvin-bot[bot] commented 3 weeks ago

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

melvin-bot[bot] commented 3 weeks ago

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

melvin-bot[bot] commented 3 weeks ago

Upwork job price has been updated to $125

dominictb commented 3 weeks ago

Proposal

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

"Receipt" label does not appear above receipt placeholder for p2p expense.

What is the root cause of that problem?

We're only showing the Receipt label and Note violation if the report is paid group policy

https://github.com/Expensify/App/blob/4fcc5a9fbef90114ab4f38a1be698c471e3e75d1/src/components/ReportActionItem/MoneyRequestView.tsx#L328-L329

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

We should remove the isPaidGroupPolicy check on both shouldShowNotesViolations and shouldShowReceiptHeader to show the Note violation if exists and Receipt label for all expense cases.

https://github.com/Expensify/App/blob/4fcc5a9fbef90114ab4f38a1be698c471e3e75d1/src/components/ReportActionItem/MoneyRequestView.tsx#L328-L329

What alternative solutions did you explore? (Optional)

NA

CortneyOfstad commented 3 weeks ago

@getusha — we have a few proposals — can you provide feedback on those by EOD? Thank you!

getusha commented 3 weeks ago

This has a straightforward fix @neonbhai's proposal looks good to me 🎀 👀 🎀 C+ Reviewed!

melvin-bot[bot] commented 3 weeks ago

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

melvin-bot[bot] commented 3 weeks ago

📣 @getusha 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

melvin-bot[bot] commented 3 weeks ago

📣 @neonbhai 🎉 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 📖

neonbhai commented 3 weeks ago

Raising PR soon!

CortneyOfstad commented 3 weeks ago

Awesome — thanks @neonbhai!

getusha commented 3 weeks ago

Waiting a PR from @neonbhai

getusha commented 2 weeks ago

@neonbhai any updates?

neonbhai commented 2 weeks ago

PR is ready for review!

CortneyOfstad commented 1 week ago

PR is still being reviewed!

CortneyOfstad commented 1 week ago

PR deployed to staging 4 days ago 🎉

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

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

https://github.com/Expensify/App/pull/43405

If no regressions arise, payment will be issued on 2024-07-02. :confetti_ball:

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

@neonbhai requires payment automatic offer (Contributor)
@getusha requires payment automatic offer (Reviewer)

melvin-bot[bot] commented 1 week ago

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:

[ ] [@getusha] The PR that introduced the bug has been identified. Link to the PR:
[ ] [@getusha] 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:
[ ] [@getusha] 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:
[ ] [@getusha] Determine if we should create a regression test for this bug.
[ ] [@getusha] 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.
[ ] [@CortneyOfstad] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

CortneyOfstad commented 4 days ago

@getusha can you please complete the checklist above before beginning of next week, so there is no delay in payment? Thanks!

getusha commented 4 days ago

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:

[@getusha] The PR that introduced the bug has been identified. Link to the PR: https://github.com/Expensify/App/pull/41635 [@getusha] 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: https://github.com/Expensify/App/pull/41635#issuecomment-2197523544 [@getusha] 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: N/a [@getusha] Determine if we should create a regression test for this bug. No, this bug isn't impactful enough to create a regression test [@getusha] 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. N/a

CortneyOfstad commented 4 hours ago

@getusha looks like we're still waiting for you to accept the payment in Upwork. Please let me know once you accept and I can get that paid ASAP. Thank you!

Expensify / App