Skip to content
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

add new tcf fields to system model #173

Merged
merged 1 commit into from
Oct 5, 2023
Merged

Conversation

adamsachs
Copy link
Contributor

@adamsachs adamsachs commented Oct 5, 2023

Closes #172

Description Of Changes

See issue description

Code Changes

  • update System model
  • assert fields can be instantiated via automated tests

Steps to Confirm

n/a, automated test coverage should be good enough for us here as this is a pretty straightforward additive change

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Documentation Updated
  • Issue Requirements are Met
  • Relevant Follow-Up Issues Created
  • Update CHANGELOG.md

@@ -1138,10 +1138,10 @@ class System(FidesModel):
description="Deprecated. The responsibility or role over the system that processes personal data",
)
egress: Optional[List[DataFlow]] = Field(
description="The resources to which the System sends data."
description="The resources to which the system sends data."
Copy link
Contributor Author

Choose a reason for hiding this comment

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

decided to standardize capitalization of system across the descriptions in this model class while i was here

@@ -1225,6 +1225,24 @@ class System(FidesModel):
data_security_practices: Optional[str] = Field(
description="The data security practices employed by this system."
)
cookie_max_age_seconds: Optional[int] = Field(
description="The maximum storage duration, in seconds, for cookies used by this system."
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Descriptions were informed by official spec's descriptions here

@adamsachs adamsachs marked this pull request as ready for review October 5, 2023 15:05
@adamsachs
Copy link
Contributor Author

@pattisdr i think this should be a quick one, mind just sanity checking me that i haven't missed something silly?

@rsilvery i'd like a quick eye check on the descriptions i added for the new fields from a product POV please, thank you!

Copy link
Contributor

@pattisdr pattisdr left a comment

Choose a reason for hiding this comment

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

looks good

@adamsachs adamsachs merged commit 2f364dc into main Oct 5, 2023
31 checks passed
@adamsachs adamsachs deleted the asachs/172-addtl-tcf-fields branch October 5, 2023 19:07
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.

Add some missing TCF-related fields to System model
2 participants