Full Delete Course

[frasermuller]

Description

Closes #450

This PR adds a new feature of being able to fully delete a course and everything related to it. Below are my changes from my notes:

1. Wrote a query in beekeeper studio to find all the tables that have a connection to the course_model table so we can change all the tables that have NO ACTION -> CASCADE because they will block course deletion:

2. Created a new migration file to change the 10 tables ABOVE with NO ACTION to CASCADE.
   confirming changes worked on beekeeper studio:

3. Made backend endpoint that finds the course and calls .remove(). and the CASCADE should handle the rest. NOTE: only organization level admins can fully delete courses. so if this should be changed to professors aswell or some other role let me know.

4. Made frontend API function that calls the backend endpoint. nothing new to note here.

5. Made the delete button component, it is very similar to the archive course, but with a warning message/confirmation before calling the api function. Also redirects back to /courses after.

6. Added the delete button to the edit course page, but only appears for admins.

SCREENSHOTS OF NEW FEATURE:

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update
• This requires a run of yarn install
• This change requires an addition/change to the production .env variables. These changes are below:
• This change requires developers to add new .env variables. The file and variables needed are below:
• This change requires a database query to update old data on production. This query is below:

How Has This Been Tested?

Please describe how you tested this PR (both manually and with tests)
Provide instructions so we can reproduce.

TEST 1:

After deleting COSC 310 and going back into beekeeper studio I ran

SELECT id, name FROM course_model;

Which shows

Showing it was deleted

TEST 2:

SELECT * FROM course_model WHERE id = 50;
SELECT * FROM queue_model WHERE "courseId" = 50;
SELECT * FROM user_course_model WHERE "courseId" = 50;

Shows nothing returned, which cosc 310’s id was 50, so the CASCADE worked since the other tables with the referenced coursed were deleted.

Checklist:

• I have performed a code review of my own code (under the "Files Changed" tab on github) to ensure nothing is committed that shouldn't be (e.g. leftover console.logs, leftover unused logic, or anything else that was accidentally committed)
• I have commented my code where needed
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• I have checked that new and existing tests pass locally with my changes
• Any work that this PR is dependent on has been merged into the main branch
• Any UI changes have been checked to work on desktop, tablet, and mobile

[AdamFipke]

NOTE: only organization level admins can fully delete courses. so if this should be changed to professors aswell or some other role let me know.

This is a good question that I think should be brought up during the meeting.

Technically speaking, profs won't be able to create their own courses soon (they will need to email admins), in which case yeah it's probably best to leave it as admin-only

[AdamFipke]

Looks good! I like the changes, I would just add a couple integration tests for the new endpoint

[AdamFipke]

if we wanted to get really fancy with it we could have them re-type out the name of the course or something, but meh

[AdamFipke]

in this instance, I would probably want to add some tests for this. Stuff like

• Make sure only org admins can call the endpoint
• Try making a course with a bunch of random connected entities (queues, events, user_course, etc.) and then call this endpoint to make sure that all of those entities are deleted, but it probably doesn't need to be too fancy. In an ideal world, I would also want some way of making sure that no unintended database entities were affected (for example, if the user entities were deleted due to some ON DELETE CASCADE messup in another entity), but that would require a fair bit of work (plus there's a LOT of other tests should also check this).

[frasermuller]

I added 2 integration tests:

403 for non-admins (only org admins can delete)
200 for successful deletion by org admin

I wasn't able to test the CASCADE behavior (verifying that queues, user_courses, etc. get deleted too). The test DB uses: synchronize: true, which creates tables from entity decorators, but it doesn't run migrations. The CASCADE constraints are in my migration file but not in the entity decorators, so the test DB doesn't have them.

Tested manually and the CASCADE works, but if there's a better approach for testing this, let me know.

Also had to move it up in the file, cause CI was timing out.

[AdamFipke]

I wasn't able to test the CASCADE behavior (verifying that queues, user_courses, etc. get deleted too). The test DB uses: synchronize: true, which creates tables from entity decorators, but it doesn't run migrations. The CASCADE constraints are in my migration file but not in the entity decorators, so the test DB doesn't have them.

Oh right! Sorry I realised I gave the wrong feedback here initially. So usually you don't write migrations for most database schemas manually. You usually change the entity files themselves and then use the command to automatically make them (except for some niche cases, like if you need to run a specific 1-time query in production to update existing production data).

The way to do this issue is to modify the entities themselves (modify the options of OneToMany to include onDelete CASCADE. I believe some of our entities have it already but not all of them.

From there, you can try re-creating the migration file using the command. It should basically be the same as the one you've manually created, but it's probably better to use the autogenerated one