Merge branch 'master' into chris/FAL-4266-add-title-to-course-details

Author: ChrisChV

cms/djangoapps/contentstore/api/tests/test_validation.py

@@ -2,20 +2,25 @@
 Tests for the course import API views
 """
 
-
+import factory
 from datetime import datetime
+from django.conf import settings
 
+import ddt
 from django.test.utils import override_settings
 from django.urls import reverse
 from rest_framework import status
 from rest_framework.test import APITestCase
 from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory
 
+from common.djangoapps.course_modes.models import CourseMode
+from common.djangoapps.course_modes.tests.factories import CourseModeFactory
 from common.djangoapps.student.tests.factories import StaffFactory
 from common.djangoapps.student.tests.factories import UserFactory
 
 
+@ddt.ddt
 @override_settings(PROCTORING_BACKENDS={'DEFAULT': 'proctortrack', 'proctortrack': {}})
 class CourseValidationViewTest(SharedModuleStoreTestCase, APITestCase):
     """
@@ -82,39 +87,54 @@ def test_student_fails(self):
         resp = self.client.get(self.get_url(self.course_key))
         self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
 
-    def test_staff_succeeds(self):
-        self.client.login(username=self.staff.username, password=self.password)
-        resp = self.client.get(self.get_url(self.course_key), {'all': 'true'})
-        self.assertEqual(resp.status_code, status.HTTP_200_OK)
-        expected_data = {
-            'assignments': {
-                'total_number': 1,
-                'total_visible': 1,
-                'assignments_with_dates_before_start': [],
-                'assignments_with_dates_after_end': [],
-                'assignments_with_ora_dates_after_end': [],
-                'assignments_with_ora_dates_before_start': [],
-            },
-            'dates': {
-                'has_start_date': True,
-                'has_end_date': False,
-            },
-            'updates': {
-                'has_update': True,
-            },
-            'certificates': {
-                'is_enabled': False,
-                'is_activated': False,
-                'has_certificate': False,
-            },
-            'grades': {
-                'has_grading_policy': False,
-                'sum_of_weights': 1.0,
-            },
-            'proctoring': {
-                'needs_proctoring_escalation_email': True,
-                'has_proctoring_escalation_email': True,
-            },
-            'is_self_paced': True,
-        }
-        self.assertDictEqual(resp.data, expected_data)
+    @ddt.data(
+        (False, False),
+        (True, False),
+        (False, True),
+        (True, True),
+    )
+    @ddt.unpack
+    def test_staff_succeeds(self, certs_html_view, with_modes):
+        features = dict(settings.FEATURES, CERTIFICATES_HTML_VIEW=certs_html_view)
+        with override_settings(FEATURES=features):
+            if with_modes:
+                CourseModeFactory.create_batch(
+                    2,
+                    course_id=self.course.id,
+                    mode_slug=factory.Iterator([CourseMode.AUDIT, CourseMode.VERIFIED]),
+                )
+            self.client.login(username=self.staff.username, password=self.password)
+            resp = self.client.get(self.get_url(self.course_key), {'all': 'true'})
+            self.assertEqual(resp.status_code, status.HTTP_200_OK)
+            expected_data = {
+                'assignments': {
+                    'total_number': 1,
+                    'total_visible': 1,
+                    'assignments_with_dates_before_start': [],
+                    'assignments_with_dates_after_end': [],
+                    'assignments_with_ora_dates_after_end': [],
+                    'assignments_with_ora_dates_before_start': [],
+                },
+                'dates': {
+                    'has_start_date': True,
+                    'has_end_date': False,
+                },
+                'updates': {
+                    'has_update': True,
+                },
+                'certificates': {
+                    'is_enabled': with_modes,
+                    'is_activated': False,
+                    'has_certificate': False,
+                },
+                'grades': {
+                    'has_grading_policy': False,
+                    'sum_of_weights': 1.0,
+                },
+                'proctoring': {
+                    'needs_proctoring_escalation_email': True,
+                    'has_proctoring_escalation_email': True,
+                },
+                'is_self_paced': True,
+            }
+            self.assertDictEqual(resp.data, expected_data)

cms/djangoapps/contentstore/models.py

@@ -7,12 +7,12 @@
 
 from config_models.models import ConfigurationModel
 from django.db import models
