New TCF Purpose Header Field on Translations #5246
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
fides
|
||||||||||||||||||||||||||||||
| Project |
fides
|
| Branch Review |
refs/pull/5246/merge
|
| Run status |
|
| Run duration | 00m 36s |
| Commit |
|
| Committer | Dawn Pattison |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
4
|
Upgrade your plan to view test results. | |
| View all changes introduced in this branch ↗︎ | |
| @@ -155,6 +155,7 @@ class ExperienceTranslationBase: | |||
| privacy_policy_link_label = Column(String) | |||
| privacy_policy_url = Column(String) | |||
| privacy_preferences_link_label = Column(String) | |||
| purpose_header = Column(String) | |||
There was a problem hiding this comment.
Brand new purpose_header field showing up on the translations, the field is added here, on ExperienceTranslatoin, and on the ExperienceConfigHistory table, which stores every version. We have to keep a record of these changes so we can demonstrate the specifics of what was shown to the customer at a point in time.
|
|
||
|
|
||
| def downgrade(): | ||
| pass |
There was a problem hiding this comment.
Considering what to add to downgrade here, maybe clear the purpose header, but I don't want to update descriptions necessarily in case they were updated by the end user too. The purpose header field is also entirely deleted altogether in the previous downgrade on the previous migration.
There was a problem hiding this comment.
Is there a situation where we would want/need to downgrade only cc37 and leave 1332 in place? I feel like it's enough to say that if this was reverted both migrations would end up downgraded and the field would be deleted entirely, in which case leaving this downgrade() empty is fine (I think? Not super familiar with alembic). Unless we want to combine the two to make it impossible to downgrade only one.
There was a problem hiding this comment.
yes all good questions, I can probably add the downgrade on just the purpose header just in case. Matter of preference, I like separating schema and data migrations into separate files for their distinct operations!
There was a problem hiding this comment.
Hmm I think it's okay to just rely on the column delete on the previous one, if purpose_header is cleared, there's still an ExperienceConfigHistory record with the purpose_header on it. The downrev could delete the historical record, but it's possible other historical records were added in the interim, we can't assume there's just been one created. I think there's less meddling if we drop the field altogether -
| depends_on = None | ||
|
|
||
| # fmt: off | ||
| updated_translations = { |
There was a problem hiding this comment.
These translations are normally stored in Fidesplus, but adding a subset of them here to facilitate adding them directly to existing ExperienceTranslations
|
|
||
|
|
||
| def downgrade(): | ||
| pass |
There was a problem hiding this comment.
Is there a situation where we would want/need to downgrade only cc37 and leave 1332 in place? I feel like it's enough to say that if this was reverted both migrations would end up downgraded and the field would be deleted entirely, in which case leaving this downgrade() empty is fine (I think? Not super familiar with alembic). Unless we want to combine the two to make it impossible to downgrade only one.
- Adds a new purpose_header field on the ExperienceTranslation and on the historical record, the PrivacyExperienceConfigHistory table - Adds a data migration to populate purpose_header fields on all existing TCF translations as well as update the existing description, provided the customer is still on version 1 for that translation, meaning they've made no edits
fides
|
||||||||||||||||||||||||||||||
| Project |
fides
|
| Branch Review |
main
|
| Run status |
|
| Run duration | 00m 36s |
| Commit |
|
| Committer | Dawn Pattison |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
Failures
|
0
|
|
Flaky
|
0
|
|
Pending
|
0
|
|
Skipped
|
0
|
|
Passing
|
4
|
Upgrade your plan to view test results. | |
| View all changes introduced in this branch ↗︎ | |
Closes #PROD-2616
Description Of Changes
Adds/updates TCF translation fields in support of the new minimal TCF banner
Code Changes
purpose_headerfield on theExperienceTranslationand on the historical record, thePrivacyExperienceConfigHistorytablepurpose_headerfields on all existing TCF translations as well as update the existing description, provided the customer is still on version 1 for that translation, meaning they've made no editsSteps to Confirm
Pre-Merge Checklist
CHANGELOG.mdmaindowngrade()migration is correct and works