-
Notifications
You must be signed in to change notification settings - Fork 56
Multitenant backend #775
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
Multitenant backend #775
Conversation
…adet into dev/multitenent
…ssessment config and types updates
I think the backend is ready for a MVP merge(since we have deployed it and all the test and checks are passing). More tests and swagger documentation will be added after this. |
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.
Thank you again for all the hard work. The overall changes are good.
Minor nits:
- Missing some Swagger documentation
- Commented out code should just be removed
- Some inconsistencies - some places use user_id, others use course_reg_id
- Some validation is in the controller when it should be in the model changeset function
Two bigger issues:
-
Need to see if there are any other cases where there is a reference to the course registration ID that will prevent a user from being removed from a course. E.g. last grader ID, unsubmitted by ID.
I actually think now that it was not necessary (or in fact a bad idea) to make everything key on the course registration ID rather than the user ID + course ID, but it's too late to change that now.
I think what should be done now is to add a
deleted
orremoved
column to the course registrations table so removing a user doesn't actually mean everything they have done in the course has to be deleted. -
The auth provider should be a separate column in the users table, not prefixed to the username.
I have not looked at the tests; I'll do so soon.
Documentation can be done later but let's resolve the other (non-documentation) issues first before merging.
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.
Just one outstanding issue, otherwise LGTM
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.
Couple more nits
test/cadet_web/admin_controllers/admin_goals_controller_test.exs
Outdated
Show resolved
Hide resolved
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.
I think this is good for merging now.
Outstanding issues:
- Remove use of Ecto Schemas from the migrations
- Split authentication provider into a separate column
- Namespacing the assets bucket
- Fix removal of staff/admin from course (and possible other issues)
- Move role checking from model to controller (stories and Sourcecasts; any others?)
- Show staff groups in /users
- Fix semantics of the
public
flag in Sourcecasts
* update test.exs to include optional test setting * initial schema change to account * added schema for course_registration * updated accounts query * Removed settings context and shifted sublanguages to courses context * Added initial tests for courses context and shifted settings tests over * Added initial tests for courses context controllers * Added course table changeset tests * updated course_registration context function/getters * Updated course config functions and tests * Fix filename typo * Updated endpoint url for course config update to prevent clash with assessment config and types updates * Added assesment config routing, functions and tests * Temporarily fix references to Courses context in miscellaneous files * Added context functions and tests for assessment types * adding course_registration test set * Updated courses context tests * Added update assessment types routing and routing tests * updated course_registration test * Updated get course config tests to include assessment types for the specified course * added pipeline for course_registration to assign course_reg in conn * Temporary edits to make tests compilable * Updated admin courses controller routes and test urls * Updated courses controller routes and test urls * Update admin courses controller tests * Updated courses context tests * Updated course_registrations function * Follow up on router assign_course * addded course_registration test for changeset * fix group factory, partial fix for the ecto.setup flow * updated course_registration context function test getters * Updated sourcecast context functions and tests * updated test for course_registration insert and update * updated course_registration test * updated course_registration test * Updated sourcecast tests * Updated assessment types tests * fix course_registration_test.exs * Updated sourcecast changeset * Updated stories schema and tests * upadted assessment schema and relevant test * update assessment factory * updated submissions schema with test, and added db migration * added latest_viewed_course in user and course fk to group * fix snake case * updated accounts context function and adminUserController with test * update accounts_test with get_users_by and fix queries with no groups * update userController with test(except for stories) * fix accounts test * added latest_viewed context functions with test * fix snake_case issue * update seed.exs for initial test with frontend * updated seed with latest_viewed * fix user view snake_case issue * refactor assessment_config to belong to assessment type with test and seeds * update_assessment_config: move order from url param to json boday * upadte courseName and courseShortName * update assessment_type test in user controller * update seed * update admin controller test to test updated result and fix course field names * refactor admin_course_controller test * fix change of field name in course in user view * remove grade/max_grade/adjustment in assessments context * fix assessments show & update submission_votes voter * fix assessment submit and query avenger_of? with test * updated bonus_xp logit with assessment config * update formatting and seed * Updated google claim extractor * Added github auth provider * Namespacing for different auth providers * Updated dev.secrets.exs.example * Updated seed * Update github auth config * Update user seeds * added get assessment config * updated config route, combine assessment config table * debuging for working with frontend * remove decay rate and update assessment config reorder logit with test * Updated sign_in flow and user table to allow for NULL name * refactor reorder logit with test * Added update_role and delete_user endpoints (#781) * Update views to match frontend requirements * Added update user role route * Added delete user from course * tested reorder and mass_upsert_reorder * Refactor update_role and delete_course_registration * update test cases and fix some credo issues * updated bonus_xp logic * update notifications with test * Added add_users endpoint and tests * Format * Added create course endpoint and tests * updated notification controller with test * updated assessments context functions with test(skipping contest) * updated answer controller with test * added delete assessment_config route with test * Updated provider tests * Format * Temp updates to migration file * Complete migration file * remove required in notification * fixing test seed and update with migration * update assessment controller with test + alter assessment config table * finalise assessment controller with test * Updated assessment config json to proper camelCase * Remove capitalisation of assessment types inside assessment_config changeset * update /user call * update xml parser for assessment with test * updated assessment config booleans * rename /user view field * updated /user endpoint with skippable sent * updated admin assessment controller with test * update /user call and remove settings test * finalised /user call * fix auth_controller_test * updated contest test * remove mentor from group * setting up admin grading controller test * Updated admin PUT /users endpoint * Namespace existing usernames in migration file * Updated migration file assessment type configs * Updated add users logic * updated admin grading controller with test(less grading summary) * merging * Updated devices * refactor assessment config booleans to question * code formatting * Fix migration file * Fix errors when testing with frontend * added isManuallyGraded field to admin grading endpoint * fix testcases * delete assessment config deletes relations + tested * fix assessment grading view * fix format and credo issues * Update migration file * added unique constaints for assessment number and course_id * rename user view crId to courseRegId * updated jobs and autograder with test * update assessments controller test * updated grading summary with test * fix spec and rename field * reorder grading summary * fix grader_id in migration * grading index filter by course * read all notifications in the previous course * prepare for multitenant deploy * prepare for multitenant deploy * Debug workflow * Specify --overwrite * update init.sh url * add role in /user call course array * add role in /user call course array * clean up unused code * set latest_viewed_id to nil in users for non-viewable course * update question testcases format * add migration for update testcase format * update migration * Increase upsert users and groups transaction timeout to handle large requests of up to 1000 entries * update admin assets controller test * update achievement for multitenant upgrade * fix formatting * renaming and cleaning up code * rename latest viewed course * add course_reg_id to admin user routes * update migration * update migration * update migration * update achievement migration * update contest leaderboard * update contest test * fix assessment config update * fix source chapter and variant changeset condition * fix course changeset and validation * simplify function * update delete route * fix issues * fix migration Co-authored-by: Chow En Rong <[email protected]> Co-authored-by: angelsl <[email protected]>
This is the initial changes for the backend, we mostly covered accounts and courses context, we will move on to assessment.
all test under /courses and /accounts are passing.
All the tests should be passing.
(seeds.exs is working partially, but migration from old db id not working, please use
MIX_ENV=test mix ecto.reset
before testing)