feat!: remove Studio editing for Old Mongo Courses

This removes user-facing Studio edit support for Old Mongo courses
(courses that have a CourseKey of the format {org}/{course}/{run}).
This does not affect our normal courses, which have CourseKeys
starting with "course-v1:".

After this commit:

* Old Mongo courses will continue to appear on the Studio course
  listing page, but are not clickable.
* Any attempt to directly access an Old Mongo course in Studio via URL
  fail with a 404 error.
* Course certificates will still be available for Old Mongo courses.
* Old Mongo courses will continue to be returned by CourseOverviews
  and get_course_summaries() calls.

We decided against removing Old Mongo courses from the listing entirely
because that would require very expensive CourseOverviews query to
filter them out. Making that query more efficient would involve a
database migration to add appropriate indexing, which is something else
that we are looking to avoid. CourseOverviews are used everywhere in
the system, so we want to avoid changing how they work so that we can
minimize risk.

This is part of the Old Mongo Modulestore deprecation effort:
  https://github.com/openedx/public-engineering/issues/62

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -1485,6 +1485,12 @@ class ContentStoreTest(ContentStoreTestCase):
         """Test viewing the course overview page with an existing course"""
         course = CourseFactory.create()
         resp = self._show_course_overview(course.id)
+
+        # course_handler raise 404 for old mongo course
+        if course.id.deprecated:
+            self.assertEqual(resp.status_code, 404)
+            return
+
         self.assertContains(
             resp,
             '<article class="outline outline-complex outline-course" data-locator="{locator}" data-course-key="{course_key}">'.format(  # lint-amnesty, pylint: disable=line-too-long
@@ -1550,6 +1556,12 @@ class ContentStoreTest(ContentStoreTestCase):
         course_key = course_items[0].id
 
         resp = self._show_course_overview(course_key)
+
+        # course_handler raise 404 for old mongo course
+        if course_key.deprecated:
+            self.assertEqual(resp.status_code, 404)
+            return
+
         self.assertEqual(resp.status_code, 200)
         self.assertContains(resp, 'Chapter 2')
 
@@ -1879,6 +1891,10 @@ class RerunCourseTest(ContentStoreTestCase):
         rerun_course_data.update(destination_course_data)
         destination_course_key = _get_course_id(self.store, destination_course_data)
 
+        # course_handler raise 404 for old mongo course
+        if destination_course_key.deprecated:
+            raise SkipTest('OldMongo Deprecation')
+
         # post the request
         course_url = get_url('course_handler', destination_course_key, 'course_key_string')
         response = self.client.ajax_post(course_url, rerun_course_data)
@@ -2198,6 +2214,10 @@ def _create_course(test, course_key, course_data):
     """
     Creates a course via an AJAX request and verifies the URL returned in the response.
     """
+    # course_handler raise 404 for old mongo course
+    if course_key.deprecated:
+        raise SkipTest('OldMongo Deprecation')
+
     course_url = get_url('course_handler', course_key, 'course_key_string')
     response = test.client.ajax_post(course_url, course_data)
     test.assertEqual(response.status_code, 200)

cms/djangoapps/contentstore/tests/test_course_listing.py

@@ -185,6 +185,7 @@ class TestCourseListing(ModuleStoreTestCase):
 
         # Fetch accessible courses list & verify their count
         courses_list_by_staff, __ = get_courses_accessible_to_user(self.request)
+
         self.assertEqual(len(list(courses_list_by_staff)), TOTAL_COURSES_COUNT)
 
         # Verify fetched accessible courses list is a list of CourseSummery instances
@@ -194,7 +195,7 @@ class TestCourseListing(ModuleStoreTestCase):
         with check_mongo_calls(mongo_calls):
             list(_accessible_courses_summary_iter(self.request))
 
-    @ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo)
+    @ddt.data(ModuleStoreEnum.Type.split)
     def test_get_course_list_with_invalid_course_location(self, store):
         """
         Test getting courses with invalid course location (course deleted from modulestore).

cms/djangoapps/contentstore/views/course.py

@@ -271,6 +271,11 @@ def course_handler(request, course_key_string=None):
         json: delete this branch from this course (leaving off /branch/draft would imply delete the course)
     """
     try:
+        if course_key_string:
+            course_key = CourseKey.from_string(course_key_string)
+            if course_key.deprecated:
+                logging.error(f"User {request.user.id} tried to access Studio for Old Mongo course {course_key}.")
+                return HttpResponseNotFound()
         response_format = request.GET.get('format') or request.POST.get('format') or 'html'
         if response_format == 'json' or 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'):
             if request.method == 'GET':
@@ -783,7 +788,7 @@ def _process_courses_list(courses_iter, in_process_course_actions, split_archive
         """
         Return a dict of the data which the view requires for each course
         """
-        return {
+        course_context = {
             'display_name': course.display_name,
             'course_key': str(course.location.course_key),
             'url': reverse_course_url('course_handler', course.id),
@@ -793,6 +798,13 @@ def _process_courses_list(courses_iter, in_process_course_actions, split_archive
             'number': course.display_number_with_default,
             'run': course.location.run
         }
+        if course.id.deprecated:
+            course_context.update({
+                'url': None,
+                'lms_link': None,
+                'rerun_link': None
+            })
+        return course_context
 
     in_process_action_course_keys = {uca.course_key for uca in in_process_course_actions}
     active_courses = []

cms/djangoapps/contentstore/views/tests/test_course_index.py

@@ -5,7 +5,7 @@ Unit tests for getting the list of courses and the course outline.
 
 import datetime
 import json
-from unittest import mock
+from unittest import SkipTest, mock, skip
 from unittest.mock import patch
 
 import ddt
@@ -58,7 +58,7 @@ class TestCourseIndex(CourseTestCase):
             display_name='dotted.course.name-2',
         )
 
-    def check_courses_on_index(self, authed_client):
+    def check_courses_on_index(self, authed_client, expected_course_tab_len):
         """
         Test that the React course listing is present.
         """
@@ -66,7 +66,7 @@ class TestCourseIndex(CourseTestCase):
         index_response = authed_client.get(index_url, {}, HTTP_ACCEPT='text/html')
         parsed_html = lxml.html.fromstring(index_response.content)
         courses_tab = parsed_html.find_class('react-course-listing')
-        self.assertEqual(len(courses_tab), 1)
+        self.assertEqual(len(courses_tab), expected_course_tab_len)
 
     def test_libraries_on_index(self):
         """
@@ -100,7 +100,7 @@ class TestCourseIndex(CourseTestCase):
         """
         Test that people with is_staff see the courses and can navigate into them
         """
-        self.check_courses_on_index(self.client)
+        self.check_courses_on_index(self.client, 1)
 
     def test_negative_conditions(self):
         """
@@ -110,7 +110,10 @@ class TestCourseIndex(CourseTestCase):
         # register a non-staff member and try to delete the course branch
         non_staff_client, _ = self.create_non_staff_authed_user_client()
         response = non_staff_client.delete(outline_url, {}, HTTP_ACCEPT='application/json')
-        self.assertEqual(response.status_code, 403)
+        if self.course.id.deprecated:
+            self.assertEqual(response.status_code, 404)
+        else:
+            self.assertEqual(response.status_code, 403)
 
     def test_course_staff_access(self):
         """
@@ -128,9 +131,10 @@ class TestCourseIndex(CourseTestCase):
             )
 
         # test access
-        self.check_courses_on_index(course_staff_client)
+        self.check_courses_on_index(course_staff_client, 1)
 
     def test_json_responses(self):
+
         outline_url = reverse_course_url('course_handler', self.course.id)
         chapter = ItemFactory.create(parent_location=self.course.location, category='chapter', display_name="Week 1")
         lesson = ItemFactory.create(parent_location=chapter.location, category='sequential', display_name="Lesson 1")
@@ -142,6 +146,11 @@ class TestCourseIndex(CourseTestCase):
         ItemFactory.create(parent_location=subsection.location, category="video", display_name="My Video")
 
         resp = self.client.get(outline_url, HTTP_ACCEPT='application/json')
+
+        if self.course.id.deprecated:
+            self.assertEqual(resp.status_code, 404)
+            return
+
         json_response = json.loads(resp.content.decode('utf-8'))
 
         # First spot check some values in the root response
@@ -307,6 +316,11 @@ class TestCourseIndex(CourseTestCase):
         course_outline_url = reverse_course_url('course_handler', updated_course.id)
         response = self.client.get_html(course_outline_url)
 
+        # course_handler raise 404 for old mongo course
+        if self.course.id.deprecated:
+            self.assertEqual(response.status_code, 404)
+            return
+
         # Assert that response code is 200.
         self.assertEqual(response.status_code, 200)
 
@@ -401,6 +415,7 @@ class TestCourseIndexArchived(CourseTestCase):
         archived_course_tab = parsed_html.find_class('archived-courses')
         self.assertEqual(len(archived_course_tab), 1 if separate_archived_courses else 0)
 
+    @skip('Skip test for old mongo course')
     @ddt.data(
         # Staff user has course staff access
         (True, 'staff', None, 0, 20),
@@ -469,6 +484,10 @@ class TestCourseOutline(CourseTestCase):
         outline_url = reverse_course_url('course_handler', self.course.id)
         outline_url = outline_url + '?format=concise' if is_concise else outline_url
         resp = self.client.get(outline_url, HTTP_ACCEPT='application/json')
+        if self.course.id.deprecated:
+            self.assertEqual(resp.status_code, 404)
+            return
+
         json_response = json.loads(resp.content.decode('utf-8'))
 
         # First spot check some values in the root response
@@ -613,6 +632,8 @@ class TestCourseOutline(CourseTestCase):
         """
         Test to check proctored exam settings mfe url is rendering properly
         """
+        if self.course.id.deprecated:
+            raise SkipTest("Skip test for old mongo course")
         mock_validate_proctoring_settings.return_value = [
             {
                 'key': 'proctoring_provider',

cms/djangoapps/contentstore/views/tests/test_exam_settings_view.py

@@ -1,7 +1,7 @@
 """
 Exam Settings View Tests
 """
-
+from unittest import SkipTest
 from unittest.mock import patch
 
 import ddt
@@ -104,6 +104,10 @@ class TestExamSettingsView(CourseTestCase, UrlResetMixin):
         self.course.enable_proctored_exams = True
         self.save_course()
 
+        # course_handler raise 404 for old mongo course
+        if self.course.id.deprecated:
+            raise SkipTest("course_handler raise 404 for old mongo course")
+
         url = reverse_course_url(page_handler, self.course.id)
         resp = self.client.get(url, HTTP_ACCEPT='text/html')
         alert_text = self._get_exam_settings_alert_text(resp.content)
@@ -139,6 +143,9 @@ class TestExamSettingsView(CourseTestCase, UrlResetMixin):
         self.course.enable_proctored_exams = True
         self.save_course()
 
+        # course_handler raise 404 for old mongo course
+        if self.course.id.deprecated and page_handler == 'course_handler':
+            raise SkipTest("course_handler raise 404 for old mongo course")
         url = reverse_course_url(page_handler, self.course.id)
         resp = self.client.get(url, HTTP_ACCEPT='text/html')
         alert_text = self._get_exam_settings_alert_text(resp.content)
@@ -163,6 +170,9 @@ class TestExamSettingsView(CourseTestCase, UrlResetMixin):
         """
         Alert should not be visible if no proctored exam setting error exists
         """
+        # course_handler raise 404 for old mongo course
+        if self.course.id.deprecated and page_handler == 'course_handler':
+            raise SkipTest("course_handler raise 404 for old mongo course")
         url = reverse_course_url(page_handler, self.course.id)
         resp = self.client.get(url, HTTP_ACCEPT='text/html')
         parsed_html = lxml.html.fromstring(resp.content)

cms/djangoapps/contentstore/views/tests/test_header_menu.py

@@ -1,7 +1,7 @@
 """
 Course Header Menu Tests.
 """
-
+from unittest import SkipTest
 
 from django.conf import settings
 from django.test.utils import override_settings
@@ -37,6 +37,9 @@ class TestHeaderMenu(CourseTestCase, UrlResetMixin):
         Tests course header menu should not have `Certificates` menu item
         if course has not web/HTML certificates enabled.
         """
+        # course_handler raise 404 for old mongo course
+        if self.course.id.deprecated:
+            raise SkipTest("course_handler raise 404 for old mongo course")
         self.course.cert_html_view_enabled = False
         self.save_course()
         outline_url = reverse_course_url('course_handler', self.course.id)
@@ -49,6 +52,9 @@ class TestHeaderMenu(CourseTestCase, UrlResetMixin):
         Tests course header menu should have `Certificates` menu item
         if course has web/HTML certificates enabled.
         """
+        # course_handler raise 404 for old mongo course
+        if self.course.id.deprecated:
+            raise SkipTest("course_handler raise 404 for old mongo course")
         outline_url = reverse_course_url('course_handler', self.course.id)
         resp = self.client.get(outline_url, HTTP_ACCEPT='text/html')
         self.assertEqual(resp.status_code, 200)
@@ -60,6 +66,9 @@ class TestHeaderMenu(CourseTestCase, UrlResetMixin):
         Tests course header menu should not have `Exam Settings` menu item
         if course does not have the Exam Settings view enabled.
         """
+        # course_handler raise 404 for old mongo course
+        if self.course.id.deprecated:
+            raise SkipTest("course_handler raise 404 for old mongo course")
         outline_url = reverse_course_url('course_handler', self.course.id)
         resp = self.client.get(outline_url, HTTP_ACCEPT='text/html')
         self.assertEqual(resp.status_code, 200)
@@ -71,6 +80,9 @@ class TestHeaderMenu(CourseTestCase, UrlResetMixin):
         Tests course header menu should have `Exam Settings` menu item
         if course does have Exam Settings view enabled.
         """
+        # course_handler raise 404 for old mongo course
+        if self.course.id.deprecated:
+            raise SkipTest("course_handler raise 404 for old mongo course")
         outline_url = reverse_course_url('course_handler', self.course.id)
         resp = self.client.get(outline_url, HTTP_ACCEPT='text/html')
         self.assertEqual(resp.status_code, 200)

cms/static/js/features_jsx/studio/CourseOrLibraryListing.jsx

@@ -10,34 +10,45 @@ export function CourseOrLibraryListing(props) {
   const linkClass = props.linkClass;
   const idBase = props.idBase;
 
+  const renderCourseMetadata = (item, i) => (
+    <div>
+      <h3 className="course-title" id={`title-${idBase}-${i}`}>{item.display_name}</h3>
+        <div className="course-metadata">
+          <span className="course-org metadata-item">
+            <span className="label">{gettext('Organization:')}</span>
+            <span className="value">{item.org}</span>
+          </span>
+          <span className="course-num metadata-item">
+            <span className="label">{gettext('Course Number:')}</span>
+            <span className="value">{item.number}</span>
+          </span>
+          { item.run &&
+            <span className="course-run metadata-item">
+              <span className="label">{gettext('Course Run:')}</span>
+              <span className="value">{item.run}</span>
+            </span>
+          }
+          { item.can_edit === false &&
+            <span className="extra-metadata">{gettext('(Read-only)')}</span>
+          }
+      </div>
+    </div>
+  );
+
   return (
     <ul className="list-courses">
       {
         props.items.map((item, i) =>
           (
             <li key={i} className="course-item" data-course-key={item.course_key}>
-              <a className={linkClass} href={item.url}>
-                <h3 className="course-title" id={`title-${idBase}-${i}`}>{item.display_name}</h3>
-                <div className="course-metadata">
-                  <span className="course-org metadata-item">
-                    <span className="label">{gettext('Organization:')}</span>
-                    <span className="value">{item.org}</span>
-                  </span>
-                  <span className="course-num metadata-item">
-                    <span className="label">{gettext('Course Number:')}</span>
-                    <span className="value">{item.number}</span>
-                  </span>
-                  { item.run &&
-                  <span className="course-run metadata-item">
-                    <span className="label">{gettext('Course Run:')}</span>
-                    <span className="value">{item.run}</span>
-                  </span>
-                  }
-                  { item.can_edit === false &&
-                  <span className="extra-metadata">{gettext('(Read-only)')}</span>
-                  }
-                </div>
-              </a>
+              {item.url
+                ? (
+                  <a className={linkClass} href={item.url}>
+                    {renderCourseMetadata(item, i)}
+                  </a>
+                )
+                : renderCourseMetadata(item, i)
+              }
               { item.lms_link && item.rerun_link &&
               <ul className="item-actions course-actions">
                 { allowReruns &&

lms/djangoapps/courseware/management/commands/tests/test_dump_course.py

@@ -107,7 +107,7 @@ class CommandsTestBase(SharedModuleStoreTestCase):
 
     def test_dump_course_ids(self):
         output = self.call_command('dump_course_ids')
-        dumped_courses = output.strip().split('\n')
+        dumped_courses = (output.strip() or []) and output.strip().split('\n')
         course_ids = {str(course_id) for course_id in self.loaded_courses}
         dumped_ids = set(dumped_courses)
         assert course_ids == dumped_ids

openedx/core/djangoapps/content/course_overviews/management/commands/tests/test_simulate_publish.py

@@ -30,12 +30,10 @@ class TestSimulatePublish(SharedModuleStoreTestCase):
         """
         super().setUpClass()
         cls.command = Command()
-        # org.0/course_0/Run_0
-        cls.course_key_1 = CourseFactory.create(default_store=ModuleStoreEnum.Type.mongo).id
         # course-v1:org.1+course_1+Run_1
-        cls.course_key_2 = CourseFactory.create(default_store=ModuleStoreEnum.Type.split).id
+        cls.course_key_1 = CourseFactory.create(default_store=ModuleStoreEnum.Type.split).id
         # course-v1:org.2+course_2+Run_2
-        cls.course_key_3 = CourseFactory.create(default_store=ModuleStoreEnum.Type.split).id
+        cls.course_key_2 = CourseFactory.create(default_store=ModuleStoreEnum.Type.split).id
 
     def setUp(self):
         """
@@ -108,12 +106,11 @@ class TestSimulatePublish(SharedModuleStoreTestCase):
         """Test sending only to specific courses."""
         self.command.handle(
             **self.options(
-                courses=[str(self.course_key_1), str(self.course_key_2)]
+                courses=[str(self.course_key_1)]
             )
         )
         assert self.course_key_1 in self.received_1
-        assert self.course_key_2 in self.received_1
-        assert self.course_key_3 not in self.received_1
+        assert self.course_key_2 not in self.received_1
         assert self.received_1 == self.received_2
 
     def test_specific_receivers(self):
@@ -125,7 +122,6 @@ class TestSimulatePublish(SharedModuleStoreTestCase):
         )
         assert self.course_key_1 in self.received_1
         assert self.course_key_2 in self.received_1
-        assert self.course_key_3 in self.received_1
         assert not self.received_2
 
     def test_course_overviews(self):
@@ -139,7 +135,7 @@ class TestSimulatePublish(SharedModuleStoreTestCase):
                 ]
             )
         )
-        assert CourseOverview.objects.all().count() == 3
+        assert CourseOverview.objects.all().count() == 2
         assert not self.received_1
         assert not self.received_2
 

xmodule/modulestore/tests/test_semantics.py

@@ -446,9 +446,17 @@ class TestSplitDirectOnlyCategorySemantics(DirectOnlyCategorySemantics):
                                    self.ASIDE_DATA_FIELD.field_name, self.ASIDE_DATA_FIELD.updated)
 
 
+@ddt.ddt
 class TestMongoDirectOnlyCategorySemantics(DirectOnlyCategorySemantics):
     """
     Verify DIRECT_ONLY_CATEGORY semantics against the MongoModulestore
     """
     MODULESTORE = TEST_DATA_MONGO_MODULESTORE
     __test__ = True
+
+    @ddt.data(ModuleStoreEnum.Branch.draft_preferred, ModuleStoreEnum.Branch.published_only)
+    def test_course_summaries(self, branch):
+        """ Test that `get_course_summaries` method in modulestore work as expected. """
+        with self.store.branch_setting(branch_setting=branch):
+            course_summaries = self.store.get_course_summaries()
+            assert len(course_summaries) == 1