Skip to content

ENG-2470: update idp monitor form to use generic discovery monitor endpoint#7276

Merged
jpople merged 5 commits intomainfrom
asachs/ENG-2470
Feb 2, 2026
Merged

ENG-2470: update idp monitor form to use generic discovery monitor endpoint#7276
jpople merged 5 commits intomainfrom
asachs/ENG-2470

Conversation

@adamsachs
Copy link
Copy Markdown
Contributor

@adamsachs adamsachs commented Jan 30, 2026

Ticket ENG-2470

Description Of Changes

Update Okta monitor config to use generic discovery monitor endpoints on okta monitor config/create form to avoid bugs

Code Changes

  • Update Okta monitor submission logic to use standard monitor endpoint
  • Remove several now-unused endpoints

Steps to Confirm

  1. Create an Okta integration
  2. Create a monitor for the integration
  3. Edit the monitor config
  4. Should be able to create the monitor and your changes should save when editing

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 Jan 30, 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 Feb 2, 2026 4:06pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
fides-privacy-center Ignored Ignored Feb 2, 2026 4:06pm

Request Review

@jpople jpople marked this pull request as ready for review January 30, 2026 17:51
@jpople jpople requested a review from a team as a code owner January 30, 2026 17:51
@jpople jpople requested review from speaker-ender and removed request for a team January 30, 2026 17:51
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Jan 30, 2026

Greptile Overview

Greptile Summary

Consolidated Okta monitor configuration to use standard discovery monitor endpoints, removing technical debt from Okta-specific endpoint implementations.

Key Changes:

  • Removed three unused Okta-specific RTK Query endpoints (createIdentityProviderMonitor, putIdentityProviderMonitor, getIdentityProviderMonitors)
  • Simplified ConfigureMonitorModal.tsx by removing conditional logic that used different endpoints for Okta vs other integrations
  • All monitor types (including Okta) now use the unified /plus/discovery-monitor endpoint

Analysis:
The changes eliminate unnecessary code duplication and align with the existing pattern already used in useMonitorConfigTable.tsx. The generic EditableMonitorConfig type contains all necessary fields previously used by the Okta-specific interface. No other code references the removed endpoints, confirming this is a clean refactoring.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk - it's a clean refactoring that removes unused code
  • The changes are straightforward: removing duplicate Okta-specific endpoint code and consolidating to use the generic discovery monitor endpoint. No usages of the removed code exist elsewhere in the codebase, and the generic endpoint supports all necessary fields.
  • No files require special attention

Important Files Changed

Filename Overview
changelog/7276-okta-monitor-form-update.yaml Standard changelog entry documenting the Okta monitor form update
clients/admin-ui/src/features/data-discovery-and-detection/discovery-detection.slice.ts Removed Okta-specific endpoint interfaces and hooks that are no longer needed
clients/admin-ui/src/features/integrations/configure-monitor/ConfigureMonitorModal.tsx Simplified modal to use generic discovery monitor endpoint for all integrations including Okta

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

MonitorFrequency.NOT_SCHEDULED
}
showTime
className="w-full"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Drive-by fix, this has been bothering me forever.

Before:
Image

After:
Image

Copy link
Copy Markdown
Contributor

@speaker-ender speaker-ender left a comment

Choose a reason for hiding this comment

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

Tested locally and works as expected.
Appreciate the reduction of code/logic.

@jpople jpople added this pull request to the merge queue Feb 2, 2026
Merged via the queue into main with commit d8a8496 Feb 2, 2026
46 checks passed
@jpople jpople deleted the asachs/ENG-2470 branch February 2, 2026 18:22
@greptile-apps greptile-apps bot mentioned this pull request Feb 6, 2026
18 tasks
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.

3 participants