Merge branch 'master' into master

This commit is contained in:
Santhosh Kumar
2025-09-18 04:49:46 +05:30
committed by GitHub
28 changed files with 1329 additions and 461 deletions

View File

@@ -2,6 +2,7 @@
Helper methods for Studio views.
"""
from __future__ import annotations
import json
import logging
import pathlib
import urllib
@@ -493,6 +494,14 @@ def _fetch_and_set_upstream_link(
# new values from the published upstream content.
if isinstance(upstream_link.upstream_key, UsageKey): # only if upstream is a block, not a container
fetch_customizable_fields_from_block(downstream=temp_xblock, user=user, upstream=temp_xblock)
# Although the above function will set all customisable fields to match its upstream_* counterpart
# We copy the downstream_customized list to the new block to avoid overriding user customisations on sync
# So we will have:
# temp_xblock.display_name == temp_xblock.upstream_display_name
# temp_xblock.data == temp_xblock.upstream_data # for html blocks
# Even then we want to set `downstream_customized` value to avoid overriding user customisations on sync
downstream_customized = temp_xblock.xml_attributes.get("downstream_customized", '[]')
temp_xblock.downstream_customized = json.loads(downstream_customized)
def _import_xml_node_to_parent(

View File

@@ -394,7 +394,7 @@ class DownstreamView(DeveloperErrorViewMixin, APIView):
Inspect an XBlock's link to upstream content.
"""
downstream = _load_accessible_block(request.user, usage_key_string, require_write_access=False)
return Response(UpstreamLink.try_get_for_block(downstream).to_json())
return Response(UpstreamLink.try_get_for_block(downstream).to_json(include_child_info=True))
def put(self, request: _AuthenticatedRequest, usage_key_string: str) -> Response:
"""
@@ -499,6 +499,12 @@ class SyncFromUpstreamView(DeveloperErrorViewMixin, APIView):
def post(self, request: _AuthenticatedRequest, usage_key_string: str) -> Response:
"""
Pull latest updates to the block at {usage_key_string} from its linked upstream content.
Optionally accepts json data in below format to control override of locally customized fields
{
"override_customizations": True or False (default: False),
"keep_custom_fields": [] (If override_customizations is True, this key can be used to preserve
some fields from override).
}
"""
downstream = _load_accessible_block(request.user, usage_key_string, require_write_access=True)
try:

View File

@@ -15,6 +15,7 @@ from cms.djangoapps.contentstore.xblock_storage_handlers.xblock_helpers import g
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory
from xmodule.xml_block import serialize_field
@ddt.ddt
@@ -78,6 +79,29 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
data = self._api('get', f'/api/olx-export/v1/xblock/{usage_key}/', {}, expect_response=200)
return data["blocks"][data["root_block_id"]]["olx"]
def _copy_course_block(self, usage_key: str):
"""
Copy a course block to the clipboard
"""
data = self._api(
'post',
"/api/content-staging/v1/clipboard/",
{"usage_key": usage_key},
expect_response=200
)
return data
def _paste_course_block(self, parent_usage_key: str):
"""
Paste a course block from the clipboard
"""
return self._api(
'post',
'/xblock/',
{"parent_locator": parent_usage_key, "staged_content": "clipboard"},
expect_response=200
)
# def _get_course_block_fields(self, usage_key: str):
# return self._api('get', f'/xblock/{usage_key}', {}, expect_response=200)
@@ -173,6 +197,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
'version_declined': None,
'ready_to_sync': False,
'error_message': None,
'is_modified': False,
# 'upstream_link': 'http://course-authoring-mfe/library/lib:CL-TEST:testlib/components?usageKey=...'
})
assert status["upstream_link"].startswith("http://course-authoring-mfe/library/")
@@ -216,6 +241,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
upstream="{self.upstream_problem1['id']}"
upstream_display_name="Problem 1 Display Name"
upstream_version="2"
downstream_customized="["display_name"]"
{self.standard_capa_attributes}
>multiple choice...</problem>
""")
@@ -228,6 +254,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
'version_declined': None,
'ready_to_sync': True, # <--- updated
'error_message': None,
'is_modified': True,
})
# 3⃣ Now, sync and check the resulting OLX of the downstream
@@ -247,6 +274,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
max_attempts="3"
upstream="{self.upstream_problem1['id']}"
upstream_display_name="Problem 1 NEW name"
downstream_customized="[&quot;display_name&quot;]"
upstream_version="3"
{self.standard_capa_attributes}
>multiple choice v2...</problem>
@@ -268,9 +296,9 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
parent_usage_key=str(self.course_subsection.usage_key),
upstream_key=self.upstream_unit["id"],
)
downstream_unit_block_key = get_block_key_dict(
downstream_unit_block_key = serialize_field(get_block_key_dict(
UsageKey.from_string(downstream_unit["locator"]),
)
)).replace('"', '&quot;')
status = self._get_sync_status(downstream_unit["locator"])
self.assertDictContainsEntries(status, {
'upstream_ref': self.upstream_unit["id"], # e.g. 'lct:CL-TEST:testlib:unit:u1'
@@ -279,6 +307,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
'version_declined': None,
'ready_to_sync': False,
'error_message': None,
'is_modified': False,
# 'upstream_link': 'http://course-authoring-mfe/library/lib:CL-TEST:testlib/units/...'
})
assert status["upstream_link"].startswith("http://course-authoring-mfe/library/")
@@ -302,6 +331,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
editor="visual"
upstream="{self.upstream_html1['id']}"
upstream_version="2"
upstream_data="This is the HTML."
top_level_downstream_parent_key="{downstream_unit_block_key}"
>This is the HTML.</html>
<problem
@@ -426,6 +456,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
'version_declined': None,
'ready_to_sync': True, # <--- It's the top-level parent of the block
'error_message': None,
'is_modified': False,
})
# Check the upstream/downstream status of [one of] the children
@@ -437,6 +468,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
'version_declined': None,
'ready_to_sync': False, # <-- It has top-level parent, the parent is the one who must synchronize
'error_message': None,
'is_modified': False,
})
# Sync and check the resulting OLX of the downstream
@@ -455,6 +487,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
editor="visual"
upstream="{self.upstream_html1['id']}"
upstream_version="2"
upstream_data="This is the HTML."
top_level_downstream_parent_key="{downstream_unit_block_key}"
>This is the HTML.</html>
<!-- 🟢 the problem below has been updated: -->
@@ -580,6 +613,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
'version_declined': None,
'ready_to_sync': True,
'error_message': None,
'is_modified': False,
})
# Sync and check the resulting OLX of the downstream
@@ -598,6 +632,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
editor="visual"
upstream="{self.upstream_html1['id']}"
upstream_version="2"
upstream_data="This is the HTML."
top_level_downstream_parent_key="{downstream_unit_block_key}"
>This is the HTML.</html>
<problem
@@ -748,6 +783,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
editor="visual"
upstream="{self.upstream_html1['id']}"
upstream_version="2"
upstream_data="This is the HTML."
top_level_downstream_parent_key="{downstream_unit_block_key}"
>This is the HTML.</html>
</vertical>
@@ -831,6 +867,313 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
self.assertEqual(data["count"], 4)
self.assertListEqual(data["results"], expected_downstreams)
def test_unit_sync_with_modified_downstream(self):
"""
Test that modified children component is not overridden when syncing parent container like unit
"""
# pylint: disable=too-many-statements
# 1⃣ Create a "vertical" block in the course based on a "unit" container:
downstream_unit = self._create_block_from_upstream(
# The API consumer needs to specify "vertical" here, even though upstream is "unit".
# In the future we could create a nicer REST API endpoint for this that's not part of
# the messy '/xblock/' API and which auto-detects the types based on the upstream_key.
block_category="vertical",
parent_usage_key=str(self.course_subsection.usage_key),
upstream_key=self.upstream_unit["id"],
)
downstream_unit_block_key = serialize_field(get_block_key_dict(
UsageKey.from_string(downstream_unit["locator"]),
)).replace('"', '&quot;')
status = self._get_sync_status(downstream_unit["locator"])
self.assertDictContainsEntries(status, {
'upstream_ref': self.upstream_unit["id"], # e.g. 'lct:CL-TEST:testlib:unit:u1'
'version_available': 2,
'version_synced': 2,
'version_declined': None,
'ready_to_sync': False,
'error_message': None,
'is_modified': False,
# 'upstream_link': 'http://course-authoring-mfe/library/lib:CL-TEST:testlib/units/...'
})
assert status["upstream_link"].startswith("http://course-authoring-mfe/library/")
assert status["upstream_link"].endswith(f"/units/{self.upstream_unit['id']}")
# Check that the downstream container matches our expectations.
# Note that:
# (1) Every XBlock has an "upstream" field
# (2) some "downstream only" fields like weight and max_attempts are omitted.
# (3) The "top_level_downstream_parent" is the container created
self.assertXmlEqual(self._get_course_block_olx(downstream_unit["locator"]), f"""
<vertical
display_name="Unit 1 Title"
upstream_display_name="Unit 1 Title"
upstream="{self.upstream_unit['id']}"
upstream_version="2"
>
<html
display_name="Text Content"
upstream_display_name="Text Content"
editor="visual"
upstream="{self.upstream_html1['id']}"
upstream_version="2"
upstream_data="This is the HTML."
top_level_downstream_parent_key="{downstream_unit_block_key}"
>This is the HTML.</html>
<problem
display_name="Problem 1 Display Name"
upstream_display_name="Problem 1 Display Name"
markdown="MD 1"
{self.standard_capa_attributes}
upstream="{self.upstream_problem1['id']}"
upstream_version="2"
top_level_downstream_parent_key="{downstream_unit_block_key}"
>multiple choice...</problem>
<problem
display_name="Problem 2 Display Name"
upstream_display_name="Problem 2 Display Name"
markdown="null"
{self.standard_capa_attributes}
upstream="{self.upstream_problem2['id']}"
upstream_version="2"
top_level_downstream_parent_key="{downstream_unit_block_key}"
>multi select...</problem>
</vertical>
""")
children_downstream_keys = self._get_course_block_children(downstream_unit["locator"])
downstream_html1 = children_downstream_keys[0]
assert "type@html" in downstream_html1
downstream_problem1 = children_downstream_keys[1]
assert "type@problem" in downstream_problem1
downstream_problem2 = children_downstream_keys[2]
assert "type@problem" in downstream_problem2
# 2⃣ Now, lets modify the upstream problem 1 and upstream html 1:
self._set_library_block_olx(
self.upstream_problem1["id"],
'<problem display_name="Problem 1 NEW name" markdown="updated">multiple choice v2...</problem>'
)
self._set_library_block_olx(
self.upstream_html1["id"],
'<html display_name="Text content upstream 1">updated content upstream 1</html>'
)
self._publish_container(self.upstream_unit["id"])
status = self._get_sync_status(downstream_unit["locator"])
self.assertDictContainsEntries(status, {
'upstream_ref': self.upstream_unit["id"], # e.g. 'lct:CL-TEST:testlib:unit:u1'
'version_available': 2, # <--- not updated since we didn't directly modify the unit
'version_synced': 2,
'version_declined': None,
'ready_to_sync': True, # <--- It's the top-level parent of the block
'error_message': None,
'is_modified': False,
})
# Check the upstream/downstream status of [one of] the children
self.assertDictContainsEntries(self._get_sync_status(downstream_problem1), {
'upstream_ref': self.upstream_problem1["id"],
'version_available': 3, # <--- updated since we modified the problem
'version_synced': 2,
'version_declined': None,
'ready_to_sync': False, # <-- It has top-level parent, the parent is the one who must synchronize
'error_message': None,
'is_modified': False,
})
self.assertDictContainsEntries(self._get_sync_status(downstream_html1), {
'upstream_ref': self.upstream_html1["id"],
'version_available': 3, # <--- updated since we modified the problem
'version_synced': 2,
'version_declined': None,
'ready_to_sync': False, # <-- It has top-level parent, the parent is the one who must synchronize
'error_message': None,
'is_modified': False,
})
# Now let's modify course html block
self._update_course_block_fields(downstream_html1, {
"data": "The new downstream data.",
})
# Sync and check the resulting OLX of the downstream
self._sync_downstream(downstream_unit["locator"])
# Notice how the customization of html block i.e. modified child block is preserved by
# not updating any of the fields except for upstream_* fields
self.assertXmlEqual(self._get_course_block_olx(downstream_unit["locator"]), f"""
<vertical
display_name="Unit 1 Title"
upstream_display_name="Unit 1 Title"
upstream="{self.upstream_unit['id']}"
upstream_version="2"
>
<html
display_name="Text content upstream 1"
upstream_display_name="Text content upstream 1"
editor="visual"
upstream="{self.upstream_html1['id']}"
upstream_version="3"
upstream_data="updated content upstream 1"
top_level_downstream_parent_key="{downstream_unit_block_key}"
downstream_customized="[&quot;data&quot;]"
>The new downstream data.</html>
<!-- 🟢 the problem below has been updated: -->
<problem
display_name="Problem 1 NEW name"
upstream_display_name="Problem 1 NEW name"
markdown="updated"
{self.standard_capa_attributes}
upstream="{self.upstream_problem1['id']}"
upstream_version="3"
top_level_downstream_parent_key="{downstream_unit_block_key}"
>multiple choice v2...</problem>
<problem
display_name="Problem 2 Display Name"
upstream_display_name="Problem 2 Display Name"
markdown="null"
{self.standard_capa_attributes}
upstream="{self.upstream_problem2['id']}"
upstream_version="2"
top_level_downstream_parent_key="{downstream_unit_block_key}"
>multi select...</problem>
</vertical>
""")
def test_modified_html_copy_paste(self):
"""
Test that we can sync a html from a library into a course.
"""
# 1⃣ First, create the html in the course, using the upstream problem as a template:
downstream_html1 = self._create_block_from_upstream(
block_category="html",
parent_usage_key=str(self.course_subsection.usage_key),
upstream_key=self.upstream_html1["id"],
)
status = self._get_sync_status(downstream_html1["locator"])
self.assertDictContainsEntries(status, {
'upstream_ref': self.upstream_html1["id"], # e.g. 'lb:CL-TEST:testlib:html:html1'
'version_available': 2,
'version_synced': 2,
'version_declined': None,
'ready_to_sync': False,
'error_message': None,
'is_modified': False,
# 'upstream_link': 'http://course-authoring-mfe/library/lib:CL-TEST:testlib/components?usageKey=...'
})
assert status["upstream_link"].startswith("http://course-authoring-mfe/library/")
assert status["upstream_link"].endswith(f"/components?usageKey={self.upstream_html1['id']}")
# Check the OLX of the downstream block. Notice that:
# (1) fields display_name and data (content/body of the <html>) are synced.
self.assertXmlEqual(self._get_course_block_olx(downstream_html1["locator"]), f"""
<html
display_name="Text Content"
editor="visual"
upstream="{self.upstream_html1['id']}"
upstream_display_name="Text Content"
upstream_version="2"
upstream_data="This is the HTML."
>This is the HTML.</html>
""")
# 2⃣ Now, lets modify the upstream html AND the downstream display_name:
self._update_course_block_fields(downstream_html1["locator"], {
"display_name": "New Text Content",
})
self._set_library_block_olx(
self.upstream_html1["id"],
'<html display_name="HTML 1 NEW name">The new upstream data.</html>'
)
self._publish_library_block(self.upstream_html1["id"])
# Here's how the downstream OLX looks now, before we sync:
self.assertXmlEqual(self._get_course_block_olx(downstream_html1["locator"]), f"""
<html
display_name="New Text Content"
downstream_customized="[&quot;display_name&quot;]"
editor="visual"
upstream="{self.upstream_html1['id']}"
upstream_display_name="Text Content"
upstream_version="2"
upstream_data="This is the HTML."
>This is the HTML.</html>
""")
status = self._get_sync_status(downstream_html1["locator"])
self.assertDictContainsEntries(status, {
'upstream_ref': self.upstream_html1["id"], # e.g. 'lb:CL-TEST:testlib:html:html1'
'version_available': 3, # <--- updated
'version_synced': 2,
'version_declined': None,
'ready_to_sync': True, # <--- updated
'error_message': None,
'is_modified': True,
})
# 3⃣ Now, sync and check the resulting OLX of the downstream
self._sync_downstream(downstream_html1["locator"])
# Here's how the downstream OLX looks now, after we synced it.
# All customizations are preserved based on the post data
self.assertXmlEqual(self._get_course_block_olx(downstream_html1["locator"]), f"""
<html
display_name="New Text Content"
editor="visual"
upstream="{self.upstream_html1['id']}"
upstream_display_name="HTML 1 NEW name"
upstream_version="3"
upstream_data="The new upstream data."
downstream_customized="[&quot;display_name&quot;]"
>The new upstream data.</html>
""")
self._update_course_block_fields(downstream_html1["locator"], {
"data": "The new downstream data.",
})
# Here's how the downstream OLX looks now
self.assertXmlEqual(self._get_course_block_olx(downstream_html1["locator"]), f"""
<html
display_name="New Text Content"
editor="visual"
upstream="{self.upstream_html1['id']}"
upstream_display_name="HTML 1 NEW name"
upstream_version="3"
upstream_data="The new upstream data."
downstream_customized="[&quot;display_name&quot;, &quot;data&quot;]"
>The new downstream data.</html>
""")
# Copy this modified html block
self._copy_course_block(downstream_html1["locator"])
# Paste it
pasted_block = self._paste_course_block(str(self.course_subsection.usage_key))
# The pasted block will have same fields as the above block except the
# upstream_* fields will now have the original blocks base value, so
# pasted_block.upstream_display_name == downstream_html1.display_name
# pasted_block.upstream_data == downstream_html1.data
# while still have `downstream_customized` same to avoid overriding during sync
# to allow authors to revert back to back to the original copied customized value
# See `upstream_data` below is same as how downstream_html1.data is set.
self.assertXmlEqual(self._get_course_block_olx(pasted_block["locator"]), f"""
<html
display_name="New Text Content"
editor="visual"
upstream="{self.upstream_html1['id']}"
upstream_display_name="New Text Content"
upstream_version="3"
upstream_data="The new downstream data."
downstream_customized="[&quot;display_name&quot;, &quot;data&quot;]"
>The new downstream data.</html>
""")
def test_unit_decline_sync(self):
"""
Test that we can decline sync a unit from the library into the course
@@ -844,9 +1187,9 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
parent_usage_key=str(self.course_subsection.usage_key),
upstream_key=self.upstream_unit["id"],
)
downstream_unit_block_key = get_block_key_dict(
downstream_unit_block_key = serialize_field(get_block_key_dict(
UsageKey.from_string(downstream_unit["locator"]),
)
)).replace('"', '&quot;')
children_downstream_keys = self._get_course_block_children(downstream_unit["locator"])
downstream_problem1 = children_downstream_keys[1]
assert "type@problem" in downstream_problem1
@@ -882,6 +1225,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
upstream="{self.upstream_html1['id']}"
upstream_version="2"
top_level_downstream_parent_key="{downstream_unit_block_key}"
upstream_data="This is the HTML."
>This is the HTML.</html>
<problem
display_name="Problem 1 Display Name"
@@ -971,6 +1315,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
upstream_version="2"
upstream_version_declined="2"
top_level_downstream_parent_key="{downstream_unit_block_key}"
upstream_data="This is the HTML."
>This is the HTML.</html>
<problem
display_name="Problem 1 Display Name"

View File

@@ -48,6 +48,7 @@ def _get_upstream_link_good_and_syncable(downstream):
version_available=(downstream.upstream_version or 0) + 1,
version_declined=downstream.upstream_version_declined,
error_message=None,
is_modified=False,
has_top_level_parent=False,
)
@@ -625,6 +626,29 @@ class GetUpstreamViewTest(
data["use_top_level_parents"] = str(use_top_level_parents)
return self.client.get("/api/contentstore/v2/downstreams/", data=data)
def test_200_single_upstream_container(self):
"""
Test single upstream container link provides children info as well.
"""
self.client.login(username="superuser", password="password")
# Publish components
self._set_library_block_olx(self.html_lib_id_2, "<html><b>Hello world!</b></html>")
self._publish_library_block(self.html_lib_id_2)
response = self.client.get(f"/api/contentstore/v2/downstreams/{self.top_level_downstream_unit.usage_key}")
assert response.status_code == 200
data = response.json()
assert data['upstream_ref'] == self.top_level_unit_id
assert data['error_message'] is None
assert data['ready_to_sync'] is True
assert len(data['ready_to_sync_children']) == 1
html_block = modulestore().get_item(self.top_level_downstream_html_key)
self.assertDictEqual(data['ready_to_sync_children'][0], {
'name': html_block.display_name,
'upstream': str(self.html_lib_id_2),
'id': str(html_block.usage_key),
})
def test_200_all_downstreams_for_a_course(self):
"""
Returns all links for given course

