[HOLD for payment 2024-07-17] [$250] Receipt required over undefined is shown in the violation message

[m-natarajan]

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.85-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: @puneetlath
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1718718827355089

Action Performed:

Precondition : As a admin set up expense violation as " Receipt required amount" to "0" in OldDot control policy

1. Invite an employee to the above policy
2. As a employee submit a report without receipt
3. As an admin open the report submitted

Expected Result:

Correct error message for the receipt required violation should appear

Actual Result:

"receipt required over undefined" message appears

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e9b622536ce232c5
• Upwork Job ID: 1803138142417847354
• Last Price Increase: 2024-06-18
• Automatic offers:
• ahmedGaber93 | Reviewer | 102810235
• abzokhattab | Contributor | 102810237

Issue Owner

Current Issue Owner: @puneetlath

[melvin-bot]

Triggered auto assignment to @puneetlath (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/~01e9b622536ce232c5

[melvin-bot]

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

[abzokhattab]

Proposal

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

Receipt required over undefined is shown in the violation message

What is the root cause of that problem?

in the current case, the user submits a report without a receipt meaning the expected error should be Receipt required

Currently, when a user submits a report without a receipt, the expected error message should be "Receipt required." However, the translation logic solely checks if the params object exists but does not verify if formattedLimit is undefined.

App/src/languages/en.ts

Line 3140 in ec9ce7c

receiptRequired: (params: ViolationsReceiptRequiredParams) => `Receipt required${params ? ` over ${params.formattedLimit}${params.category ? ' category limit' : ''}` : ''}`,

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

we need to change the param check to check the inner values of the params object since the params object will be always true as we are passing the object here:

        receiptRequired: ({formattedLimit, category}: ViolationsReceiptRequiredParams) =>
            `Receipt required${formattedLimit ?? category ? ` over${formattedLimit ? ` ${formattedLimit}` : ''}${category ? ` ${category} category limit` : ''}` : ''}`,

this is compliant with the translation guide here: #33969 (comment)

or alternatively, we can change the way this translation is called so that it passes a null value to the params object if both of the category and the formattedLimit are null

poc

[garatkarr]

I'm new here so not sure how the backend works yet but it seems to me this may be a backend issue. Onyx Pub/Sub sends the following when "Receipt Required Amount" is greater than 0:

{ "onyxMethod": "mergecollection", "key": "transactionViolations_", "value": { "transactionViolations_5942621983798817006": [ { "type": "violation", "name": "receiptRequired", "data": { "formattedLimit": "$0.01" } } ] } }

but when it is 0, data is null like in this example:
{ "onyxMethod": "mergecollection", "key": "transactionViolations_", "value": { "transactionViolations_5942621983798817006": [ { "type": "violation", "name": "receiptRequired", "data": null } ] } },

This is probably a backend issue with the common mistake of using something like if(dollarAmount) because a $0.00 amount is still a value and should not be treated as false. The backend code needs to perhaps check for not null and not undefined using === (strict equality)

[ahmedGaber93]

reviewing

[ahmedGaber93]

@abzokhattab Thanks for the proposal.

Your proposal looks good, but I think your code logic not cover all cases here #33969 (comment). Could you please update it?

Try this in your browser console.

const receiptRequired = (params) =>
          `Receipt required${params.category ?? params.formattedLimit ? ` over ${params.formattedLimit ?? ''}${params.category ? ' category limit' : ''}` : ''}`;


console.log(receiptRequired({}))
console.log(receiptRequired({ formattedLimit: '$500' }))
console.log(receiptRequired({ category: 'travel' }))
console.log(receiptRequired({ formattedLimit: '$500', category: 'travel' }))

[abzokhattab]

okay i updated the proposal to produce exactly the same expected output

[ahmedGaber93]

We can move forward with @abzokhattab's proposal, It cover all cases here #33969 (comment)

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 @ahmedGaber93 🎉 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]

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

[abzokhattab]

The PR is ready.

[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 9.0.5-13 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix Required Receipt Translations #44220

If no regressions arise, payment will be issued on 2024-07-17. 🎊

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

• @ahmedGaber93 requires payment automatic offer (Reviewer)
• @abzokhattab requires payment automatic offer (Contributor)

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

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

[melvin-bot]

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

• Fix Required Receipt Translations #44220

If no regressions arise, payment will be issued on 2024-07-22. 🎊

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

• @ahmedGaber93 requires payment automatic offer (Reviewer)
• @abzokhattab requires payment automatic offer (Contributor)

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

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

[puneetlath]

@ahmedGaber93 friendly reminder about the checklist.

[ahmedGaber93]

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:

• [@ahmedGaber93] The PR that introduced the bug has been identified. Link to the PR: Add violation message to MoneyRequestPreview #33969
• [@ahmedGaber93] 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: Add violation message to MoneyRequestPreview #33969 (comment)
• [@ahmedGaber93] 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:
• [@ahmedGaber93] Determine if we should create a regression test for this bug. Yes
• [@ahmedGaber93] 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.

Regression Test Proposal

Precondition : As a admin set up expense violation as " Receipt required amount" to "0" in OldDot control policy

1. Invite an employee to the above policy
2. As a employee submit a report without receipt
3. As an admin open the report submitted
4. Validate that the correct translation is shown ("receipt required").

Do we agree 👍 or 👎

[puneetlath]

Regression test issue: https://github.com/Expensify/Expensify/issues/413396

All paid. Thanks everyone!