Skip to content

Conversation

@jpople
Copy link
Contributor

@jpople jpople commented Dec 11, 2025

Ticket ENG-2212

Description Of Changes

When viewing taxonomies on /taxonomy, the URL now updates when clicking between them and you can link to e.g. /taxonomy/data_subject to navigate directly to one.

Code Changes

  • Extracted taxonomy view page into a separate component that now takes an optional initialTaxonomy prop
  • Created a dynamic route to handle linking directly to taxonomies

Steps to Confirm

  1. View taxonomy
  2. Click around to different taxonomies in the floating menu
  3. Should see URL update
  4. On refreshing the page or linking directly to a taxonomy key, that taxonomy should be active on initial page load

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

@jpople jpople requested a review from a team as a code owner December 11, 2025 22:40
@jpople jpople requested review from speaker-ender and removed request for a team December 11, 2025 22:40
@vercel
Copy link

vercel bot commented Dec 11, 2025

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

Project Deployment Review Updated (UTC)
fides-plus-nightly Ready Ready Preview, Comment Dec 15, 2025 10:18pm
1 Skipped Deployment
Project Deployment Review Updated (UTC)
fides-privacy-center Ignored Ignored Dec 15, 2025 10:18pm

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 11, 2025

Greptile Overview

Greptile Summary

This PR adds URL tracking for the active taxonomy in the taxonomy view, allowing users to navigate directly to specific taxonomies (e.g., /taxonomy/data_subject) and see the URL update when switching between taxonomies.

Changes:

  • Extracted the taxonomy page content into a reusable TaxonomyPageContent component that accepts an optional initialTaxonomy prop
  • Created a dynamic route [key].tsx to handle direct navigation to specific taxonomy types
  • Simplified the index page to use the new shared component

Issues found:

  • The current implementation has a hydration bug: when navigating directly to a taxonomy URL, router.query is empty on initial render. The component initializes state to the default taxonomy, and there's no effect to sync state when the prop updates after hydration.

Confidence Score: 2/5

  • PR has a functional bug that prevents the main feature from working correctly on direct navigation
  • The core feature (linking directly to taxonomy types) won't work reliably due to a Next.js hydration issue with router.query. Direct navigation will show the default taxonomy instead of the requested one.
  • clients/admin-ui/src/features/taxonomy/components/TaxonomyPageContent.tsx needs a useEffect to sync state after router hydration

Important Files Changed

File Analysis

Filename Score Overview
clients/admin-ui/src/pages/taxonomy/[key].tsx 2/5 New dynamic route for direct taxonomy navigation. Has a hydration issue: router.query.key is undefined on initial render, and the component doesn't sync state after router is ready.
clients/admin-ui/src/features/taxonomy/components/TaxonomyPageContent.tsx 3/5 Extracted taxonomy view logic into reusable component. Missing useEffect to sync taxonomyType state when initialTaxonomy prop changes after hydration.
clients/admin-ui/src/pages/taxonomy/index.tsx 5/5 Simplified to render TaxonomyPageContent without initialTaxonomy prop, correctly using the default taxonomy.

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.

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

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

Works as intended.
Small code nits but approving as is.

Comment on lines +228 to +238
// Update URL when taxonomy changes, preserving query parameters
const query = { ...router.query };
delete query.key;
router.replace(
{
pathname: `/taxonomy/${key}`,
query,
},
undefined,
{ shallow: true },
);
Copy link
Contributor

Choose a reason for hiding this comment

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

What query params need to be preserved?
This could be simplified if there is no need to preserve.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This view supports a show_disabled_items query param which needs to be preserved, do you prefer handling that explicitly?

https://github.com/ethyca/fides/pull/7113/changes#diff-7341123e02fae338ea5f227aa6179f7d6bfc16925a858ebb63d1645c44b684baR84 #

@jpople jpople added this pull request to the merge queue Dec 16, 2025
Merged via the queue into main with commit d7388c4 Dec 16, 2025
47 checks passed
@jpople jpople deleted the jpople/2025-12-11/track-taxonomy-in-url branch December 16, 2025 18:46
@jpople jpople mentioned this pull request Dec 17, 2025
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