Merge branch 'master' into jci/issue35245

jciasenza · web-flow · commit 34b532ed863e · 2025-01-27T12:42:12.000-03:00
diff --git a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py
@@ -555,7 +555,6 @@ def _copy_paste_and_assert_link(key_to_copy):
             assert new_block.upstream == str(self.lib_block_key)
             assert new_block.upstream_version == 3
             assert new_block.upstream_display_name == "MCQ-draft"
-            assert new_block.upstream_max_attempts == 5
             return new_block_key
 
         # first verify link for copied block from library
diff --git a/cms/lib/xblock/test/test_upstream_sync.py b/cms/lib/xblock/test/test_upstream_sync.py
@@ -1,7 +1,9 @@
 """
 Test CMS's upstream->downstream syncing system
 """
+import datetime
 import ddt
+from pytz import utc
 
 from organizations.api import ensure_organization
 from organizations.models import Organization
@@ -42,13 +44,33 @@ def setUp(self):
             title="Test Upstream Library",
         )
         self.upstream_key = libs.create_library_block(self.library.key, "html", "test-upstream").usage_key
-        libs.create_library_block(self.library.key, "video", "video-upstream")
 
         upstream = xblock.load_block(self.upstream_key, self.user)
         upstream.display_name = "Upstream Title V2"
         upstream.data = "<html><body>Upstream content V2</body></html>"
         upstream.save()
 
