Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

Add days_left field #1281

Merged
merged 10 commits into from
Sep 9, 2022
Merged

Add days_left field #1281

merged 10 commits into from
Sep 9, 2022

Conversation

TheAndrewJackson
Copy link
Contributor

@TheAndrewJackson TheAndrewJackson commented Sep 8, 2022

Purpose

Add dynamic days_left field that is generated based off of the current due_date value of a privacy request.

Changes

  • Add new property to PrivacyRequest model
  • Add new tests and update existing tests to work with new update

Checklist

  • Update CHANGELOG.md file
    • Merge in main so the most recent CHANGELOG.md file is being appended to
    • Add description within the Unreleased section in an appropriate category. Add a new category from the list at the top of the file if the needed one isn't already there.
    • Add a link to this PR at the end of the description with the PR number as the text. example: #1
  • Applicable documentation updated (guides, quickstart, postman collections, tutorial, fidesdemo, database diagram.
  • If docs updated (select one):
    • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
    • documentation issue created (tag docs-team to complete issue separately)
  • Good unit test/integration test coverage
  • This PR contains a DB migration. If checked, the reviewer should confirm with the author that the down_revision correctly references the previous migration before merging
  • The Run Unsafe PR Checks label has been applied, and checks have passed, if this PR touches any external services

Ticket

Fixes #1148

@TheAndrewJackson TheAndrewJackson marked this pull request as ready for review September 8, 2022 20:04
@TheAndrewJackson
Copy link
Contributor Author

@sanders41 I updated the days_left field to use the property decorator and moved the function body inside the model. Since the function is inside the model now I left the parameter as self. If that doesn't make sense for the property decorator I can push up a quick update changing it

@sanders41
Copy link
Contributor

The property update looks good. I'll approve and merge as soon as CI finishes.

@sanders41
Copy link
Contributor

sanders41 commented Sep 9, 2022

@TheAndrewJackson one thing I just realized, this updates a database model. Do we need a migration? Maybe not since it is just a property?

@TheAndrewJackson
Copy link
Contributor Author

TheAndrewJackson commented Sep 9, 2022

@sanders41 AFAIK we don't need a migration. We decided to calculate this property at the api level because it's a value that changes every day and there could be many changes if the org has a lot of privacy requests. It doesn't exist in the DB right now.

If at a later date we decided to move this property into the DB and have an automated job that updated all the due dates we would need a migration.

@sanders41 sanders41 merged commit 33552ef into main Sep 9, 2022
@sanders41 sanders41 deleted the 1148_days_left_field branch September 9, 2022 14:21
sanders41 pushed a commit that referenced this pull request Sep 22, 2022
* Add `days_left` field

* Fix lints and update tests

* Move property to model

* Run black

* Update CHANGELOG.md

* Add new test and fix issue

* Fix pylint issue

* Fix brittle tests issue

* Address PR feedback
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add days_left to PrivacyRequest API
2 participants