[HOLD for payment 2024-04-03] [Report Fields] [Onyx] Move `policyReportFields_` key back into the `policy_` key

[flodnv]

Problem & Solution

Sister issue: #36171

Discussed internally, summarised here and concluded there

• We added the onyx keys policyTaxRates_ and policyReportFields_ for no good reason. There's no size to optimize for in these collections.
• This makes my life harder in trying to fix an architectural problem of sending policy updates in https://github.com/Expensify/Auth/pull/9773 (see https://expensify.slack.com/archives/C03TQ48KC/p1706285244529629 & [Backwards Compatibility] [HIGH] Push all policy updates to all policy members forever #34834)
• This makes code more complex than it needs to be (lots of existing data massaging for really no gain)
• As such, we should kill and fix this unnecessarily complex code that was recently introduced. This means, at a minimum (I might be missing things):
  • Moving this data back into the policy_ key
  • Removing any data massaging from the backend
  • Have the frontend accept the same data as what's in the database
  • Where we should not display disabled fields, then let's use the disabled property to hide them. We will need to display them to admins soon enough
• No need to worry about sending updates to policy members when the policy changes, that will be done as part of https://github.com/Expensify/Auth/pull/9773, which includes doing this for all current and future policy attributes

cc @iwiznia @puneetlath @tgolen @marcaaron @francoisl @dangrous @johnmlee101 @MonilBhavsar

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~010e62ab2c798627fa
• Upwork Job ID: 1755672881949298688
• Last Price Increase: 2024-02-08

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~010e62ab2c798627fa

[melvin-bot]

Triggered auto assignment to Contributor Plus for review of internal employee PR - @mollfpr (Internal)

[thienlnam]

Sibtain will be handling the FE changes and I will take the BE changes

[marcaaron]

I am not aware of any documented problem with storing everything related to a policy in the policy_ key.

Some hypothetical problems to watch out for could be:

• Large policy JSON is inefficient to modify in Onyx because we do a deep merge (maybe some copying still happens in some places as well).
• Optimizations exist to deep compare to see if we should update subscribers. Larger the JSON the slower these comparisons would be.
• Subscribing to a subset of data should result in faster updates than subscribing to a large JSON object. Even with the use of a selector. I think it would still be faster.

That's all I have to say on it though. I have not instrumented any of that and it's all just theoretical. This proposal looks fine.

[flodnv]

Large policy JSON is inefficient to modify in Onyx

How large? Tags is the most bloating data and it's already in its own key (and for now we'll leave it there)

because we do a deep merge

Can you expand on "we do a deep merge"? Where and why?

---

Thanks for having a look and commenting 🙇

[marcaaron]

How large? Tags is the most bloating data and it's already in its own key (and for now we'll leave it there)

I haven't instrumented anything so I can't answer that question.

Can you expand on "we do a deep merge"? Where and why?

There is a memory cache layer. Before we save things to the Onyx dbs we merge them into the existing objects in the cache. The smaller those objects are the faster they would be to merge (hypothetically). I am not aware that we've hit any kind of limit there yet - but I do know that it was slow enough in the past that we optimized it to make it faster. We could have someone instrument to see at what point the merges get slow - maybe it's a lot or not.