DESENG-632 Add DB tables & API routes #2533
Conversation
| @@ -0,0 +1,27 @@ | |||
| """ | |||
| Relax language table constraints. | |||
There was a problem hiding this comment.
like, just chill out, dude
| branch_labels = None | ||
| depends_on = None | ||
|
|
||
| with open("migrations/supported_languages.json", "r") as read_file: |
There was a problem hiding this comment.
Has maintainability been considered here? What will it look like when we need to add or remove a language? Can we still use the same JSON file or will we be manually migrating to match any edits made?
There was a problem hiding this comment.
Great point!
Adding/removing languages in the future would be done with a DB migration. I don't expect us to have to do so that often. Given that, maybe it would be better to rename supported_languages to initial_languages or something, knowing that it may not be the exhaustive in the long term.
I could have also not had a separate file but I felt the size of the migration file would be super huge and less readable that way. Do you think maybe I should just move the list into the migration file given that they'll be the ultimate source of truth?
| request_json, 'language' | ||
| languages = LanguageService.get_languages() | ||
| return ( | ||
| jsonify(LanguageSchema(many=True).dump(languages)), |
There was a problem hiding this comment.
Is this call to jsonify necessary if dump() already produces a json string?
There was a problem hiding this comment.
Oh I missed this, this might just solve an issue I was having... Thanks
| """Resourse for removing language-tenant relationships.""" | ||
|
|
||
| @staticmethod | ||
| # @_jwt.requires_auth |
There was a problem hiding this comment.
I realize this is temporary, but do remember to add appropriate authentication to your routes before merging to main
There was a problem hiding this comment.
I made a note about it in the PR description but yes for sure, I'll have to circle back to auth once I push changes for the frontend. It'll be easier to test it when I'm making requests from the browser
| @@ -24,22 +23,6 @@ def get_languages(): | |||
| languages_records = Language.get_languages() | |||
| return LanguageSchema(many=True).dump(languages_records) | |||
|
|
|||
| @staticmethod | |||
| def create_language(language_data): | |||
| """Create language.""" | |||
There was a problem hiding this comment.
If we can delete languages, we should probably still be able to add them as well, otherwise the number of languages can only decrease with time. Is there a reason all this code was removed?
There was a problem hiding this comment.
If I left in a call to deleting languages themselves then I'll be removing it. Great point
| if success: | ||
| return 'Successfully deleted language', HTTPStatus.NO_CONTENT | ||
| raise ValueError('Language not found') | ||
| return 'Successfully deleted language-tenant mapping', HTTPStatus.NO_CONTENT |
There was a problem hiding this comment.
I appreciate you updating the language to remain accurate :)
|
…odes (#2540) * DESENG-632 Add DB tables & API routes (#2533) * DESENG-632 Add DB tables & API routes * DESENG-623 fix linting * DESENG-623 Move language to migration file, remove delete crud language route * DESENG-623 Remove ability to delete language --------- Co-authored-by: Alex <[email protected]> * Feature/deseng623 - Add frontend, update backend routes to use tenant short name (#2538) * DESENG-623 Add frontend, update backend routes * DESENG-623 Add api route updates, correct linting * DESENG-623 Correctly invoke autocomplete api to disable claering --------- Co-authored-by: Alex <[email protected]> * DESENG-623 Add frontend tests, trim api tests * DESENG-623 Add frontend tests * DESENG-623 Add new tests, fix broken ones, merge migrations * DESENG-623 Update changelog * DESENG-623 fix linting and tests, clean up code further --------- Co-authored-by: Alex <[email protected]>
Issue #: https://github.com/bcgov/met-public/issues/
Description of changes:
This is a partial PR.