App Client-side violations for money request updates

Client-side violations for money request updates

Open lindboe opened this issue 5 months ago • 47 comments

Details

Adds optimistic client-side updates for money requests for missing tags, categories, or disabled tags/categories. This PR handles edits, while https://github.com/Expensify/App/pull/32528 handled creation of money requests. Requires setting up a policy in old dot and configuring it to have a workspace (see testing instructions). This PR does not make any visible changes; that is scoped to other tickets.

Fixed Issues

$ https://github.com/Expensify/App/issues/31093 PROPOSAL:

Tests

[x] Verify that no errors appear in the JS console

Setup:

Follow the instructions here to set up a policy with a workspace in New Dot.
Within the policy in Old Dot, navigate Settings > Categories and verify that People must categorize expenses is enabled, and make sure there's at least two categories on the policy
Within the policy in Old Dot, navigate Settings > Tags and verify that People must tag expenses is enabled, and make sure there's at least two tags enabled on the policy

Instructions for all tests

Expectations for violations, generally:

Violations should be visible when viewing the request on each menu item with text and a RBR indicator. Here, we're generally concerned the following violations that we calculate client-side: missing category, missing tag, category out of policy, and tag out of policy. Other visible violations should not be affected by this PR.

Testing the violations were stored correctly (web only):

Navigate to the ONYXDB -> keyvaluepairs within your browser storage tab
Identify the transaction that was created with the dollar amount you specified, should look like transactions_someID. If you open chrome dev tools before making the money request, you can inspect the response which should have the transactionID
Verify there's a transaction violations object with the corresponding ID like transactionViolations_SameIDasTransaction, with the array content specified by the test

Test no client-side violations

Create a request and request money from the policy you created in setup. Set an amount, merchant, category, and tag. Ensure your request amount is not above the maximum set by the policy, and not above the amount the policy requires for a receipt.
(Web) Once the request is created, check your browser storage tab. You should see either no matching transactionViolations_ID key, or, if you've navigated to edit the request, you may see a transaction violations list with only a cashExpenseWithNoReceipt violation.
Edit the request's date and save. There should be no visible errors on the request or the LHN.
(Web) inspect the transaction violations key again. The violations list should still only contain a cashExpenseWithNoReceipt violation.

Test missing tag

Create a request and request money from the policy you created in setup. Set an amount, merchant, and category, but DO NOT set tag. Save the request.
(Web) inspect the transaction violations object. You should see a matching transactionViolations_ID key, with an array containing a missingTag violation, and possibly also a cashExpenseWithNoReceipt violation.
Edit the request to add a tag. There should be no visible errors on the request or the LHN.
1. (Web) inspect the transaction violations key again. You should see either no matching transactionViolations_ID key, or, if you've navigated to edit the request, you may see a transaction violations list with only a cashExpenseWithNoReceipt violation.

Test missing category

Create a request and request money from the policy you created in setup. Set an amount, merchant, and tag, but DO NOT set category. Save the request.
(Web) inspect the transaction violations object. You should see a matching transactionViolations_ID key, with an array containing a missingCategory violation, and possibly also a cashExpenseWithNoReceipt violation.
Edit the request to add a category. There should be no visible errors on the request or the LHN.
1. (Web) inspect the transaction violations key again. You should see either no matching transactionViolations_ID key, or, if you've navigated to edit the request, you may see a transaction violations list with only a cashExpenseWithNoReceipt violation, or no violations.

Test tag out of policy

Create a request and request money from the policy you created in setup. Set an amount, merchant, and tag and category that are enabled on old dot. Save the request.
On old dot, disable the tag you used. Refresh new dot, and ensure a tagOutOfPolicy violation is shown in the transaction violations list.
Edit the request to use a tag that is enabled in the policy, and ensure that tagOutOfPolicy is no longer in the transaction violations list.

Test category out of policy

Create a request and request money from the policy you created in setup. Set an amount, merchant, and tag and category that are enabled on old dot. Save the request.
On old dot, disable the category you used. Refresh new dot, and ensure a categoryOutOfPolicy violation is shown in the transaction violations list.
Edit the request to use a category that is enabled in the policy, and ensure that categoryOutOfPolicy is no longer in the transaction violations list.

