[Publisher][Admin Panel] Add Metrics Reporting Agency tab #1665

nasaownsky · 2025-02-06T16:10:10Z

Description of the change

Added Metrics Reporting Agency feature UI and logic

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

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

nasaownsky · 2025-02-06T16:26:43Z

"Hi @mxosman and @nichelle-hall! Aside from some small UI bugs that I'll address tomorrow, I think it turned out quite solid!

The only issue I've noticed with the API behavior is that it seems like I cannot save the "None" option to the BE. It acts like it's saved (with a 200 code), but it's not having any effect on the actual data. I'm sending reporting_agency_id: null with the updated reporting_agencies array when "None" is selected, so maybe this is the issue. Could you please check this out, @nichelle-hall?

Anyway, please check my changes and let me know your thoughts on them. Thank you!"

Edit: I also noticed that if we assign vendor as reporting agency -> save changes -> then delete this vendor from "Manage Vendors" modal -- the BE will preserve is_self_reported and reporting_agency_id values when we go back to "Metrics Reporting Agency" tab.

mxosman · 2025-02-07T20:48:03Z

"Hi @mxosman and @nichelle-hall! Aside from some small UI bugs that I'll address tomorrow, I think it turned out quite solid!

The only issue I've noticed with the API behavior is that it seems like I cannot save the "None" option to the BE. It acts like it's saved (with a 200 code), but it's not having any effect on the actual data. I'm sending reporting_agency_id: null with the updated reporting_agencies array when "None" is selected, so maybe this is the issue. Could you please check this out, @nichelle-hall?

Anyway, please check my changes and let me know your thoughts on them. Thank you!"

Edit: I also noticed that if we assign vendor as reporting agency -> save changes -> then delete this vendor from "Manage Vendors" modal -- the BE will preserve is_self_reported and reporting_agency_id values when we go back to "Metrics Reporting Agency" tab.

Thank you so much, @nasaownsky! I'll take a closer look at this on Monday.

Great call on the deleting the vendor - I didn't give this too much thought, but I'm thinking we don't need to preserve these if a vendor gets deleted, and the UI can just show None. What do you think, @nichelle-hall?

mxosman · 2025-02-10T23:44:24Z

Thank you for your patience, @nasaownsky - I'll get to this tomorrow as soon as I can! @nichelle-hall will follow up with you as well!

nichelle-hall · 2025-02-11T00:19:04Z

@nasaownsky

Sorry I didn't get time to look at this today! I was heads-down in another task. I'll respond first thing tomorrow!

mxosman

Amazing! Works great (aside from the None)!

A couple of things:

Can we include ungating the Vendor Management so we can release all of this once we merge this PR in?
@nichelle-hall - when you get a chance, would you mind playing around with this and double checking that the saved selections propagate to the dashboard API correctly?

mxosman · 2025-02-11T15:31:43Z

publisher/src/components/AdminPanel/AgencyProvisioning.tsx

+        selected:
+          currentSettingType ===
+          AgencyProvisioningSettings.METRICS_REPORTING_AGENCY,
+        /** Hide metrics reporting agency tab when creating a new agency */


Great call! Thanks for thinking of this.

mxosman · 2025-02-11T15:38:34Z

publisher/src/components/AdminPanel/MetricsReportingAgency.tsx

+                  const updatedReportingAgency = reportingAgenciesUpdates.find(
+                    (agency) => agency.metric_key === metric.key
+                  );
+                  const selecteReportingdAgency =


typo: selectedReportingAgency

mxosman · 2025-02-11T15:44:05Z

