Add Template_Categories table #2193
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Initial work on the models
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- Fix typos - Update sqlalchemy model
jzbahrai
reviewed
Jun 13, 2024
jzbahrai
reviewed
Jun 13, 2024
- Added template_category_id to the template_history table - Adjusted model class name TemplateCategories -> TemplateCategory - WIP: Added some basic tests for the TemplateCategory dao
- Added a schema for TemplateCategory - Added from_json and serialize to the TemplateCategory model - Added the template/category Blueprint - Add some initial CRUD api endpoints
jzbahrai
reviewed
Jun 17, 2024
app/models.py
Outdated
| @@ -1179,6 +1213,7 @@ class Template(TemplateBase): | |||
|
|
|||
| service = db.relationship("Service", backref="templates") | |||
| version = db.Column(db.Integer, default=0, nullable=False) | |||
| category = db.relationship("TemplateCategory", backref="templates") | |||
There was a problem hiding this comment.
I would change this to template_category. Ignore my relationship comment above, this should work!
- Changed the category relationship in Template from TemplateCategory to template_category - dao_get_template_category_by_template_id now simply selects the template, and returns the associated template_category instead of using an expensive join
- Moved the default categories into our config file - Added dao_update_template_category - Added dao_delete_template_category_by_id - Added routes to: get and delete a template category - Added a route to get all template categories - Updated the migration file to use default category uuid's from the config file
- Squash more bugs - Fix formatting - Added a couple more tests for the filters on dao_get_all_template_categories
- Excluded the template_category table from deletion in notify_db_session to preserve the 3 generic template categories between test runs - Fixed inserts in the migration, apparently alembic / sqlalchemy doesn't like multi-line f-strings - Made a few tests shorter by excluding the description_en and description_fr columns as they are optional
- Allow passing of a uuid to dao_create_template_category - Fixed issues with get_template_categories and delete_template_category filters / flags - Added a fixture to re-populate the template_category table with generic categories and removed template_categories from the list of tables that are excluded from the post-test db clear
jzbahrai
reviewed
Jun 25, 2024
6 tasks
- Remove NOT NULL constraint on templates.process_type
- Simplify the dao delete logic
- Adjust tests - formatting
5 tasks
jzbahrai
reviewed
Jun 26, 2024
jzbahrai
reviewed
Jun 26, 2024
| @@ -241,6 +335,7 @@ def create_sample_template( | |||
| subject_line="Subject", | |||
| user=None, | |||
| service=None, | |||
| category=None, | |||
There was a problem hiding this comment.
Do we need template_category_id here instead of category?
There was a problem hiding this comment.
Just setting the category_id should work fine. What we could also do is if the category is None we can set the category_id to one of the default categories.
whabanks
commented
Jun 26, 2024
andrewleith
reviewed
Jun 27, 2024
Clarify that new category assignments aren't done at random but are chosen from one of the default categories.
jzbahrai
reviewed
Jul 3, 2024
There was a problem hiding this comment.
Excellent Job! I liked the way the delete worked and thinking of the cascade usecases.
1 requirement:
When doing the delete - we update the Templates table. When I checked the DB - the updated_at didn't change. We should fix that
1 suggestion (please make a ticket for if you think we should do this - not blocking this PR)
- Add a created_at/ updated_at on the TemplateCategory table (this should also update with the API accordingly)
- Set a default category id if a category is not passed to create_sample_template
jzbahrai
reviewed
Jul 3, 2024
jzbahrai
approved these changes
Jul 3, 2024
- Create a template category when creating a sample template and do not pass in a category as we cannot rely on the defaults existing in the test db
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary | Résumé
This PR implements the backend work for Template Categories per the Short code ADR.
Change Details
template_categoriestable to the DB in a migrationTemplateCategorymodel and associations withTemplateandTemplateHistory/template-category/template-category/template-category/template-category/<template_category_id>/template-category/by-template-id/<template-id>/template-category/<template_category_id>/template-category/<template_category_id>?cascade=[True | False]/service/<id>/template/<template_id>/category/<template_category_id>Related Issues | Cartes liées
Test instructions | Instructions pour tester la modification
If you're using Postman, you can import this request collection to hit each new endpoint. I've included example payloads for create and update
Default generic Template Category id's you can use in the requests
/template-category/template-category/<template_category_id>Getting a Category
/template-category/<template_category_id>/service/<service_id>/template/<template_id>/category/<template_category_id>/template-category/by-template-id/<template-id>Getting all Categories
You'll need to create some new categories to fully test the gets with filtering.
hiddencategory - associate it with an SMS templatenot hiddencategory - associate it with an email template/template-category/template-category?hidden=[True | False | None]/template-category?template_type=[sms | email]/template-category?hidden=[True | False | None]&template_type=[sms | email]Deleting Categories
/template-category/template_category_id?cascade=Falsecategory_idfrom the earlier test where you associated it with a template/template-category/<template_category_id>?cascade=Falsemaking surecascade=Falseor omitted entirelycascade=Truein the query string and try deleting the same category againRelease Instructions | Instructions pour le déploiement
Reviewer checklist | Liste de vérification du réviseur