-from django.db.models import QuerySet, OuterRef, Case, When, Exists, Value, ExpressionWrapper
-from django.db.models.fields import IntegerField, TextField, BooleanField
+from django.db.models import Case, Exists, ExpressionWrapper, OuterRef, Q, QuerySet, Value, When
+from django.db.models.fields import BooleanField, IntegerField, TextField
 from django.db.models.functions import Coalesce
 from django.db.models.lookups import GreaterThan
 from django.utils.translation import gettext_lazy as _
-from opaque_keys.edx.django.models import CourseKeyField, ContainerKeyField, UsageKeyField
+from opaque_keys.edx.django.models import ContainerKeyField, CourseKeyField, UsageKeyField
 from opaque_keys.edx.keys import CourseKey, UsageKey
 from opaque_keys.edx.locator import LibraryContainerLocator
 from openedx_learning.api.authoring import get_published_version
@@ -23,7 +23,6 @@
     manual_date_time_field,
 )
 
-
 logger = logging.getLogger(__name__)
 
 
@@ -391,7 +390,7 @@ def filter_links(
             cls.objects.filter(**link_filter).select_related(*RELATED_FIELDS),
         )
         if ready_to_sync is not None:
-            result = result.filter(ready_to_sync=ready_to_sync)
+            result = result.filter(Q(ready_to_sync=ready_to_sync) | Q(ready_to_sync_from_children=ready_to_sync))
 
         # Handle top-level parents logic
         if use_top_level_parents:
@@ -436,6 +435,11 @@ def _annotate_query_with_ready_to_sync(cls, query_set: QuerySet["EntityLinkBase"
                     ),
                     then=1
                 ),
+                # If upstream block was deleted, set ready_to_sync = True
+                When(
+                    Q(upstream_container__publishable_entity__published__version__version_num__isnull=True),
+                    then=1
+                ),
                 default=0,
                 output_field=models.IntegerField()
             )
@@ -457,6 +461,11 @@ def _annotate_query_with_ready_to_sync(cls, query_set: QuerySet["EntityLinkBase"
                     ),
                     then=1
                 ),
+                # If upstream block was deleted, set ready_to_sync = True
+                When(
+                    Q(upstream_block__publishable_entity__published__version__version_num__isnull=True),
+                    then=1
+                ),
                 default=0,
                 output_field=models.IntegerField()
             )

cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py

@@ -23,7 +23,7 @@
 from common.djangoapps.student.tests.factories import UserFactory
 from openedx.core.djangoapps.content_libraries import api as lib_api
 from xmodule.modulestore.django import modulestore
-from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase, ImmediateOnCommitMixin
+from xmodule.modulestore.tests.django_utils import ImmediateOnCommitMixin, SharedModuleStoreTestCase
 from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory
 
 from .. import downstreams as downstreams_views
@@ -32,6 +32,7 @@
 URL_PREFIX = '/api/libraries/v2/'
 URL_LIB_CREATE = URL_PREFIX
 URL_LIB_BLOCKS = URL_PREFIX + '{lib_key}/blocks/'
+URL_LIB_BLOCK = URL_PREFIX + 'blocks/{block_key}/'
 URL_LIB_BLOCK_PUBLISH = URL_PREFIX + 'blocks/{block_key}/publish/'
 URL_LIB_BLOCK_OLX = URL_PREFIX + 'blocks/{block_key}/olx/'
 URL_LIB_CONTAINER = URL_PREFIX + 'containers/{container_key}/'  # Get a container in this library
