Skip to content

Allow non-flexible Purposes to add Purpose Restriction#6066

Merged
gilluminate merged 3 commits intomainfrom
gill/LJ-694/inflexible-purpose-restriction
Apr 21, 2025
Merged

Allow non-flexible Purposes to add Purpose Restriction#6066
gilluminate merged 3 commits intomainfrom
gill/LJ-694/inflexible-purpose-restriction

Conversation

@gilluminate
Copy link
Copy Markdown
Contributor

@gilluminate gilluminate commented Apr 18, 2025 •
edited
Loading

Closes LJ-694

Description Of Changes

Removes restrictions on editing purpose restrictions for non-flexible purposes in TCF consent settings. Instead of blocking editing entirely, the UI now allows users to create restrictions for these purposes but limits them to only use Purpose Restriction type.

Code Changes

  • Removed code that hid the "Edit" button for non-flexible purposes in PublisherRestrictionsTable
  • Added state and logic in PurposeRestrictionFormModal to disable the Restriction Type dropdown for non-flexible purposes
  • Added a tooltip explanation for why the Restriction Type is disabled
  • Removed the error toast and redirect in the purpose config page ([purpose_id].tsx) that prevented accessing non-flexible purposes

Steps to Confirm

  1. Navigate to a TCF consent configuration (/settings/consent)
  2. Attempt to edit restrictions for a non-flexible purpose (those with "No" in the Flexible column)
  3. Verify that the Edit button is visible and the form opens
  4. Confirm the Restriction type is set to Purpose restriction and cannot be changed
    NOTE: Backend validation with throw an error for now, @JadeCara is working to address this

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
    • Bug fix for current release work, no entry required
  • Followup issues:
    • Followup issues created (include link)
    • 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 Apr 18, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
fides-plus-nightly ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 21, 2025 8:46pm
fides-privacy-center ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 21, 2025 8:46pm

@gilluminate gilluminate force-pushed the gill/LJ-694/inflexible-purpose-restriction branch from 256e9fe to 9026bcd Compare April 18, 2025 21:22
@gilluminate gilluminate force-pushed the gill/LJ-694/inflexible-purpose-restriction branch from c716b42 to dc6694b Compare April 21, 2025 15:33
@lucanovera lucanovera self-requested a review April 21, 2025 15:41
lucanovera
lucanovera reviewed Apr 21, 2025
View reviewed changes
Comment thread clients/admin-ui/src/features/consent-settings/tcf/PurposeRestrictionFormModal.tsx Outdated
const toast = useToast();
const [createRestriction] = useCreatePublisherRestrictionMutation();
const [updateRestriction] = useUpdatePublisherRestrictionMutation();
const [isPurposeFlexible, setIsPurposeFlexible] = useState(true);
Copy link
Copy Markdown
Contributor

@lucanovera lucanovera Apr 21, 2025

Choose a reason for hiding this comment

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

I've tested the feature and it works correctly. However, I found a bit odd that the during the first render the initialValues are set to the original values, then the useEffect runs, and then the initialValues get changed.
Captura de pantalla 2025-04-21 a la(s) 4 51 45 p  m

Apparently this works fine with Formik, but I think with a couple small improvement this can be cleaned and avoid the changing values during rendering (and also avoiding the eslint rule disable).

The purpose flexible can be derived from the prop without needing a state
const isPurposeFlexible = !( purposeId && FORBIDDEN_LEGITIMATE_INTEREST_PURPOSE_IDS.includes(+purposeId) );
so it's value is the correct one even during the first render.

and the initial values is only used in the Formik component so it can be overriden there:
<Formik initialValues={{ ...initialValues, restriction_type: isPurposeFlexible ? initialValues.restriction_type : TCFRestrictionType.PURPOSE_RESTRICTION, }}

What do you think? Let me know if you see any issues with this approach.

Copy link
Copy Markdown
Contributor Author

@gilluminate gilluminate Apr 21, 2025

Choose a reason for hiding this comment

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

This is a great suggestion. Thank you! Updated.

lucanovera
lucanovera reviewed Apr 21, 2025
View reviewed changes
Copy link
Copy Markdown
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.

@gilluminate I've tested and everything is working correctly 💯
Captura de pantalla 2025-04-21 a la(s) 4 38 16 p  m

I left a suggestion for the code and don't forget to add the changelog entry.

@gilluminate gilluminate force-pushed the gill/LJ-694/inflexible-purpose-restriction branch from d744e1c to 82ec62c Compare April 21, 2025 20:39
lucanovera
lucanovera approved these changes Apr 21, 2025
View reviewed changes
Copy link
Copy Markdown
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.

Approved 💯

@gilluminate gilluminate merged commit 71ea780 into main Apr 21, 2025
17 checks passed
@gilluminate gilluminate deleted the gill/LJ-694/inflexible-purpose-restriction branch April 21, 2025 21:05
@cypress
Copy link
Copy Markdown

cypress Bot commented Apr 21, 2025

fides    Run #12850

Run Properties:  status check passed Passed #12850  •  git commit 71ea7802c4: Allow non-flexible Purposes to add Purpose Restriction (#6066)
Project fides
Branch Review main
Run status status check passed Passed #12850
Run duration 00m 54s
Commit git commit 71ea7802c4: Allow non-flexible Purposes to add Purpose Restriction (#6066)
Committer Jason Gill
View all properties for this run ↗︎
Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 5
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.
View all changes introduced in this branch ↗︎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lucanovera lucanovera lucanovera approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@gilluminate @lucanovera