navin/fal 4077/backport/follow up

cms/djangoapps/contentstore/models.py

@@ -106,6 +106,34 @@ class EntityLinkBase(models.Model):
     class Meta:
         abstract = True
 
+
+class ComponentLink(EntityLinkBase):
+    """
+    This represents link between any two publishable entities or link between publishable entity and a course
+    XBlock. It helps in tracking relationship between XBlocks imported from libraries and used in different courses.
+    """
+    upstream_block = models.ForeignKey(
+        Component,
+        on_delete=models.SET_NULL,
+        related_name="links",
+        null=True,
+        blank=True,
+    )
+    upstream_usage_key = UsageKeyField(
+        max_length=255,
+        help_text=_(
+            "Upstream block usage key, this value cannot be null"
+            " and useful to track upstream library blocks that do not exist yet"
+        )
+    )
+
+    class Meta:
+        verbose_name = _("Component Link")
+        verbose_name_plural = _("Component Links")
+
+    def __str__(self):
+        return f"ComponentLink<{self.upstream_usage_key}->{self.downstream_usage_key}>"
+
     @property
     def upstream_version_num(self) -> int | None:
         """
@@ -177,34 +205,6 @@ def summarize_by_downstream_context(cls, downstream_context_key: CourseKey) -> Q
         )
         return result
 
-
-class ComponentLink(EntityLinkBase):
-    """
-    This represents link between any two publishable entities or link between publishable entity and a course
-    XBlock. It helps in tracking relationship between XBlocks imported from libraries and used in different courses.
-    """
-    upstream_block = models.ForeignKey(
-        Component,
-        on_delete=models.SET_NULL,
-        related_name="links",
-        null=True,
-        blank=True,
-    )
-    upstream_usage_key = UsageKeyField(
-        max_length=255,
-        help_text=_(
-            "Upstream block usage key, this value cannot be null"
-            " and useful to track upstream library blocks that do not exist yet"
-        )
-    )
-
-    class Meta:
-        verbose_name = _("Component Link")
-        verbose_name_plural = _("Component Links")
-
-    def __str__(self):
-        return f"ComponentLink<{self.upstream_usage_key}->{self.downstream_usage_key}>"
-
     @classmethod
     def update_or_create(
         cls,
@@ -232,25 +232,15 @@ def update_or_create(
             'version_declined': version_declined,
         }
         if upstream_block:
-            new_values.update(
-                {
-                    'upstream_block': upstream_block,
-                }
-            )
+            new_values['upstream_block'] = upstream_block
         try:
             link = cls.objects.get(downstream_usage_key=downstream_usage_key)
-            # TODO: until we save modified datetime for course xblocks in index, the modified time for links are updated
-            # everytime a downstream/course block is updated. This allows us to order links[1] based on recently
-            # modified downstream version.
-            # pylint: disable=line-too-long
-            # 1. https://github.com/open-craft/frontend-app-course-authoring/blob/0443d88824095f6f65a3a64b77244af590d4edff/src/course-libraries/ReviewTabContent.tsx#L222-L233
-            has_changes = True  # change to false once above condition is met.
-            for key, value in new_values.items():
-                prev = getattr(link, key)
-                # None != None is True, so we need to check for it specially
-                if prev != value and ~(prev is None and value is None):
+            has_changes = False
+            for key, new_value in new_values.items():
+                prev_value = getattr(link, key)
+                if prev_value != new_value:
                     has_changes = True
-                    setattr(link, key, value)
+                    setattr(link, key, new_value)
             if has_changes:
                 link.updated = created
                 link.save()
@@ -290,6 +280,77 @@ class Meta:
     def __str__(self):
         return f"ContainerLink<{self.upstream_container_key}->{self.downstream_usage_key}>"
 
+    @property
+    def upstream_version_num(self) -> int | None:
+        """
+        Returns upstream container version number if available.
+        """
+        published_version = get_published_version(self.upstream_container.publishable_entity.id)
+        return published_version.version_num if published_version else None
+
+    @property
+    def upstream_context_title(self) -> str:
+        """
+        Returns upstream context title.
+        """
+        return self.upstream_container.publishable_entity.learning_package.title
+
+    @classmethod
+    def filter_links(
+        cls,
+        **link_filter,
+    ) -> QuerySet["EntityLinkBase"]:
+        """
+        Get all links along with sync flag, upstream context title and version, with optional filtering.
+        """
+        ready_to_sync = link_filter.pop('ready_to_sync', None)
+        result = cls.objects.filter(**link_filter).select_related(
+            "upstream_container__publishable_entity__published__version",
+            "upstream_container__publishable_entity__learning_package"
+        ).annotate(
+            ready_to_sync=(
+                GreaterThan(
+                    Coalesce("upstream_container__publishable_entity__published__version__version_num", 0),
+                    Coalesce("version_synced", 0)
+                ) & GreaterThan(
+                    Coalesce("upstream_container__publishable_entity__published__version__version_num", 0),
+                    Coalesce("version_declined", 0)
+                )
+            )
+        )
+        if ready_to_sync is not None:
+            result = result.filter(ready_to_sync=ready_to_sync)
+        return result
+
+    @classmethod
+    def summarize_by_downstream_context(cls, downstream_context_key: CourseKey) -> QuerySet:
+        """
+        Returns a summary of links by upstream context for given downstream_context_key.
+        Example:
+        [
+            {
+                "upstream_context_title": "CS problems 3",
+                "upstream_context_key": "lib:OpenedX:CSPROB3",
+                "ready_to_sync_count": 11,
+                "total_count": 14
+            },
+            {
+                "upstream_context_title": "CS problems 2",
+                "upstream_context_key": "lib:OpenedX:CSPROB2",
+                "ready_to_sync_count": 15,
+                "total_count": 24
+            },
+        ]
+        """
+        result = cls.filter_links(downstream_context_key=downstream_context_key).values(
+            "upstream_context_key",
+            upstream_context_title=F("upstream_container__publishable_entity__learning_package__title"),
+        ).annotate(
+            ready_to_sync_count=Count("id", Q(ready_to_sync=True)),
+            total_count=Count('id')
+        )
+        return result
+
     @classmethod
     def update_or_create(
         cls,
@@ -317,25 +378,15 @@ def update_or_create(
             'version_declined': version_declined,
         }
         if upstream_container:
-            new_values.update(
-                {
-                    'upstream_container': upstream_container,
-                }
-            )
+            new_values['upstream_container'] = upstream_container
         try:
             link = cls.objects.get(downstream_usage_key=downstream_usage_key)
-            # TODO: until we save modified datetime for course xblocks in index, the modified time for links are updated
-            # everytime a downstream/course block is updated. This allows us to order links[1] based on recently
-            # modified downstream version.
-            # pylint: disable=line-too-long
-            # 1. https://github.com/open-craft/frontend-app-course-authoring/blob/0443d88824095f6f65a3a64b77244af590d4edff/src/course-libraries/ReviewTabContent.tsx#L222-L233
-            has_changes = True  # change to false once above condition is met.
-            for key, value in new_values.items():
-                prev = getattr(link, key)
-                # None != None is True, so we need to check for it specially
-                if prev != value and ~(prev is None and value is None):
+            has_changes = False
+            for key, new_value in new_values.items():
+                prev_value = getattr(link, key)
+                if prev_value != new_value:
                     has_changes = True
-                    setattr(link, key, value)
+                    setattr(link, key, new_value)
             if has_changes:
                 link.updated = created
                 link.save()

cms/djangoapps/contentstore/utils.py

@@ -2429,8 +2429,5 @@ def create_or_update_xblock_upstream_link(xblock, course_key: CourseKey, created
         _create_or_update_component_link(course_key, created, xblock)
     except InvalidKeyError:
         # It is possible that the upstream is a container and UsageKeyV2 parse failed
-        # Create upstream container link
-        try:
-            _create_or_update_container_link(course_key, created, xblock)
-        except InvalidKeyError:
-            log.error(f"Invalid key: {xblock.upstream}")
+        # Create upstream container link and raise InvalidKeyError if xblock.upstream is a valid key.
+        _create_or_update_container_link(course_key, created, xblock)

cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py

@@ -548,8 +548,7 @@ def sync_library_content(downstream: XBlock, request, store) -> StaticFileNotice
             notices = []
             # Store final children keys to update order of components in unit
             children = []
-            for i in range(len(upstream_children)):
-                upstream_child = upstream_children[i]
+            for i, upstream_child in enumerate(upstream_children):
                 assert isinstance(upstream_child, LibraryXBlockMetadata)  # for now we only support units
                 if upstream_child.usage_key not in downstream_children_keys:
                     # This upstream_child is new, create it.
@@ -574,6 +573,7 @@ def sync_library_content(downstream: XBlock, request, store) -> StaticFileNotice
             for child in downstream_children:
                 if child.usage_key not in children:
                     # This downstream block was added, or deleted from upstream block.
+                    # NOTE: This will also delete any local additions to a unit in the next upstream sync.
                     store.delete_item(child.usage_key, user_id=request.user.id)
             downstream.children = children
             store.update_item(downstream, request.user.id)

cms/static/sass/course-unit-mfe-iframe-bundle.scss

@@ -8,7 +8,7 @@
 
 html {
   body {
-    min-width: 800px;
+    min-width: 560px;
     background: transparent;
     &.openassessment_full_height.view-container {
         overflow-y: hidden;
@@ -39,11 +39,19 @@ body,
       padding: ($baseline * 1.2) ($baseline * 1.2) ($baseline / 1.67);
       border-bottom: none;
 
-      .header-details .xblock-display-name {
-        font-size: 22px;
-        line-height: 28px;
-        font-weight: 700;
-        color: $black;
+      .header-details {
+        .xblock-display-title {
+          white-space: nowrap;
+          overflow: hidden;
+          text-overflow: ellipsis;
+        }
+
+        .xblock-display-name {
+          font-size: 22px;
+          line-height: 28px;
+          font-weight: 700;
+          color: $black;
+        }
       }
     }
 
@@ -345,7 +353,6 @@ body,
     }
 
     .tip.setting-help {
-      color: $border-color;
       font-size: 14px;
       line-height: $base-font-size;
     }
@@ -452,6 +459,11 @@ body,
 
   .modal-lg.modal-window.confirm.openassessment_modal_window {
     height: 635px;
+    max-height: 100vh;
+
+    .edit-xblock-modal .modal-content {
+      max-height: 100%;
+    }
   }
 
   // Additions for the xblock editor on the Library Authoring
@@ -672,11 +684,21 @@ body [class*="view-"] .openassessment_editor_buttons.xblock-actions {
     max-width: 1200px;
   }
 
-  .modal-lg.modal-editor .modal-header .editor-modes .action-item {
-    .editor-button,
-    .settings-button {
-      @extend %light-button;
-    }
+  .modal-lg.modal-editor {
+      .modal-header .editor-modes .action-item {
+        .editor-button,
+        .settings-button {
+          @extend %light-button;
+        }
+      }
+
+      .edit-xblock-modal .modal-content {
+        max-height: calc(100vh - 144px);
+
+        .editor-with-buttons.wrapper-comp-settings .list-input.settings-list {
+          max-height: calc(100vh - 205px);
+        }
+      }
   }
 
   .wrapper.wrapper-modal-window .modal-window .modal-actions .action-primary {

openedx/core/djangoapps/content/search/documents.py

@@ -212,6 +212,8 @@ class implementation returns only:
         Fields.access_id: _meili_access_id_from_context_key(block.usage_key.context_key),
         Fields.breadcrumbs: [],
     }
+    if hasattr(block, "edited_on"):
+        block_data[Fields.modified] = block.edited_on.timestamp()
     # Get the breadcrumbs (course, section, subsection, etc.):
     if block.usage_key.context_key.is_course:  # Getting parent is not yet implemented in Learning Core (for libraries).
         cur_block = block

openedx/core/djangoapps/content/search/tests/test_api.py

@@ -61,19 +61,27 @@ def setUp(self):
         # Clear the Meilisearch client to avoid side effects from other tests
         api.clear_meilisearch_client()
 
+        modified_date = datetime(2024, 5, 6, 7, 8, 9, tzinfo=timezone.utc)
         # Create course
-        self.course = self.store.create_course(
-            "org1",
-            "test_course",
-            "test_run",
-            self.user_id,
-            fields={"display_name": "Test Course"},
-        )
-        course_access, _ = SearchAccess.objects.get_or_create(context_key=self.course.id)
-        self.course_block_key = "block-v1:org1+test_course+test_run+type@course+block@course"
-
-        # Create XBlocks
-        self.sequential = self.store.create_child(self.user_id, self.course.location, "sequential", "test_sequential")
+        with freeze_time(modified_date):
+            self.course = self.store.create_course(
+                "org1",
+                "test_course",
+                "test_run",
+                self.user_id,
+                fields={"display_name": "Test Course"},
+            )
+            course_access, _ = SearchAccess.objects.get_or_create(context_key=self.course.id)
+            self.course_block_key = "block-v1:org1+test_course+test_run+type@course+block@course"
+
+            # Create XBlocks
+            self.sequential = self.store.create_child(
+                self.user_id,
+                self.course.location,
+                "sequential",
+                "test_sequential"
+            )
+            self.store.create_child(self.user_id, self.sequential.location, "vertical", "test_vertical")
         self.doc_sequential = {
             "id": "block-v1org1test_coursetest_runtypesequentialblocktest_sequential-f702c144",
             "type": "course_block",
@@ -90,8 +98,8 @@ def setUp(self):
             ],
             "content": {},
             "access_id": course_access.id,
+            "modified": modified_date.timestamp(),
         }
-        self.store.create_child(self.user_id, self.sequential.location, "vertical", "test_vertical")
         self.doc_vertical = {
             "id": "block-v1org1test_coursetest_runtypeverticalblocktest_vertical-e76a10a4",
             "type": "course_block",
@@ -112,6 +120,7 @@ def setUp(self):
             ],
             "content": {},
             "access_id": course_access.id,
+            "modified": modified_date.timestamp(),
         }
         # Make sure the CourseOverview for the course is created:
         CourseOverview.get_from_id(self.course.id)
@@ -130,7 +139,6 @@ def setUp(self):
             self.problem1 = library_api.create_library_block(self.library.key, "problem", "p1")
             self.problem2 = library_api.create_library_block(self.library.key, "problem", "p2")
         # Update problem1, freezing the date so we can verify modified date serializes correctly.
-        modified_date = datetime(2024, 5, 6, 7, 8, 9, tzinfo=timezone.utc)
         with freeze_time(modified_date):
             library_api.set_library_block_olx(self.problem1.usage_key, "<problem />")