Remove retired Data Catalog beta feature#8149
Conversation
dsill-ethyca
commented
May 8, 2026
dsill-ethyca
commented
- Remove Data Catalog beta UI pages and feature module
- Remove Data Catalog nav entry, route, flag, and orphaned tag types
- Remove Data Catalog tests
- remove orphaned helpers
dsill-ethyca
requested review from
gilluminate and
thabofletcher
and removed request for
a team
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
dsill-ethyca
requested review from
adamsachs
and removed request for
gilluminate and
thabofletcher
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8149 +/- ##
==========================================
+ Coverage 85.23% 85.47% +0.24%
==========================================
Files 638 651 +13
Lines 42011 42434 +423
Branches 4937 4982 +45
==========================================
+ Hits 35808 36271 +463
+ Misses 5096 5055 -41
- Partials 1107 1108 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
adamsachs
left a comment
adamsachs
left a comment
There was a problem hiding this comment.
generally lgtm, thank you!
just one note on a few of the cypress test removals, not sure if we'd like a FE expert to weigh in there. pretty minor in any case.
| it("Set flags before visiting any page for feature-specific tests", () => { | ||
| stubOpenIdProviders(); | ||
| stubPlusAuth(); | ||
| cy.login(); | ||
| stubPlus(true); | ||
|
|
||
| // This pattern is useful when testing features behind flags | ||
| // Set the flag before navigating to test the enabled state | ||
| cy.overrideFeatureFlag("dataCatalog", false); | ||
|
|
||
| // Now visit a page that uses this flag | ||
| cy.visit("/data-catalog"); | ||
| // The feature will be disabled from the start | ||
| cy.getByTestId("Data catalog").should("not.exist"); | ||
| }); |
There was a problem hiding this comment.
seems like this may be a 'generic' test that is just using the datacatalog as an example/test case? if so, i expect we may want to switch it to use something else rather than removing it? 🤷
There was a problem hiding this comment.
Good catch — restored the test using policies as the example flag. While restoring it I noticed the original assertion (cy.getByTestId("Data catalog").should("not.exist")) was actually targeting Layout's page-level testid, which doesn't check flags — so the test wasn't really verifying flag gating. Reframed it to assert on the gated nav link ([data-testid="DSR policies-nav-link"]) which is actually gated by the flag, so the test now meaningfully exercises the flag-off path.
| it("includes feature flagged routes when enabled", () => { | ||
| const navGroups = configureNavGroups({ | ||
| config: NAV_CONFIG, | ||
| userScopes: ALL_SCOPES, | ||
| flags: { | ||
| dataCatalog: true, | ||
| }, | ||
| hasPlus: true, | ||
| }); | ||
|
|
||
| expect( | ||
| findGroup(navGroups, "Detection & Discovery").children, | ||
| ).toMatchObject([ | ||
| { title: "Action center", path: routes.ACTION_CENTER_ROUTE }, | ||
| { title: "Data catalog", path: routes.DATA_CATALOG_ROUTE }, |
There was a problem hiding this comment.
again if this is a more generic feature flag test, may need to be adjusted rather than removed? just a guess.
There was a problem hiding this comment.
Good catch — restored the positive-case test using alphaPurposeBasedAccessControl (which gates the "Access control" route in the same "Detection & Discovery" group), so it mirrors the negative-case test directly above it.
speaker-ender
left a comment
speaker-ender
left a comment
There was a problem hiding this comment.
Approving as is and looks good.
Adam's comments around the tests seem valid.
Per review feedback (@adamsachs): the deleted tests demonstrated a generic feature-flag mechanism using dataCatalog as the example. Restored both with currently-active flags: - nav-config.test.ts: positive-case configureNavGroups test now uses alphaPurposeBasedAccessControl to mirror the negative-case test in the same Detection & Discovery group - feature-flags.cy.ts: rewrote assertion to target the flag-gated nav link (DSR policies-nav-link) instead of Layout's unconditional page testid, so the test actually exercises the flag-off path
3 participants