View File

@@ -292,6 +292,8 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False):
"""
Wraps the results of rendering an XBlock view in a div which adds a header and Studio action buttons.
"""
# Allow some imported components to be edited by authors in course.
editable_library_components = ["html"]
# Only add the Studio wrapper when on the container page. The "Pages" page will remain as is for now.
if not context.get('is_pages_view', None) and view in PREVIEW_VIEWS:
root_xblock = context.get('root_xblock')
@@ -304,13 +306,22 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False):
can_edit = context.get('can_edit', True)
can_add = context.get('can_add', True)
upstream_link = UpstreamLink.try_get_for_block(root_xblock, log_error=False)
if upstream_link.error_message is None and isinstance(upstream_link.upstream_key, LibraryContainerLocator):
can_move = context.get('can_move', True)
root_upstream_link = UpstreamLink.try_get_for_block(root_xblock, log_error=False)
upstream_link = UpstreamLink.try_get_for_block(xblock, log_error=False)
if (
root_upstream_link.error_message is None
and isinstance(root_upstream_link.upstream_key, LibraryContainerLocator)
):
# If this unit is linked to a library unit, for now we make it completely read-only
# because when it is synced, all local changes like added components will be lost.
# (This is only on the frontend; the backend doesn't enforce it)
can_edit = False
can_add = False
can_move = False
if upstream_link.error_message is None and upstream_link.upstream_ref:
can_edit = xblock.category in editable_library_components
# Is this a course or a library?
is_course = xblock.context_key.is_course
@@ -336,10 +347,11 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False):
# Generally speaking, "if you can add, you can delete". One exception is itembank (Problem Bank)
# which has its own separate "add" workflow but uses the normal delete workflow for its child blocks.
'can_delete': can_add or (root_xblock and root_xblock.scope_ids.block_type == "itembank" and can_edit),
'can_move': context.get('can_move', is_course),
'can_move': can_move,
'language': getattr(course, 'language', None),
'is_course': is_course,
'tags_count': tags_count,
'can_edit_title': True, # This is always true even for imported components
}
add_webpack_js_to_fragment(frag, "js/factories/xblock_validation")

View File