publisher/src/stores/AdminPanelStore.ts

    try {
+      this.reportingAgencyMetadataLoading = true;


I think we need to wrap this in an action as well since this is an async method.

mxosman · 2025-02-11T15:48:05Z

publisher/src/components/AdminPanel/MetricsReportingAgency.tsx

+                      <Styled.ReportingAgencyDropdownWrapper>
+                        <DropdownContainer>
+                          <Dropdown
+                            label={reportingAgencyName ?? "Please select..."}


I was going back and forth with this one - I change my mind on the "Please select..." - can we just label this as "None"?

nichelle-hall · 2025-02-11T17:10:12Z

@mxosman @nasaownsky

This message is regarding not being able to save null. I'm still thinking about the deleting vendors issue and if there is a quick solution on the BE.

I think we should remove the None option.

On the backend, we don’t currently handle saving "None" as a valid value for metric settings changes—particularly when an agency wants to null out a previously configured metric.

From my perspective, "None" shouldn’t be an available option. Here’s why:

It’s technically not valid to transition from a non-null reporting agency to a null value. There are really only two states: either the agency is reporting the metric itself, or it’s being reported by a vendor or super agency.
"None" or null would indicate the metric is "unconfigured," but that doesn’t fit how we handle other metric configurations. (IE un-configuring a metric. It's either not available, reported monthly, or reported annually.). Once configured, you can switch between valid options, but you don’t "unconfigure" it.
I’m also struggling to see a use case where an agency would intentionally select "None," given that the reporting agency options we provide are the only valid ones.

What do you think about this approach? I’d be happy to discuss if you see something I might be missing.

nichelle-hall · 2025-02-11T19:47:58Z

I went ahead and handled the reporting agency once a vendor is deleted cleanup on the backend. I know you came up with a fix on the frontend, but it’s important to me that the database stays consistent, so I wanted to make sure this was handled properly at the BE level.

If you want to test it out, you can point the BE URL of your local application to https://nichelletest---publisher-web-b47yvyxs3q-uc.a.run.app/.

Let me know if you run into any issues or if anything needs adjusting! 🚀😊

mxosman · 2025-02-11T20:22:45Z

@mxosman @nasaownsky

This message is regarding not being able to save null. I'm still thinking about the deleting vendors issue and if there is a quick solution on the BE.

I think we should remove the None option.

On the backend, we don’t currently handle saving "None" as a valid value for metric settings changes—particularly when an agency wants to null out a previously configured metric.

From my perspective, "None" shouldn’t be an available option. Here’s why:

It’s technically not valid to transition from a non-null reporting agency to a null value. There are really only two states: either the agency is reporting the metric itself, or it’s being reported by a vendor or super agency.

"None" or null would indicate the metric is "unconfigured," but that doesn’t fit how we handle other metric configurations. (IE un-configuring a metric. It's either not available, reported monthly, or reported annually.). Once configured, you can switch between valid options, but you don’t "unconfigure" it.

I’m also struggling to see a use case where an agency would intentionally select "None," given that the reporting agency options we provide are the only valid ones.

What do you think about this approach? I’d be happy to discuss if you see something I might be missing.

That makes sense. I do recall Chris mentioning wanting the ability to do this more than once though (every time I presented the Figma) - would you mind checking with him to see if he's OK with us not including a 'None' option with your lovely explanation? I'm not sure what the need would be either, but I'm wondering if the use case was mostly to unset it so it doesn't display in the dashboard UI for some reason? Or if they're not sure about the source and accidentally make a selection?

nasaownsky · 2025-02-12T09:54:04Z

I went ahead and handled the reporting agency once a vendor is deleted cleanup on the backend. I know you came up with a fix on the frontend, but it’s important to me that the database stays consistent, so I wanted to make sure this was handled properly at the BE level.

@nichelle-hall Thank you for your fix! Unfortunately, when performing action chain with vendor removal (on your test URL) I described earlier, the server now responds with 500 error when I'm trying to load the agency, in which the removed vendor was selected as reporting agency.

Edit: Oi, not only on your test URL, but also in staging environment I get server error when trying to load such agency.

nasaownsky · 2025-02-12T11:06:17Z

@mxosman @nichelle-hall

Thank you both so much! Fixes have been applied, except for the vendor and 'None' case issues -- waiting on the decisions for that part.

Add Metrics Reporting Agency logic & UI

bfd5fbd

nasaownsky added the Type: Feature label Feb 6, 2025

nasaownsky requested review from nichelle-hall and mxosman February 6, 2025 16:10

nasaownsky added 4 commits February 7, 2025 15:43

Fixes & improvements

a015d10

Add separate loading store prop

74c6dc4

Fix tests

d120d86

Hide reporting tab from create new agency flow

5418e26

mxosman reviewed Feb 11, 2025

View reviewed changes

Fixes

056fa26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Publisher][Admin Panel] Add Metrics Reporting Agency tab #1665

[Publisher][Admin Panel] Add Metrics Reporting Agency tab #1665

nasaownsky commented Feb 6, 2025

nasaownsky commented Feb 6, 2025 •

edited

Loading

mxosman commented Feb 7, 2025

mxosman commented Feb 10, 2025

nichelle-hall commented Feb 11, 2025

mxosman left a comment

mxosman Feb 11, 2025

mxosman Feb 11, 2025

mxosman Feb 11, 2025

mxosman Feb 11, 2025

nichelle-hall commented Feb 11, 2025

nichelle-hall commented Feb 11, 2025

mxosman commented Feb 11, 2025 •

edited

Loading

nasaownsky commented Feb 12, 2025 •

edited

Loading

nasaownsky commented Feb 12, 2025

[Publisher][Admin Panel] Add Metrics Reporting Agency tab #1665

Are you sure you want to change the base?

[Publisher][Admin Panel] Add Metrics Reporting Agency tab #1665

Conversation

nasaownsky commented Feb 6, 2025

Description of the change

Type of change

Related issues

Checklists

Development

Code review

nasaownsky commented Feb 6, 2025 • edited Loading

mxosman commented Feb 7, 2025

mxosman commented Feb 10, 2025

nichelle-hall commented Feb 11, 2025

mxosman left a comment

Choose a reason for hiding this comment

mxosman Feb 11, 2025

Choose a reason for hiding this comment

mxosman Feb 11, 2025

Choose a reason for hiding this comment

mxosman Feb 11, 2025

Choose a reason for hiding this comment

mxosman Feb 11, 2025

Choose a reason for hiding this comment

nichelle-hall commented Feb 11, 2025

nichelle-hall commented Feb 11, 2025

mxosman commented Feb 11, 2025 • edited Loading

nasaownsky commented Feb 12, 2025 • edited Loading

nasaownsky commented Feb 12, 2025

nasaownsky commented Feb 6, 2025 •

edited

Loading

mxosman commented Feb 11, 2025 •

edited

Loading

nasaownsky commented Feb 12, 2025 •

edited

Loading