Skip to content

Conversation

@dsill-ethyca
Copy link
Contributor

@dsill-ethyca dsill-ethyca commented Jan 7, 2026

Ticket ENG-2270

Description Of Changes

Add support for the infrastructure monitor type in the action center, gated behind the oktaMonitor feature flag. Infrastructure monitor results (used for Okta monitors) will now appear in the action center when the feature flag is enabled, consistent with how website and datastore monitor types are handled.

Code Changes

  • Extract oktaMonitor feature flag from flags object
  • Add MONITOR_TYPES.INFRASTRUCTURE to monitor types array conditionally when oktaMonitorEnabled is true

Steps to Confirm

  1. Verify infrastructure monitor results appear in action center when oktaMonitor feature flag is enabled
  2. Verify infrastructure monitor results do not appear in action center when oktaMonitor feature flag is disabled

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
Contributor

vercel bot commented Jan 7, 2026

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

Project Deployment Review Updated (UTC)
fides-plus-nightly Ready Ready Preview, Comment Jan 9, 2026 7:21pm
1 Skipped Deployment
Project Deployment Review Updated (UTC)
fides-privacy-center Ignored Ignored Jan 9, 2026 7:21pm

@dsill-ethyca dsill-ethyca marked this pull request as ready for review January 7, 2026 18:36
@dsill-ethyca dsill-ethyca requested a review from a team as a code owner January 7, 2026 18:36
@dsill-ethyca dsill-ethyca requested review from jpople and removed request for a team January 7, 2026 18:36
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 7, 2026

Greptile Summary

This PR adds support for infrastructure monitor type (used for Okta monitors) in the action center, gated behind the oktaMonitor feature flag. The implementation follows the existing pattern used for WEBSITE and DATASTORE monitor types.

Key changes:

  • Extracts oktaMonitor flag from flags object
  • Conditionally adds MONITOR_TYPES.INFRASTRUCTURE to the monitor types array when flag is enabled
  • Updates changelog appropriately

Issue found:

  • The disabled page check on line 74 doesn't include oktaMonitorEnabled, which means users with only the oktaMonitor flag enabled will incorrectly see the disabled page instead of infrastructure monitor results

Confidence Score: 2/5

  • This PR has a logic bug that will cause incorrect behavior for users with only the oktaMonitor flag enabled
  • The implementation correctly adds infrastructure monitor type support, but the disabled page check on line 74 doesn't account for oktaMonitorEnabled. This means users who only have the oktaMonitor feature flag enabled (without webMonitor or heliosV2) will see the "Action center is currently disabled" page instead of their infrastructure monitor results. This is a clear logic error that breaks the feature's intended functionality.
  • Pay close attention to clients/admin-ui/src/pages/data-discovery/action-center/index.tsx - the disabled page check needs to be updated

Important Files Changed

Filename Overview
clients/admin-ui/src/pages/data-discovery/action-center/index.tsx Added oktaMonitor flag and INFRASTRUCTURE monitor type, but disabled page check doesn't include oktaMonitorEnabled
CHANGELOG.md Standard changelog entry added correctly
changelog/7191.yaml Changelog YAML file created correctly

Copy link
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.

Additional Comments (1)

  1. clients/admin-ui/src/pages/data-discovery/action-center/index.tsx, line 74-76 (link)

    logic: this check doesn't include oktaMonitorEnabled, so if only the oktaMonitor flag is enabled, users will see the disabled page even though infrastructure monitors should be available

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@lucanovera lucanovera left a comment

Choose a reason for hiding this comment

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

The infrastructure monitor results now appear when the flag is on, code changes are simple and correct.

This is approved for me, but we shouldn't merge this until the BE changes are merged. If we merge this without BE support, the action center endpoint breaks because of the lack of support for the 'infrastructure' value.

Let me know when that is approved and I'll approve this.

Copy link
Contributor

@lucanovera lucanovera left a comment

Choose a reason for hiding this comment

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

BE is merged already. Approved!

@dsill-ethyca dsill-ethyca added this pull request to the merge queue Jan 9, 2026
Merged via the queue into main with commit 835a4ce Jan 9, 2026
45 checks passed
@dsill-ethyca dsill-ethyca deleted the ENG-2270-be-scan-results-for-infra branch January 9, 2026 21:21
@greptile-apps greptile-apps bot mentioned this pull request Jan 14, 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