Recidiviz / justice-counts

Technical infrastructure for the Justice Counts initiative
GNU General Public License v3.0
2 stars 0 forks source link

[Publisher] Persist current metric after switching to a different child agency via the dropdown #1378

Closed nasaownsky closed 5 months ago

nasaownsky commented 5 months ago

Description of the change

Added initial logic for metric persistence

Type of change

All pull requests must have at least one of the following labels applied (otherwise the PR will fail):

Label Description
Type: Bug non-breaking change that fixes an issue
Type: Feature non-breaking change that adds functionality
Type: Breaking Change fix or feature that would cause existing functionality to not work as expected
Type: Non-breaking refactor change addresses some tech debt item or prepares for a later change, but does not change functionality
Type: Configuration Change adjusts configuration to achieve some end related to functionality, development, performance, or security
Type: Dependency Upgrade upgrades a project dependency - these changes are not included in release notes

Related issues

closes #1332

Checklists

Development

This box MUST be checked by the submitter prior to merging:

These boxes should be checked by the submitter prior to merging:

Code review

These boxes should be checked by reviewers prior to merging:

nasaownsky commented 5 months ago

Hi again, @mxosman! After tweaking agencies dropdown I came up with this solution. It persists metric pretty well, but the thing is - if metric is enabled for current child agency we viewing and not enabled for the one we want to select next - it will also persists. I tried various approaches, but so far had no success. Mostly I tried to find out if metric is enabled utilizing agencyMetrics and metricsBySystem, but the problem is that this metrics info will only update in the future, when we actually select child agency we want to visit. Maybe you have some ideas regarding this issue? I will appreciate any help at this point. Thank you so much!

mxosman commented 5 months ago

Hi @nasaownsky! Thank you so much for working on this and for the comment. I'm working on a focused task and will follow up with you as soon as I break away!

mxosman commented 5 months ago

I totally see what you're talking about - such a good point. We query the metrics based on the current agency... so there is no way to know in advance what the child agency's list of metrics will be until we switch to it and a request is made. Hmm... still chewing on this.

mxosman commented 5 months ago

@nasaownsky - here's an idea. It looks like there's some logic where if the metric search param doesn't exist, it falls back on the first enabled metric. Maybe that will be useful in our case, and we don't need to know whether or not the metric is enabled in advance.

So there's a useEffect here that we might be able to use:

https://github.com/Recidiviz/justice-counts/blob/eb6bb4f75ab9585f044bb64d0802e1806e1b334e/publisher/src/components/DataViz/MetricsDataChart.tsx#L189-L208

I can't remember all of the details of why this is setup this way, but this runs every time there's a change in the metrics/system params to keep the data viz store and the URL in sync. I think we can just do a check in L199 - where it's setting the search params for the metric - and see if the currentMetric metric is enabled. If it is enabled, we can keep using that key -- but if it's not enabled, we can pass undefined and hopefully it resolves to the first available metric on the next re-render.

LMK if this makes sense?

nasaownsky commented 5 months ago

@mxosman Yes, totally worked, thank you! Please, test my changes and see if there are any issues they can cause!