[#9534] Deleting the last instructor in the course leaves a dangling course object in the database #11930
Conversation
|
Just noticed end-to-end testing needs to be updated as well, working on that now |
|
The only E2E test that should be affected now passes, and the consistently failing test seems to fail both on my feature branch and master |
|
@mattlim1207 will be checking this out Edit: reran the E2E test and all seem ok |
| "name": "Teammates Instr3", | ||
| "email": "AAccounts.instr3@gmail.tmt", | ||
| "role": "Co-owner", | ||
| "isDisplayedToStudents": true, |
There was a problem hiding this comment.
Any reason why an additional instructor account for courseId tm.e2e.AAccounts.CS2103 is needed?
There was a problem hiding this comment.
So from what I gathered about E2E tests and the "happy path," I figured a test should check the admin can successfully delete an instructor, as deleting a last instructor of a course is more of an edge case. Thus, it required another instructor to be created
There was a problem hiding this comment.
Which E2E test are you referring to? From what I understand this PR only touches the unit test DeleteInstructorActionTest.java
There was a problem hiding this comment.
It's the AdminAccountsPageE2ETest. I figured I should fix the issue, and all associated unit tests and E2E tests, in one PR. The E2E test was set up to create a course with one invisible instructor, then simulate the admin deleting the instructor. Because this behavior was changed with this PR, I modified the initial parameters.
There was a problem hiding this comment.
I see, so the changes you have made in DeleteInstructorAction would cause AdminAccountsPageE2ETest to fail as the there is currently 1 instructor attached to the course CS2103 (they are invisible as well, causing the test to fail too).
Thanks for clarifying, this makes sense to me
Co-authored-by: Nicolas Chang Weng Yew <nicolas.chang99@gmail.com>
ziqing26
added
s.ToMerge
Fixes #9534
Outline of Solution
DeleteInstructorActionthat allowed admins to bypass the truth checking stopping the last instructor of a course from being deleted from said course.testExecute_deleteLastInstructorByGoogleId_shouldPass, by flipping asserts, and renaming accordingly.