@@ -277,6 +278,10 @@ def _create_container(self, lib_key, container_type, slug: str | None, display_n
             data["slug"] = slug
         return self._api('post', URL_LIB_CONTAINERS.format(lib_key=lib_key), data, expect_response)
 
+    def _delete_component(self, block_key, expect_response=200):
+        """ Publish all changes in the specified container + children """
+        return self._api('delete', URL_LIB_BLOCK.format(block_key=block_key), None, expect_response)
+
 
 class SharedErrorTestCases(_BaseDownstreamViewTestMixin):
     """
@@ -1503,3 +1508,109 @@ def test_200_summary(self):
             'last_published_at': self.now.strftime('%Y-%m-%dT%H:%M:%S.%fZ'),
         }]
         self.assertListEqual(data, expected)
+
+
+class GetDownstreamDeletedUpstream(
+    _BaseDownstreamViewTestMixin,
+    ImmediateOnCommitMixin,
+    SharedModuleStoreTestCase,
+):
+    """
+    Test that parent container is marked ready_to_sync when even when the only change is a deleted component under it
+    """
+    def call_api(
+        self,
+        course_id: str | None = None,
+        ready_to_sync: bool | None = None,
+        upstream_key: str | None = None,
+        item_type: str | None = None,
+        use_top_level_parents: bool | None = None,
+    ):
+        data = {}
+        if course_id is not None:
+            data["course_id"] = str(course_id)
+        if ready_to_sync is not None:
+            data["ready_to_sync"] = str(ready_to_sync)
+        if upstream_key is not None:
+            data["upstream_key"] = str(upstream_key)
+        if item_type is not None:
+            data["item_type"] = str(item_type)
+        if use_top_level_parents is not None:
+            data["use_top_level_parents"] = str(use_top_level_parents)
+        return self.client.get("/api/contentstore/v2/downstreams/", data=data)
+
+    def test_delete_component_should_be_ready_to_sync(self):
+        """
+        Test deleting a component from library should mark the entire section container ready to sync
+        """
+        # Create blocks
+        section_id = self._create_container(self.library_id, "section", "section-12", "Section 12")["id"]
+        subsection_id = self._create_container(self.library_id, "subsection", "subsection-12", "Subsection 12")["id"]
+        unit_id = self._create_container(self.library_id, "unit", "unit-12", "Unit 12")["id"]
+        video_id = self._add_block_to_library(self.library_id, "video", "video-bar-13")["id"]
+        section_key = ContainerKey.from_string(section_id)
+        subsection_key = ContainerKey.from_string(subsection_id)
+        unit_key = ContainerKey.from_string(unit_id)
+        video_key = LibraryUsageLocatorV2.from_string(video_id)
+
+        # Set children
+        lib_api.update_container_children(section_key, [subsection_key], None)
+        lib_api.update_container_children(subsection_key, [unit_key], None)
+        lib_api.update_container_children(unit_key, [video_key], None)
+        self._publish_container(unit_id)
+        self._publish_container(subsection_id)
+        self._publish_container(section_id)
+        self._publish_library_block(video_id)
+        course = CourseFactory.create(display_name="Course New")
+        add_users(self.superuser, CourseStaffRole(course.id), self.course_user)
+        chapter = BlockFactory.create(
+            category='chapter', parent=course, upstream=section_id, upstream_version=2,
+        )
+        sequential = BlockFactory.create(
+            category='sequential',
+            parent=chapter,
+            upstream=subsection_id,
+            upstream_version=2,
+            top_level_downstream_parent_key=get_block_key_string(chapter.usage_key),
+        )
+        vertical = BlockFactory.create(
+            category='vertical',
+            parent=sequential,
+            upstream=unit_id,
+            upstream_version=2,
+            top_level_downstream_parent_key=get_block_key_string(chapter.usage_key),
+        )
+        BlockFactory.create(
+            category='video',
+            parent=vertical,
+            upstream=video_id,
+            upstream_version=1,
+            top_level_downstream_parent_key=get_block_key_string(chapter.usage_key),
+        )
+        self._delete_component(video_id)
+        self._publish_container(unit_id)
+        response = self.call_api(course_id=course.id, ready_to_sync=True, use_top_level_parents=True)
+        assert response.status_code == 200
+        data = response.json()['results']
+        assert len(data) == 1
+        date_format = self.now.isoformat().split("+")[0] + 'Z'
+        expected_results = {
+            'created': date_format,
+            'downstream_context_key': str(course.id),
+            'downstream_usage_key': str(chapter.usage_key),
+            'downstream_customized': [],
+            'id': 8,
+            'ready_to_sync': False,
+            'ready_to_sync_from_children': True,
+            'top_level_parent_usage_key': None,
+            'updated': date_format,
+            'upstream_context_key': self.library_id,
+            'upstream_context_title': self.library_title,
+            'upstream_key': section_id,
+            'upstream_type': 'container',
+            'upstream_version': 2,
+            'version_declined': None,
+            'version_synced': 2,
+        }
+
+        self.assertDictEqual(data[0], expected_results)

cms/djangoapps/contentstore/views/certificate_manager.py

@@ -121,7 +121,7 @@ def is_activated(course):
         along with the certificates.
         """
         is_active = False
-        certificates = None
+        certificates = []
         if settings.FEATURES.get('CERTIFICATES_HTML_VIEW', False):
             certificates = CertificateManager.get_certificates(course)
             # we are assuming only one certificate in certificates collection.