Courses migration of create, delete and get course by id #341
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
armintalaie
approved these changes
Mar 14, 2025
There was a problem hiding this comment.
Looks good @VictorAKang.
I pushed some changes:
- API had some type issues
- moved default roles to
seed.sql - left some comments that can be later addressed. look for
@victorin code (ideally it would be a PR comment but I wanted to get this in asap)
everything else looks really good!! thanks
armintalaie
added a commit
that referenced
this pull request
Mar 14, 2025
* server code checkpoint * shared package update * users api tests * Initial migration of courses APIs. Create course, get course by id, and add user to course migrated. Still require tests and a bit of refactoring * Fix bug * test * Add post test setup * Courses migration of create, delete and get course by id (#341) * Added deffault roles and fixed bug on course creation * Added tests for create, delete, get, get all courses * moved init data to seed; syntax fixes; notes --------- Co-authored-by: Armin T <[email protected]> --------- Co-authored-by: Victor Akita Kang <[email protected]> Co-authored-by: Boris Wang <[email protected]> Co-authored-by: Victor Akita Kang <[email protected]>
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.
Proposed Changes
Start of courses migration. Migrated create, delete, and get course by id.
Coverage of new files is at 33% however most of the branches that are being missed are the ones that test for sanity checking (e.g. if default roles are created) or for disastrous cases (i.e. database is broken) so unless requested to test this scenarios I will keep the low test coverage.
Still missing transforming the functions into APIs. Will do in next PR.