-
Notifications
You must be signed in to change notification settings - Fork 72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update privacy center patch call for new API response #4235
Update privacy center patch call for new API response #4235
Conversation
@@ -171,7 +171,7 @@ const NoticeDrivenConsent = () => { | |||
(n) => n.privacy_notice_history_id === historyKey | |||
); | |||
const servedNotice = servedNotices?.find( | |||
(sn) => sn.privacy_notice_history.id === historyKey | |||
(sn) => sn.privacy_notice_history?.id === historyKey |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this file is the only one with "actual" changes. there were two API changes we account for here:
- served notices now return optional
privacy_notice_history
(since TCF doesn't have those) - the preferences patch now returns the data we expect nested under
preferences
(so that tcf data can be nested in its own subsections)
every other change in this PR is pretty much type wrangling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, it looks like a lot of good type updates on the privacy center side made it into this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, the privacy center types were a bit out of date 😓
Passing run #4507 ↗︎
Details:
Review all test suite changes for PR #4235 ↗︎ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did a lot of testing and looked closely at this file: clients/privacy-center/components/consent/NoticeDrivenConsent.tsx
I just have one question really!
@@ -1565,7 +1565,7 @@ describe("Consent banner", () => { | |||
cy.wait("@patchPrivacyPreference").then((preferenceInterception) => { | |||
const { preferences } = preferenceInterception.request.body; | |||
const expected = interception.response?.body.map( | |||
(s: LastServedNoticeSchema) => s.served_notice_history_id | |||
(s: LastServedConsentSchema) => s.served_notice_history_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a similar LastServedNoticeSchema
still being imported into clients/privacy-center/cypress/e2e/consent-notices.cy.ts - does that also need to be updated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahh good eye!! fixed: fad6a78
tcf_purpose_consents?: Array<TCFPurposeConsentRecord>; | ||
tcf_purpose_legitimate_interests?: Array<TCFPurposeLegitimateInterestsRecord>; | ||
tcf_special_purposes?: Array<TCFSpecialPurposeRecord>; | ||
tcf_features?: Array<TCFFeatureRecord>; | ||
tcf_special_features?: Array<TCFSpecialFeatureRecord>; | ||
tcf_vendor_consents?: Array<TCFVendorConsentRecord>; | ||
tcf_vendor_legitimate_interests?: Array<TCFVendorLegitimateInterestsRecord>; | ||
tcf_vendor_relationships?: Array<TCFVendorRelationships>; | ||
tcf_system_consents?: Array<TCFVendorConsentRecord>; | ||
tcf_system_legitimate_interests?: Array<TCFVendorLegitimateInterestsRecord>; | ||
tcf_system_relationships?: Array<TCFVendorRelationships>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do you generate these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I typically:
- tag
fides
- Use that version in fidesplus
- in admin-ui, run
npm run openapi:generate
, thennpm run format
. This will autogenerate all the TS types based off of the backend'sopenapi.json
file. we run it off of fidesplus since fidesplus has a superset of fides types - that sets admin-ui up nicely for TS types (which I did in the branch this PR is based off of)
- unfortunately for apps other than admin-ui, we manually copy and paste the relevant files over 🥲 there should be a way to share types one day, but that's been an open issue for a while now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahh thank you, I'll save this somewhere 👍
export type SavePrivacyPreferencesResponse = { | ||
preferences?: Array<CurrentPrivacyPreferenceSchema>; | ||
purpose_consent_preferences?: Array<CurrentPrivacyPreferenceSchema>; | ||
purpose_legitimate_interests_preferences?: Array<CurrentPrivacyPreferenceSchema>; | ||
special_purpose_preferences?: Array<CurrentPrivacyPreferenceSchema>; | ||
vendor_consent_preferences?: Array<CurrentPrivacyPreferenceSchema>; | ||
vendor_legitimate_interests_preferences?: Array<CurrentPrivacyPreferenceSchema>; | ||
feature_preferences?: Array<CurrentPrivacyPreferenceSchema>; | ||
special_feature_preferences?: Array<CurrentPrivacyPreferenceSchema>; | ||
system_consent_preferences?: Array<CurrentPrivacyPreferenceSchema>; | ||
system_legitimate_interests_preferences?: Array<CurrentPrivacyPreferenceSchema>; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
@@ -171,7 +171,7 @@ const NoticeDrivenConsent = () => { | |||
(n) => n.privacy_notice_history_id === historyKey | |||
); | |||
const servedNotice = servedNotices?.find( | |||
(sn) => sn.privacy_notice_history.id === historyKey | |||
(sn) => sn.privacy_notice_history?.id === historyKey |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, it looks like a lot of good type updates on the privacy center side made it into this PR
5f614aa
into
aking/4209/integrate-vendor-legal-basis
Closes #4225
Description Of Changes
The privacy center uses the response of patching notices with user preferences in order to create/update its cookie. The response payload format changed in #4201 to nest preferences under an object. This PR allows the FE to handle this new expected payload type.
Code Changes
Steps to Confirm
Everything should work as before! The main thing that changed is the API returns a new type, so now the FE handles that. To prove it's working, you'll want to observe that your cookie updates according to your saved preferences. This is for notices only, not TCF.
{"consent":{"data_sales_and_sharing":false}}
Pre-Merge Checklist
CHANGELOG.md