[Publisher][Bug] Fix metric definitions display for agencies with supervision + other non-supervision sectors

Description of the change

Fix metric definitions display for agencies with supervision + other non-supervision sectors with similar metrics (e.g. Funding, Staff, etc.) that are disaggregated by subpopulation in the supervision sector.

Huge shout out to @lilidworkin's playbook, as I stumbled into this while testing another PR going through the various agencies.

This one is super sneaky... If you are in an agency that has, for example, Law Enforcement & Supervision sectors - both of which have a Funding metric - and within the Supervision sector, you disaggregate the Funding metric by subpopulations; when you look at the Funding metric's definitions within the Law Enforcement sector, you'll unfortunately see the list of definitions for the first disaggregated supervision subpopulation, and NOT the list of Law Enforcement Funding metric definitions. This only affects multi-sector agencies w/ a supervision sector w/ subpopulations and with similar metrics that are disaggregated in the supervision sector.

This change adds a check for the active system before determining which system-metric key to use. I think originally when I worked on this, I must've mistakenly assumed that if you were in a different sector that checks like agencySupervisionSubsystems would come up empty, but this is not true because it's scanning all the sectors belonging to an agency and will always come up if an agency has a supervision agency w/ subsectors.

Before: Screenshot 2024-05-30 at 10 52 28 AM

After: Screenshot 2024-05-30 at 11 05 28 AM

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 #1350

Checklists

Development

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

[ ] Double- and triple-checked that there is no Personally Identifiable Information (PII) being mistakenly added in this pull request

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

[ ] Tests have been written to cover the code changed/added as part of this pull request

Code review

These boxes should be checked by reviewers prior to merging:

[ ] This pull request has a descriptive title and information useful to a reviewer
[ ] This pull request has been moved out of a Draft state, has no "Work In Progress" label, and has assigned reviewers
[ ] Potential security implications or infrastructural changes have been considered, if relevant

Thanks so much for catching this, @mxosman ! Could you explain why this fixes it? I'm having trouble intuiting why the updated logic is the right fix. It's not clear to me why the replace SystemMetricKey logic should only be run if the system is Supervision?

Absolutely! It might be helpful to bring in the few lines above it to add more context (this is before this change):

https://github.com/Recidiviz/justice-counts/blob/c7bea896e57415e894b3a31a6fa8a5bc75ded3db/publisher/src/components/MetricsConfiguration/MetricDefinitions.tsx#L76-L86

When a supervision system is disaggregated, we need to replace the systemMetricKey because it'll be something like SUPERVISION-SUPERVISION_FUNDING and in disaggregated-mode, we need it to be something like PROBATION-PROBATION_FUNDING.

The above chunk of code naively assumed it was OK to do a replaceSystemMetricKeyWithNewSystem on all systemMetricKeys regardless of sector (even if it was a no-op) because it trusted the useState check in L79-81 would cover all cases:

If a supervision system was combined and not disaggregated (subsystems are disabled), the initial value of selectedSupervisionSubsystem would be 'SUPERVISION', and the activeSystemMetricKey would be the same as the systemMetricKey: SUPERVISION-SUPERVISION_FUNDING.
If a supervision system was disaggregated, the initial value of selectedSupervisionSubsystem would be the first subsystem in the agency's list (e.g. 'PAROLE'), and the activeSystemMetricKey would spit out something like PAROLE_PAROLE_FUNDING
If it was a non-supervision system, the initial value of selectedSupervisionSubsystem would be the current non-supervision system (for most cases - more below), and the activeSystemMetricKey would be the same as the systemMetricKey: JAILS-JAILS_FUNDING (for example).

What the L79-81 check does not cover is an agency with multiple systems that include a supervision agency w/ enabled subsystems (disaggregated) for a metric. In those cases, the initial value of selectedSupervisionSubsystem will always be the first enabled subsystem in the agency system's list, because agencySupervisionSubsystems will have enabled subsystems for the metric - and will replace the systemMetricKey (e.g. LAW_ENFORCEMENT-LAW_ENFORCEMENT_FUNDING) with PAROLE_PAROLE_FUNDING (assuming Parole was the first supervision subsystem on the list of the agency's systems).

With this change, the replacement of the system metric key is now isolated to supervision systems and will not run for other sectors. I'm not sure if this long explanation made any sense - please let me know if there are any areas I can clarify further or any thoughts on adjustments to make!

In the meantime, I'll adjust the selectedSupervisionSubsystem state to be more clear and true to what it is, and add a clarifying comment to help clarify this behavior.

Recidiviz / justice-counts