Merge branch 'master' into master

This commit is contained in:
Santhosh Kumar
2025-09-01 10:03:31 +05:30
committed by GitHub
31 changed files with 450 additions and 231 deletions

View File

@@ -128,6 +128,10 @@ class EntityLinkBase(models.Model):
class Meta:
abstract = True
@classmethod
def get_by_downstream_usage_key(cls, downstream_usage_key: UsageKey):
return cls.objects.get(downstream_usage_key=downstream_usage_key)
class ComponentLink(EntityLinkBase):
"""
@@ -523,10 +527,6 @@ class ContainerLink(EntityLinkBase):
link.save()
return link
@classmethod
def get_by_downstream_usage_key(cls, downstream_usage_key: UsageKey):
return cls.objects.get(downstream_usage_key=downstream_usage_key)
class LearningContextLinksStatusChoices(models.TextChoices):
"""

View File

@@ -455,16 +455,39 @@ class DownstreamView(DeveloperErrorViewMixin, APIView):
Sever an XBlock's link to upstream content.
"""
downstream = _load_accessible_block(request.user, usage_key_string, require_write_access=True)
affected_blocks: list[XBlock] = []
try:
sever_upstream_link(downstream)
# Try to get the upstream key before severing the link, so we can delete
# the corresponding ComponentLink or ContainerLink below.
try:
upstream_key = UpstreamLink.get_for_block(downstream).upstream_key
except NoUpstream:
# Even if we don't have an UpstreamLink, we still need to check
# if the block has the upstream key set, so we don't want to
# raise an exception here.
upstream_key = None
affected_blocks = sever_upstream_link(downstream)
# Remove the ComponentLink or ContainerLink, if it exists.
if upstream_key:
if isinstance(upstream_key, LibraryUsageLocatorV2):
ComponentLink.get_by_downstream_usage_key(downstream.usage_key).delete()
elif isinstance(upstream_key, LibraryContainerLocator):
ContainerLink.get_by_downstream_usage_key(downstream.usage_key).delete()
except NoUpstream:
logger.exception(
"Tried to DELETE upstream link of '%s', but it wasn't linked to anything in the first place. "
"Will do nothing. ",
usage_key_string,
)
else:
modulestore().update_item(downstream, request.user.id)
finally:
if affected_blocks:
# If we successfully severed the upstream link, then we need to update the affected blocks.
with modulestore().bulk_operations(downstream.usage_key.context_key):
for block in affected_blocks:
modulestore().update_item(block, request.user.id)
return Response(status=204)

View File

@@ -2,29 +2,29 @@
Unit tests for /api/contentstore/v2/downstreams/* JSON APIs.
"""
import json
import ddt
from datetime import datetime, timezone
from unittest.mock import patch, MagicMock
from unittest.mock import MagicMock, patch
import ddt
from django.conf import settings
from django.urls import reverse
from freezegun import freeze_time
from opaque_keys.edx.keys import ContainerKey, UsageKey
from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2
from organizations.models import Organization
from cms.djangoapps.contentstore.helpers import StaticFileNotices
from cms.lib.xblock.upstream_sync import BadUpstream, UpstreamLink
from cms.djangoapps.contentstore.tests.utils import CourseTestCase
from cms.djangoapps.contentstore.xblock_storage_handlers import view_handlers as xblock_view_handlers
from cms.djangoapps.contentstore.xblock_storage_handlers.xblock_helpers import get_block_key_dict
from opaque_keys.edx.keys import ContainerKey, UsageKey
from opaque_keys.edx.locator import LibraryLocatorV2
from common.djangoapps.student.tests.factories import UserFactory
from cms.lib.xblock.upstream_sync import BadUpstream, UpstreamLink
from common.djangoapps.student.auth import add_users
from common.djangoapps.student.roles import CourseStaffRole
from common.djangoapps.student.tests.factories import UserFactory
from openedx.core.djangoapps.content_libraries import api as lib_api
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory
from openedx.core.djangoapps.content_libraries import api as lib_api
from .. import downstreams as downstreams_views
@@ -42,7 +42,7 @@ URL_LIB_CONTAINER_PUBLISH = URL_LIB_CONTAINER + 'publish/' # Publish changes to
def _get_upstream_link_good_and_syncable(downstream):
return UpstreamLink(
upstream_ref=downstream.upstream,
upstream_key=UsageKey.from_string(downstream.upstream),
upstream_key=LibraryUsageLocatorV2.from_string(downstream.upstream),
downstream_key=str(downstream.usage_key),
version_synced=downstream.upstream_version,
version_available=(downstream.upstream_version or 0) + 1,
@@ -433,6 +433,45 @@ class DeleteDownstreamViewTest(SharedErrorTestCases, SharedModuleStoreTestCase):
assert response.status_code == 204
assert mock_sever.call_count == 1
def test_unlink_parent_should_update_children_top_level_parent(self):
"""
If we unlink a parent block, do all children get the new top-level parent?
"""
self.client.login(username="superuser", password="password")
all_downstreams = self.client.get(
"/api/contentstore/v2/downstreams/",
data={"course_id": str(self.course.id)},
)
assert all_downstreams.data["count"] == 11
response = self.call_api(self.top_level_downstream_chapter.usage_key)
assert response.status_code == 204
# Check that all children have their top_level_downstream_parent_key updated
subsection = modulestore().get_item(self.top_level_downstream_sequential.usage_key)
assert subsection.top_level_downstream_parent_key is None
unit = modulestore().get_item(self.top_level_downstream_unit_2.usage_key)
# The sequential is the top-level parent for the unit
assert unit.top_level_downstream_parent_key == {
"id": str(self.top_level_downstream_sequential.usage_key.block_id),
"type": str(self.top_level_downstream_sequential.usage_key.block_type),
}
video = modulestore().get_item(self.top_level_downstream_video_key)
# The sequential is the top-level parent for the video
assert video.top_level_downstream_parent_key == {
"id": str(self.top_level_downstream_sequential.usage_key.block_id),
"type": str(self.top_level_downstream_sequential.usage_key.block_type),
}
all_downstreams = self.client.get(
"/api/contentstore/v2/downstreams/",
data={"course_id": str(self.course.id)},
)
assert all_downstreams.data["count"] == 10
class _DownstreamSyncViewTestMixin(SharedErrorTestCases):
"""
@@ -562,7 +601,7 @@ class GetUpstreamViewTest(
SharedModuleStoreTestCase,
):
"""
Test that `GET /api/v2/contentstore/downstreams-all?...` returns list of links based on the provided filter.
Test that `GET /api/v2/contentstore/downstreams?...` returns list of links based on the provided filter.
"""
def call_api(

View File

@@ -1654,7 +1654,7 @@ def handle_create_xblock_upstream_link(usage_key):
# The top-level parent link does not exist yet,
# it is necessary to create it first.
handle_create_xblock_upstream_link(str(top_level_parent_usage_key))
create_or_update_xblock_upstream_link(xblock, xblock.course_id)
create_or_update_xblock_upstream_link(xblock)
@shared_task
@@ -1671,7 +1671,7 @@ def handle_update_xblock_upstream_link(usage_key):
return
if not xblock.upstream or not xblock.upstream_version:
return
create_or_update_xblock_upstream_link(xblock, xblock.course_id)
create_or_update_xblock_upstream_link(xblock)
@shared_task
@@ -1711,7 +1711,7 @@ def create_or_update_upstream_links(
course_status.update_status(LearningContextLinksStatusChoices.FAILED)
return
for xblock in xblocks:
create_or_update_xblock_upstream_link(xblock, course_key, created)
create_or_update_xblock_upstream_link(xblock, created)
course_status.update_status(LearningContextLinksStatusChoices.COMPLETED)

View File

@@ -2385,7 +2385,7 @@ def get_xblock_render_error(request, xblock):
return ""
def _create_or_update_component_link(course_key: CourseKey, created: datetime | None, xblock):
def _create_or_update_component_link(created: datetime | None, xblock):
"""
Create or update upstream->downstream link for components in database for given xblock.
"""
@@ -2399,7 +2399,7 @@ def _create_or_update_component_link(course_key: CourseKey, created: datetime |
top_level_parent_usage_key = None
if xblock.top_level_downstream_parent_key is not None:
top_level_parent_usage_key = BlockUsageLocator(
course_key,
xblock.usage_key.course_key,
xblock.top_level_downstream_parent_key.get('type'),
xblock.top_level_downstream_parent_key.get('id'),
)
@@ -2408,7 +2408,7 @@ def _create_or_update_component_link(course_key: CourseKey, created: datetime |
lib_component,
upstream_usage_key=upstream_usage_key,
upstream_context_key=str(upstream_usage_key.context_key),
downstream_context_key=course_key,
downstream_context_key=xblock.usage_key.course_key,
downstream_usage_key=xblock.usage_key,
top_level_parent_usage_key=top_level_parent_usage_key,
version_synced=xblock.upstream_version,
@@ -2417,7 +2417,7 @@ def _create_or_update_component_link(course_key: CourseKey, created: datetime |
)
def _create_or_update_container_link(course_key: CourseKey, created: datetime | None, xblock):
def _create_or_update_container_link(created: datetime | None, xblock):
"""
Create or update upstream->downstream link for containers in database for given xblock.
"""
@@ -2431,7 +2431,7 @@ def _create_or_update_container_link(course_key: CourseKey, created: datetime |
top_level_parent_usage_key = None
if xblock.top_level_downstream_parent_key is not None:
top_level_parent_usage_key = BlockUsageLocator(
course_key,
xblock.usage_key.course_key,
xblock.top_level_downstream_parent_key.get('type'),
xblock.top_level_downstream_parent_key.get('id'),
)
@@ -2440,7 +2440,7 @@ def _create_or_update_container_link(course_key: CourseKey, created: datetime |
lib_component,
upstream_container_key=upstream_container_key,
upstream_context_key=str(upstream_container_key.context_key),
downstream_context_key=course_key,
downstream_context_key=xblock.usage_key.course_key,
downstream_usage_key=xblock.usage_key,
version_synced=xblock.upstream_version,
top_level_parent_usage_key=top_level_parent_usage_key,
@@ -2449,7 +2449,7 @@ def _create_or_update_container_link(course_key: CourseKey, created: datetime |
)
def create_or_update_xblock_upstream_link(xblock, course_key: CourseKey, created: datetime | None = None) -> None:
def create_or_update_xblock_upstream_link(xblock, created: datetime | None = None) -> None:
"""
Create or update upstream->downstream link in database for given xblock.
"""
@@ -2457,11 +2457,11 @@ def create_or_update_xblock_upstream_link(xblock, course_key: CourseKey, created
return None
try:
# Try to create component link
_create_or_update_component_link(course_key, created, xblock)
_create_or_update_component_link(created, xblock)
except InvalidKeyError:
# It is possible that the upstream is a container and UsageKeyV2 parse failed
# Create upstream container link and raise InvalidKeyError if xblock.upstream is a valid key.
_create_or_update_container_link(course_key, created, xblock)
_create_or_update_container_link(created, xblock)
def get_previous_run_course_key(course_key):

View File

@@ -1118,11 +1118,14 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements
# defining the default value 'True' for delete, duplicate, drag and add new child actions
# in xblock_actions for each xblock.
# The unlinkable action is set to None by default, which means the action is not applicable for
# any xblock unless explicitly set to True or False for a specific xblock condition.
xblock_actions = {
"deletable": True,
"draggable": True,
"childAddable": True,
"duplicable": True,
"unlinkable": None,
}
explanatory_message = None
@@ -1320,9 +1323,13 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements
# Also add upstream info
upstream_info = UpstreamLink.try_get_for_block(xblock, log_error=False).to_json()
xblock_info["upstream_info"] = upstream_info
# Disable adding or removing children component if xblock is imported from library
if upstream_info["upstream_ref"]:
# Disable adding or removing children component if xblock is imported from library
xblock_actions["childAddable"] = False
# Enable unlinking only for top level imported components
xblock_actions["unlinkable"] = not upstream_info["has_top_level_parent"]
if is_xblock_unit:
# if xblock is a Unit we add the discussion_enabled option
xblock_info["discussion_enabled"] = xblock.discussion_enabled

View File

@@ -0,0 +1,65 @@
# Generated by Django 4.2.23 on 2025-08-19 16:21
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [
('import_from_modulestore', '0001_initial'),
]
operations = [
migrations.AlterUniqueTogether(
name='publishableentityimport',
unique_together=None,
),
migrations.RemoveField(
model_name='publishableentityimport',
name='import_event',
),
migrations.RemoveField(
model_name='publishableentityimport',
name='resulting_change',
),
migrations.RemoveField(
model_name='publishableentityimport',
name='resulting_mapping',
),
migrations.AlterUniqueTogether(
name='publishableentitymapping',
unique_together=None,
),
migrations.RemoveField(
model_name='publishableentitymapping',
name='target_entity',
),
migrations.RemoveField(
model_name='publishableentitymapping',
name='target_package',
),
migrations.AlterUniqueTogether(
name='stagedcontentforimport',
unique_together=None,
),
migrations.RemoveField(
model_name='stagedcontentforimport',
name='import_event',
),
migrations.RemoveField(
model_name='stagedcontentforimport',
name='staged_content',
),
migrations.DeleteModel(
name='Import',
),
migrations.DeleteModel(
name='PublishableEntityImport',
),
migrations.DeleteModel(
name='PublishableEntityMapping',
),
migrations.DeleteModel(
name='StagedContentForImport',
),
]

View File

@@ -1,139 +1,3 @@
"""
Models for the course to library import app.
Models for the course to library import app (TO BE DELETED)
"""
import uuid as uuid_tools
from django.contrib.auth import get_user_model
from django.db import models
from django.utils.translation import gettext_lazy as _
from model_utils.models import TimeStampedModel
from opaque_keys.edx.django.models import (
LearningContextKeyField,
UsageKeyField,
)
from openedx_learning.api.authoring_models import LearningPackage, PublishableEntity
from .data import ImportStatus
User = get_user_model()
class Import(TimeStampedModel):
"""
Represents the action of a user importing a modulestore-based course or legacy
library into a learning-core based learning package (today, that is always a content library).
"""
uuid = models.UUIDField(default=uuid_tools.uuid4, editable=False, unique=True)
status = models.CharField(
max_length=100,
choices=ImportStatus.choices,
default=ImportStatus.NOT_STARTED,
db_index=True
)
user = models.ForeignKey(User, on_delete=models.CASCADE)
# Note: For now, this will always be a course key. In the future, it may be a legacy library key.
source_key = LearningContextKeyField(help_text=_('The modulestore course'), max_length=255, db_index=True)
target_change = models.ForeignKey(to='oel_publishing.DraftChangeLog', on_delete=models.SET_NULL, null=True)
class Meta:
verbose_name = _('Import from modulestore')
verbose_name_plural = _('Imports from modulestore')
def __str__(self):
return f'{self.source_key}{self.target_change}'
def set_status(self, status: ImportStatus):
"""
Set import status.
"""
self.status = status
self.save()
if status in [ImportStatus.IMPORTED, ImportStatus.CANCELED]:
self.clean_related_staged_content()
def clean_related_staged_content(self) -> None:
"""
Clean related staged content.
"""
for staged_content_for_import in self.staged_content_for_import.all():
staged_content_for_import.staged_content.delete()
class PublishableEntityMapping(TimeStampedModel):
"""
Represents a mapping between a source usage key and a target publishable entity.
"""
source_usage_key = UsageKeyField(
max_length=255,
help_text=_('Original usage key/ID of the thing that has been imported.'),
)
target_package = models.ForeignKey(LearningPackage, on_delete=models.CASCADE)
target_entity = models.ForeignKey(PublishableEntity, on_delete=models.CASCADE)
class Meta:
unique_together = ('source_usage_key', 'target_package')
def __str__(self):
return f'{self.source_usage_key}{self.target_entity}'
class PublishableEntityImport(TimeStampedModel):
"""
Represents a publishableentity version that has been imported into a learning package (e.g. content library)
This is a many-to-many relationship between a container version and a course to library import.
"""
import_event = models.ForeignKey(Import, on_delete=models.CASCADE)
resulting_mapping = models.ForeignKey(PublishableEntityMapping, on_delete=models.SET_NULL, null=True, blank=True)
resulting_change = models.OneToOneField(
to='oel_publishing.DraftChangeLogRecord',
# a changelog record can be pruned, which would set this to NULL, but not delete the
# entire import record
null=True,
on_delete=models.SET_NULL,
)
class Meta:
unique_together = (
('import_event', 'resulting_mapping'),
)
def __str__(self):
return f'{self.import_event}{self.resulting_mapping}'
class StagedContentForImport(TimeStampedModel):
"""
Represents m2m relationship between an import and staged content created for that import.
"""
import_event = models.ForeignKey(
Import,
on_delete=models.CASCADE,
related_name='staged_content_for_import',
)
staged_content = models.OneToOneField(
to='content_staging.StagedContent',
on_delete=models.CASCADE,
related_name='staged_content_for_import',
)
# Since StagedContent stores all the keys of the saved blocks, this field was added to optimize search.
source_usage_key = UsageKeyField(
max_length=255,
help_text=_(
'The original Usage key of the highest-level component that was saved in StagedContent.'
),
)
class Meta:
unique_together = (
('import_event', 'staged_content'),
)
def __str__(self):
return f'{self.import_event}{self.staged_content}'

View File

@@ -440,7 +440,6 @@ FEATURES:
ENABLE_SERVICE_STATUS: true
ENABLE_SPECIAL_EXAMS: true
ENABLE_THIRD_PARTY_AUTH: true
ENABLE_V2_CERT_DISPLAY_SETTINGS: true
ENABLE_VERIFIED_CERTIFICATES: true
ENABLE_VIDEO_ABSTRACTION_LAYER_API: true
ENABLE_VIDEO_BUMPER: true

View File

@@ -28,6 +28,7 @@ from opaque_keys.edx.keys import UsageKey
from xblock.exceptions import XBlockNotFoundError
from xblock.fields import Scope, String, Integer, Dict
from xblock.core import XBlockMixin, XBlock
from xmodule.util.keys import BlockKey
if t.TYPE_CHECKING:
from django.contrib.auth.models import User # pylint: disable=imported-auth-user
@@ -124,7 +125,7 @@ class UpstreamLink:
for child in downstream_children:
if child.upstream:
child_upstream_link = UpstreamLink.get_for_block(child)
child_upstream_link = UpstreamLink.try_get_for_block(child)
child_ready_to_sync = bool(
child_upstream_link.upstream_ref and
@@ -190,14 +191,14 @@ class UpstreamLink:
downstream.upstream,
)
return cls(
upstream_ref=getattr(downstream, "upstream", ""),
upstream_ref=getattr(downstream, "upstream", None),
upstream_key=None,
downstream_key=str(getattr(downstream, "usage_key", "")),
version_synced=getattr(downstream, "upstream_version", None),
version_available=None,
version_declined=None,
error_message=str(exc),
has_top_level_parent=False,
has_top_level_parent=getattr(downstream, "top_level_downstream_parent_key", None) is not None,
)
@classmethod
@@ -311,7 +312,37 @@ def decline_sync(downstream: XBlock, user_id=None) -> None:
store.update_item(downstream, user_id)
def sever_upstream_link(downstream: XBlock) -> None:
def _update_children_top_level_parent(
downstream: XBlock,
new_top_level_parent_key: dict[str, str] | None
) -> list[XBlock]:
"""
Given a new top-level parent block, update the `top_level_downstream_parent_key` field on the downstream block
and all of its children.
If `new_top_level_parent_key` is None, use the current downstream block's usage_key for its children.
Returns a list of all affected blocks.
"""
if not downstream.has_children:
return []
affected_blocks = []
for child in downstream.get_children():
child.top_level_downstream_parent_key = new_top_level_parent_key
affected_blocks.append(child)
# If the `new_top_level_parent_key` is None, the current level assume the top-level
# parent key for its children.
child_top_level_parent_key = new_top_level_parent_key if new_top_level_parent_key is not None else (
BlockKey.from_usage_key(child.usage_key)._asdict()
)
affected_blocks.extend(_update_children_top_level_parent(child, child_top_level_parent_key))
return affected_blocks
def sever_upstream_link(downstream: XBlock) -> list[XBlock]:
"""
Given an XBlock that is linked to upstream content, disconnect the link, such that authors are never again prompted
to sync upstream updates. Erase all `.upstream*` fields from the downtream block.
@@ -320,10 +351,12 @@ def sever_upstream_link(downstream: XBlock) -> None:
because once a downstream block has been de-linked from source (e.g., a Content Library block), it is no different
than if the block had just been copy-pasted in the first place.
Does not save `downstream` to the store. That is left up to the caller.
Does not save `downstream` (or its children) to the store. That is left up to the caller.
If `downstream` lacks a link, then this raises NoUpstream (though it is reasonable for callers to handle such
exception and ignore it, as the end result is the same: `downstream.upstream is None`).
Returns a list of affected blocks, which includes the `downstream` block itself and all of its children.
"""
if not downstream.upstream:
raise NoUpstream()
@@ -336,6 +369,14 @@ def sever_upstream_link(downstream: XBlock) -> None:
continue
setattr(downstream, fetched_upstream_field, None) # Null out upstream_display_name, et al.
# Set the top_level_dowwnstream_parent_key to None, and calls `_update_children_top_level_parent` to
# update all children with the new top_level_dowwnstream_parent_key for each of them.
downstream.top_level_downstream_parent_key = None
affected_blocks = _update_children_top_level_parent(downstream, None)
# Return the list of affected blocks, which includes the `downstream` block itself.
return [downstream, *affected_blocks]
def _get_library_xblock_url(usage_key: LibraryUsageLocatorV2):
"""

View File

@@ -30,6 +30,7 @@ function($, _, Backbone, gettext, BasePage,
'click .copy-button': 'copyXBlock',
'click .move-button': 'showMoveXBlockModal',
'click .delete-button': 'deleteXBlock',
'click .unlink-button': 'unlinkXBlock',
'click .library-sync-button': 'showXBlockLibraryChangesPreview',
'click .problem-bank-v2-add-button': 'showSelectV2LibraryContent',
'click .show-actions-menu-button': 'showXBlockActionsMenu',
@@ -922,6 +923,25 @@ function($, _, Backbone, gettext, BasePage,
this.deleteComponent(this.findXBlockElement(event.target));
},
unlinkXBlock: function(event) {
event.preventDefault();
const primaryHeader = $(event.target).closest('.xblock-header-primary, .nav-actions');
const usageId = encodeURI(primaryHeader.attr('data-usage-id'));
try {
if (this.options.isIframeEmbed) {
window.parent.postMessage(
{
type: 'unlinkXBlock',
message: 'Unlink the XBlock',
payload: { usageId }
}, document.referrer
);
}
} catch (e) {
console.error(e);
}
},
createComponent: function(template, target, iframeMessageData) {
// A placeholder element is created in the correct location for the new xblock
// and then onNewXBlock will replace it with a rendering of the xblock. Note that

View File

@@ -26,6 +26,7 @@ has_not_configured_message = messages.get('summary',{}).get('type', None) == 'no
block_is_unit = is_unit(xblock)
upstream_info = UpstreamLink.try_get_for_block(xblock, log_error=False)
can_unlink = upstream_info.upstream_ref and not upstream_info.has_top_level_parent
%>
<%namespace name='static' file='static_content.html'/>
@@ -202,6 +203,11 @@ upstream_info = UpstreamLink.try_get_for_block(xblock, log_error=False)
<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>

View File

@@ -32,6 +32,7 @@ from django.views.decorators.csrf import ensure_csrf_cookie
from django.views.decorators.http import require_GET, require_http_methods, require_POST
from django.views.generic import View
from edx_django_utils.monitoring import set_custom_attribute, set_custom_attributes_for_course_key
from edx_django_utils.plugins import pluggable_override
from ipware.ip import get_client_ip
from xblock.core import XBlock
@@ -2082,6 +2083,7 @@ def financial_assistance(request, course_id=None):
@login_required
@require_POST
@pluggable_override('OVERRIDE_FINANCIAL_ASSISTANCE_REQUEST')
def financial_assistance_request(request):
"""Submit a request for financial assistance to Zendesk."""
try:

View File

@@ -2,6 +2,7 @@
Discussion notifications sender util.
"""
import re
import html
from bs4 import BeautifulSoup, Tag
from django.conf import settings
@@ -447,7 +448,9 @@ def clean_thread_html_body(html_body):
"""
Get post body with tags removed and limited to 500 characters
"""
html_body = BeautifulSoup(Truncator(html_body).chars(500, html=True), 'html.parser')
truncated_body = Truncator(html_body).chars(500, html=True)
truncated_body = html.unescape(truncated_body)
html_body = BeautifulSoup(truncated_body, 'html.parser')
tags_to_remove = [
"a", "link", # Link Tags

View File

@@ -2,7 +2,6 @@
Unit tests for the DiscussionNotificationSender class
"""
import re
import django
import unittest
from unittest.mock import MagicMock, patch
@@ -109,13 +108,12 @@ class TestCleanThreadHtmlBody(unittest.TestCase):
<p>Script test: <script>alert("hello");</script></p>
<p>Some other content that should remain.</p>
"""
excepted_script_quot = 'alert(&amp;quot;hello&amp;quot;);' if django.VERSION >= (5, 0) else 'alert("hello");'
expected_output = (
f'<p style="margin: 0">This is a link to a page.</p>'
f'<p style="margin: 0">Here is an image: </p>'
f'<p style="margin: 0">Embedded video: </p>'
f'<p style="margin: 0">Script test: {excepted_script_quot}</p>'
f'<p style="margin: 0">Some other content that should remain.</p>'
'<p style="margin: 0">This is a link to a page.</p>'
'<p style="margin: 0">Here is an image: </p>'
'<p style="margin: 0">Embedded video: </p>'
'<p style="margin: 0">Script test: alert("hello");</p>'
'<p style="margin: 0">Some other content that should remain.</p>'
)
result = clean_thread_html_body(html_body)

View File

@@ -0,0 +1,18 @@
# Generated by Django 4.2.23 on 2025-08-29 12:43
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('lti_provider', '0004_require_user_account'),
]
operations = [
migrations.AddField(
model_name='lticonsumer',
name='use_lti_pii',
field=models.BooleanField(blank=True, default=False, help_text='When checked, the platform will automatically use any personal information provided via LTI to create an account. Otherwise an anonymous account will be created.'),
),
]

View File

@@ -40,6 +40,10 @@ class LtiConsumer(models.Model):
"in this instance. This is required only for linking learner accounts with "
"the LTI consumer. See the Open edX LTI Provider documentation for more details."
))
use_lti_pii = models.BooleanField(blank=True, default=False, help_text=_(
"When checked, the platform will automatically use any personal information provided "
"via LTI to create an account. Otherwise an anonymous account will be created."
))
@staticmethod
def get_or_supplement(instance_guid, consumer_key):

View File

@@ -2,10 +2,11 @@
Tests for the LTI user management functionality
"""
import itertools
import string
from unittest.mock import MagicMock, PropertyMock, patch
import ddt
import pytest
from django.contrib.auth.models import AnonymousUser, User # lint-amnesty, pylint: disable=imported-auth-user
from django.core.exceptions import PermissionDenied
@@ -73,6 +74,7 @@ class UserManagementHelperTest(TestCase):
f"Username has forbidden character '{username[char]}'"
@ddt.ddt
@patch('lms.djangoapps.lti_provider.users.switch_user', autospec=True)
@patch('lms.djangoapps.lti_provider.users.create_lti_user', autospec=True)
class AuthenticateLtiUserTest(TestCase):
@@ -156,7 +158,10 @@ class AuthenticateLtiUserTest(TestCase):
create_user.assert_called_with(self.lti_user_id, self.lti_consumer)
users.authenticate_lti_user(request, self.lti_user_id, self.auto_linking_consumer)
create_user.assert_called_with(self.lti_user_id, self.auto_linking_consumer, self.old_user.email)
create_user.assert_called_with(self.lti_user_id, self.auto_linking_consumer, {
"email": self.old_user.email,
"full_name": "",
})
def test_auto_linking_of_users_using_lis_person_contact_email_primary_case_insensitive(self, create_user, switch_user): # pylint: disable=line-too-long
request = RequestFactory().post("/", {"lis_person_contact_email_primary": self.old_user.email.upper()})
@@ -166,7 +171,10 @@ class AuthenticateLtiUserTest(TestCase):
create_user.assert_called_with(self.lti_user_id, self.lti_consumer)
users.authenticate_lti_user(request, self.lti_user_id, self.auto_linking_consumer)
create_user.assert_called_with(self.lti_user_id, self.auto_linking_consumer, request.user.email)
create_user.assert_called_with(self.lti_user_id, self.auto_linking_consumer, {
"email": self.old_user.email,
"full_name": "",
})
def test_raise_exception_trying_to_auto_link_unauthenticate_user(self, create_user, switch_user):
request = RequestFactory().post("/")
@@ -190,7 +198,57 @@ class AuthenticateLtiUserTest(TestCase):
assert not create_user.called
switch_user.assert_called_with(self.request, lti_user, self.auto_linking_consumer)
@ddt.data(
*itertools.product(
(
(
{
"lis_person_contact_email_primary": "some_email@example.com",
"lis_person_name_given": "John",
"lis_person_name_family": "Doe",
},
"some_email@example.com",
"John Doe",
),
(
{
"lis_person_contact_email_primary": "some_email@example.com",
"lis_person_name_full": "John Doe",
"lis_person_name_given": "Jacob",
},
"some_email@example.com",
"John Doe",
),
(
{"lis_person_contact_email_primary": "some_email@example.com", "lis_person_name_full": "John Doe"},
"some_email@example.com",
"John Doe",
),
({"lis_person_contact_email_primary": "some_email@example.com"}, "some_email@example.com", ""),
({"lis_person_contact_email_primary": ""}, "", ""),
({"lis_person_contact_email_primary": ""}, "", ""),
({}, "", ""),
),
[True, False],
)
)
@ddt.unpack
def test_create_user_when_user_account_not_required(self, params, enable_lti_pii, create_user, switch_user):
post_params, email, name = params
self.auto_linking_consumer.require_user_account = False
self.auto_linking_consumer.use_lti_pii = enable_lti_pii
self.auto_linking_consumer.save()
request = RequestFactory().post("/", post_params)
request.user = AnonymousUser()
users.authenticate_lti_user(request, self.lti_user_id, self.auto_linking_consumer)
if enable_lti_pii:
profile = {"email": email, "full_name": name, "username": self.lti_user_id}
create_user.assert_called_with(self.lti_user_id, self.auto_linking_consumer, profile)
else:
create_user.assert_called_with(self.lti_user_id, self.auto_linking_consumer)
@ddt.ddt
class CreateLtiUserTest(TestCase):
"""
Tests for the create_lti_user function in users.py
@@ -222,22 +280,22 @@ class CreateLtiUserTest(TestCase):
@patch('lms.djangoapps.lti_provider.users.generate_random_edx_username', side_effect=['edx_id', 'new_edx_id'])
def test_unique_username_created(self, username_mock):
User(username='edx_id').save()
users.create_lti_user('lti_user_id', self.lti_consumer)
users.create_lti_user('lti_user_id', self.lti_consumer, None)
assert username_mock.call_count == 2
assert User.objects.count() == 3
user = User.objects.get(username='new_edx_id')
assert user.email == 'new_edx_id@lti.example.com'
def test_existing_user_is_linked(self):
lti_user = users.create_lti_user('lti_user_id', self.lti_consumer, self.existing_user.email)
lti_user = users.create_lti_user('lti_user_id', self.lti_consumer, {"email": self.existing_user.email})
assert lti_user.lti_consumer == self.lti_consumer
assert lti_user.edx_user == self.existing_user
def test_only_one_lti_user_edx_user_for_each_lti_consumer(self):
users.create_lti_user('lti_user_id', self.lti_consumer, self.existing_user.email)
users.create_lti_user('lti_user_id', self.lti_consumer, {"email": self.existing_user.email})
with pytest.raises(IntegrityError):
users.create_lti_user('lti_user_id', self.lti_consumer, self.existing_user.email)
users.create_lti_user('lti_user_id', self.lti_consumer, {"email": self.existing_user.email})
def test_create_multiple_lti_users_for_edx_user_if_lti_consumer_varies(self):
lti_consumer_2 = LtiConsumer(
@@ -247,11 +305,42 @@ class CreateLtiUserTest(TestCase):
)
lti_consumer_2.save()
lti_user_1 = users.create_lti_user('lti_user_id', self.lti_consumer, self.existing_user.email)
lti_user_2 = users.create_lti_user('lti_user_id', lti_consumer_2, self.existing_user.email)
lti_user_1 = users.create_lti_user('lti_user_id', self.lti_consumer, {"email": self.existing_user.email})
lti_user_2 = users.create_lti_user('lti_user_id', lti_consumer_2, {"email": self.existing_user.email})
assert lti_user_1.edx_user == lti_user_2.edx_user
def test_create_lti_user_with_full_profile(self):
lti_user = users.create_lti_user('lti_user_id', self.lti_consumer, {
"email": "some.user@example.com",
"full_name": "John Doe",
"username": "john_doe",
})
assert lti_user.edx_user.email == "some.user@example.com"
assert lti_user.edx_user.username == "john_doe"
assert lti_user.edx_user.profile.name == "John Doe"
@patch('lms.djangoapps.lti_provider.users.generate_random_edx_username', side_effect=['edx_id'])
def test_create_lti_user_with_missing_username_in_profile(self, mock):
lti_user = users.create_lti_user('lti_user_id', self.lti_consumer, {
"email": "some.user@example.com",
"full_name": "John Doe",
})
assert lti_user.edx_user.email == "some.user@example.com"
assert lti_user.edx_user.username == "edx_id"
assert lti_user.edx_user.profile.name == "John Doe"
@patch('lms.djangoapps.lti_provider.users.generate_random_edx_username', side_effect=['edx_id', 'edx_id123'])
def test_create_lti_user_with_duplicate_username_in_profile(self, mock):
lti_user = users.create_lti_user('lti_user_id', self.lti_consumer, {
"email": "some.user@example.com",
"full_name": "John Doe",
"username": self.existing_user.username,
})
assert lti_user.edx_user.email == "some.user@example.com"
assert lti_user.edx_user.username == "edx_id"
assert lti_user.edx_user.profile.name == "John Doe"
class LtiBackendTest(TestCase):
"""

View File

@@ -19,6 +19,20 @@ from lms.djangoapps.lti_provider.models import LtiUser
from openedx.core.djangoapps.safe_sessions.middleware import mark_user_change_as_expected
def get_lti_user_details(request):
"""
Returns key LTI user details from the LTI launch request.
"""
post_data = request.POST
email = post_data.get("lis_person_contact_email_primary", "").lower()
full_name = post_data.get("lis_person_name_full", "")
given_name = post_data.get("lis_person_name_given", "")
family_name = post_data.get("lis_person_name_family", "")
if not full_name and given_name:
full_name = f"{given_name} {family_name}"
return dict(email=email, full_name=full_name)
def authenticate_lti_user(request, lti_user_id, lti_consumer):
"""
Determine whether the user specified by the LTI launch has an existing
@@ -28,7 +42,7 @@ def authenticate_lti_user(request, lti_user_id, lti_consumer):
If the currently logged-in user does not match the user specified by the LTI
launch, log out the old user and log in the LTI identity.
"""
lis_email = request.POST.get("lis_person_contact_email_primary")
profile = get_lti_user_details(request)
try:
lti_user = LtiUser.objects.get(
@@ -40,11 +54,14 @@ def authenticate_lti_user(request, lti_user_id, lti_consumer):
if lti_consumer.require_user_account:
# Verify that the email from the LTI Launch and the logged-in user are the same
# before linking the LtiUser with the edx_user.
if request.user.is_authenticated and request.user.email.lower() == lis_email.lower():
lti_user = create_lti_user(lti_user_id, lti_consumer, request.user.email)
if request.user.is_authenticated and request.user.email.lower() == profile["email"]:
lti_user = create_lti_user(lti_user_id, lti_consumer, profile)
else:
# Ask the user to login before linking.
raise PermissionDenied() from exc
elif lti_consumer.use_lti_pii:
profile["username"] = lti_user_id
lti_user = create_lti_user(lti_user_id, lti_consumer, profile)
else:
lti_user = create_lti_user(lti_user_id, lti_consumer)
@@ -55,11 +72,15 @@ def authenticate_lti_user(request, lti_user_id, lti_consumer):
switch_user(request, lti_user, lti_consumer)
def create_lti_user(lti_user_id, lti_consumer, email=None):
def create_lti_user(lti_user_id, lti_consumer, profile=None):
"""
Generate a new user on the edX platform with a random username and password,
and associates that account with the LTI identity.
"""
if profile is None:
profile = {}
email = profile.get("email")
edx_user_id = profile.get("username") or generate_random_edx_username()
edx_user = User.objects.filter(email=email).first() if email else None
if not edx_user:
@@ -67,8 +88,7 @@ def create_lti_user(lti_user_id, lti_consumer, email=None):
edx_password = str(uuid.uuid4())
while not created:
try:
edx_user_id = generate_random_edx_username()
edx_email = f"{edx_user_id}@{settings.LTI_USER_EMAIL_DOMAIN}"
edx_email = email if email else f"{edx_user_id}@{settings.LTI_USER_EMAIL_DOMAIN}"
with transaction.atomic():
edx_user = User.objects.create_user(
username=edx_user_id,
@@ -78,13 +98,13 @@ def create_lti_user(lti_user_id, lti_consumer, email=None):
# A profile is required if PREVENT_CONCURRENT_LOGINS flag is set.
# TODO: We could populate user information from the LTI launch here,
# but it's not necessary for our current uses.
edx_user_profile = UserProfile(user=edx_user)
edx_user_profile = UserProfile(user=edx_user, name=profile.get("full_name", ""))
edx_user_profile.save()
created = True
except IntegrityError:
edx_user_id = generate_random_edx_username()
# The random edx_user_id wasn't unique. Since 'created' is still
# False, we will retry with a different random ID.
pass
lti_user = LtiUser(
lti_consumer=lti_consumer,

View File

@@ -35,7 +35,9 @@ REQUIRED_PARAMETERS = [
OPTIONAL_PARAMETERS = [
'context_title', 'context_label', 'lis_result_sourcedid',
'lis_outcome_service_url', 'tool_consumer_instance_guid'
'lis_outcome_service_url', 'tool_consumer_instance_guid',
"lis_person_name_full", "lis_person_name_given", "lis_person_name_family",
"lis_person_contact_email_primary",
]

View File

@@ -22,6 +22,8 @@ ORA2_FILEUPLOAD_BACKEND = 'django'
DEBUG = True
INTERNAL_IPS = ('127.0.0.1',)
USE_I18N = True
DEFAULT_TEMPLATE_ENGINE['OPTIONS']['debug'] = True
LMS_BASE = 'localhost:18000'
@@ -81,13 +83,14 @@ DJFS = {
################################ DEBUG TOOLBAR ################################
INSTALLED_APPS += ['debug_toolbar']
MIDDLEWARE += [
'lms.djangoapps.discussion.django_comment_client.utils.QueryCountDebugMiddleware',
'debug_toolbar.middleware.DebugToolbarMiddleware',
]
INTERNAL_IPS = ('127.0.0.1',)
# The Django Debug Toolbar is disabled by default, if you need to enable it for
# debugging. Simply uncomment the INSTALLED_APPS and MIDDLEWARE section below.
#
# INSTALLED_APPS += ['debug_toolbar']
# MIDDLEWARE += [
# 'lms.djangoapps.discussion.django_comment_client.utils.QueryCountDebugMiddleware',
# 'debug_toolbar.middleware.DebugToolbarMiddleware',
# ]
DEBUG_TOOLBAR_PANELS = (
'debug_toolbar.panels.versions.VersionsPanel',

View File

@@ -596,7 +596,6 @@ FEATURES:
ENABLE_SERVICE_STATUS: true
ENABLE_SPECIAL_EXAMS: true
ENABLE_THIRD_PARTY_AUTH: true
ENABLE_V2_CERT_DISPLAY_SETTINGS: true
ENABLE_VERIFIED_CERTIFICATES: true
ENABLE_VIDEO_ABSTRACTION_LAYER_API: true
ENABLE_VIDEO_BUMPER: true

View File

@@ -30,7 +30,10 @@ from openedx.core.djangoapps.notifications.email.utils import encrypt_string
from openedx.core.djangoapps.notifications.models import (
CourseNotificationPreference, Notification, NotificationPreference
)
from openedx.core.djangoapps.notifications.serializers import add_non_editable_in_preference
from openedx.core.djangoapps.notifications.serializers import (
add_info_to_notification_config,
add_non_editable_in_preference
)
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
@@ -711,6 +714,7 @@ class TestNotificationPreferencesView(ModuleStoreTestCase):
expected_data = exclude_inaccessible_preferences(self.default_data['data'], self.user)
expected_data = add_non_editable_in_preference(expected_data)
expected_data = add_info_to_notification_config(expected_data)
self.assertEqual(response.data['data'], expected_data)
@@ -768,25 +772,30 @@ class TestNotificationPreferencesView(ModuleStoreTestCase):
"web": False,
"email": False,
"push": False,
"email_cadence": "Daily"
"email_cadence": "Daily",
"info": ""
},
"new_question_post": {
"web": False,
"email": False,
"push": False,
"email_cadence": "Daily"
"email_cadence": "Daily",
"info": ""
},
"new_instructor_all_learners_post": {
"web": False,
"email": False,
"push": False,
"email_cadence": "Daily"
"email_cadence": "Daily",
"info": ""
},
"core": {
"web": False,
"email": False,
"push": False,
"email_cadence": "Daily"
"email_cadence": "Daily",
"info": "Notifications for responses and comments on your posts, and the ones youre "
"following, including endorsements to your responses and on your posts."
}
},
"non_editable": {
@@ -803,13 +812,15 @@ class TestNotificationPreferencesView(ModuleStoreTestCase):
"web": False,
"email": False,
"push": False,
"email_cadence": "Daily"
"email_cadence": "Daily",
"info": ""
},
"core": {
"web": True,
"email": True,
"push": True,
"email_cadence": "Daily"
"email_cadence": "Daily",
"info": "Notifications for new announcements and updates from the course team."
}
},
"non_editable": {
@@ -824,13 +835,15 @@ class TestNotificationPreferencesView(ModuleStoreTestCase):
"web": False,
"email": False,
"push": False,
"email_cadence": "Daily"
"email_cadence": "Daily",
"info": ""
},
"core": {
"web": True,
"email": True,
"push": True,
"email_cadence": "Daily"
"email_cadence": "Daily",
"info": "Notifications for submission grading."
}
},
"non_editable": {

View File

@@ -31,6 +31,7 @@ from .models import Notification
from .serializers import (
NotificationSerializer,
UserNotificationPreferenceUpdateAllSerializer,
add_info_to_notification_config,
add_non_editable_in_preference
)
from .tasks import create_notification_preference
@@ -323,11 +324,14 @@ class NotificationPreferencesView(APIView):
type_details['push'] = user_pref.push
type_details['email_cadence'] = user_pref.email_cadence
exclude_inaccessible_preferences(structured_preferences, request.user)
structured_preferences = add_non_editable_in_preference(
add_info_to_notification_config(structured_preferences)
)
return Response({
'status': 'success',
'message': 'Notification preferences retrieved successfully.',
'show_preferences': get_show_notifications_tray(self.request.user),
'data': add_non_editable_in_preference(structured_preferences)
'data': structured_preferences
}, status=status.HTTP_200_OK)
def put(self, request):

View File

@@ -68,14 +68,14 @@ bleach[css]==6.2.0
# xblock-poll
boto==2.49.0
# via -r requirements/edx/kernel.in
boto3==1.40.19
boto3==1.40.20
# via
# -r requirements/edx/kernel.in
# django-ses
# fs-s3fs
# ora2
# snowflake-connector-python
botocore==1.40.19
botocore==1.40.20
# via
# -r requirements/edx/kernel.in
# boto3

View File

@@ -136,7 +136,7 @@ boto==2.49.0
# via
# -r requirements/edx/doc.txt
# -r requirements/edx/testing.txt
boto3==1.40.19
boto3==1.40.20
# via
# -r requirements/edx/doc.txt
# -r requirements/edx/testing.txt
@@ -144,7 +144,7 @@ boto3==1.40.19
# fs-s3fs
# ora2
# snowflake-connector-python
botocore==1.40.19
botocore==1.40.20
# via
# -r requirements/edx/doc.txt
# -r requirements/edx/testing.txt
@@ -1267,7 +1267,7 @@ meilisearch==0.37.0
# -r requirements/edx/doc.txt
# -r requirements/edx/testing.txt
# edx-search
mistune==3.1.3
mistune==3.1.4
# via
# -r requirements/edx/doc.txt
# sphinx-mdinclude

View File

@@ -103,14 +103,14 @@ bleach[css]==6.2.0
# xblock-poll
boto==2.49.0
# via -r requirements/edx/base.txt
boto3==1.40.19
boto3==1.40.20
# via
# -r requirements/edx/base.txt
# django-ses
# fs-s3fs
# ora2
# snowflake-connector-python
botocore==1.40.19
botocore==1.40.20
# via
# -r requirements/edx/base.txt
# boto3
@@ -926,7 +926,7 @@ meilisearch==0.37.0
# via
# -r requirements/edx/base.txt
# edx-search
mistune==3.1.3
mistune==3.1.4
# via sphinx-mdinclude
mongoengine==0.29.1
# via -r requirements/edx/base.txt

View File

@@ -113,7 +113,7 @@ ruamel-yaml==0.18.15
# via semgrep
ruamel-yaml-clib==0.2.12
# via ruamel-yaml
semgrep==1.133.0
semgrep==1.134.0
# via -r requirements/edx/semgrep.in
tomli==2.0.2
# via semgrep

View File

@@ -100,14 +100,14 @@ bleach[css]==6.2.0
# xblock-poll
boto==2.49.0
# via -r requirements/edx/base.txt
boto3==1.40.19
boto3==1.40.20
# via
# -r requirements/edx/base.txt
# django-ses
# fs-s3fs
# ora2
# snowflake-connector-python
botocore==1.40.19
botocore==1.40.20
# via
# -r requirements/edx/base.txt
# boto3

View File

@@ -10,9 +10,9 @@ attrs==25.3.0
# via zeep
backoff==2.2.1
# via -r scripts/user_retirement/requirements/base.in
boto3==1.40.19
boto3==1.40.20
# via -r scripts/user_retirement/requirements/base.in
botocore==1.40.19
botocore==1.40.20
# via
# boto3
# s3transfer

View File

@@ -14,11 +14,11 @@ attrs==25.3.0
# zeep
backoff==2.2.1
# via -r scripts/user_retirement/requirements/base.txt
boto3==1.40.19
boto3==1.40.20
# via
# -r scripts/user_retirement/requirements/base.txt
# moto
botocore==1.40.19
botocore==1.40.20
# via
# -r scripts/user_retirement/requirements/base.txt
# boto3