Offline tests

Repeat the tests outlined above while completely offline, except for "tag/category out of policy", because you can't change policy while offline. Different steps for those tests are provided below.

Then, in addition, after each test, go online and verify the transactionViolations key remains the same, with the potential addition of a cashExpenseWithNoReceipt violation, or no violations.

Test tag out of policy (offline)

Create a request and request money from the policy you created in setup. Set an amount, merchant, and tag and category that are enabled on old dot. Save the request.
On old dot, disable the tag you used. Refresh new dot, and ensure a tagOutOfPolicy violation is shown in the transaction violations list.
On new dot, go offline. Select a tag that has remained enabled on your policy on the request instead. Save the request, and ensure the transaction violations list no longer has a tagOutOfPolicy violation.
Go online, and ensure that the transaction violations list does not change.

Test category out of policy (offline)

Create a request and request money from the policy you created in setup. Set an amount, merchant, and tag and category that are enabled on old dot. Save the request.
On old dot, disable the category you used. Refresh new dot, and ensure a categoryOutOfPolicy violation is shown in the transaction violations list.
On new dot, go offline. Select a category that has remained enabled on your policy on the request instead. Save the request, and ensure the transaction violations list no longer has a categoryOutOfPolicy violation.
Go online, and ensure that the transaction violations list does not change.

Test failure to edit request (web only, requires Chrome)

Create a request and request money from the policy you created in setup. Set an amount, merchant, and category, and tag. Save the request.
Go to old dot and disable the category you used.
Refresh new dot.
Inspect the corresponding transactionViolations_ key to ensure there is a categoryOutOfPolicy violation there.
Now, cause the request to the backend to fail:
With the Network tab open in Chrome devtools, edit a DIFFERENT request's category. Look for a request entry matching api?command=EditMoneyRequest (it is possible that instead it might be a more specific command in the future, like UpdateMoneyRequestTag). Right-click and select "Block request URL". This will cause subsequent calls to this URL to fail.
Go offline and edit the request to use an enabled category. Save the request, and inspect the transaction violations key. The categoryOutOfPolicy violation should be gone.
Go online and allow the request to be sent and fail.
Wait a little bit for the request to fail after several retries (you should see an error in the transaction thread), and verify that the transaction violations list in Onyx is reset to what it was before you fixed the request (i.e., it contains a categoryOutOfPolicy violation).
Unblock the endpoint you blocked before, and again update the request to use an enabled category. The transactionViolations list should be updated to remove the categoryOutOfPolicy violation.

QA Steps

[x] Verify that no errors appear in the JS console

Same as tests

PR Author Checklist

[x] I linked the correct issue in the ### Fixed Issues section above
[x] I wrote clear testing steps that cover the changes made in this PR
- [x] I added steps for local testing in the Tests section
- [x] I added steps for the expected offline behavior in the Offline steps section
- [x] I added steps for Staging and/or Production testing in the QA steps section
- [x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- [x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
- [x] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
[x] I included screenshots or videos for tests on all platforms
[x] I ran the tests on all platforms & verified they passed on:
- [x] Android: Native
- [x] Android: mWeb Chrome
- [x] iOS: Native
- [x] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [x] MacOS: Desktop
[x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
[x] I followed proper code patterns (see Reviewing the code)
- [x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
- [x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
- [x] I verified that comments were added to code that is not self explanatory
- [x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- [x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
  - [x] If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
- [x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- [x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
- [x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- [x] I verified the JSDocs style guidelines (in STYLE.md) were followed
[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I followed the guidelines as stated in the Review Guidelines
[x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] I verified that if a function's arguments changed that all usages have also been updated correctly
[x] If any new file was added I verified that:
- [x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
[x] If a new CSS style is added I verified that:
- [x] A similar style doesn't already exist
- [x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
[x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[x] If the PR modifies the form input styles:
- [x] I verified that all the inputs inside a form are aligned with each other.
- [x] I added Design label so the design team can review the changes.
[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.