PolicyEngine / policyengine-api

PolicyEngine's REST API for computing policy impacts.
GNU Affero General Public License v3.0
10 stars 21 forks source link

Preserve policy ID when naming the policy #1637

Open MaxGhenis opened 3 months ago

MaxGhenis commented 3 months ago

Currently naming a policy gives it a new policy ID, meaning users have to wait for another microsimulation when navigating off the page

anth-volk commented 3 months ago

Unfortunately, I don't think preserving the policy ID would be the fix here, unless we more fully embraced the logged-in user experience. At present, the API guarantees that every significant change to a policy will create a new one; this includes renaming the policy, adding a provision, etc. If we were to keep the policy ID, then anytime any user created the same policy, they would have this renamed policy as their own. This would also mean multiple users could be renaming the same policy.

A more effective fix in the short run may be to somehow abort calculation when renaming at certain points.

MaxGhenis commented 3 months ago

Huh I think it used to preserve the policy ID before the sign in model. @nikhilwoodruff is that right?

MaxGhenis commented 2 months ago

Let's please do this. I find myself frequently renaming the policy after running computations, then sending it where viewers have to wait for it to run.

nikhilwoodruff commented 2 months ago

@MaxGhenis got the concern- but talking through with Anthony there's some significant complexity (/confusion in the UI) if we did this (e.g. people renaming others' policies, making them unsearchable, etc.) We could eventually solve with permissions-based UX I guess, but seems lower priority and high-effort considering other priorities. In the meantime, could you name policies before running them? :)

MaxGhenis commented 2 months ago

How about letting anyone name a policy that hasn't yet been named?

anth-volk commented 2 months ago

That wouldn't solve your immediate issue, though, would it?

I think the way to solve this would be to equate the policy_id key of the data with an individual simulation run from an individual user with an individual label, while treating the policy_hash as one set of reforms. Any time we do something that is meant to affect all reforms of the same type (e.g., caching simulation runs), we operate based upon policy_hash, while any time we do anything that would be attributable to an individual user, we use policy_id. This would also be far more standard than our handling.

The problem underpinning all of this is is that the API only creates new records for new country-label-policy sets, but not for each new instance of a user modifying a parameter set. The impact of this is described in the user flow below:

  1. User Joe wants to increase IRS tax bracket 7 to 99%. This becomes stored as policy ID 100, with no label. Joe then wants to give this policy a name, calling it "Joe's Policy".
  2. User Melinda also wants to increase IRS tax bracket 7 to 99%. The moment she updates the IRS bracket, if we fixed #1637 without changing the policy ID structure, she would see "Joe's Policy" as the title here. If she were to change the title to "Melinda's Policy", this would be overwritten.
  3. Joe just ran the policy, so it would be returned fast for Melinda, since it's been cached.
  4. A day later, Joe revisits the policy, but for a reason unclear to him, it's now called "Melinda's Policy."

I think the long-term way to proceed with this issue (and some related) is to move to using policy IDs as individual record identifiers, as described at the top, then ensuring that the app is updated to match those assumptions. Shorter-term, renaming before running computations would avoid the issue.