App: Custom units are broken

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Open or create any workspace.
  2. Attempt to update the custom unit rates.
  3. Notice how it just doesn’t work.

https://user-images.githubusercontent.com/31285285/190010127-eab7c109-e68a-4afd-a7fa-14c3177a8030.mov

Expected Result:

Updates to the custom unit rate should be persisted.

Actual Result:

Updates to the custom unit rate aren’t persisted. The policy.customUnits is null.

Workaround:

Reverting changes from https://github.com/Expensify/App/pull/10873 fixes things locally

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number: v1.2.0-0 Reproducible in staging?: yes Reproducible in production?: no Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos: Any additional supporting documentation Expensify/Expensify Issue URL: Issue reported by: Slack conversation:

View all open jobs on GitHub

About this issue

  • Original URL
  • State: closed
  • Created 2 years ago
  • Comments: 16 (14 by maintainers)

Most upvoted comments

Notice how in the screenshot onyxRates is keyed by ID, while rates is just an array of rates. We need the rates to be keyed by ID for them to work with any Onyx updates.

This will actually be removed in #10974, so things should just work afterwards?

Yeah that sounds about right. https://github.com/Expensify/Web-Expensify/pull/34862 should get the rates keyed by ID and then our form should be working correctly again. I’ll re-test the withFullPolicy refactor after that stuff gets merged in

https://github.com/Expensify/App/pull/10873 was also the last place we called Policy.loadFullPolicy(), so there might also be additional places in the code where we rely on the whole policy being loaded that would now be broken and that we haven’t seen yet.

Let’s revert the PR to unblock the deploy so that we can look into a proper long-term fix.