define a unseeding forums permissions and call into it from delete_cours... - #954
Conversation
…urse define a unseeding forums permissions and call into it from delete_course define a unseeding forums permissions and call into it from delete_course
…se with a different casing on the courseId
…ns does not accidentially delete another course's forum seeding
There was a problem hiding this comment.
Maybe check first that permissions are seeded? Or perhaps that is well covered in another test.
If so, change the comment line to say "verify form unseeding".
|
Just some stylistic comments. I can't say whether or not this code is sufficient to completely unprovision a forum, but I assume Greg knows. |
|
👍 Once tests pass. |
There was a problem hiding this comment.
This should be combined with the other import from this module
There was a problem hiding this comment.
Actually, this should not be invoked directly in this test.
There was a problem hiding this comment.
I do think it is worth testing the unseed_permissions_roles method in isolation, though perhaps not in this file (in addition to testing that the deletion unseeds permissions).
There was a problem hiding this comment.
Agreed (including that it doesn't belong in this file)
…nction in CMS. Update tests to use new method
There was a problem hiding this comment.
Ah, this looks better! But (ducking...) can you also add a test for unseed_permissions_roles in isolation? It would be in the test file for django_comment_common/utils.py.
There was a problem hiding this comment.
Yea, I agree. But when I try to do this, I see that in django_comment_common/module.py, there's a dependency on a course to be in the modulestore (line 70) so that it can dereference course.forum_posts_allowed. I'm sure there's a way to get a course factory or something else in there, but I'd like to timebox this...
|
👍 I agree with timeboxing the investigation of the targeted unit tests for unseed_permissions_roles. |
…on-delete define a unseeding forums permissions and call into it from delete_cours...
Update rocketchat 0.2.23
3 participants
...e