+        self.upstream_problem_key = libs.create_library_block(self.library.key, "problem", "problem-upstream").usage_key
+        libs.set_library_block_olx(self.upstream_problem_key, (
+            '<problem'
+            ' attempts_before_showanswer_button="1"'
+            ' display_name="Upstream Problem Title V2"'
+            ' due="2024-01-01T00:00:00Z"'
+            ' force_save_button="false"'
+            ' graceperiod="1d"'
+            ' grading_method="last_attempt"'
+            ' matlab_api_key="abc"'
+            ' max_attempts="10"'
+            ' rerandomize="&quot;always&quot;"'
+            ' show_correctness="never"'
+            ' show_reset_button="false"'
+            ' showanswer="on_correct"'
+            ' submission_wait_seconds="10"'
+            ' use_latex_compiler="false"'
+            ' weight="1"'
+            '/>\n'
+        ))
+
         libs.publish_changes(self.library.key, self.user.id)
 
         self.taxonomy_all_org = tagging_api.create_taxonomy(
@@ -179,6 +201,100 @@ def test_sync_updates_happy_path(self):
         for object_tag in object_tags:
             assert object_tag.value in new_upstream_tags
 
+    # pylint: disable=too-many-statements
+    def test_sync_updates_to_downstream_only_fields(self):
+        """
+        If we sync to modified content, will it preserve downstream-only fields, and overwrite the rest?
+        """
+        downstream = BlockFactory.create(category='problem', parent=self.unit, upstream=str(self.upstream_problem_key))
+
+        # Initial sync
+        sync_from_upstream(downstream, self.user)
+
+        # These fields are copied from upstream
+        assert downstream.upstream_display_name == "Upstream Problem Title V2"
+        assert downstream.display_name == "Upstream Problem Title V2"
+        assert downstream.rerandomize == '"always"'
+        assert downstream.matlab_api_key == 'abc'
+        assert not downstream.use_latex_compiler
+
+        # These fields are "downstream only", so field defaults are preserved, and values are NOT copied from upstream
+        assert downstream.attempts_before_showanswer_button == 0
+        assert downstream.due is None
+        assert not downstream.force_save_button
+        assert downstream.graceperiod is None
+        assert downstream.grading_method == 'last_score'
+        assert downstream.max_attempts is None
+        assert downstream.show_correctness == 'always'
+        assert not downstream.show_reset_button
+        assert downstream.showanswer == 'finished'
+        assert downstream.submission_wait_seconds == 0
+        assert downstream.weight is None
+
+        # Upstream updates
+        libs.set_library_block_olx(self.upstream_problem_key, (
+            '<problem'
+            ' attempts_before_showanswer_button="10"'
+            ' display_name="Upstream Problem Title V3"'
+            ' due="2024-02-02T00:00:00Z"'
+            ' force_save_button="false"'
+            ' graceperiod=""'
+            ' grading_method="final_attempt"'
+            ' matlab_api_key="def"'
+            ' max_attempts="11"'
+            ' rerandomize="&quot;per_student&quot;"'
+            ' show_correctness="past_due"'
+            ' show_reset_button="false"'
+            ' showanswer="attempted"'
+            ' submission_wait_seconds="11"'
+            ' use_latex_compiler="true"'
+            ' weight="2"'
+            '/>\n'
+        ))
+        libs.publish_changes(self.library.key, self.user.id)
+
+        # Modifing downstream-only fields are "safe" customizations
+        downstream.display_name = "Downstream Title Override"
+        downstream.attempts_before_showanswer_button = 2
+        downstream.due = datetime.datetime(2025, 2, 2, tzinfo=utc)
+        downstream.force_save_button = True
+        downstream.graceperiod = '2d'
+        downstream.grading_method = 'last_score'
+        downstream.max_attempts = 100
+        downstream.show_correctness = 'always'
+        downstream.show_reset_button = True
+        downstream.showanswer = 'on_expired'
+        downstream.submission_wait_seconds = 100
+        downstream.weight = 3
+
+        # Modifying synchronized fields are "unsafe" customizations
+        downstream.rerandomize = '"onreset"'
+        downstream.matlab_api_key = 'hij'
+        downstream.save()
+
+        # Follow-up sync.
+        sync_from_upstream(downstream, self.user)
+
+        # "unsafe" customizations are overridden by upstream
+        assert downstream.upstream_display_name == "Upstream Problem Title V3"
+        assert downstream.rerandomize == '"per_student"'
+        assert downstream.matlab_api_key == 'def'
+        assert downstream.use_latex_compiler
+
+        # but "safe" customizations survive
+        assert downstream.display_name == "Downstream Title Override"
+        assert downstream.attempts_before_showanswer_button == 2
+        assert downstream.due == datetime.datetime(2025, 2, 2, tzinfo=utc)
+        assert downstream.force_save_button
+        assert downstream.graceperiod == '2d'
+        assert downstream.grading_method == 'last_score'
+        assert downstream.max_attempts == 100
+        assert downstream.show_correctness == 'always'
+        assert downstream.show_reset_button
+        assert downstream.showanswer == 'on_expired'
+        assert downstream.submission_wait_seconds == 100
+        assert downstream.weight == 3
+
     def test_sync_updates_to_modified_content(self):
         """
         If we sync to modified content, will it preserve customizable fields, but overwrite the rest?
diff --git a/cms/lib/xblock/upstream_sync.py b/cms/lib/xblock/upstream_sync.py
@@ -252,10 +252,6 @@ def _update_customizable_fields(*, upstream: XBlock, downstream: XBlock, only_fe
      * Set `course_problem.upstream_display_name = lib_problem.display_name` ("fetch").
      * If `not only_fetch`, and `course_problem.display_name` wasn't customized, then:
        * Set `course_problem.display_name = lib_problem.display_name` ("sync").
-
-     * Set `course_problem.upstream_max_attempts = lib_problem.max_attempts` ("fetch").
-     * If `not only_fetch`, and `course_problem.max_attempts` wasn't customized, then:
-       * Set `course_problem.max_attempts = lib_problem.max_attempts` ("sync").
     """
     syncable_field_names = _get_synchronizable_fields(upstream, downstream)
 
@@ -264,6 +260,10 @@ def _update_customizable_fields(*, upstream: XBlock, downstream: XBlock, only_fe
         if field_name not in syncable_field_names:
             continue
 
+        # Downstream-only fields don't have an upstream fetch field
+        if fetch_field_name is None:
+            continue
+
         # FETCH the upstream's value and save it on the downstream (ie, `downstream.upstream_$FIELD`).
         old_upstream_value = getattr(downstream, fetch_field_name)
         new_upstream_value = getattr(upstream, field_name)
@@ -361,6 +361,9 @@ def sever_upstream_link(downstream: XBlock) -> None:
     downstream.upstream = None
     downstream.upstream_version = None
     for _, fetched_upstream_field in downstream.get_customizable_fields().items():
+        # Downstream-only fields don't have an upstream fetch field
+        if fetched_upstream_field is None:
+            continue
         setattr(downstream, fetched_upstream_field, None)  # Null out upstream_display_name, et al.
 
 
@@ -414,21 +417,30 @@ class UpstreamSyncMixin(XBlockMixin):
         help=("The value of display_name on the linked upstream block."),
         default=None, scope=Scope.settings, hidden=True, enforce_type=True,
     )
-    upstream_max_attempts = Integer(
-        help=("The value of max_attempts on the linked upstream block."),
-        default=None, scope=Scope.settings, hidden=True, enforce_type=True,
-    )
 
     @classmethod
-    def get_customizable_fields(cls) -> dict[str, str]:
+    def get_customizable_fields(cls) -> dict[str, str | None]:
         """
         Mapping from each customizable field to the field which can be used to restore its upstream value.
 
+        If the customizable field is mapped to None, then it is considered "downstream only", and cannot be restored
+        from the upstream value.
+
         XBlocks outside of edx-platform can override this in order to set up their own customizable fields.
         """
         return {
             "display_name": "upstream_display_name",
-            "max_attempts": "upstream_max_attempts",
+            "attempts_before_showanswer_button": None,
+            "due": None,
+            "force_save_button": None,
+            "graceperiod": None,
+            "grading_method": None,
+            "max_attempts": None,
+            "show_correctness": None,
+            "show_reset_button": None,
+            "showanswer": None,
+            "submission_wait_seconds": None,
+            "weight": None,
         }
 
     # PRESERVING DOWNSTREAM CUSTOMIZATIONS and RESTORING UPSTREAM VALUES
@@ -485,6 +497,10 @@ def get_customizable_fields(cls) -> dict[str, str]:
     #            if field_name in self.downstream_customized:
     #                continue
     #
+    #            # If there is no restore_field name, it's a downstream-only field
+    #            if restore_field_name is None:
+    #                continue
+    #
     #            # If this field's value doesn't match the synced upstream value, then mark the field
     #            # as customized so that we don't clobber it later when syncing.
     #            # NOTE: Need to consider the performance impact of all these field lookups.
