Skip to content

fix: non-clearable config select#7799

Merged
speaker-ender merged 1 commit intomainfrom
fix/non-clearable-shared-monitor-config
Mar 31, 2026
Merged

fix: non-clearable config select#7799
speaker-ender merged 1 commit intomainfrom
fix/non-clearable-shared-monitor-config

Conversation

@speaker-ender
Copy link
Copy Markdown
Contributor

@speaker-ender speaker-ender commented Mar 31, 2026

Ticket []

Description Of Changes

Fixes small UX where shared config select component is not clearable on the monitor config form

Code Changes

Steps to Confirm

  1. Edit a monitor config
  2. Confirm that you can add and remove a shared monitor config

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Mar 31, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
fides-plus-nightly Ready Ready Preview, Comment Mar 31, 2026 3:33pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
fides-privacy-center Ignored Ignored Mar 31, 2026 3:33pm

Request Review

@speaker-ender speaker-ender force-pushed the fix/non-clearable-shared-monitor-config branch 2 times, most recently from 084ef57 to 49328e5 Compare March 31, 2026 15:22
chore: update changelog

fix: yaml format
@speaker-ender speaker-ender force-pushed the fix/non-clearable-shared-monitor-config branch from 49328e5 to 298906a Compare March 31, 2026 15:27
@speaker-ender speaker-ender marked this pull request as ready for review March 31, 2026 15:30
@speaker-ender speaker-ender requested a review from a team as a code owner March 31, 2026 15:30
@speaker-ender speaker-ender requested review from jpople and removed request for a team March 31, 2026 15:30
@github-actions
Copy link
Copy Markdown

Title Lines Statements Branches Functions
admin-ui Coverage: 7%
5.66% (2385/42079) 4.6% (1094/23761) 3.81% (477/12512)
fides-js Coverage: 78%
78.98% (1962/2484) 65.55% (1214/1852) 72.57% (336/463)
privacy-center Coverage: 86%
83.18% (287/345) 77.15% (152/197) 75% (48/64)

Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Small, targeted fix. Adding allowClear to SharedConfigSelect is the right approach — Ant Design's Select does not expose a clear button by default, and both consuming forms (ConfigureMonitorForm, ConfigureWebsiteMonitorForm) already type shared_config_id as string | undefined, so clearing emits undefined and is handled correctly downstream without any other changes.

One minor nit: The changelog filename has a typo (cofigconfig). Also worth verifying whether the existing 7786-fix-monitor-config-select.yaml entry covers a different bug — if so, a slightly more descriptive description here (e.g., "monitor config select can now be cleared after a value is set") would help distinguish the two entries in the release notes.

No blocking issues. LGTM with the nit addressed.

@@ -0,0 +1,4 @@
type: Fixed # One of: Added, Changed, Developer Experience, Deprecated, Docs, Fixed, Removed, Security
description: monitor config select is now clearable
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the filename has a typo — cofig instead of config. Also worth double-checking whether this entry overlaps with 7786-fix-monitor-config-select.yaml, which already describes a fix to the shared monitor config select. If these are two separate bug fixes it's fine to keep both entries, but the descriptions are similar enough that it may cause changelog confusion.

@speaker-ender speaker-ender added this pull request to the merge queue Mar 31, 2026
Merged via the queue into main with commit 6df0493 Mar 31, 2026
49 checks passed
@speaker-ender speaker-ender deleted the fix/non-clearable-shared-monitor-config branch March 31, 2026 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants