Commit f5abef88 by Nimisha Asthagiri

LMS-2947 remove get_error_courses in Split.

LMS-2950 move delete_course to Mongo.
parent 545cfd84
......@@ -7,6 +7,7 @@ from student.roles import CourseInstructorRole, CourseStaffRole
from opaque_keys.edx.keys import CourseKey
from opaque_keys import InvalidKeyError
from opaque_keys.edx.locations import SlashSeparatedCourseKey
from xmodule.modulestore import ModuleStoreEnum
#
......@@ -38,7 +39,7 @@ class Command(BaseCommand):
print("Cloning course {0} to {1}".format(source_course_id, dest_course_id))
with mstore.bulk_write_operations(dest_course_id):
if mstore.clone_course(source_course_id, dest_course_id, None):
if mstore.clone_course(source_course_id, dest_course_id, ModuleStoreEnum.UserID.mgmt_command):
print("copying User permissions...")
# purposely avoids auth.add_user b/c it doesn't have a caller to authorize
CourseInstructorRole(dest_course_id).add_users(
......
......@@ -7,7 +7,7 @@ from contentstore.utils import delete_course_and_groups
from opaque_keys.edx.keys import CourseKey
from opaque_keys import InvalidKeyError
from opaque_keys.edx.locations import SlashSeparatedCourseKey
from xmodule.modulestore import ModuleStoreEnum
class Command(BaseCommand):
help = '''Delete a MongoDB backed course'''
......@@ -28,6 +28,6 @@ class Command(BaseCommand):
if commit:
print('Actually going to delete the course from DB....')
if query_yes_no("Deleting course {0}. Confirm?".format(course_key), default="no"):
if query_yes_no("Are you sure. This action cannot be undone!", default="no"):
delete_course_and_groups(course_key, commit)
if query_yes_no("Deleting course {0}. Confirm?".format(course_key), default="no"):
if query_yes_no("Are you sure. This action cannot be undone!", default="no"):
delete_course_and_groups(course_key, ModuleStoreEnum.UserID.mgmt_command)
......@@ -8,6 +8,7 @@ from django_comment_common.utils import (seed_permissions_roles,
from xmodule.modulestore.xml_importer import import_from_xml
from xmodule.modulestore.django import modulestore
from xmodule.contentstore.django import contentstore
from xmodule.modulestore import ModuleStoreEnum
class Command(BaseCommand):
......@@ -37,10 +38,9 @@ class Command(BaseCommand):
data=data_dir,
courses=course_dirs,
dis=do_import_static))
mstore = modulestore()
_, course_items = import_from_xml(
mstore, "**replace_user**", data_dir, course_dirs, load_error_modules=False,
modulestore(), ModuleStoreEnum.UserID.mgmt_command, data_dir, course_dirs, load_error_modules=False,
static_content_store=contentstore(), verbose=True,
do_import_static=do_import_static,
create_new_course_if_not_present=True,
......
......@@ -30,7 +30,6 @@ from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.modulestore.inheritance import own_metadata
from opaque_keys.edx.keys import UsageKey
from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation
from xmodule.modulestore.store_utilities import delete_course
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from xmodule.modulestore.xml_exporter import export_to_xml
from xmodule.modulestore.xml_importer import import_from_xml, perform_xlint
......@@ -632,7 +631,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase):
self.store.convert_to_draft(vertical.location, self.user.id)
# delete the course
delete_course(self.store, content_store, course_id, commit=True)
self.store.delete_course(course_id, self.user.id)
# assert that there's absolutely no non-draft modules in the course
# this should also include all draft items
......@@ -697,7 +696,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase):
self.assertEqual(on_disk['course/2012_Fall'], own_metadata(course))
# remove old course
delete_course(self.store, content_store, course_id, commit=True)
self.store.delete_course(course_id, self.user.id)
# reimport over old course
self.check_import(root_dir, content_store, course_id)
......@@ -909,7 +908,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase):
# Delete the course from module store and reimport it
delete_course(self.store, content_store, course_id, commit=True)
self.store.delete_course(course_id, self.user.id)
import_from_xml(
self.store, self.user.id, root_dir, ['test_export_no_content_store'],
......@@ -997,7 +996,7 @@ class ContentStoreTest(ContentStoreTestCase):
test_course_data = self.assert_created_course(number_suffix=uuid4().hex)
course_id = _get_course_id(test_course_data)
self.assertTrue(are_permissions_roles_seeded(course_id))
delete_course_and_groups(course_id, commit=True)
delete_course_and_groups(course_id, self.user.id)
# should raise an exception for checking permissions on deleted course
with self.assertRaises(ItemNotFoundError):
are_permissions_roles_seeded(course_id)
......@@ -1009,7 +1008,7 @@ class ContentStoreTest(ContentStoreTestCase):
# unseed the forums for the first course
course_id = _get_course_id(test_course_data)
delete_course_and_groups(course_id, commit=True)
delete_course_and_groups(course_id, self.user.id)
# should raise an exception for checking permissions on deleted course
with self.assertRaises(ItemNotFoundError):
are_permissions_roles_seeded(course_id)
......@@ -1029,7 +1028,7 @@ class ContentStoreTest(ContentStoreTestCase):
self.assertTrue(CourseEnrollment.is_enrolled(self.user, course_id))
self.assertTrue(self.user.roles.filter(name="Student", course_id=course_id)) # pylint: disable=no-member
delete_course_and_groups(course_id, commit=True)
delete_course_and_groups(course_id, self.user.id)
# check that user's enrollment for this course is not deleted
self.assertTrue(CourseEnrollment.is_enrolled(self.user, course_id))
# check that user has form role "Student" for this course even after deleting it
......@@ -1051,7 +1050,7 @@ class ContentStoreTest(ContentStoreTestCase):
self.assertTrue(len(instructor_role.users_with_role()) > 0)
# Now delete course and check that user not in instructor groups of this course
delete_course_and_groups(course_id, commit=True)
delete_course_and_groups(course_id, self.user.id)
# Update our cached user since its roles have changed
self.user = User.objects.get_by_natural_key(self.user.natural_key()[0])
......
......@@ -145,7 +145,7 @@ class TestCourseListing(ModuleStoreTestCase):
self.assertEqual(courses_list, courses_list_by_groups)
# now delete this course and re-add user to instructor group of this course
delete_course_and_groups(course_key, commit=True)
delete_course_and_groups(course_key, self.user.id)
CourseInstructorRole(course_key).add_users(self.user)
......@@ -237,7 +237,7 @@ class TestCourseListing(ModuleStoreTestCase):
self.assertEqual(len(courses_list_by_groups), 2)
# now delete first course (course_location_caps) and check that it is no longer accessible
delete_course_and_groups(course_location_caps, commit=True)
delete_course_and_groups(course_location_caps, self.user.id)
# test that get courses through iterating all courses now returns one course
courses_list = _accessible_courses_list(self.request)
......@@ -269,7 +269,7 @@ class TestCourseListing(ModuleStoreTestCase):
course_location = SlashSeparatedCourseKey('testOrg', 'doomedCourse', 'RunBabyRun')
self._create_course_with_access_groups(course_location, self.user)
store.delete_course(course_location)
store.delete_course(course_location, self.user.id)
course_location = SlashSeparatedCourseKey('testOrg', 'erroredCourse', 'RunBabyRun')
course = self._create_course_with_access_groups(course_location, self.user)
......
......@@ -162,7 +162,7 @@ class TemplateTests(unittest.TestCase):
self.assertIsInstance(self.split_store.get_course(id_locator), CourseDescriptor)
# and by guid
self.assertIsInstance(self.split_store.get_item(guid_locator), CourseDescriptor)
self.split_store.delete_course(id_locator)
self.split_store.delete_course(id_locator, ModuleStoreEnum.UserID.test)
# test can no longer retrieve by id
self.assertRaises(ItemNotFoundError, self.split_store.get_course, id_locator)
# but can by guid
......
......@@ -62,7 +62,7 @@ class TestUsersDefaultRole(ModuleStoreTestCase):
# check that user has his default "Student" forum role for this course
self.assertTrue(self.user.roles.filter(name="Student", course_id=self.course_key)) # pylint: disable=no-member
delete_course_and_groups(self.course_key, commit=True)
delete_course_and_groups(self.course_key, self.user.id)
# check that user's enrollment for this course is not deleted
self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course_key))
......@@ -80,7 +80,7 @@ class TestUsersDefaultRole(ModuleStoreTestCase):
self.assertTrue(self.user.roles.filter(name="Student", course_id=self.course_key)) # pylint: disable=no-member
# delete this course and recreate this course with same user
delete_course_and_groups(self.course_key, commit=True)
delete_course_and_groups(self.course_key, self.user.id)
resp = self._create_course_with_given_location(self.course_key)
self.assertEqual(resp.status_code, 200)
......@@ -98,7 +98,7 @@ class TestUsersDefaultRole(ModuleStoreTestCase):
# check that user has enrollment and his default "Student" forum role for this course
self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course_key))
# delete this course and recreate this course with same user
delete_course_and_groups(self.course_key, commit=True)
delete_course_and_groups(self.course_key, self.user.id)
# now create same course with different name case ('uppercase')
new_course_key = self.course_key.replace(course=self.course_key.course.upper())
......
......@@ -11,12 +11,10 @@ from django.utils.translation import ugettext as _
from django.core.urlresolvers import reverse
from xmodule.contentstore.content import StaticContent
from xmodule.contentstore.django import contentstore
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError
from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location
from xmodule.modulestore.store_utilities import delete_course
from student.roles import CourseInstructorRole, CourseStaffRole
......@@ -28,27 +26,25 @@ 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):
def delete_course_and_groups(course_id, user_id):
"""
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()
content_store = contentstore()
with module_store.bulk_write_operations(course_id):
if delete_course(module_store, content_store, course_id, commit):
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:
staff_role = CourseStaffRole(course_id)
staff_role.remove_users(*staff_role.users_with_role())
instructor_role = CourseInstructorRole(course_id)
instructor_role.remove_users(*instructor_role.users_with_role())
except Exception as err:
log.error("Error in deleting course groups for {0}: {1}".format(course_id, err))
module_store.delete_course(course_id, user_id)
print 'removing User permissions from course....'
# in the django layer, we need to remove all the user permissions groups associated with this course
try:
staff_role = CourseStaffRole(course_id)
staff_role.remove_users(*staff_role.users_with_role())
instructor_role = CourseInstructorRole(course_id)
instructor_role.remove_users(*instructor_role.users_with_role())
except Exception as err:
log.error("Error in deleting course groups for {0}: {1}".format(course_id, err))
def get_lms_link_for_item(location, preview=False):
......
......@@ -12,6 +12,7 @@ from django_future.csrf import ensure_csrf_cookie
from django.views.decorators.http import require_http_methods
from edxmako.shortcuts import render_to_response
from xmodule.modulestore.django import modulestore
from xmodule.modulestore import ModuleStoreEnum
from xmodule.tabs import CourseTabList, StaticTab, CourseTab, InvalidTabsException
from opaque_keys.edx.keys import CourseKey, UsageKey
......@@ -192,7 +193,7 @@ def primitive_delete(course, num):
# Note for future implementations: if you delete a static_tab, then Chris Dodge
# points out that there's other stuff to delete beyond this element.
# This code happens to not delete static_tab so it doesn't come up.
modulestore().update_item(course, '**replace_user**')
modulestore().update_item(course, ModuleStoreEnum.UserID.primitive_command)
def primitive_insert(course, num, tab_type, name):
......@@ -201,5 +202,5 @@ def primitive_insert(course, num, tab_type, name):
new_tab = CourseTab.from_json({u'type': unicode(tab_type), u'name': unicode(name)})
tabs = course.tabs
tabs.insert(num, new_tab)
modulestore().update_item(course, '**replace_user**')
modulestore().update_item(course, ModuleStoreEnum.UserID.primitive_command)
......@@ -97,7 +97,7 @@ class TestCourseListing(ModuleStoreTestCase):
course_location = SlashSeparatedCourseKey('testOrg', 'doomedCourse', 'RunBabyRun')
self._create_course_with_access_groups(course_location)
mongo_store.delete_course(course_location)
mongo_store.delete_course(course_location, ModuleStoreEnum.UserID.test)
course_location = SlashSeparatedCourseKey('testOrg', 'erroredCourse', 'RunBabyRun')
course = self._create_course_with_access_groups(course_location)
......
......@@ -72,6 +72,21 @@ class ModuleStoreEnum(object):
draft = 'draft-branch'
published = 'published-branch'
class UserID(object):
"""
Values for user ID defaults
"""
# Note: we use negative values here to (try to) not conflict
# with user identifiers provided by an actual user service.
# user ID to use for all management commands
mgmt_command = -1
# user ID to use for primitive commands
primitive_command = -2
# user ID to use for tests that do not have a django user available
test = -3
class PublishState(object):
"""
......@@ -270,6 +285,18 @@ class ModuleStoreRead(object):
"""
pass
@abstractmethod
def compute_publish_state(self, xblock):
"""
Returns whether this xblock is draft, public, or private.
Returns:
PublishState.draft - content is in the process of being edited, but still has a previous
version deployed to LMS
PublishState.public - content is locked and deployed to LMS
PublishState.private - content is editable and not deployed to LMS
"""
pass
class ModuleStoreWrite(ModuleStoreRead):
"""
......@@ -348,7 +375,7 @@ class ModuleStoreWrite(ModuleStoreRead):
pass
@abstractmethod
def delete_course(self, course_key, user_id=None):
def delete_course(self, course_key, user_id):
"""
Deletes the course. It may be a soft or hard delete. It may or may not remove the xblock definitions
depending on the persistence layer and how tightly bound the xblocks are to the course.
......@@ -441,6 +468,12 @@ class ModuleStoreReadBase(ModuleStoreRead):
None
)
def compute_publish_state(self, xblock):
"""
Returns PublishState.public since this is a read-only store.
"""
return PublishState.public
def heartbeat(self):
"""
Is this modulestore ready?
......@@ -553,6 +586,15 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite):
super(ModuleStoreWriteBase, self).clone_course(source_course_id, dest_course_id, user_id)
return dest_course_id
def delete_course(self, course_key, user_id):
"""
This base method just deletes the assets. The lower level impls must do the actual deleting of
content.
"""
# delete the assets
self.contentstore.delete_all_course_assets(course_key)
super(ModuleStoreWriteBase, self).delete_course(course_key, user_id)
@contextmanager
def bulk_write_operations(self, course_id):
"""
......
......@@ -229,16 +229,13 @@ class MixedModuleStore(ModuleStoreWriteBase):
store = self._get_modulestore_for_courseid(course_id)
return store.has_course(course_id, ignore_case)
def delete_course(self, course_key, user_id=None):
def delete_course(self, course_key, user_id):
"""
See xmodule.modulestore.__init__.ModuleStoreWrite.delete_course
"""
assert(isinstance(course_key, CourseKey))
store = self._get_modulestore_for_courseid(course_key)
if hasattr(store, 'delete_course'):
return store.delete_course(course_key, user_id)
else:
raise NotImplementedError(u"Cannot delete a course on store {}".format(store))
return store.delete_course(course_key, user_id)
def get_parent_location(self, location, **kwargs):
"""
......@@ -290,10 +287,6 @@ class MixedModuleStore(ModuleStoreWriteBase):
Returns: a CourseDescriptor
"""
store = self._get_modulestore_for_courseid(None)
if not hasattr(store, 'create_course'):
raise NotImplementedError(u"Cannot create a course on store {}".format(store))
return store.create_course(org, offering, user_id, fields, **kwargs)
def clone_course(self, source_course_id, dest_course_id, user_id):
......@@ -456,13 +449,7 @@ class MixedModuleStore(ModuleStoreWriteBase):
"""
course_id = xblock.scope_ids.usage_id.course_key
store = self._get_modulestore_for_courseid(course_id)
if hasattr(store, 'compute_publish_state'):
return store.compute_publish_state(xblock)
elif hasattr(store, 'publish'):
raise NotImplementedError(u"Cannot compute_publish_state on store {}".format(store))
else:
# read-only store; so, everything's public
return PublishState.public
return store.compute_publish_state(xblock)
def publish(self, location, user_id):
"""
......
......@@ -892,15 +892,6 @@ class MongoModuleStore(ModuleStoreWriteBase):
return course
def delete_course(self, course_key, user_id=None):
"""
The impl removes all of the db records for the course.
:param course_key:
:param user_id:
"""
course_query = self._course_key_to_son(course_key)
self.collection.remove(course_query, multi=True)
def create_xmodule(self, location, definition_data=None, metadata=None, runtime=None, fields={}):
"""
Create the new xmodule but don't save it. Returns the new module.
......
......@@ -144,6 +144,18 @@ class DraftModuleStore(MongoModuleStore):
del key['_id.revision']
return self.collection.find(key).count() > 0
def delete_course(self, course_key, user_id):
"""
:param course_key: which course to delete
:param user_id: id of the user deleting the course
"""
# delete the assets
super(DraftModuleStore, self).delete_course(course_key, user_id)
# delete all of the db records for the course
course_query = self._course_key_to_son(course_key)
self.collection.remove(course_query, multi=True)
def clone_course(self, source_course_id, dest_course_id, user_id):
"""
Only called if cloning within this store or if env doesn't set up mixed.
......
......@@ -1380,7 +1380,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
return result
def delete_course(self, course_key, user_id=None):
def delete_course(self, course_key, user_id):
"""
Remove the given course from the course index.
......@@ -1395,6 +1395,10 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
log.info(u"deleting course from split-mongo: %s", course_key)
self.db_connection.delete_course_index(index)
# We do NOT call the super class here since we need to keep the assets
# in case the course is later restored.
# super(SplitMongoModuleStore, self).delete_course(course_key, user_id)
def revert_to_published(self, location, user_id=None):
"""
Reverts an item to its last published version (recursively traversing all of its descendants).
......@@ -1407,13 +1411,6 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
"""
raise NotImplementedError()
def get_errored_courses(self):
"""
This function doesn't make sense for the mongo modulestore, as structures
are loaded on demand, rather than up front
"""
return {}
def inherit_settings(self, block_map, block_json, inheriting_settings=None):
"""
Updates block_json with any inheritable setting set by an ancestor and recurses to children.
......
......@@ -85,25 +85,3 @@ def rewrite_nonportable_content_links(source_course_id, dest_course_id, text):
logging.warning("Error producing regex substitution %r for text = %r.\n\nError msg = %s", source_course_id, text, str(exc))
return text
def delete_course(modulestore, contentstore, course_key, commit=False):
"""
This method will actually do the work to delete all content in a course in a MongoDB backed
courseware store. BE VERY CAREFUL, this is not reversable.
"""
# check to see if the source course is actually there
if not modulestore.has_course(course_key):
raise Exception("Cannot find a course at {0}. Aborting".format(course_key))
if commit:
print "Deleting assets and thumbnails {}".format(course_key)
contentstore.delete_all_course_assets(course_key)
# finally delete the course
print "Deleting {0}...".format(course_key)
if commit:
modulestore.delete_course(course_key, '**replace-user**')
return True
......@@ -15,10 +15,9 @@ from django.core.management.base import CommandError
from django.test.utils import override_settings
from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
from xmodule.contentstore.django import contentstore
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore
from opaque_keys.edx.locations import SlashSeparatedCourseKey
from xmodule.modulestore.store_utilities import delete_course
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
import dashboard.git_import as git_import
from dashboard.git_import import GitImportError
......@@ -161,9 +160,7 @@ class TestGitAddCourse(ModuleStoreTestCase):
self.TEST_BRANCH)
# Delete to test branching back to master
delete_course(def_ms, contentstore(),
self.TEST_BRANCH_COURSE,
True)
def_ms.delete_course(self.TEST_BRANCH_COURSE, ModuleStoreEnum.UserID.test)
self.assertIsNone(def_ms.get_course(self.TEST_BRANCH_COURSE))
git_import.add_repo(self.TEST_REPO,
repo_dir / 'edx4edx_lite',
......
......@@ -38,10 +38,8 @@ from external_auth.models import ExternalAuthMap
from external_auth.views import generate_password
from student.models import CourseEnrollment, UserProfile, Registration
import track.views
from xmodule.contentstore.django import contentstore
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.store_utilities import delete_course
from xmodule.modulestore.xml import XMLModuleStore
from opaque_keys.edx.locations import SlashSeparatedCourseKey
......@@ -591,9 +589,7 @@ class Courses(SysadminDashboardView):
elif course_found and not is_xml_course:
# delete course that is stored with mongodb backend
content_store = contentstore()
commit = True
delete_course(self.def_ms, content_store, course.id, commit)
self.def_ms.delete_course(course.id, request.user.id)
# don't delete user permission groups, though
self.msg += \
u"<font color='red'>{0} {1} = {2} ({3})</font>".format(
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment