-
Notifications
You must be signed in to change notification settings - Fork 4.3k
define a unseeding forums permissions and call into it from delete_cours... #954
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
Changes from all commits
41be20a
bd69db4
615341f
481cbfd
bf67c83
2ac6d78
955e54c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,6 +55,8 @@ | |
| from pymongo import MongoClient | ||
| from student.models import CourseEnrollment | ||
|
|
||
| from contentstore.utils import delete_course_and_groups | ||
|
|
||
| TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE) | ||
| TEST_DATA_CONTENTSTORE['OPTIONS']['db'] = 'test_xcontent_%s' % uuid4().hex | ||
|
|
||
|
|
@@ -1292,6 +1294,28 @@ def test_create_course_check_forum_seeding(self): | |
| test_course_data = self.assert_created_course(number_suffix=uuid4().hex) | ||
| self.assertTrue(are_permissions_roles_seeded(self._get_course_id(test_course_data))) | ||
|
|
||
| def test_forum_unseeding_on_delete(self): | ||
| """Test new course creation and verify forum unseeding """ | ||
| test_course_data = self.assert_created_course(number_suffix=uuid4().hex) | ||
| self.assertTrue(are_permissions_roles_seeded(self._get_course_id(test_course_data))) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move this down and use course_id |
||
| course_id = self._get_course_id(test_course_data) | ||
| delete_course_and_groups(course_id, commit=True) | ||
| self.assertFalse(are_permissions_roles_seeded(course_id)) | ||
|
|
||
| def test_forum_unseeding_with_multiple_courses(self): | ||
| """Test new course creation and verify forum unseeding when there are multiple courses""" | ||
| test_course_data = self.assert_created_course(number_suffix=uuid4().hex) | ||
| second_course_data = self.assert_created_course(number_suffix=uuid4().hex) | ||
|
|
||
| # unseed the forums for the first course | ||
| course_id = self._get_course_id(test_course_data) | ||
| delete_course_and_groups(course_id, commit=True) | ||
| self.assertFalse(are_permissions_roles_seeded(course_id)) | ||
|
|
||
| second_course_id = self._get_course_id(second_course_data) | ||
| # permissions should still be there for the other course | ||
| self.assertTrue(are_permissions_roles_seeded(second_course_id)) | ||
|
|
||
| def _get_course_id(self, test_course_data): | ||
| """Returns the course ID (org/number/run).""" | ||
| return "{org}/{number}/{run}".format(**test_course_data) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,8 +32,30 @@ def seed_permissions_roles(course_id): | |
| administrator_role.inherit_permissions(moderator_role) | ||
|
|
||
|
|
||
| def are_permissions_roles_seeded(course_id): | ||
| def _remove_permission_role(course_id, name): | ||
| try: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would create a helper function (defined within this one) that does try: Then you can just call that function with each role type. |
||
| role = Role.objects.get(name=name, course_id=course_id) | ||
| if role.course_id == course_id: | ||
| role.delete() | ||
| except Role.DoesNotExist: | ||
| pass | ||
|
|
||
|
|
||
| def unseed_permissions_roles(course_id): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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... |
||
| """ | ||
| A utility method to clean up all forum related permissions and roles | ||
| """ | ||
| _remove_permission_role(name="Administrator", course_id=course_id) | ||
| _remove_permission_role(name="Moderator", course_id=course_id) | ||
| _remove_permission_role(name="Community TA", course_id=course_id) | ||
| _remove_permission_role(name="Student", course_id=course_id) | ||
|
|
||
|
|
||
| def are_permissions_roles_seeded(course_id): | ||
| """ | ||
| Returns whether the forums permissions for a course have been provisioned in | ||
| the database | ||
| """ | ||
| try: | ||
| administrator_role = Role.objects.get(name="Administrator", course_id=course_id) | ||
| moderator_role = Role.objects.get(name="Moderator", course_id=course_id) | ||
|
|
||
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.
Comment should be updated.