-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[HOLD for payment 2024-08-19] [$750] [Violations] Remove violations beta from front-end #44995
Comments
Job added to Upwork: https://www.upwork.com/jobs/~01e029ffdb5d51f383 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @getusha ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.[Violations] Remove violations beta from front-end. What is the root cause of that problem?We need to remove violations code. What changes do you think we should make in order to solve the problem?All the things are already mentioned in OP. Most important part seems to be testing.
App/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx Line 87 in da24e9a
App/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx Line 118 in da24e9a
App/src/libs/OptionsListUtils.ts Line 1746 in da24e9a
Line 22 in da24e9a
Line 74 in da24e9a
App/src/libs/__mocks__/Permissions.ts Line 15 in da24e9a
Line 1625 in da24e9a
Line 1688 in da24e9a
Line 5389 in da24e9a
Example: This would be changed to:
This would be changed to:
|
I'll update the solution in the issue description, but please include all the places in the code we need to update in your proposal |
@cead22 Updated. |
@getusha any thoughts on that proposal? |
The task looks pretty straightforward to me. the main part seems to be the testing. we can proceed with @ShridharGoel |
Current assignee @cead22 is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new. |
Correct, we can leave that and other betas alone and focus on the violations one |
📣 @ShridharGoel 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
Will open PR within a few days (once all test steps get completed). |
Thanks, let me know if I can help! |
@ShridharGoel how is the PR coming along? anything I can do to help? |
@cead22 Will open the PR soon. I was travelling yesterday, it got a bit delayed. |
@cead22 The "Missing category" and "Missing tag" messages are not shown when a receipt is scanning. Is that expected? |
Yes |
Yes probably. Which ones are failing? |
@cead22 |
Can you share some insights on how to do this? |
I will, let me make sure I can get the steps right. Is this the last thing blocking you from submitting the PR for review? If so, let's get started on the review and we can worry about that test later |
@getusha I think the PR can be reviewed while I'm working on completing the videos for all platforms. |
Update: PR is awaiting review. |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.18-10 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-08-19. 🎊 For reference, here are some details about the assignees on this issue:
|
Payment summary: Contributor: @ShridharGoel $750 @getusha I vaguely remember that as part of this issue we'd add a series of new regression tests, is that right or did I misremember that part? |
They're the ones at the top of this issue, which should be the same as these ones shared in #qa |
Nice thanks for clarifying. Contributor is paid out via Upwork. @getusha Please submit a request via Newdot and we'll get that processed. |
Contributor: @ShridharGoel paid $750 via Upwork Do we want a test case for/from this issue specifically or is that being covered more holistically as part of a project? |
$750 approved for @getusha |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Problem
Violations were implemented under a beta and we're ready to release this functionality to everyone
Solution
(related https://github.com/Expensify/Expensify/issues/338701)
Tests
Pre-testing steps
Important Notes
Missing tag and missing category violations
Manual Request
Scan Request
Distance Request
Category out of policy, tag out of policy violation, expense over limit, max age, over category limit, receipt required
Manual Request
Scan Request
Distance Request
Tag violations on workspace with Independent Tags (multi-level tags)
Manual Request
Tag violations on workspace with Dependent Tags (multi-level tags)
Manual Request
Switch off requirement for tags and categories
For the next tests, stop requiring tags and categories so you can focus on receipt related violations
Receipt Audit Scan
Receipt Audit Distance
Receipt Audit Card (Amount greater than card transaction)
Receipt Audit - Receipt Not Verified
cc @JmillsExpensify
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @JmillsExpensifyThe text was updated successfully, but these errors were encountered: