From 41be20a0008dc62d35d09c5b94bebd6273c99e1e Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 12 Sep 2013 12:31:00 -0400 Subject: [PATCH 1/7] define a unseeding forums permissions and call into it from delete_course define a unseeding forums permissions and call into it from delete_course define a unseeding forums permissions and call into it from delete_course --- .../management/commands/delete_course.py | 4 ++++ .../contentstore/tests/test_contentstore.py | 8 +++++++ .../djangoapps/django_comment_common/utils.py | 24 +++++++++++++++++++ 3 files changed, 36 insertions(+) diff --git a/cms/djangoapps/contentstore/management/commands/delete_course.py b/cms/djangoapps/contentstore/management/commands/delete_course.py index 50f9b82e80..f97cdf5130 100644 --- a/cms/djangoapps/contentstore/management/commands/delete_course.py +++ b/cms/djangoapps/contentstore/management/commands/delete_course.py @@ -7,6 +7,7 @@ from xmodule.modulestore.django import modulestore from xmodule.contentstore.django import contentstore from xmodule.course_module import CourseDescriptor from .prompt import query_yes_no +from django_comment_common.utils import unseed_permissions_roles from auth.authz import _delete_course_group @@ -40,6 +41,9 @@ class Command(BaseCommand): if query_yes_no("Are you sure. This action cannot be undone!", default="no"): loc = CourseDescriptor.id_to_location(course_id) if delete_course(ms, cs, loc, commit): + print 'removing forums permissions and roles...' + unseed_permissions_roles(course_id) + print 'removing User permissions from course....' # in the django layer, we need to remove all the user permissions groups associated with this course if commit: diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index c4f5f4ee61..b4f35bb19a 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -55,6 +55,8 @@ from uuid import uuid4 from pymongo import MongoClient from student.models import CourseEnrollment +from django_comment_common.utils import unseed_permissions_roles + TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE) TEST_DATA_CONTENTSTORE['OPTIONS']['db'] = 'test_xcontent_%s' % uuid4().hex @@ -1292,6 +1294,12 @@ class ContentStoreTest(ModuleStoreTestCase): 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(self): + """Test new course creation and verify forum seeding """ + test_course_data = self.assert_created_course(number_suffix=uuid4().hex) + unseed_permissions_roles(self._get_course_id(test_course_data)) + self.assertFalse(are_permissions_roles_seeded(self._get_course_id(test_course_data))) + def _get_course_id(self, test_course_data): """Returns the course ID (org/number/run).""" return "{org}/{number}/{run}".format(**test_course_data) diff --git a/common/djangoapps/django_comment_common/utils.py b/common/djangoapps/django_comment_common/utils.py index f74116d59f..b4c3f7ae29 100644 --- a/common/djangoapps/django_comment_common/utils.py +++ b/common/djangoapps/django_comment_common/utils.py @@ -32,6 +32,30 @@ def seed_permissions_roles(course_id): administrator_role.inherit_permissions(moderator_role) +def unseed_permissions_roles(course_id): + """ + A utility method to clean up all forum related permissions and roles + """ + administrator_role = Role.objects.get_or_create(name="Administrator", course_id=course_id)[0] + moderator_role = Role.objects.get_or_create(name="Moderator", course_id=course_id)[0] + community_ta_role = Role.objects.get_or_create(name="Community TA", course_id=course_id)[0] + student_role = Role.objects.get_or_create(name="Student", course_id=course_id)[0] + + # safety switches in case the casing does not match + + if administrator_role.course_id == course_id: + administrator_role.delete() + + if moderator_role.course_id == course_id: + moderator_role.delete() + + if community_ta_role.course_id == course_id: + community_ta_role.delete() + + if student_role.course_id == course_id: + student_role.delete() + + def are_permissions_roles_seeded(course_id): try: From bd69db4410bbd22de63354825ced239d92c0853f Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 12 Sep 2013 14:02:22 -0400 Subject: [PATCH 2/7] switch to using get() rather than get_or_create(). Also add a test case with a different casing on the courseId --- .../contentstore/tests/test_contentstore.py | 15 ++++++- .../djangoapps/django_comment_common/utils.py | 44 ++++++++++++------- 2 files changed, 41 insertions(+), 18 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index b4f35bb19a..ad8bd9d01b 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1297,8 +1297,19 @@ class ContentStoreTest(ModuleStoreTestCase): def test_forum_unseeding(self): """Test new course creation and verify forum seeding """ test_course_data = self.assert_created_course(number_suffix=uuid4().hex) - unseed_permissions_roles(self._get_course_id(test_course_data)) - self.assertFalse(are_permissions_roles_seeded(self._get_course_id(test_course_data))) + course_id = self._get_course_id(test_course_data) + unseed_permissions_roles(course_id) + self.assertFalse(are_permissions_roles_seeded(course_id)) + + def test_forum_unseeding_with_different_casing(self): + """Test new course creation and verify forum seeding """ + test_course_data = self.assert_created_course(number_suffix=uuid4().hex) + # make sure we don't delete a forum permissions set with different casing + # than the passed in course_id. This is because Mongo and MySQL are using different collations + course_id = self._get_course_id(test_course_data) + unseed_permissions_roles(course_id.upper()) + # permissions should still be there! + self.assertTrue(are_permissions_roles_seeded(course_id)) def _get_course_id(self, test_course_data): """Returns the course ID (org/number/run).""" diff --git a/common/djangoapps/django_comment_common/utils.py b/common/djangoapps/django_comment_common/utils.py index b4c3f7ae29..a73dc2aadb 100644 --- a/common/djangoapps/django_comment_common/utils.py +++ b/common/djangoapps/django_comment_common/utils.py @@ -36,28 +36,40 @@ def unseed_permissions_roles(course_id): """ A utility method to clean up all forum related permissions and roles """ - administrator_role = Role.objects.get_or_create(name="Administrator", course_id=course_id)[0] - moderator_role = Role.objects.get_or_create(name="Moderator", course_id=course_id)[0] - community_ta_role = Role.objects.get_or_create(name="Community TA", course_id=course_id)[0] - student_role = Role.objects.get_or_create(name="Student", course_id=course_id)[0] + try: + administrator_role = Role.objects.get(name="Administrator", course_id=course_id) + if administrator_role.course_id == course_id: + administrator_role.delete() + except Role.DoesNotExist: + pass - # safety switches in case the casing does not match + try: + moderator_role = Role.objects.get(name="Moderator", course_id=course_id) + if moderator_role.course_id == course_id: + moderator_role.delete() + except Role.DoesNotExist: + pass - if administrator_role.course_id == course_id: - administrator_role.delete() + try: + community_ta_role = Role.objects.get(name="Community TA", course_id=course_id) + if community_ta_role.course_id == course_id: + community_ta_role.delete() + except Role.DoesNotExist: + pass - if moderator_role.course_id == course_id: - moderator_role.delete() - - if community_ta_role.course_id == course_id: - community_ta_role.delete() - - if student_role.course_id == course_id: - student_role.delete() + try: + student_role = Role.objects.get(name="Student", course_id=course_id) + if student_role.course_id == course_id: + student_role.delete() + except Role.DoesNotExist: + pass 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) From 615341fb65242490a40e58bd6aece39031b43eee Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 12 Sep 2013 15:03:00 -0400 Subject: [PATCH 3/7] add one more test to verify that deleting one courses forum permissions does not accidentially delete another course's forum seeding --- .../contentstore/tests/test_contentstore.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index ad8bd9d01b..41e6906906 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1302,7 +1302,7 @@ class ContentStoreTest(ModuleStoreTestCase): self.assertFalse(are_permissions_roles_seeded(course_id)) def test_forum_unseeding_with_different_casing(self): - """Test new course creation and verify forum seeding """ + """Test new course creation and verify forum unseeding """ test_course_data = self.assert_created_course(number_suffix=uuid4().hex) # make sure we don't delete a forum permissions set with different casing # than the passed in course_id. This is because Mongo and MySQL are using different collations @@ -1311,6 +1311,20 @@ class ContentStoreTest(ModuleStoreTestCase): # permissions should still be there! self.assertTrue(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) + unseed_permissions_roles(course_id) + 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) From 481cbfd8a56ac5116a7e39d5e79ad7623e2601bd Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 12 Sep 2013 15:57:13 -0400 Subject: [PATCH 4/7] consolidate delete role logic --- .../djangoapps/django_comment_common/utils.py | 40 ++++++------------- 1 file changed, 13 insertions(+), 27 deletions(-) diff --git a/common/djangoapps/django_comment_common/utils.py b/common/djangoapps/django_comment_common/utils.py index a73dc2aadb..388474395c 100644 --- a/common/djangoapps/django_comment_common/utils.py +++ b/common/djangoapps/django_comment_common/utils.py @@ -32,37 +32,23 @@ def seed_permissions_roles(course_id): administrator_role.inherit_permissions(moderator_role) +def _remove_permission_role(course_id, name): + try: + 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): """ A utility method to clean up all forum related permissions and roles """ - try: - administrator_role = Role.objects.get(name="Administrator", course_id=course_id) - if administrator_role.course_id == course_id: - administrator_role.delete() - except Role.DoesNotExist: - pass - - try: - moderator_role = Role.objects.get(name="Moderator", course_id=course_id) - if moderator_role.course_id == course_id: - moderator_role.delete() - except Role.DoesNotExist: - pass - - try: - community_ta_role = Role.objects.get(name="Community TA", course_id=course_id) - if community_ta_role.course_id == course_id: - community_ta_role.delete() - except Role.DoesNotExist: - pass - - try: - student_role = Role.objects.get(name="Student", course_id=course_id) - if student_role.course_id == course_id: - student_role.delete() - except Role.DoesNotExist: - pass + _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): From bf67c833402cbcbcca64a62ce3dd752f8b37e0d4 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 12 Sep 2013 15:58:56 -0400 Subject: [PATCH 5/7] update comment and verify seeding was done --- cms/djangoapps/contentstore/tests/test_contentstore.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 41e6906906..9bea3b2784 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1295,8 +1295,9 @@ class ContentStoreTest(ModuleStoreTestCase): self.assertTrue(are_permissions_roles_seeded(self._get_course_id(test_course_data))) def test_forum_unseeding(self): - """Test new course creation and verify forum seeding """ + """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))) course_id = self._get_course_id(test_course_data) unseed_permissions_roles(course_id) self.assertFalse(are_permissions_roles_seeded(course_id)) From 2ac6d78a9061ef962bc40e317a1629688e75312f Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 12 Sep 2013 16:01:51 -0400 Subject: [PATCH 6/7] update comment --- cms/djangoapps/contentstore/tests/test_contentstore.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 9bea3b2784..b5b8bab587 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1303,7 +1303,7 @@ class ContentStoreTest(ModuleStoreTestCase): self.assertFalse(are_permissions_roles_seeded(course_id)) def test_forum_unseeding_with_different_casing(self): - """Test new course creation and verify forum unseeding """ + """Make sure that we honor case sensitivity when unseeding forums""" test_course_data = self.assert_created_course(number_suffix=uuid4().hex) # make sure we don't delete a forum permissions set with different casing # than the passed in course_id. This is because Mongo and MySQL are using different collations From 955e54c2e6b9839c139841d2eaa11f5860275e1d Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 13 Sep 2013 09:07:01 -0400 Subject: [PATCH 7/7] refactor some code out of the django-admin entry point into a help function in CMS. Update tests to use new method --- .../management/commands/delete_course.py | 27 ++-------------- .../contentstore/tests/test_contentstore.py | 18 +++-------- cms/djangoapps/contentstore/utils.py | 31 ++++++++++++++++++- 3 files changed, 36 insertions(+), 40 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/delete_course.py b/cms/djangoapps/contentstore/management/commands/delete_course.py index f97cdf5130..085fce5fe5 100644 --- a/cms/djangoapps/contentstore/management/commands/delete_course.py +++ b/cms/djangoapps/contentstore/management/commands/delete_course.py @@ -2,14 +2,8 @@ ### Script for cloning a course ### from django.core.management.base import BaseCommand, CommandError -from xmodule.modulestore.store_utilities import delete_course -from xmodule.modulestore.django import modulestore -from xmodule.contentstore.django import contentstore -from xmodule.course_module import CourseDescriptor from .prompt import query_yes_no -from django_comment_common.utils import unseed_permissions_roles - -from auth.authz import _delete_course_group +from contentstore.utils import delete_course_and_groups # @@ -31,23 +25,6 @@ class Command(BaseCommand): if commit: print 'Actually going to delete the course from DB....' - ms = modulestore('direct') - cs = contentstore() - - org, course_num, run = course_id.split("/") - ms.ignore_write_events_on_courses.append('{0}/{1}'.format(org, course_num)) - if query_yes_no("Deleting course {0}. Confirm?".format(course_id), default="no"): if query_yes_no("Are you sure. This action cannot be undone!", default="no"): - loc = CourseDescriptor.id_to_location(course_id) - if delete_course(ms, cs, loc, commit): - print 'removing forums permissions and roles...' - unseed_permissions_roles(course_id) - - print 'removing User permissions from course....' - # in the django layer, we need to remove all the user permissions groups associated with this course - if commit: - try: - _delete_course_group(loc) - except Exception as err: - print("Error in deleting course groups for {0}: {1}".format(loc, err)) + delete_course_and_groups(course_id, commit) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index b5b8bab587..c4e6ff23dd 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -55,7 +55,7 @@ from uuid import uuid4 from pymongo import MongoClient from student.models import CourseEnrollment -from django_comment_common.utils import unseed_permissions_roles +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 @@ -1294,24 +1294,14 @@ class ContentStoreTest(ModuleStoreTestCase): 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(self): + 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))) course_id = self._get_course_id(test_course_data) - unseed_permissions_roles(course_id) + delete_course_and_groups(course_id, commit=True) self.assertFalse(are_permissions_roles_seeded(course_id)) - def test_forum_unseeding_with_different_casing(self): - """Make sure that we honor case sensitivity when unseeding forums""" - test_course_data = self.assert_created_course(number_suffix=uuid4().hex) - # make sure we don't delete a forum permissions set with different casing - # than the passed in course_id. This is because Mongo and MySQL are using different collations - course_id = self._get_course_id(test_course_data) - unseed_permissions_roles(course_id.upper()) - # permissions should still be there! - self.assertTrue(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) @@ -1319,7 +1309,7 @@ class ContentStoreTest(ModuleStoreTestCase): # unseed the forums for the first course course_id = self._get_course_id(test_course_data) - unseed_permissions_roles(course_id) + 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) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index e5ae6bb66b..09d04e4929 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -5,12 +5,16 @@ from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.contentstore.content import StaticContent -from django.core.urlresolvers import reverse +from xmodule.contentstore.django import contentstore import copy import logging import re from xmodule.modulestore.draft import DIRECT_ONLY_CATEGORIES from django.utils.translation import ugettext as _ +from django_comment_common.utils import unseed_permissions_roles +from auth.authz import _delete_course_group +from xmodule.modulestore.store_utilities import delete_course +from xmodule.course_module import CourseDescriptor log = logging.getLogger(__name__) @@ -20,6 +24,31 @@ NOTES_PANEL = {"name": _("My Notes"), "type": "notes"} EXTRA_TAB_PANELS = dict([(p['type'], p) for p in [OPEN_ENDED_PANEL, NOTES_PANEL]]) +def delete_course_and_groups(course_id, commit=False): + """ + This deletes the courseware associated with a course_id as well as cleaning update_item + the various user table stuff (groups, permissions, etc.) + """ + module_store = modulestore('direct') + content_store = contentstore() + + org, course_num, run = course_id.split("/") + module_store.ignore_write_events_on_courses.append('{0}/{1}'.format(org, course_num)) + + loc = CourseDescriptor.id_to_location(course_id) + if delete_course(module_store, content_store, loc, commit): + print 'removing forums permissions and roles...' + unseed_permissions_roles(course_id) + + print 'removing User permissions from course....' + # in the django layer, we need to remove all the user permissions groups associated with this course + if commit: + try: + _delete_course_group(loc) + except Exception as err: + log.error("Error in deleting course groups for {0}: {1}".format(loc, err)) + + def get_modulestore(category_or_location): """ Returns the correct modulestore to use for modifying the specified location