-
Notifications
You must be signed in to change notification settings - Fork 77
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
PROD-2058 implement endpoints for property specific messaging #4983
PROD-2058 implement endpoints for property specific messaging #4983
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
05bdb13
to
37c4f59
Compare
Passing run #8366 ↗︎
Details:
Review all test suite changes for PR #4983 ↗︎ |
5c46156
to
dec5f1d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4983 +/- ##
==========================================
+ Coverage 86.54% 86.59% +0.04%
==========================================
Files 351 351
Lines 21739 21818 +79
Branches 2878 2881 +3
==========================================
+ Hits 18814 18893 +79
Misses 2420 2420
Partials 505 505 ☔ View full report in Codecov by Sentry. |
Pipeline failures dependent on #4993 being merged |
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.
Very straightforward set of endpoints to manage all of these templates! Everything worked as expected so feel free to address my comments only if time permits. The two things that stuck out to me were:
- Having these endpoints in fides, in my opinion, since they are for plus functionality, the endpoints should be defined in fidesplus
- Using a more constrained enum instead of
MessagingActionType
for theGET/POST /api/v1/messaging/templates/{template_type}
endpoints
…ts-property-specific-messaging
Ticket to move endpoints to plus as a follow-up: https://ethyca.atlassian.net/browse/PROD-2211 |
…ts-property-specific-messaging
Passing run #8367 ↗︎
Details:
Review all test suite changes for PR #4983 ↗︎ |
Closes https://ethyca.atlassian.net/browse/PROD-2058
Description Of Changes
Implements API endpoints for property-specific messaging. The following new endpoints were added:
/messaging/templates/summary
- get paginated list of all templates/messaging/templates/default/{template_type}
- get the default messaging template by type. This is not dependent on the DB at all, rather hard-coded in the BE./messaging/templates/{template_type}
- create a new messaging template . Use req body formatted like:/messaging/templates/{id}
- update an existing messaging template. Use req body formatted like:/messaging/templates/{id}
- update an existing messaging template. Use req body formatted like:/messaging/templates/{id}
- get an existing messaging template by id/messaging/templates/{id}
- delete a messaging template by idCode Changes
Steps to Confirm
ops/api/v1/endpoints/test_messaging_endpoints.py
for details of how the endpoints should function.Pre-Merge Checklist
CHANGELOG.md
main
downgrade()
migration is correct and works