@@ -301,14 +301,11 @@ def modify_xblock(usage_key, request):
)
def _update_with_callback(xblock, user, old_metadata=None, old_content=None):
def save_xblock_with_callback(xblock, user, old_metadata=None, old_content=None):
"""
Updates the xblock in the modulestore.
But before doing so, it calls the xblock's editor_saved callback function,
and after doing so, it calls the xblock's post_editor_saved callback function.
TODO: Remove getattrs from this function.
See https://github.com/openedx/edx-platform/issues/33715
"""
if old_metadata is None:
old_metadata = own_metadata(xblock)
@@ -377,7 +374,7 @@ def _save_xblock(
if old_parent_location:
old_parent = store.get_item(old_parent_location)
old_parent.children.remove(new_child)
old_parent = _update_with_callback(old_parent, user)
old_parent = save_xblock_with_callback(old_parent, user)
else:
# the Studio UI currently doesn't present orphaned children, so assume this is an error
return JsonResponse(
@@ -447,7 +444,7 @@ def _save_xblock(
validate_and_update_xblock_due_date(xblock)
# update the xblock and call any xblock callbacks
xblock = _update_with_callback(xblock, user, old_metadata, old_content)
xblock = save_xblock_with_callback(xblock, user, old_metadata, old_content)
# for static tabs, their containing course also records their display name
course = store.get_course(xblock.location.course_key)
@@ -533,7 +530,7 @@ def sync_library_content(
downstream: XBlock,
request,
store,
top_level_parent: XBlock | None = None
top_level_parent: XBlock | None = None,
) -> StaticFileNotices:
"""
Handle syncing library content for given xblock depending on its upstream type.
@@ -541,9 +538,21 @@ def sync_library_content(
"""
link = UpstreamLink.get_for_block(downstream)
upstream_key = link.upstream_key
request_data = getattr(request, "json", getattr(request, "data", {}))
override_customizations = request_data.get("override_customizations", False)
keep_custom_fields = request_data.get("keep_custom_fields", [])
if isinstance(upstream_key, LibraryUsageLocatorV2):
lib_block = sync_from_upstream_block(downstream=downstream, user=request.user)
static_file_notices = import_static_assets_for_library_sync(downstream, lib_block, request)
lib_block = sync_from_upstream_block(
downstream=downstream,
user=request.user,
top_level_parent=top_level_parent,
override_customizations=override_customizations,
keep_custom_fields=keep_custom_fields,
)
if lib_block:
static_file_notices = import_static_assets_for_library_sync(downstream, lib_block, request)
else:
static_file_notices = StaticFileNotices()
store.update_item(downstream, request.user.id)
else:
with store.bulk_operations(downstream.usage_key.context_key):
@@ -1678,7 +1687,7 @@ def _get_release_date(xblock, user=None):
if reset_to_default and user:
xblock.start = DEFAULT_START_DATE
xblock = _update_with_callback(xblock, user)
xblock = save_xblock_with_callback(xblock, user)
# Treat DEFAULT_START_DATE as a magic number that means the release date has not been set
return (

View File

@@ -844,8 +844,8 @@ XBLOCK_MIXINS = (
ResourceTemplates,
XModuleMixin,
EditInfoMixin,
UpstreamSyncMixin, # Should be above AuthoringMixin for UpstreamSyncMixin.editor_saved to take effect
AuthoringMixin,
UpstreamSyncMixin,
)
# .. setting_name: XBLOCK_EXTRA_MIXINS

View File

@@ -17,6 +17,7 @@ from cms.lib.xblock.upstream_sync import (
sever_upstream_link,
)
from cms.lib.xblock.upstream_sync_block import sync_from_upstream_block, fetch_customizable_fields_from_block
from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import save_xblock_with_callback
from common.djangoapps.student.tests.factories import UserFactory
from openedx.core.djangoapps.content_libraries import api as libs
from openedx.core.djangoapps.content_tagging import api as tagging_api
@@ -305,7 +306,7 @@ class UpstreamTestCase(ModuleStoreTestCase):
# Modifying synchronized fields are "unsafe" customizations
downstream.rerandomize = '"onreset"'
downstream.matlab_api_key = 'hij'
downstream.save()
save_xblock_with_callback(downstream, self.user)
# Follow-up sync.
sync_from_upstream_block(downstream, self.user)
@@ -349,73 +350,96 @@ class UpstreamTestCase(ModuleStoreTestCase):
upstream.save()
libs.publish_changes(self.library.key, self.user.id)
# Downstream modifications
downstream.display_name = "Downstream Title Override" # "safe" customization
downstream.data = "Downstream content override" # "unsafe" override
downstream.save()
# Downstream modifications, currently all fields are overridden on individual sync
downstream.display_name = "Downstream Title Override"
downstream.data = "Downstream content override"
save_xblock_with_callback(downstream, self.user)
# Follow-up sync. Assert that updates are pulled into downstream, but customizations are saved.
sync_from_upstream_block(downstream, self.user)
assert downstream.upstream_display_name == "Upstream Title V3"
assert downstream.display_name == "Downstream Title Override" # "safe" customization survives
assert downstream.data == "<html><body>Upstream content V3</body></html>" # "unsafe" override is gone
assert downstream.data == "Downstream content override" # "safe" customization survives
# Verify hidden field has latest upstream value
assert downstream.upstream_data == "<html><body>Upstream content V3</body></html>"
assert downstream.upstream_display_name == "Upstream Title V3"
# For the Content Libraries Relaunch Beta, we do not yet need to support this edge case.
# See "PRESERVING DOWNSTREAM CUSTOMIZATIONS and RESTORING UPSTREAM DEFAULTS" in cms/lib/xblock/upstream_sync.py.
#
# def test_sync_to_downstream_with_subtle_customization(self):
# """
# Edge case: If our downstream customizes a field, but then the upstream is changed to match the
# customization do we still remember that the downstream field is customized? That is,
# if the upstream later changes again, do we retain the downstream customization (rather than
# following the upstream update?)
# """
# # Start with an uncustomized downstream block.
# downstream = BlockFactory.create(category='html', parent=self.unit, upstream=str(self.upstream_key))
# sync_from_upstream_block(downstream, self.user)
# assert downstream.downstream_customized == []
# assert downstream.display_name == downstream.upstream_display_name == "Upstream Title V2"
#
# # Then, customize our downstream title.
# downstream.display_name = "Title V3"
# downstream.save()
# assert downstream.downstream_customized == ["display_name"]
#
# # Syncing should retain the customization.
# sync_from_upstream_block(downstream, self.user)
# assert downstream.upstream_version == 2
# assert downstream.upstream_display_name == "Upstream Title V2"
# assert downstream.display_name == "Title V3"
#
# # Whoa, look at that, the upstream has updated itself to the exact same title...
# upstream = xblock.load_block(self.upstream_key, self.user)
# upstream.display_name = "Title V3"
# upstream.save()
#
# # ...which is reflected when we sync.
# sync_from_upstream_block(downstream, self.user)
# assert downstream.upstream_version == 3
# assert downstream.upstream_display_name == downstream.display_name == "Title V3"
#
# # But! Our downstream knows that its title is still customized.
# assert downstream.downstream_customized == ["display_name"]
# # So, if the upstream title changes again...
# upstream.display_name = "Title V4"
# upstream.save()
#
# # ...then the downstream title should remain put.
# sync_from_upstream_block(downstream, self.user)
# assert downstream.upstream_version == 4
# assert downstream.upstream_display_name == "Title V4"
# assert downstream.display_name == "Title V3"
#
# # Finally, if we "de-customize" the display_name field, then it should go back to syncing normally.
# downstream.downstream_customized = []
# upstream.display_name = "Title V5"
# upstream.save()
# sync_from_upstream_block(downstream, self.user)
# assert downstream.upstream_version == 5
# assert downstream.upstream_display_name == downstream.display_name == "Title V5"
def test_sync_to_downstream_with_subtle_customization(self):
"""
Edge case: If our downstream customizes a field, but then the upstream is changed to match the
customization do we still remember that the downstream field is customized? That is,
if the upstream later changes again, do we retain the downstream customization (rather than
following the upstream update?)
"""
# Start with an uncustomized downstream block.
downstream = BlockFactory.create(category='html', parent=self.unit, upstream=str(self.upstream_key))
sync_from_upstream_block(downstream, self.user)
assert downstream.downstream_customized == []
assert downstream.display_name == downstream.upstream_display_name == "Upstream Title V2"
# Then, customize our downstream title.
downstream.display_name = "Title V3"
save_xblock_with_callback(downstream, self.user)
assert downstream.downstream_customized == ["display_name"]
# Syncing should retain the customization if we allow display name customization.
sync_from_upstream_block(
downstream,
self.user,
override_customizations=True,
keep_custom_fields=["display_name"]
)
assert downstream.upstream_version == 2
assert downstream.upstream_display_name == "Upstream Title V2"
assert downstream.display_name == "Title V3"
# Whoa, look at that, the upstream has updated itself to the exact same title...
upstream = xblock.load_block(self.upstream_key, self.user)
upstream.display_name = "Title V3"
upstream.save()
libs.publish_changes(self.library.key, self.user.id)
# ...which is reflected when we sync.
sync_from_upstream_block(
downstream,
self.user,
override_customizations=True,
keep_custom_fields=["display_name"]
)
assert downstream.upstream_version == 3
assert downstream.upstream_display_name == downstream.display_name == "Title V3"
# But! Our downstream knows that its title is still customized.
assert downstream.downstream_customized == ["display_name"]
# So, if the upstream title changes again...
upstream.display_name = "Title V4"
upstream.save()
libs.publish_changes(self.library.key, self.user.id)
# ...then the downstream title should remain put.
sync_from_upstream_block(
downstream,
self.user,
override_customizations=True,
keep_custom_fields=["display_name"]
)
assert downstream.upstream_version == 4
assert downstream.upstream_display_name == "Title V4"
assert downstream.display_name == "Title V3"
# Finally, if we don't allow keeping any customizations
upstream.display_name = "Title V5"
upstream.save()
libs.publish_changes(self.library.key, self.user.id)
sync_from_upstream_block(
downstream,
self.user,
override_customizations=True,
keep_custom_fields=[]
) # No customizations!
assert downstream.upstream_version == 5
assert downstream.upstream_display_name == downstream.display_name == "Title V5"
# Clears downstream_customized field as well
assert downstream.downstream_customized == []
@ddt.data(None, "Title From Some Other Upstream Version")
def test_update_customizable_fields(self, initial_upstream_display_name):
@@ -587,3 +611,33 @@ class UpstreamTestCase(ModuleStoreTestCase):
# `edx_video_id` doesn't change
assert downstream.edx_video_id == "test_video_id"
def test_sync_keep_customizaton_option(self):
"""
Test that when an upstream block has a customized downstream block, we keep
the customized options when syncing based on keep_custom_fields option.
"""
# Start with an uncustomized downstream block.
downstream = BlockFactory.create(category='html', parent=self.unit, upstream=str(self.upstream_key))
sync_from_upstream_block(downstream, self.user)
assert downstream.downstream_customized == []
assert downstream.display_name == downstream.upstream_display_name == "Upstream Title V2"
# Then, customize our downstream title and content
downstream.display_name = "Title V3"
downstream.data = "<html><data>Some content</data></html>"
save_xblock_with_callback(downstream, self.user)
assert downstream.downstream_customized == ["display_name", "data"]
# Now, sync the upstream block with `keep_custom_fields=["display_name"] only`.
# And let data be overridden
sync_from_upstream_block(
downstream,
self.user,
override_customizations=True,
keep_custom_fields=['display_name']
)
assert downstream.display_name == "Title V3"
# data is overridden
assert downstream.data == "<html><body>Upstream content V2</body></html>"
assert downstream.downstream_customized == ["display_name"]

View File

@@ -26,7 +26,7 @@ from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locator import LibraryContainerLocator, LibraryUsageLocatorV2
from opaque_keys.edx.keys import UsageKey
from xblock.exceptions import XBlockNotFoundError
from xblock.fields import Scope, String, Integer, Dict
from xblock.fields import Scope, String, Integer, Dict, List
from xblock.core import XBlockMixin, XBlock
from xmodule.util.keys import BlockKey
@@ -84,10 +84,11 @@ class UpstreamLink:
version_available: int | None # Latest version of the upstream that's available, or None if it couldn't be loaded.
version_declined: int | None # Latest version which the user has declined to sync with, if any.
error_message: str | None # If link is valid, None. Otherwise, a localized, human-friendly error message.
is_modified: bool | None # If modified in course, True. Otherwise, False.
has_top_level_parent: bool # True if this Upstream link has a top-level parent
@property
def _is_ready_to_sync_individually(self) -> bool:
def is_ready_to_sync_individually(self) -> bool:
return bool(
self.upstream_ref and
self.version_available and
@@ -95,6 +96,44 @@ class UpstreamLink:
self.version_available > (self.version_declined or 0)
)
def _check_children_ready_to_sync(self, xblock_downstream: XBlock, return_fast: bool) -> list[dict[str, str]]:
"""
Check if all the children of the current XBlock are ready to be synced individually.
Args:
xblock_downstream (XBlock): The XBlock mixin instance whose children need to be checked.
return_fast (bool): If True, return the first child that is ready to sync.
Returns:
list[dict]: A list of children id and names that ready to sync.
"""
if not xblock_downstream.has_children:
return []
downstream_children = xblock_downstream.get_children()
child_info = []
for child in downstream_children:
if child.upstream:
child_upstream_link = UpstreamLink.try_get_for_block(child)
# If one child needs sync, it is not needed to check more children
if child_upstream_link.is_ready_to_sync_individually:
child_info.append({
'name': child.display_name,
'upstream': getattr(child, 'upstream', None),
'id': str(child.usage_key),
})
if return_fast:
return child_info
grand_children_info = self._check_children_ready_to_sync(child, return_fast)
child_info.extend(grand_children_info)
if return_fast and len(grand_children_info) > 0:
# If one child needs sync, it is not needed to check more children
return child_info
return child_info
@property
def ready_to_sync(self) -> bool:
"""
@@ -108,45 +147,16 @@ class UpstreamLink:
return False
if isinstance(self.upstream_key, LibraryUsageLocatorV2):
return self._is_ready_to_sync_individually
return self.is_ready_to_sync_individually
elif isinstance(self.upstream_key, LibraryContainerLocator):
# The container itself has changes to update, it is not necessary to review its children
if self._is_ready_to_sync_individually:
return True
def check_children_ready_to_sync(xblock_downstream):
"""
Checks if one of the children of `xblock_downstream` is ready to sync
"""
if not xblock_downstream.has_children:
return False
downstream_children = xblock_downstream.get_children()
for child in downstream_children:
if child.upstream:
child_upstream_link = UpstreamLink.try_get_for_block(child)
child_ready_to_sync = bool(
child_upstream_link.upstream_ref and
child_upstream_link.version_available and
child_upstream_link.version_available > (child_upstream_link.version_synced or 0) and
child_upstream_link.version_available > (child_upstream_link.version_declined or 0)
)
# If one child needs sync, it is not needed to check more children
if child_ready_to_sync:
return True
if check_children_ready_to_sync(child):
# If one child needs sync, it is not needed to check more children
return True
return False
if self.downstream_key is not None:
return check_children_ready_to_sync(
modulestore().get_item(UsageKey.from_string(self.downstream_key))
)
return self.is_ready_to_sync_individually or (
self.downstream_key is not None
and len(self._check_children_ready_to_sync(
modulestore().get_item(UsageKey.from_string(self.downstream_key)),
return_fast=True,
)) > 0
)
return False
@property
@@ -162,7 +172,7 @@ class UpstreamLink:
return _get_library_container_url(self.upstream_key)
return None
def to_json(self) -> dict[str, t.Any]:
def to_json(self, include_child_info=False) -> dict[str, t.Any]:
"""
Get an JSON-API-friendly representation of this upstream link.
"""
@@ -171,6 +181,18 @@ class UpstreamLink:
"ready_to_sync": self.ready_to_sync,
"upstream_link": self.upstream_link,
}
if (
include_child_info
and self.ready_to_sync
and isinstance(self.upstream_key, LibraryContainerLocator)
and self.downstream_key is not None
):
from xmodule.modulestore.django import modulestore
data["ready_to_sync_children"] = self._check_children_ready_to_sync(
modulestore().get_item(UsageKey.from_string(self.downstream_key)),
return_fast=False,
)
del data["upstream_key"] # As JSON (string), this would be redundant with upstream_ref
return data
@@ -198,6 +220,7 @@ class UpstreamLink:
version_available=None,
version_declined=None,
error_message=str(exc),
is_modified=len(getattr(downstream, "downstream_customized", [])) > 0,
has_top_level_parent=getattr(downstream, "top_level_downstream_parent_key", None) is not None,
)
@@ -280,6 +303,7 @@ class UpstreamLink:
version_available=version_available,
version_declined=downstream.upstream_version_declined,
error_message=None,
is_modified=len(getattr(downstream, "downstream_customized", [])) > 0,
has_top_level_parent=downstream.top_level_downstream_parent_key is not None,
)
@@ -453,6 +477,27 @@ class UpstreamSyncMixin(XBlockMixin):
default=None, scope=Scope.settings, hidden=True, enforce_type=True,
)
# PRESERVING DOWNSTREAM CUSTOMIZATIONS and RESTORING UPSTREAM VALUES
#
# For the full Content Libraries Relaunch, we would like to keep track of which customizable fields the user has
# actually customized. The idea is: once an author has customized a customizable field....
#
# - future upstream syncs will NOT blow away the customization,
# - but future upstream syncs WILL fetch the upstream values and tuck them away in a hidden field,
# - and the author can can revert back to said fetched upstream value at any point.
#
# Now, whether field is "customized" (and thus "revertible") is dependent on whether they have ever edited it.
# To instrument this, we need to keep track of which customizable fields have been edited using a new XBlock field:
# `downstream_customized`
downstream_customized = List(
help=(
"Names of the fields which have values set on the upstream block yet have been explicitly "
"overridden on this downstream block. Unless explicitly cleared by the user, these customizations "
"will persist even when updates are synced from the upstream."
),
default=[], scope=Scope.settings, hidden=True, enforce_type=True,
)
@classmethod
def get_customizable_fields(cls) -> dict[str, str | None]:
"""
@@ -478,66 +523,32 @@ class UpstreamSyncMixin(XBlockMixin):
"weight": None,
}
# PRESERVING DOWNSTREAM CUSTOMIZATIONS and RESTORING UPSTREAM VALUES
#
# For the full Content Libraries Relaunch, we would like to keep track of which customizable fields the user has
# actually customized. The idea is: once an author has customized a customizable field....
#
# - future upstream syncs will NOT blow away the customization,
# - but future upstream syncs WILL fetch the upstream values and tuck them away in a hidden field,
# - and the author can can revert back to said fetched upstream value at any point.
#
# Now, whether field is "customized" (and thus "revertible") is dependent on whether they have ever edited it.
# To instrument this, we need to keep track of which customizable fields have been edited using a new XBlock field:
# `downstream_customized`
#
# Implementing `downstream_customized` has proven difficult, because there is no simple way to keep it up-to-date
# with the many different ways XBlock fields can change. The `.save()` and `.editor_saved()` methods are promising,
# but we need to do more due diligence to be sure that they cover all cases, including API edits, import/export,
# copy/paste, etc. We will figure this out in time for the full Content Libraries Relaunch (related ticket:
# https://github.com/openedx/frontend-app-authoring/issues/1317). But, for the Beta realease, we're going to
# implement something simpler:
#
# - We fetch upstream values for customizable fields and tuck them away in a hidden field (same as above).
# - If a customizable field DOES match the fetched upstream value, then future upstream syncs DO update it.
# - If a customizable field does NOT the fetched upstream value, then future upstream syncs DO NOT update it.
# - There is no UI option for explicitly reverting back to the fetched upstream value.
#
# For future reference, here is a partial implementation of what we are thinking for the full Content Libraries
# Relaunch::
#
# downstream_customized = List(
# help=(
# "Names of the fields which have values set on the upstream block yet have been explicitly "
# "overridden on this downstream block. Unless explicitly cleared by the user, these customizations "
# "will persist even when updates are synced from the upstream."
# ),
# default=[], scope=Scope.settings, hidden=True, enforce_type=True,
# )
#
# def save(self, *args, **kwargs):
# """
# Update `downstream_customized` when a customizable field is modified.
#
# NOTE: This does not work, because save() isn't actually called in all the cases that we'd want it to be.
# """
# super().save(*args, **kwargs)
# customizable_fields = self.get_customizable_fields()
#
# # Loop through all the fields that are potentially cutomizable.
# for field_name, restore_field_name in self.get_customizable_fields():
#
# # If the field is already marked as customized, then move on so that we don't
# # unneccessarily query the block for its current value.
# 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.
# if getattr(self, field_name) != getattr(self, restore_field_name):
# self.downstream_customized.append(field_name)
def editor_saved(self, user, old_metadata, old_content):
"""
Update `downstream_customized` when a customizable field is modified.
"""
super().editor_saved(user, old_metadata, old_content)
customizable_fields = self.get_customizable_fields()
new_data = (
self.get_explicitly_set_fields_by_scope(Scope.settings)
| self.get_explicitly_set_fields_by_scope(Scope.content)
)
old_data = old_metadata | old_content
# Loop through all the fields that are potentially cutomizable.
for field_name, restore_field_name in customizable_fields.items():
# If the field is already marked as customized, then move on so that we don't
# unneccessarily query the block for its current value.
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.
if new_data.get(field_name) != old_data.get(restore_field_name):
self.downstream_customized.append(field_name)

View File

@@ -6,22 +6,33 @@ upstream is a container, not an XBlock.
"""
from __future__ import annotations
import logging
import typing as t
from django.core.exceptions import PermissionDenied
from django.utils.translation import gettext_lazy as _
from rest_framework.exceptions import NotFound
from opaque_keys.edx.locator import LibraryUsageLocatorV2
from xblock.fields import Scope
from rest_framework.exceptions import NotFound
from xblock.core import XBlock
from xblock.fields import Scope
from .upstream_sync import UpstreamLink, BadUpstream
from .upstream_sync import BadDownstream, BadUpstream, UpstreamLink
if t.TYPE_CHECKING:
from django.contrib.auth.models import User # pylint: disable=imported-auth-user
def sync_from_upstream_block(downstream: XBlock, user: User) -> XBlock:
logger = logging.getLogger(__name__)
def sync_from_upstream_block(
downstream: XBlock,
user: User,
*,
top_level_parent: XBlock | None = None,
override_customizations: bool = False,
keep_custom_fields: list[str] | None = None,
) -> XBlock | None:
"""
Update `downstream` with content+settings from the latest available version of its linked upstream content.
@@ -36,9 +47,16 @@ def sync_from_upstream_block(downstream: XBlock, user: User) -> XBlock:
link = UpstreamLink.get_for_block(downstream) # can raise UpstreamLinkException
if not isinstance(link.upstream_key, LibraryUsageLocatorV2):
raise TypeError("sync_from_upstream_block() only supports XBlock upstreams, not containers")
# Upstream is a library block:
upstream = _load_upstream_block(downstream, user)
_update_customizable_fields(upstream=upstream, downstream=downstream, only_fetch=False)
# Upstream is a library block:
# Sync all fields from the upstream block and override customizations
_update_customizable_fields(
upstream=upstream,
downstream=downstream,
only_fetch=False,
override_customizations=override_customizations,
keep_custom_fields=keep_custom_fields,
)
_update_non_customizable_fields(upstream=upstream, downstream=downstream)
_update_tags(upstream=upstream, downstream=downstream)
downstream.upstream_version = link.version_available
@@ -57,6 +75,17 @@ def fetch_customizable_fields_from_block(*, downstream: XBlock, user: User, upst
_update_customizable_fields(upstream=upstream, downstream=downstream, only_fetch=True)
def _verify_modification_to(downstream: XBlock, allowed_fields: list[str]):
"""
Raise error if any field except for fields in allowed_fields is modified in course locally.
"""
if len(downstream.downstream_customized) > len(allowed_fields):
raise BadDownstream("Too many fields modified, skip sync operation")
not_allowed_modified = set(downstream.downstream_customized).difference(allowed_fields)
if len(not_allowed_modified) > 0:
raise BadDownstream(f"{not_allowed_modified} fields are modified locally")
def _load_upstream_block(downstream: XBlock, user: User) -> XBlock:
"""
Load the upstream metadata and content for a downstream block.
@@ -80,12 +109,21 @@ def _load_upstream_block(downstream: XBlock, user: User) -> XBlock:
return lib_block
def _update_customizable_fields(*, upstream: XBlock, downstream: XBlock, only_fetch: bool) -> None:
def _update_customizable_fields(
*,
upstream: XBlock,
downstream: XBlock,
only_fetch: bool,
override_customizations: bool = False,
keep_custom_fields: list[str] | None = None,
) -> None:
"""
For each customizable field:
* Save the upstream value to a hidden field on the downstream ("FETCH").
* If `not only_fetch`, and if the field *isn't* customized on the downstream, then:
* If `not only_fetch`, and if the field *isn't* customized on the downstream
or if override_customizations=True and keep_custom_fields does not contain the field name, then:
* Update it the downstream field's value from the upstream field ("SYNC").
* Remove the field from downstream.downstream_customized field if exists.
Concrete example: Imagine `lib_problem` is our upstream and `course_problem` is our downstream.
@@ -107,7 +145,6 @@ def _update_customizable_fields(*, upstream: XBlock, downstream: XBlock, only_fe
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)
setattr(downstream, fetch_field_name, new_upstream_value)
@@ -116,19 +153,15 @@ def _update_customizable_fields(*, upstream: XBlock, downstream: XBlock, only_fe
# Okay, now for the nuanced part...
# We need to update the downstream field *iff it has not been customized**.
# Determining whether a field has been customized will differ in Beta vs Future release.
# (See "PRESERVING DOWNSTREAM CUSTOMIZATIONS" comment below for details.)
## FUTURE BEHAVIOR: field is "customized" iff we have noticed that the user edited it.
# if field_name in downstream.downstream_customized:
# continue
if field_name in downstream.downstream_customized:
if not override_customizations or keep_custom_fields and field_name in keep_custom_fields:
continue
else:
# Remove the field from downstream_customized field as it can be overridden
downstream.downstream_customized.remove(field_name)
## BETA BEHAVIOR: field is "customized" iff we have the prev upstream value, but field doesn't match it.
downstream_value = getattr(downstream, field_name)
if old_upstream_value and downstream_value != old_upstream_value:
continue # Field has been customized. Don't touch it. Move on.
# Field isn't customized -- SYNC it!
# Field isn't customized or is can be overridden -- SYNC it!
setattr(downstream, field_name, new_upstream_value)
@@ -137,7 +170,10 @@ def _update_non_customizable_fields(*, upstream: XBlock, downstream: XBlock) ->
For each field `downstream.blah` that isn't customizable: set it to `upstream.blah`.
"""
syncable_fields = _get_synchronizable_fields(upstream, downstream)
customizable_fields = set(downstream.get_customizable_fields().keys())
# Remove both field_name and its upstream_* counterpart from the list of fields to copy
customizable_fields = set(downstream.get_customizable_fields().keys()) | set(
downstream.get_customizable_fields().values()
)
# TODO: resolve this so there's no special-case happening for video block.
# e.g. by some non_cloneable_fields property of the XBlock class?
is_video_block = downstream.usage_key.block_type == "video"

View File

@@ -25,6 +25,7 @@ function($, _, Backbone, gettext, BasePage,
events: {
'click .edit-button': 'editXBlock',
'click .title-edit-button': 'clickTitleButton',
'click .access-button': 'editVisibilitySettings',
'click .duplicate-button': 'duplicateXBlock',
'click .copy-button': 'copyXBlock',
@@ -1274,6 +1275,37 @@ function($, _, Backbone, gettext, BasePage,
scrollToNewComponentButtons: function(event) {
event.preventDefault();
$.scrollTo(this.$('.add-xblock-component'), {duration: 250});
},
clickTitleButton: function(event) {
const xblockElement = this.findXBlockElement(event.target);
const xblockInfo = XBlockUtils.findXBlockInfo(xblockElement, this.model);
var self = this,
oldTitle = xblockInfo.get('display_name'),
titleElt = $(xblockElement).find('.xblock-display-name'),
buttonElt = $(xblockElement).find('.title-edit-button'),
$input = $('<input class="xblock-inline-title-editor" type="text" />'),
changeFunc = function(evt) {
var newTitle = $(evt.target).val();
if (oldTitle !== newTitle) {
xblockInfo.set('display_name', newTitle);
return XBlockUtils.updateXBlockField(xblockInfo, "display_name", newTitle).done(function() {
self.refreshXBlock(xblockElement, false);
});
} else {
titleElt.html(newTitle); // xss-lint: disable=javascript-jquery-html
$(buttonElt).show();
}
return true;
};
event.preventDefault();
$input.val(oldTitle);
$input.change(changeFunc).blur(changeFunc);
titleElt.html($input); // xss-lint: disable=javascript-jquery-html
$input.focus().select();
$(buttonElt).hide();
return true;
}
});

View File

@@ -567,11 +567,18 @@ body,
background-color: #f8f7f6;
}
input.xblock-inline-title-editor {
padding-top: 0px;
padding-bottom: 0px;
font-size: 18px;
height: 50px;
font-weight: 700;
}
.btn-default.action-edit.title-edit-button {
@extend %button-styles;
position: relative;
top: -7px;
.fa-pencil {
display: none;

View File

@@ -120,10 +120,14 @@ can_unlink = upstream_info.upstream_ref and not upstream_info.has_top_level_pare
<span class="sr-only">${_("Sourced from a library - but the upstream link is broken/invalid.")}</span>
% else:
<!-- "library" icon from https://fonts.google.com/icons?selected=Material+Symbols+Outlined:newsstand:FILL@0;wght@400;GRAD@0;opsz@24&icon.size=24 -->
<svg role="img" data-tooltip="${_("Sourced from a library.")}" aria-hidden="true" xmlns="http://www.w3.org/2000/svg" width="24px" height="24px" viewBox="0 -960 960 960" fill="currentColor" style="vertical-align: middle; padding-bottom: 4px;">
<svg role="img" data-tooltip="${_("Sourced from a library - but has been modified locally." if upstream_info.is_modified else "Sourced from a library.")}" aria-hidden="true" xmlns="http://www.w3.org/2000/svg" width="24px" height="24px" viewBox="0 -960 960 960" fill="currentColor" style="vertical-align: middle; padding-bottom: 4px;">
<path d="M80-160v-80h800v80H80Zm80-160v-320h80v320h-80Zm160 0v-480h80v480h-80Zm160 0v-480h80v480h-80Zm280 0L600-600l70-40 160 280-70 40Z"/>
</svg>
% if upstream_info.is_modified:
<span class="sr-only">${_("Sourced from a library - but has been modified locally.")}</span>
% else:
<span class="sr-only">${_("Sourced from a library.")}</span>
% endif
% endif
% endif
<span class="xblock-display-name">${label}</span>
@@ -135,88 +139,94 @@ can_unlink = upstream_info.upstream_ref and not upstream_info.has_top_level_pare
</div>
<div class="header-actions">
<ul class="actions-list nav-dd ui-right">
% if can_edit_title and not can_edit:
<li class="action-item action-edit">
<button data-tooltip=${_('Edit Title')} class="btn-default action-edit title-edit-button only-for-lib-components">
<span class="icon fa fa-pencil" aria-hidden="true"></span>
<span class="action-button-text">${_("Edit Title")}</span>
</button>
</li>
% endif
% if not is_root:
% if can_edit:
% if upstream_info.ready_to_sync:
<li class="action-item">
<button
class="btn-default library-sync-button action-button"
data-tooltip="${_("Update available - click to sync")}"
>
<span class="icon fa fa-refresh" aria-hidden="true"></span>
<span class="action-button-text">${_("Update available")}</span>
</button>
</li>
% endif
% if use_tagging:
<li class="action-item tag-count" data-locator="${xblock.location}"></li>
% endif
% if not show_inline:
<li class="action-item action-edit">
<button class="btn-default edit-button action-button" data-usage-id=${xblock.scope_ids.usage_id}>
<span class="icon fa fa-pencil" aria-hidden="true"></span>
<span class="action-button-text">${_("Edit")}</span>
</button>
</li>
% endif
<li class="action-item action-actions-menu nav-item">
<button data-tooltip="${_("Actions")}" class="btn-default show-actions-menu-button action-button">
<span class="icon fa fa-ellipsis-v" aria-hidden="true"></span>
<span class="sr">${_("Actions")}</span>
% if upstream_info.ready_to_sync:
<li class="action-item">
<button
class="btn-default library-sync-button action-button"
data-tooltip="${_("Update available - click to sync")}"
>
<span class="icon fa fa-refresh" aria-hidden="true"></span>
<span class="action-button-text">${_("Update available")}</span>
</button>
<div class="wrapper wrapper-nav-sub" style="right: -10px; top: 45px;">
<div class="nav-sub">
<ul>
% if not show_inline:
% if can_edit_visibility:
<li class="nav-item">
<a class="access-button" href="#" role="button">${_("Manage Access")}</a>
</li>
% endif
% if can_move:
<li class="nav-item">
<a class="move-button" href="#" role="button">${_("Move")}</a>
</li>
% endif
% if use_tagging:
<li class="nav-item">
<a class="manage-tags-button" href="#" role="button">${_("Manage Tags")}</a>
</li>
% endif
% if is_course:
<!--
Only show the "Copy to Clipboard" button for xblocks inside courses since
the copy/paste functionality is not yet implemented for LibraryContent.
-->
<li class="nav-item">
<a class="copy-button" href="#" role="button">${_("Copy to Clipboard")}</a>
</li>
% endif
% if can_add:
<li class="nav-item">
<a class="duplicate-button" href="#" role="button">${_("Duplicate")}</a>
</li>
% endif
% endif
% if can_delete:
<li class="nav-item">
<a class="delete-button" href="#" role="button">${_("Delete")}</a>
</li>
% endif
% if can_unlink:
<li class="nav-item">
<a class="unlink-button" href="#" role="button">${_("Unlink from Library")}</a>
</li>
% endif
</ul>
</div>
</div>
</li>
% if is_reorderable:
<li class="action-item action-drag">
<span data-tooltip="${_('Drag to reorder')}" class="drag-handle action"></span>
</li>
% endif
% endif
% if use_tagging:
<li class="action-item tag-count" data-locator="${xblock.location}"></li>
% endif
% if not show_inline and can_edit:
<li class="action-item action-edit">
<button class="btn-default edit-button action-button" data-usage-id=${xblock.scope_ids.usage_id}>
<span class="icon fa fa-pencil" aria-hidden="true"></span>
<span class="action-button-text">${_("Edit")}</span>
</button>
</li>
% endif
<li class="action-item action-actions-menu nav-item">
<button data-tooltip="${_("Actions")}" class="btn-default show-actions-menu-button action-button">
<span class="icon fa fa-ellipsis-v" aria-hidden="true"></span>
<span class="sr">${_("Actions")}</span>
</button>
<div class="wrapper wrapper-nav-sub" style="right: -10px; top: 45px;">
<div class="nav-sub">
<ul>
% if not show_inline:
% if can_edit_visibility:
<li class="nav-item">
<a class="access-button" href="#" role="button">${_("Manage Access")}</a>
</li>
% endif
% if can_move:
<li class="nav-item">
<a class="move-button" href="#" role="button">${_("Move")}</a>
</li>
% endif
% if use_tagging:
<li class="nav-item">
<a class="manage-tags-button" href="#" role="button">${_("Manage Tags")}</a>
</li>
% endif
% if is_course:
<!--
Only show the "Copy to Clipboard" button for xblocks inside courses since
the copy/paste functionality is not yet implemented for LibraryContent.
-->
<li class="nav-item">
<a class="copy-button" href="#" role="button">${_("Copy to Clipboard")}</a>
</li>
% endif
% if can_add:
<li class="nav-item">
<a class="duplicate-button" href="#" role="button">${_("Duplicate")}</a>
</li>
% endif
% endif
% if can_delete:
<li class="nav-item">
<a class="delete-button" href="#" role="button">${_("Delete")}</a>
</li>
% endif
% if can_unlink:
<li class="nav-item">
<a class="unlink-button" href="#" role="button">${_("Unlink from Library")}</a>
</li>
% endif
</ul>
</div>
</div>
</li>
% if is_reorderable and can_move:
<li class="action-item action-drag">
<span data-tooltip="${_('Drag to reorder')}" class="drag-handle action"></span>
</li>
% endif
% endif
</ul>

View File

@@ -6,6 +6,7 @@ Management commands for third_party_auth
import logging
from django.core.management.base import BaseCommand, CommandError
from edx_django_utils.monitoring import set_custom_attribute
from common.djangoapps.third_party_auth.tasks import fetch_saml_metadata
from common.djangoapps.third_party_auth.models import SAMLProviderConfig, SAMLConfiguration
@@ -18,34 +19,24 @@ class Command(BaseCommand):
def add_arguments(self, parser):
parser.add_argument('--pull', action='store_true', help="Pull updated metadata from external IDPs")
parser.add_argument(
'--fix-references',
'--run-checks',
action='store_true',
help="Fix SAMLProviderConfig references to use current SAMLConfiguration versions"
)
parser.add_argument(
'--site-id',
type=int,
help='Only fix configurations for a specific site ID (to be used with --fix-references)'
)
parser.add_argument(
'--dry-run',
action='store_true',
help='Show what would be changed, but do not make any changes.'
help="Run checks on SAMLProviderConfig configurations and report potential issues"
)
def handle(self, *args, **options):
should_pull_saml_metadata = options.get('pull', False)
should_fix_references = options.get('fix_references', False)
dry_run = options.get('dry_run', False)
if not should_pull_saml_metadata and not should_fix_references:
raise CommandError("Command must be used with '--pull' or '--fix-references' option.")
should_run_checks = options.get('run_checks', False)
if should_pull_saml_metadata:
self._handle_pull_metadata()
return
if should_fix_references:
self._handle_fix_references(options, dry_run=dry_run)
if should_run_checks:
self._handle_run_checks()
return
raise CommandError("Command must be used with '--pull' or '--run-checks' option.")
def _handle_pull_metadata(self):
"""
@@ -76,45 +67,139 @@ class Command(BaseCommand):
)
)
def _handle_fix_references(self, options, dry_run=False):
"""Handle the --fix-references option for fixing outdated SAML configuration references."""
site_id = options.get('site_id')
updated_count = 0
error_count = 0
def _handle_run_checks(self):
"""
Handle the --run-checks option for checking SAMLProviderConfig configuration issues.
This is a report-only command. It identifies potential configuration problems such as:
- Outdated SAMLConfiguration references (provider pointing to old config version)
- Site ID mismatches between SAMLProviderConfig and its SAMLConfiguration
- Slug mismatches (except 'default' slugs) # noqa: E501
- SAMLProviderConfig objects with null SAMLConfiguration references (informational)
Includes observability attributes for monitoring.
"""
# Set custom attributes for monitoring the check operation
# .. custom_attribute_name: saml_management_command.operation
# .. custom_attribute_description: Records current SAML operation ('run_checks').
set_custom_attribute('saml_management_command.operation', 'run_checks')
metrics = self._check_provider_configurations()
self._report_check_summary(metrics)
def _check_provider_configurations(self):
"""
Check each provider configuration for potential issues.
Returns a dictionary of metrics about the found issues.
"""
outdated_count = 0
site_mismatch_count = 0
slug_mismatch_count = 0
null_config_count = 0
error_count = 0
total_providers = 0
# Filter by site if specified
provider_configs = SAMLProviderConfig.objects.current_set()
if site_id:
provider_configs = provider_configs.filter(site_id=site_id)
self.stdout.write(self.style.SUCCESS("SAML Configuration Check Report"))
self.stdout.write("=" * 50)
self.stdout.write("")
for provider_config in provider_configs:
if provider_config.saml_configuration:
try:
current_config = SAMLConfiguration.current(
provider_config.site_id,
provider_config.saml_configuration.slug
)
total_providers += 1
provider_info = (
f"Provider (id={provider_config.id}, name={provider_config.name}, "
f"slug={provider_config.slug}, site_id={provider_config.site_id})"
)
if current_config and current_config.id != provider_config.saml_configuration_id:
if not provider_config.saml_configuration:
self.stdout.write(
f"[INFO] {provider_info} has no SAML configuration because "
"a matching default was not found."
)
null_config_count += 1
continue
try:
current_config = SAMLConfiguration.current(
provider_config.saml_configuration.site_id,
provider_config.saml_configuration.slug
)
# Check for outdated configuration references
if current_config:
if current_config.id != provider_config.saml_configuration_id:
self.stdout.write(
f"Provider '{provider_config.slug}' (site {provider_config.site_id}) "
f"has outdated config (ID: {provider_config.saml_configuration_id} -> {current_config.id})"
f"[WARNING] {provider_info} "
f"has outdated SAML config (id={provider_config.saml_configuration_id} which "
f"should be updated to the current SAML config (id={current_config.id})."
)
outdated_count += 1
if not dry_run:
provider_config.saml_configuration = current_config
provider_config.save()
updated_count += 1
except Exception as e: # pylint: disable=broad-except
self.stderr.write(
f"Error processing provider '{provider_config.slug}': {e}"
if provider_config.saml_configuration.site_id != provider_config.site_id:
config_site_id = provider_config.saml_configuration.site_id
provider_site_id = provider_config.site_id
self.stdout.write(
f"[WARNING] {provider_info} "
f"SAML config (id={provider_config.saml_configuration_id}, site_id={config_site_id}) "
"does not match the provider's site_id."
)
error_count += 1
site_mismatch_count += 1
style = self.style.SUCCESS
if dry_run:
msg = f"[DRY RUN] Would update {updated_count} provider configurations. {error_count} errors encountered."
saml_configuration_slug = provider_config.saml_configuration.slug
provider_config_slug = provider_config.slug
if saml_configuration_slug not in (provider_config_slug, 'default'):
self.stdout.write(
f"[WARNING] {provider_info} "
f"SAML config (id={provider_config.saml_configuration_id}, slug='{saml_configuration_slug}') "
"does not match the provider's slug."
)
slug_mismatch_count += 1
except Exception as e: # pylint: disable=broad-except
self.stderr.write(f"[ERROR] Error processing {provider_info}: {e}")
error_count += 1
metrics = {
'total_providers': {'count': total_providers, 'requires_attention': False},
'outdated_count': {'count': outdated_count, 'requires_attention': True},
'site_mismatch_count': {'count': site_mismatch_count, 'requires_attention': True},
'slug_mismatch_count': {'count': slug_mismatch_count, 'requires_attention': True},
'null_config_count': {'count': null_config_count, 'requires_attention': False},
'error_count': {'count': error_count, 'requires_attention': True},
}
for key, metric_data in metrics.items():
# .. custom_attribute_name: saml_management_command.{key}
# .. custom_attribute_description: Records metrics from SAML configuration checks.
set_custom_attribute(f'saml_management_command.{key}', metric_data['count'])
return metrics
def _report_check_summary(self, metrics):
"""
Print a summary of the check results and set the total_requiring_attention custom attribute.
"""
total_requiring_attention = sum(
metric_data['count'] for metric_data in metrics.values()
if metric_data['requires_attention']
)
# .. custom_attribute_name: saml_management_command.total_requiring_attention
# .. custom_attribute_description: The total number of configuration issues requiring attention.
set_custom_attribute('saml_management_command.total_requiring_attention', total_requiring_attention)
self.stdout.write(self.style.SUCCESS("CHECK SUMMARY:"))
self.stdout.write(f" Providers checked: {metrics['total_providers']['count']}")
self.stdout.write(f" Null configs: {metrics['null_config_count']['count']}")
if total_requiring_attention > 0:
self.stdout.write("\nIssues requiring attention:")
self.stdout.write(f" Outdated: {metrics['outdated_count']['count']}")
self.stdout.write(f" Site mismatches: {metrics['site_mismatch_count']['count']}")
self.stdout.write(f" Slug mismatches: {metrics['slug_mismatch_count']['count']}")
self.stdout.write(f" Errors: {metrics['error_count']['count']}")
self.stdout.write(f"\nTotal issues requiring attention: {total_requiring_attention}")
else:
msg = f"Updated {updated_count} provider configurations. {error_count} errors encountered."
self.stdout.write(style(msg))
self.stdout.write(self.style.SUCCESS("\nNo configuration issues found!"))

View File

@@ -8,7 +8,7 @@ import os
from io import StringIO
from unittest import mock
from ddt import ddt, data, unpack
from ddt import ddt
from django.contrib.sites.models import Site
from django.core.management import call_command
from django.core.management.base import CommandError
@@ -18,8 +18,6 @@ from requests.models import Response
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms
from common.djangoapps.third_party_auth.tests.factories import SAMLConfigurationFactory, SAMLProviderConfigFactory
from common.djangoapps.third_party_auth.models import SAMLProviderConfig
def mock_get(status_code=200):
"""
@@ -64,6 +62,7 @@ class TestSAMLCommand(CacheIsolationTestCase):
self.stdout = StringIO()
self.site = Site.objects.get_current()
self.other_site = Site.objects.create(domain='other.example.com', name='Other Site')
# We are creating SAMLConfiguration instance here so that there is always at-least one
# disabled saml configuration instance, this is done to verify that disabled configurations are
@@ -82,9 +81,9 @@ class TestSAMLCommand(CacheIsolationTestCase):
metadata_source='https://www.testshib.org/metadata/testshib-providers.xml',
)
def _setup_test_configs_for_fix_references(self):
def _setup_test_configs_for_run_checks(self):
"""
Helper method to create SAML configurations for fix-references tests.
Helper method to create SAML configurations for run-checks tests.
Returns tuple of (old_config, new_config, provider_config)
@@ -108,7 +107,7 @@ class TestSAMLCommand(CacheIsolationTestCase):
entity_id='https://updated.example.com'
)
# Create a provider config that references the old config for fix-references tests
# Create a provider config that references the old config for run-checks tests
test_provider_config = SAMLProviderConfigFactory.create(
site=self.site,
slug='test-provider',
@@ -148,14 +147,10 @@ class TestSAMLCommand(CacheIsolationTestCase):
This test would fail with an error if ValueError is raised.
"""
# Call `saml` command without any argument so that it raises a CommandError
with self.assertRaisesMessage(CommandError, "Command must be used with '--pull' or '--fix-references' option."):
# Call `saml` command without any arguments so that it raises a CommandError
with self.assertRaisesMessage(CommandError, "Command must be used with '--pull' or '--run-checks' option."):
call_command("saml")
# Call `saml` command without any argument so that it raises a CommandError
with self.assertRaisesMessage(CommandError, "Command must be used with '--pull' or '--fix-references' option."):
call_command("saml", pull=False)
def test_no_saml_configuration(self):
"""
Test that management command completes without errors and logs correct information when no
@@ -334,59 +329,144 @@ class TestSAMLCommand(CacheIsolationTestCase):
call_command("saml", pull=True, stdout=self.stdout)
assert expected in self.stdout.getvalue()
@data(
(True, '[DRY RUN]', 'should not update provider configs'),
(False, '', 'should create new provider config for new version')
)
@unpack
def test_fix_references(self, dry_run, expected_output_marker, test_description):
def _run_checks_command(self):
"""
Test the --fix-references command with and without --dry-run option.
Args:
dry_run (bool): Whether to run with --dry-run flag
expected_output_marker (str): Expected marker in output
test_description (str): Description of what the test should do
Helper method to run the --run-checks command and return output.
"""
old_config, new_config, test_provider_config = self._setup_test_configs_for_fix_references()
new_config_id = new_config.id
original_config_id = old_config.id
out = StringIO()
if dry_run:
call_command('saml', '--fix-references', '--dry-run', stdout=out)
else:
call_command('saml', '--fix-references', stdout=out)
call_command('saml', '--run-checks', stdout=out)
return out.getvalue()
output = out.getvalue()
@mock.patch('common.djangoapps.third_party_auth.management.commands.saml.set_custom_attribute')
def test_run_checks_outdated_configs(self, mock_set_custom_attribute):
"""
Test the --run-checks command identifies outdated configurations.
"""
old_config, new_config, test_provider_config = self._setup_test_configs_for_run_checks()
output = self._run_checks_command()
self.assertIn('[WARNING]', output)
self.assertIn('test-provider', output)
if expected_output_marker:
self.assertIn(expected_output_marker, output)
self.assertIn(
f'id={old_config.id} which should be updated to the current SAML config (id={new_config.id})',
output
)
self.assertIn('CHECK SUMMARY:', output)
self.assertIn('Providers checked: 2', output)
self.assertIn('Outdated: 1', output)
test_provider_config.refresh_from_db()
# Check key observability calls
expected_calls = [
mock.call('saml_management_command.operation', 'run_checks'),
mock.call('saml_management_command.total_providers', 2),
mock.call('saml_management_command.outdated_count', 1),
mock.call('saml_management_command.site_mismatch_count', 0),
mock.call('saml_management_command.slug_mismatch_count', 1),
mock.call('saml_management_command.null_config_count', 1),
mock.call('saml_management_command.error_count', 0),
mock.call('saml_management_command.total_requiring_attention', 2),
]
mock_set_custom_attribute.assert_has_calls(expected_calls, any_order=False)
if dry_run:
# For dry run, ensure the provider config was NOT updated
self.assertEqual(
test_provider_config.saml_configuration_id,
original_config_id,
"Provider config should not be updated in dry run mode"
)
else:
# For actual run, check that a new provider config was created
new_provider = SAMLProviderConfig.objects.filter(
site=self.site,
slug='test-provider',
saml_configuration_id=new_config_id
).exclude(id=test_provider_config.id).first()
@mock.patch('common.djangoapps.third_party_auth.management.commands.saml.set_custom_attribute')
def test_run_checks_site_mismatches(self, mock_set_custom_attribute):
"""
Test the --run-checks command identifies site ID mismatches.
"""
config = SAMLConfigurationFactory.create(
site=self.other_site,
slug='test-config',
entity_id='https://example.com'
)
self.assertIsNotNone(new_provider, "New provider config should be created")
self.assertEqual(new_provider.saml_configuration_id, new_config_id)
SAMLProviderConfigFactory.create(
site=self.site,
slug='test-provider',
saml_configuration=config
)
# Original provider config should still reference the old config
self.assertEqual(
test_provider_config.saml_configuration_id,
original_config_id,
"Original provider config should still reference old config"
)
output = self._run_checks_command()
self.assertIn('[WARNING]', output)
self.assertIn('test-provider', output)
self.assertIn('does not match the provider\'s site_id', output)
# Check observability calls
expected_calls = [
mock.call('saml_management_command.operation', 'run_checks'),
mock.call('saml_management_command.total_providers', 2),
mock.call('saml_management_command.outdated_count', 0),
mock.call('saml_management_command.site_mismatch_count', 1),
mock.call('saml_management_command.slug_mismatch_count', 1),
mock.call('saml_management_command.null_config_count', 1),
mock.call('saml_management_command.error_count', 0),
mock.call('saml_management_command.total_requiring_attention', 2),
]
mock_set_custom_attribute.assert_has_calls(expected_calls, any_order=False)
@mock.patch('common.djangoapps.third_party_auth.management.commands.saml.set_custom_attribute')
def test_run_checks_slug_mismatches(self, mock_set_custom_attribute):
"""
Test the --run-checks command identifies slug mismatches.
"""
config = SAMLConfigurationFactory.create(
site=self.site,
slug='config-slug',
entity_id='https://example.com'
)
SAMLProviderConfigFactory.create(
site=self.site,
slug='provider-slug',
saml_configuration=config
)
output = self._run_checks_command()
self.assertIn('[WARNING]', output)
self.assertIn('provider-slug', output)
self.assertIn('does not match the provider\'s slug', output)
# Check observability calls
expected_calls = [
mock.call('saml_management_command.operation', 'run_checks'),
mock.call('saml_management_command.total_providers', 2),
mock.call('saml_management_command.outdated_count', 0),
mock.call('saml_management_command.site_mismatch_count', 0),
mock.call('saml_management_command.slug_mismatch_count', 1),
mock.call('saml_management_command.null_config_count', 1),
mock.call('saml_management_command.error_count', 0),
mock.call('saml_management_command.total_requiring_attention', 1),
]
mock_set_custom_attribute.assert_has_calls(expected_calls, any_order=False)
@mock.patch('common.djangoapps.third_party_auth.management.commands.saml.set_custom_attribute')
def test_run_checks_null_configurations(self, mock_set_custom_attribute):
"""
Test the --run-checks command identifies providers with null configurations.
"""
SAMLProviderConfigFactory.create(
site=self.site,
slug='null-provider',
saml_configuration=None
)
output = self._run_checks_command()
self.assertIn('[INFO]', output)
self.assertIn('null-provider', output)
self.assertIn('has no SAML configuration because a matching default was not found', output)
# Check observability calls
expected_calls = [
mock.call('saml_management_command.operation', 'run_checks'),
mock.call('saml_management_command.total_providers', 2),
mock.call('saml_management_command.outdated_count', 0),
mock.call('saml_management_command.site_mismatch_count', 0),
mock.call('saml_management_command.slug_mismatch_count', 0),
mock.call('saml_management_command.null_config_count', 2),
mock.call('saml_management_command.error_count', 0),
mock.call('saml_management_command.total_requiring_attention', 0),
]
mock_set_custom_attribute.assert_has_calls(expected_calls, any_order=False)

View File

@@ -153,9 +153,12 @@ class TestSAMLConfigurationSignalHandlers(TestCase):
current_provider = self._get_current_provider(provider_slug)
mock_set_custom_attribute.assert_any_call('saml_config_signal.enabled', True)
mock_set_custom_attribute.assert_any_call('saml_config_signal.new_config_id', new_saml_config.id)
mock_set_custom_attribute.assert_any_call('saml_config_signal.slug', signal_saml_slug)
expected_calls = [
call('saml_config_signal.enabled', True),
call('saml_config_signal.new_config_id', new_saml_config.id),
call('saml_config_signal.slug', signal_saml_slug),
]
mock_set_custom_attribute.assert_has_calls(expected_calls, any_order=False)
if is_provider_updated:
mock_set_custom_attribute.assert_any_call('saml_config_signal.updated_count', 1)

View File

@@ -66,31 +66,23 @@ from openedx.core.djangolib.js_utils import (
% endif
% if sharing_sites_info:
<div class="wrapper-social-share">
<button
style="background-image: none; background-color: rgb(0, 38, 43); border-radius: 0px; color: white"
class="social-toggle-btn btn"
>
<span class="icon fa fa-share-alt mr-2" style="text-shadow: none"></span>
<button class="social-toggle-btn btn">
<span class="icon fa fa-share-alt"></span>
${_('Share this video')}
</button>
<div
hidden
class="container-social-share color-black p-2"
style="width: 300px; border-radius: 6px; background-color: white; box-shadow: 0 .5rem 1rem rgba(0,0,0,.15),0 .25rem .625rem rgba(0,0,0,.15)"
>
<div hidden class="container-social-share">
${_('Share this video')}
<div class="btn-link close-btn float-right">
<span style="color: black" class="icon fa fa-close" />
<div class="btn-link close-btn">
<span class="icon fa fa-close"></span>
</div>
% for sharing_site_info in sharing_sites_info:
<a
class="btn-link social-share-link"
data-source="${sharing_site_info['name']}"
href="${sharing_site_info['sharing_url']}"
target="_blank"
rel="noopener noreferrer"
style="font-size: 1.5rem"
class="social-share-link"
data-source="${sharing_site_info['name']}"
href="${sharing_site_info['sharing_url']}"
target="_blank"
rel="noopener noreferrer"
>
% if (sharing_site_info['name'] == 'twitter'):
<!--
@@ -110,23 +102,14 @@ from openedx.core.djangolib.js_utils import (
<span class="sr">${_("Share on {site}").format(site=sharing_site_info['name'])}</span>
</a>
% endfor
<div style="background-color: #F2F0EF" class="public-video-url-container p-2">
<a href=${public_video_url} class="d-inline-block align-middle" style="width: 200px">
<div
class="text-nowrap"
style="color: black; overflow: hidden; text-overflow: ellipsis; vertical-align: middle"
>
${public_video_url}
</div>
<div class="public-video-url-container">
<a href=${public_video_url} class="public-video-url-link">
${public_video_url}
</a>
<div
class="public-video-copy-btn btn-link d-inline-block float-right"
data-url=${public_video_url}
>
<span class="icon fa fa-link pr-1"></span>
<div class="public-video-copy-btn" data-url=${public_video_url}>
<span class="icon fa fa-link"></span>
<span>${_('Copy')}</span>
</div>
<span>
</div>
</div>
</div>

View File

@@ -29,12 +29,28 @@ class SendEmailBaseCommand(PrefixedDebugLoggerMixin, BaseCommand): # lint-amnes
'--override-recipient-email',
help='Send all emails to this address instead of the actual recipient'
)
parser.add_argument('site_domain_name')
parser.add_argument(
'site_domain_name',
nargs='?',
default=None,
help=(
'Domain name for the site to use. '
'Do not provide a domain if you wish to run this for all sites'
)
)
parser.add_argument(
'--weeks',
type=int,
help='Number of weekly emails to be sent',
)
parser.add_argument(
'--override-middlewares',
action='append',
help=(
'Use this middleware when emulating http requests. '
'To use multiple middlewares, provide this argument multiple times'
)
)
def handle(self, *args, **options):
self.log_debug('Args = %r', options)
@@ -49,19 +65,26 @@ class SendEmailBaseCommand(PrefixedDebugLoggerMixin, BaseCommand): # lint-amnes
tzinfo=pytz.UTC
)
self.log_debug('Current date = %s', current_date.isoformat())
site = Site.objects.get(domain__iexact=options['site_domain_name'])
self.log_debug('Running for site %s', site.domain)
override_recipient_email = options.get('override_recipient_email')
self.send_emails(site, current_date, override_recipient_email)
override_middlewares = options.get('override_middlewares')
def enqueue(self, day_offset, site, current_date, override_recipient_email=None):
site_domain_name = options['site_domain_name']
sites = Site.objects.filter(domain__iexact=site_domain_name) if site_domain_name else Site.objects.all()
if sites:
for site in sites:
self.log_debug('Running for site %s', site.domain)
self.send_emails(site, current_date, override_recipient_email, override_middlewares)
else:
self.log_info("No matching site found")
def enqueue(self, day_offset, site, current_date, override_recipient_email=None, override_middlewares=None):
self.async_send_task.enqueue(
site,
current_date,
day_offset,
override_recipient_email,
override_middlewares,
)
def send_emails(self, *args, **kwargs):

View File

@@ -10,6 +10,7 @@ from unittest.mock import DEFAULT, Mock, patch
import ddt
import pytz
from django.conf import settings
from django.contrib.sites.models import Site
from openedx.core.djangoapps.schedules.management.commands import SendEmailBaseCommand
from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory
@@ -33,9 +34,23 @@ class TestSendEmailBaseCommand(CacheIsolationTestCase): # lint-amnesty, pylint:
send_emails.assert_called_once_with(
self.site,
datetime.datetime(2017, 9, 29, tzinfo=pytz.UTC),
None,
None
)
def test_handle_all_sites(self):
with patch.object(self.command, 'send_emails') as send_emails:
self.command.handle(site_domain_name=None, date='2017-09-29')
expected_sites = Site.objects.all()
for expected_site in expected_sites:
send_emails.assert_any_call(
expected_site,
datetime.datetime(2017, 9, 29, tzinfo=pytz.UTC),
None,
None
)
assert send_emails.call_count == len(expected_sites)
def test_weeks_option(self):
with patch.object(self.command, 'enqueue') as enqueue:
self.command.handle(site_domain_name=self.site.domain, date='2017-09-29', weeks=12)

View File

@@ -20,6 +20,7 @@ from edx_django_utils.monitoring import (
set_custom_attribute
)
from eventtracking import tracker
from importlib import import_module
from opaque_keys.edx.keys import CourseKey
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
@@ -103,7 +104,7 @@ class BinnedScheduleMessageBaseTask(ScheduleMessageBaseTask):
task_instance = None
@classmethod
def enqueue(cls, site, current_date, day_offset, override_recipient_email=None): # lint-amnesty, pylint: disable=missing-function-docstring
def enqueue(cls, site, current_date, day_offset, override_recipient_email=None, override_middlewares=None): # lint-amnesty, pylint: disable=missing-function-docstring
set_code_owner_attribute_from_module(__name__)
current_date = resolvers._get_datetime_beginning_of_day(current_date) # lint-amnesty, pylint: disable=protected-access
@@ -120,6 +121,7 @@ class BinnedScheduleMessageBaseTask(ScheduleMessageBaseTask):
day_offset,
bin,
override_recipient_email,
override_middlewares,
)
cls.log_info('Launching task with args = %r', task_args)
cls.task_instance.apply_async(
@@ -128,16 +130,17 @@ class BinnedScheduleMessageBaseTask(ScheduleMessageBaseTask):
)
def run( # lint-amnesty, pylint: disable=arguments-differ
self, site_id, target_day_str, day_offset, bin_num, override_recipient_email=None,
self, site_id, target_day_str, day_offset, bin_num, override_recipient_email=None, override_middlewares=None,
):
set_code_owner_attribute_from_module(__name__)
site = Site.objects.select_related('configuration').get(id=site_id)
with emulate_http_request(site=site):
middlewares = [self.class_from_classpath(cls) for cls in override_middlewares] if override_middlewares else None
with emulate_http_request(site=site, middleware_classes=middlewares) as request:
msg_type = self.make_message_type(day_offset)
_annotate_for_monitoring(msg_type, site, bin_num, target_day_str, day_offset)
_annotate_for_monitoring(msg_type, request.site, bin_num, target_day_str, day_offset)
return self.resolver( # lint-amnesty, pylint: disable=not-callable
self.async_send_task,
site,
request.site,
deserialize(target_day_str),
day_offset,
bin_num,
@@ -147,6 +150,11 @@ class BinnedScheduleMessageBaseTask(ScheduleMessageBaseTask):
def make_message_type(self, day_offset):
raise NotImplementedError
def class_from_classpath(self, class_path):
module_name, klass = class_path.rsplit('.', 1)
module = import_module(module_name)
return getattr(module, klass)
@shared_task(base=LoggedTask, ignore_result=True)
@set_code_owner_attribute

View File

@@ -45,7 +45,7 @@ def emulate_http_request(site=None, user=None, middleware_classes=None):
_run_method_if_implemented(middleware, 'process_request', request)
try:
yield
yield request
except Exception as exc:
for middleware in reversed(middleware_instances):
_run_method_if_implemented(middleware, 'process_exception', request, exc)

View File

@@ -9,6 +9,7 @@ from lxml import etree
from opaque_keys.edx.locator import LibraryLocatorV2
from openedx.core.djangoapps.content_tagging.api import get_all_object_tags, TagValuesByObjectIdDict
from xmodule.xml_block import serialize_field
from .data import StaticFile
from . import utils
@@ -140,7 +141,7 @@ class XBlockSerializer:
if "top_level_downstream_parent_key" in block.fields \
and block.fields["top_level_downstream_parent_key"].is_set_on(block):
olx_node.attrib["top_level_downstream_parent_key"] = str(block.top_level_downstream_parent_key)
olx_node.attrib["top_level_downstream_parent_key"] = serialize_field(block.top_level_downstream_parent_key)
return olx_node
@@ -166,9 +167,10 @@ class XBlockSerializer:
if block.use_latex_compiler:
olx_node.attrib["use_latex_compiler"] = "true"
for field_name in block.fields:
if (field_name.startswith("upstream") or field_name == "top_level_downstream_parent_key") \
and block.fields[field_name].is_set_on(block):
olx_node.attrib[field_name] = str(getattr(block, field_name))
if (
field_name.startswith(("upstream", "downstream")) or field_name == "top_level_downstream_parent_key"
) and block.fields[field_name].is_set_on(block):
olx_node.attrib[field_name] = serialize_field(getattr(block, field_name))
# Escape any CDATA special chars
escaped_block_data = block.data.replace("]]>", "]]&gt;")

11
package-lock.json generated
View File

@@ -14,7 +14,7 @@
"@babel/plugin-transform-object-assign": "^7.18.6",
"@babel/preset-env": "^7.19.0",
"@babel/preset-react": "7.26.3",
"@edx/brand-edx.org": "^2.0.7",
"@edx/brand": "npm:@openedx/brand-openedx@^1.2.2",
"@edx/edx-bootstrap": "1.0.4",
"@edx/edx-proctoring": "^4.18.1",
"@edx/frontend-component-cookie-policy-banner": "2.2.0",
@@ -2020,10 +2020,11 @@
"url": "https://github.com/sponsors/wooorm"
}
},
"node_modules/@edx/brand-edx.org": {
"version": "2.1.3",
"resolved": "https://registry.npmjs.org/@edx/brand-edx.org/-/brand-edx.org-2.1.3.tgz",
"integrity": "sha512-1TwUwW7YVgvhh7aO1ql1poAosUCA0zd7/DNuqeSO0wXui0oCHL+WQW8b9tXS2tRJ5BMRKjG8t22fcOcKX3ljbg=="
"node_modules/@edx/brand": {
"name": "@openedx/brand-openedx",
"version": "1.2.3",
"resolved": "https://registry.npmjs.org/@openedx/brand-openedx/-/brand-openedx-1.2.3.tgz",
"integrity": "sha512-Dn9CtpC8fovh++Xi4NF5NJoeR9yU2yXZnV9IujxIyGd/dn0Phq5t6dzJVfupwq09mpDnzJv7egA8Znz/3ljO+w=="
},
"node_modules/@edx/edx-bootstrap": {
"version": "1.0.4",

View File

@@ -39,7 +39,7 @@
"@babel/plugin-transform-object-assign": "^7.18.6",
"@babel/preset-env": "^7.19.0",
"@babel/preset-react": "7.26.3",
"@edx/brand-edx.org": "^2.0.7",
"@edx/brand": "npm:@openedx/brand-openedx@^1.2.2",
"@edx/edx-bootstrap": "1.0.4",
"@edx/edx-proctoring": "^4.18.1",
"@edx/frontend-component-cookie-policy-banner": "2.2.0",

View File

@@ -61,6 +61,13 @@ class HtmlBlockMixin( # lint-amnesty, pylint: disable=abstract-method
default=_("Text")
)
data = String(help=_("Html contents to display for this block"), default="", scope=Scope.content)
upstream_data = String(
help=_("Upstream html contents to store upstream data field"),
default=None,
hidden=True,
enforce_type=True,
scope=Scope.content,
)
source_code = String(
help=_("Source code for LaTeX documents. This feature is not well-supported."),
scope=Scope.settings
@@ -143,6 +150,13 @@ class HtmlBlockMixin( # lint-amnesty, pylint: disable=abstract-method
shim_xmodule_js(fragment, 'HTMLEditingDescriptor')
return fragment
@classmethod
def get_customizable_fields(cls) -> dict[str, str | None]:
return {
"display_name": "upstream_display_name",
"data": "upstream_data",
}
uses_xmodule_styles_setup = True
mako_template = "widgets/html-edit.html"

View File

@@ -290,14 +290,21 @@
Problem.prototype.updateProgress = function(response) {
if (response.progress_changed) {
this.el.data('problem-score', response.current_score);
this.el.data('problem-total-possible', response.total_possible);
this.el.data('problem-score', this.convertToFloat(response.current_score));
this.el.data('problem-total-possible', this.convertToFloat(response.total_possible));
this.el.data('attempts-used', response.attempts_used);
this.el.trigger('progressChanged');
}
return this.renderProgressState();
};
Problem.prototype.convertToFloat = function(num) {
if (typeof num !== 'number' || !Number.isInteger(num)) {
return num;
}
return num.toFixed(1);
};
Problem.prototype.forceUpdate = function(response) {
this.el.data('problem-score', response.current_score);
this.el.data('problem-total-possible', response.total_possible);

View File

@@ -179,6 +179,7 @@
.xmodule_display.xmodule_ProblemBlock div.problem .choicegroup,
.xmodule_display.xmodule_ProblemBlock div.problem .choicetextgroup {
margin: var(--baseline, 20px) 0 0 0;
min-width: 100px;
width: auto !important;
width: 100px;
@@ -405,7 +406,7 @@
.xmodule_display.xmodule_ProblemBlock div.problem .choicegroup input[type="checkbox"] {
left: 0.5625em;
position: absolute;
top: 0.35em;
top: 0.43em;
width: calc(var(--baseline, 20px) * 1.1);
height: calc(var(--baseline, 20px) * 1.1);
z-index: 1;
@@ -636,10 +637,10 @@
}
.xmodule_display.xmodule_ProblemBlock div.problem div .grader-status .grading {
margin: 0px 7px 0 0;
margin: 0 7px 0 0;
padding-left: 25px;
background: var(--icon-info) left center no-repeat;
text-indent: 0px;
text-indent: 0;
}
.xmodule_display.xmodule_ProblemBlock div.problem div .grader-status p {
@@ -701,7 +702,7 @@
}
.xmodule_display.xmodule_ProblemBlock div.problem div .submit-message-container {
margin: var(--baseline, 20px) 0px;
margin: var(--baseline, 20px) 0;
}
.xmodule_display.xmodule_ProblemBlock div.problem div.inline>span {
@@ -878,6 +879,11 @@
content: "";
}
.xmodule_display.xmodule_ProblemBlock .problem .capa_inputtype.textline>.submitted,
.xmodule_display.xmodule_ProblemBlock .problem .inputtype.formulaequationinput>.submitted {
margin: var(--baseline, 20px) 0 0 0;
}
.xmodule_display.xmodule_ProblemBlock .problem .capa_inputtype.textline>.submitted input,
.xmodule_display.xmodule_ProblemBlock .problem .inputtype.formulaequationinput>.submitted input {
border: 2px solid var(--submitted, #0075b4);
@@ -914,7 +920,7 @@
}
.xmodule_display.xmodule_ProblemBlock .problem .inputtype.option-input {
margin: 0 0 0 0 !important;
margin: var(--baseline, 20px) 0 0 0 !important;
}
.xmodule_display.xmodule_ProblemBlock .problem .inputtype.option-input .indicator-container {
@@ -975,7 +981,7 @@
}
.xmodule_display.xmodule_ProblemBlock div.problem .CodeMirror-scroll {
margin-right: 0px;
margin-right: 0;
}
.xmodule_display.xmodule_ProblemBlock .capa-message {

View File

@@ -1163,3 +1163,89 @@
bottom: 0;
background-color: transparent;
}
.xmodule_display.xmodule_VideoBlock .wrapper-social-share .social-toggle-btn {
background: var(--primary);
font-size: 13px;
font-weight: 700;
padding: calc(var(--baseline) * 0.35) calc(var(--baseline) * 0.9);
color: var(--white);
box-shadow: none;
text-shadow: none;
border-radius: 3px;
border: none;
}
.xmodule_display.xmodule_VideoBlock .wrapper-social-share .social-toggle-btn:hover,
.xmodule_display.xmodule_VideoBlock .wrapper-social-share .social-toggle-btn:focus {
background: var(--btn-brand-focus-background);
}
.xmodule_display.xmodule_VideoBlock .wrapper-social-share .social-toggle-btn .fa {
margin-right: calc(var(--baseline) * 0.4);
}
.xmodule_display.xmodule_VideoBlock .wrapper-social-share .container-social-share {
padding: calc(var(--baseline) * 0.4);
width: 300px;
border-radius: 6px;
background-color: var(--white);
box-shadow: rgba(0, 0, 0, 0.15) 0 0.5rem 1rem, rgba(0, 0, 0, 0.15) 0 0.25rem 0.625rem;
}
.xmodule_display.xmodule_VideoBlock .wrapper-social-share .container-social-share .close-btn {
float: right;
cursor: pointer;
vertical-align: top;
display: inline-flex;
color: var(--black);
text-decoration: none !important;
}
.xmodule_display.xmodule_VideoBlock .wrapper-social-share .container-social-share .social-share-link {
margin-right: calc(var(--baseline) * 0.2);
font-size: 24px;
height: 24px;
vertical-align: middle;
text-decoration: none;
display: inline-flex;
}
.xmodule_display.xmodule_VideoBlock .wrapper-social-share .container-social-share .social-share-link > span > svg {
width: auto;
height: 24px;
vertical-align: top;
display: inline-flex;
}
.xmodule_display.xmodule_VideoBlock .wrapper-social-share .container-social-share .public-video-url-container {
padding: calc(var(--baseline) * 0.4);
display: flex;
align-items: center;
justify-content: space-between;
background-color: #f2f0ef;
}
.xmodule_display.xmodule_VideoBlock .wrapper-social-share .container-social-share .public-video-url-link {
color: var(--black);
overflow: hidden;
text-overflow: ellipsis;
vertical-align: middle;
white-space: nowrap;
}
.xmodule_display.xmodule_VideoBlock .wrapper-social-share .container-social-share .public-video-url-link:hover {
text-decoration: underline;
}
.xmodule_display.xmodule_VideoBlock .wrapper-social-share .container-social-share .public-video-copy-btn {
margin-left: calc(var(--baseline) * 0.7);
flex-shrink: 0;
color: var(--primary);
cursor: pointer;
}
.xmodule_display.xmodule_VideoBlock .wrapper-social-share .container-social-share .public-video-copy-btn:hover {
text-decoration: none;
color: var(--link-hover-color);
}