ENG-2920: Allow for showing multiple links in privacy center + show them below forms too#7612
ENG-2920: Allow for showing multiple links in privacy center + show them below forms too#7612nreyes-dev merged 13 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds typed multi-link support for the Privacy Center: new PrivacyCenterLink type, a links field on PrivacyCenterConfig (legacy privacy_policy_* fields deprecated), a getEffectivePrivacyCenterLinks utility, updated example config, and components updated to render multiple footer links. Changes
Sequence Diagram(s)sequenceDiagram
participant Config as Config (runtime / config.json)
participant Util as getEffectivePrivacyCenterLinks
participant Component as HomePage / AuthFormLayout
participant Browser as Rendered UI
Config->>Component: provide config
Component->>Util: getEffectivePrivacyCenterLinks(config)
Util-->>Component: PrivacyCenterLink[] (links or synthesized fallback)
Component->>Browser: render each link (label → url)
Browser-->>Component: user clicks link (external navigation)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
clients/privacy-center/common/config-links.ts (1)
3-7: Consider usingPrivacyCenterLinkdirectly in the type definition.The
linksfield uses an inline typeArray<{ label: string; url: string }>that duplicates the shape ofPrivacyCenterLink. Using the imported type directly would improve consistency and reduce maintenance burden ifPrivacyCenterLinkevolves.♻️ Suggested refactor
import { PrivacyCenterLink } from "~/types/config"; type ConfigWithLinks = { - links?: Array<{ label: string; url: string }>; + links?: PrivacyCenterLink[]; privacy_policy_url?: string | null; privacy_policy_url_text?: string | null; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/privacy-center/common/config-links.ts` around lines 3 - 7, Replace the inline link shape in the ConfigWithLinks type with the existing PrivacyCenterLink type to avoid duplication: change links?: Array<{ label: string; url: string }>; to use PrivacyCenterLink (e.g., links?: PrivacyCenterLink[] or Array<PrivacyCenterLink>), ensuring you import or reference the existing PrivacyCenterLink type and update any related usage to match the new type name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@clients/privacy-center/common/config-links.ts`:
- Around line 3-7: Replace the inline link shape in the ConfigWithLinks type
with the existing PrivacyCenterLink type to avoid duplication: change links?:
Array<{ label: string; url: string }>; to use PrivacyCenterLink (e.g., links?:
PrivacyCenterLink[] or Array<PrivacyCenterLink>), ensuring you import or
reference the existing PrivacyCenterLink type and update any related usage to
match the new type name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5c915a08-b200-4c72-9b60-2029b673e0a1
📒 Files selected for processing (2)
clients/privacy-center/common/config-links.tsclients/privacy-center/config/config.json
|
@greptileai review |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
clients/privacy-center/__tests__/common/config-links.test.ts (1)
3-52: Well-structured test suite with good coverage of the main logic paths.The tests comprehensively cover:
- Non-empty
linksarray returns as-is- Fallback to deprecated fields when
linksis absent- Precedence of
linksover deprecated fields- Empty array handling with fallback behavior
Test data appropriately uses RFC 2606 reserved domain (
example.com) and generic labels.Consider adding tests for partial deprecated fields (only
privacy_policy_urlor onlyprivacy_policy_url_textpresent) to explicitly document that incomplete deprecated config returns an empty array:💡 Optional: Add edge case tests for partial deprecated fields
+ it("returns empty array when only privacy_policy_url is present", () => { + const result = getEffectivePrivacyCenterLinks({ + privacy_policy_url: "https://example.com/privacy", + }); + expect(result).toEqual([]); + }); + + it("returns empty array when only privacy_policy_url_text is present", () => { + const result = getEffectivePrivacyCenterLinks({ + privacy_policy_url_text: "Privacy Policy", + }); + expect(result).toEqual([]); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/privacy-center/__tests__/common/config-links.test.ts` around lines 3 - 52, Add explicit tests to cover partial deprecated-field cases for getEffectivePrivacyCenterLinks: add one test where only privacy_policy_url is provided and assert it returns [], and another where only privacy_policy_url_text is provided and assert it returns []; place them alongside the existing cases in the describe("getEffectivePrivacyCenterLinks") suite and give clear it(...) names like "returns empty array when only privacy_policy_url is present" and "returns empty array when only privacy_policy_url_text is present" so the behavior for incomplete deprecated config is documented.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@clients/privacy-center/__tests__/common/config-links.test.ts`:
- Around line 3-52: Add explicit tests to cover partial deprecated-field cases
for getEffectivePrivacyCenterLinks: add one test where only privacy_policy_url
is provided and assert it returns [], and another where only
privacy_policy_url_text is provided and assert it returns []; place them
alongside the existing cases in the describe("getEffectivePrivacyCenterLinks")
suite and give clear it(...) names like "returns empty array when only
privacy_policy_url is present" and "returns empty array when only
privacy_policy_url_text is present" so the behavior for incomplete deprecated
config is documented.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5142cad7-91ea-4768-8fc6-48db36d3e499
📒 Files selected for processing (3)
clients/privacy-center/__tests__/common/config-links.test.tsclients/privacy-center/common/config-links.tssrc/fides/api/schemas/privacy_center_config.py
🚧 Files skipped from review as they are similar to previous changes (2)
- clients/privacy-center/common/config-links.ts
- src/fides/api/schemas/privacy_center_config.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/privacy-center/components/common/AuthFormLayout.tsx`:
- Around line 33-34: AuthFormLayout currently calls
getEffectivePrivacyCenterLinks(config) without ensuring config is non-null,
which can throw during initial renders; modify AuthFormLayout so you only
compute policyLinks when config exists (e.g., guard useConfig() result before
calling getEffectivePrivacyCenterLinks or default policyLinks to an empty
array). Locate the useConfig() call and the policyLinks variable in
AuthFormLayout and change the logic to: const config = useConfig(); const
policyLinks = config ? getEffectivePrivacyCenterLinks(config) : []; (or
equivalent optional chaining/guard) so the footer rendering safely omits links
when config is not yet available.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: be468602-2718-41ef-8ffa-eefd93ed65ba
📒 Files selected for processing (2)
clients/privacy-center/components/HomePage.tsxclients/privacy-center/components/common/AuthFormLayout.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- clients/privacy-center/components/HomePage.tsx
jpople
left a comment
There was a problem hiding this comment.
Tested locally, looking good to me!
Ticket ENG-2920
Description Of Changes
The Privacy Center previously supported only a single configurable link (Privacy Policy) rendered at the bottom of the home page. This adds a new
linksarray field to the config schema, allowing any number of named links to be displayed. Links now also appear on privacy request form pages, where they were previously absent. The legacyprivacy_policy_url/privacy_policy_url_textfields continue to work unchanged, but are marked as deprecated.Code Changes
PrivacyCenterLinkschema (Python + TypeScript) andlinks: List[PrivacyCenterLink] = []field toPrivacyCenterConfighttp/httpsonly) onPrivacyCenterLink.urlprivacy_policy_url/privacy_policy_url_textwith backwards-compatible fallback viagetEffectivePrivacyCenterLinkshelperHomePage.tsxto render all links verticallyAuthFormLayout.tsxto render links below the form card (new — previously no links appeared on form pages); links open in a new tab withrel="noopener noreferrer"getEffectivePrivacyCenterLinksSteps to Confirm
"links": [{"label": "Privacy Policy", "url": "https://example.com"}, {"label": "Terms of Service", "url": "https://example.com/tos"}]toconfig.jsonand verify both links render on the home page and on a privacy request form pagelinksand use the legacyprivacy_policy_url/privacy_policy_url_textfields — verify the single link still renders on both pagesPre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and worksSummary by CodeRabbit
New Features
Deprecations
Tests
Changelog