The default start and end date in the policy parameter editor don't reflect previously input value

[anth-volk]

I believe this may be a regression. Currently, if a user clicks on a preexisting parameter, instead of the default start and end dates being populated from that parameter, they default to 2024-01-01 and 2024-12-31, respectively. An example of that behavior is shown in the image below. Ideally, they should be populated based on that parameter.

[cats256]

Doing this is a lot more tricky than I thought. I managed to get the start date and end date to reflect the previously input value. However, the input number (like the integer) didn't change as can be seen below.

https://github.com/PolicyEngine/policyengine-app/assets/59489624/7d991972-6a41-44df-8f33-455126e44a6f

I make a PR (https://github.com/PolicyEngine/policyengine-app/pull/1526) here. As of the initial PR, this correctly makes the start date and end date reflect the previously input value. However, it does not update the input number correctly when clicking a preexisting parameter and then another preexisting parameter, since the way the app passes in the input number is through StableInputNumber defaultValue prop, instead of the value prop, which causes the value state variable to be unchanged even when what's passed into defaultValue prop changed.

I am not sure how to approach this since StableInputNumber is also used by VariableEditor. Do I make changes to StableInputNumber or do I just directly use InputNumber component instead of StableInputNumber in ParameterEditor and make startValue a state variable? I prefer the latter solution since it seems less likely to break things

[abhcs]

@anth-volk Would it be accurate to assume that you do not see the dates you expect to see when accessing an existing parameter (and when creating/editing a reformed parameter everything is fine)? If yes, then note that we do not store the last edit in the reformed parameter, so we cannot know which interval the user was working on previously. We can implement a heuristic solution but it will break down for complex parameters with multiple edited intervals.

It would also be useful if you described a complete workflow where the above bug causes problems.

[anth-volk]

Perhaps I was unclear in the issue description, as @cats256's demonstration is actually quite a bit different from the user flow I was talking about.

Let's say a user searches an existing policy, e.g., "Reform UK's manifesto policy," then clicks on one of the linked parameters on the right side. Instead of displaying that the parameter is valid from date X to date Y, the ParameterEditor will default to a start date of 2024-01-01 and an end date of 2024-12-31, no matter what the given dates are.

That said, I'm actually in the process of working on #1261 and forgot to update the post here to say that I'm working on that issue within those fixes, since it's a critical portion of the rewiring being done on that issue. I would rather separate it out, but I'm not sure that it's possible to do so.

[anth-volk]

That said, the linked PR may resolve a slightly different set of issues. More broadly, the comments from @abhcs also point to another issue - when a user with a set of multiple changes to the same parameter clicks on a link within one of them, the component doesn't populate those changes.

I'm wondering if there isn't a better, more fundamental approach that could change all of these, perhaps something like allowing the user to click a "+" icon to the right to add another change or something like that? @cats256 I'm going to wait on reviewing your PR until we can discuss a potentially more comprehensive set of changes, as well as until #1261 can be finished and reviewed, as that would change some portion of the component's behavior. That said, we do appreciate your input, just think it's best to fix this interconnected issue as methodically as possible.

[abhcs]

I'm wondering if there isn't a better, more fundamental approach that could change all of these, perhaps something like allowing the user to click a "+" icon to the right to add another change or something like that

A parameter looks like this:

2017-01-01: 18340

2018-01-01: 18700

2019-01-01: 19030

2020-01-01: 19330

2021-01-01: 19520

2022-01-01: 20130

2023-01-01: 21560

2024-01-01: 22720

You can simply expose this representation directly to the user, and allow them to add a new <date, value> pair to the list, using the + icon. This is simpler than the edit-one-interval-at-a-time approach. Moreover, you can make the components of the pair editable. This will take away the dull aspect of entering dates to edit a new interval. I am assuming this is the design you are hinting towards.

[anth-volk]

Exactly. I'm hoping to finish #1261 first before opening a new issue on this, as I think we will have to discuss whether that methodology suits clients, from the experience of those who work more closely with them, but otherwise, this is roughly what I had in mind.

[anth-volk]

Right, something I had forgotten to mention, @abhcs: while a parameter's official values do look like that, policy reforms created by the user do not; instead they're stored as follows:

{
  2019-01-01.2020-12-31: value1
}

The values are also stored as such within the parameter object that's passed as a prop to ParameterEditor. This makes doing this sort of thing all the more challenging, as it's unclear what to treat as a source of truth and which time standard (start only, or start-end pair) to employ

[abhcs]

The base parameter has the <date, value>[] form, and the reforms have a <startDate, endDate, value>[] form. Since the base parameter has a <date, value>[] form and since reform parameter = base parameter + reforms, the reform parameter also has a <date, value> form. Lines 29--34 in ParameterEditor compute this form (in fact, they do more by finding a minimal canonical form) in the front end. In the back end, the reform parameter is created here by repeatedly calling this. This explains why the reform has the current form. However, the final form of the reformed parameter is still <date, value>[], if I understand the pe-core code correctly. In summary, you should be able to use the <date, value>[] form for both the baseline and reform parameters without any semantic issues (changes must be made in the REST API.)

The disadvantage in exposing the reform in the <date, value>[] form is that if the user wants to change multiple intervals at once they will need to add and delete entries in the list. For instance, to implement the reform { 2019-01-01.2022-12-31: 50000 } the user will need to delete the three entries

2020-01-01: 19330

2021-01-01: 19520

2022-01-01: 20130

and change the value for 2019-01-01 to 50000. This can be less intuitive than the current editing mechanism. On the other hand, the user can change the last value to say 10000 easily, which is not possible currently.

Adding another possibility to the mix, we can expose all intervals in reform to the user as a list:

start_date_1, end_date_1, value_1
...
start_date_n, end_date_n, value_n

This most closely matches the representation in the REST API, and allows quick edits to each interval without entering dates. Changes to specific intervals can also be reverted quickly by deleting say start_date_k, end_date_k, value_k. I think that this may be the easiest to understand for a user, but I may be wrong.

[anth-volk]

There has been some discussion in #1261 about migrating to a structure whereby parameter reforms and parameters themselves are both represented as merely a list of start dates. However, altering the API to utilize this structure, in place of its current, poses compatibility issues for users who previously created reforms within the older structure.

Furthermore, imagine a user wants a time-limited reform, after which the value of a parameter reverts to baseline: how do we categorize that within this structure in a way that is different from a case where a user wants one data value, followed by another? For example, let's say there's a tax parameter with some sort of inflation-adjusted value, and user A wants the value of it to be 5000 until 2025-12-31, then a reversion back to the calculated value, while user B wants it to be 5000 until the same date, followed by 6000 into perpetuity; how would we model that within such a structure?

Just curious to hear your thoughts.

[abhcs]

poses compatibility issues for users who previously created reforms within the older structure

All old reforms can be stored as version 1, while new reforms are stored as version 2. Any edits to a reform transition it to version 2; this strategy should work if you decide to change the API.

Furthermore, imagine a user wants a time-limited reform...

In these situations, exposing all edit intervals to the user as a list can work, as mentioned in the last option in my previous comment.

user A wants the value of it to be 5000 until 2025-12-31

In this case, the user will add the row <2024-01-01, 2025-12-31, 5000> to the table.

user B wants it to be 5000 until the same date, followed by 6000 into perpetuity;

In this case, the user will add the rows <2024-01-01, 2025-12-31, 5000> and <2026-01-01, inf, 6000> to the table. The infinity can be entered by adding a checkbox next to the end date saying No end date (checking the box disables the end date). This is pretty similar to job application websites where you have to describe the durations of your previous roles. There is an option for saying 'I work here currently', which disables the end date for a role.

[anth-volk]

Do you imagine there being any value in adding merely <start date, value>, mirroring the baseMap structure? Also happy to move this discussion elsewhere, as this is more off-topic.

[abhcs]

The <startDate, value>[] asks the user to directly specify the final reform, while the <startDate, endDate, value>[] form asks the user to list all modified intervals. Both ideas have their benefits. We have already considered some cases when the latter form is more efficient (in the previous comments), but the former can be useful too. Imagine that user C wants to change the value in 2020-01-01.2020-12-31 to 20000. Then, they can delete 19330 from the <startDate, value>[] table and type 20000 instead. In the other representation, the user would also have to enter the end date. Thus, some edits can be more efficient in the <startDate, value>[] form.

Both forms by themselves can be confusing to a new user. For instance, we know that the end date is inclusive in the <startDate, endDate, value>[] form but the user does not know that -- we need to communicate this fact to the user. The other form avoids this issue but it is potentially a bit more difficult to keep track of which intervals you have edited (although the chart does show the edited intervals). The base map in <startDate, value>[] form could be displayed beside the reform map <startDate, value>[] form to alleviate this problem.

In summary, both forms are semantically equivalent and can serve the user quite well (both would be an improvement over the current interface). We can use Discussions or Slack to continue if needed.