Skip to content

Commit

Permalink
fix: linter and tests
Browse files Browse the repository at this point in the history
  • Loading branch information
DanielVZ96 committed Jan 27, 2025
1 parent a730dad commit 3355545
Show file tree
Hide file tree
Showing 9 changed files with 56 additions and 38 deletions.
3 changes: 2 additions & 1 deletion cms/djangoapps/contentstore/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) ->

return new_xblock, notices


def import_staged_content_for_library_sync(new_xblock: XBlock, lib_block: XBlock, request) -> StaticFileNotices:
"""
Import a block (along with its children and any required static assets) from
Expand Down Expand Up @@ -355,7 +356,7 @@ def import_staged_content_for_library_sync(new_xblock: XBlock, lib_block: XBlock
# Rewrite the OLX's static asset references to point to the new
# locations for those assets. See _import_files_into_course for more
# info on why this is necessary.
if hasattr(new_xblock, 'data') and substitutions:
if hasattr(new_xblock, "data") and substitutions:
data_with_substitutions = new_xblock.data
for old_static_ref, new_static_ref in substitutions.items():
data_with_substitutions = data_with_substitutions.replace(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
"ready_to_sync": Boolean
}
"""
from dataclasses import asdict

import logging

from attrs import asdict as attrs_asdict
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from unittest.mock import patch
from django.conf import settings

from cms.djangoapps.contentstore.helpers import StaticFileNotices
from cms.lib.xblock.upstream_sync import UpstreamLink, BadUpstream
from common.djangoapps.student.tests.factories import UserFactory
from xmodule.modulestore.django import modulestore
Expand Down Expand Up @@ -247,14 +248,16 @@ def call_api(self, usage_key_string):

@patch.object(UpstreamLink, "get_for_block", _get_upstream_link_good_and_syncable)
@patch.object(downstreams_views, "sync_from_upstream")
def test_200(self, mock_sync_from_upstream):
@patch.object(downstreams_views, "import_staged_content_for_library_sync", return_value=StaticFileNotices())
def test_200(self, mock_sync_from_upstream, mock_import_staged_content):
"""
Does the happy path work?
"""
self.client.login(username="superuser", password="password")
response = self.call_api(self.downstream_video_key)
assert response.status_code == 200
assert mock_sync_from_upstream.call_count == 1
assert mock_import_staged_content.call_count == 1


class DeleteDownstreamSyncViewtest(_DownstreamSyncViewTestMixin, SharedModuleStoreTestCase):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -609,8 +609,8 @@ def _create_block(request):
return JsonResponse({"error": str(exc)}, status=400)
static_file_notices = import_staged_content_for_library_sync(created_block, lib_block, request)
modulestore().update_item(created_block, request.user.id)
response['upstreamRef'] = upstream_ref
response['static_file_notices'] = asdict(static_file_notices)
response["upstreamRef"] = upstream_ref
response["static_file_notices"] = asdict(static_file_notices)

return JsonResponse(response)

Expand Down
33 changes: 14 additions & 19 deletions openedx/core/djangoapps/content_staging/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,18 @@
from xblock.core import XBlock

from openedx.core.lib.xblock_serializer.api import StaticFile, XBlockSerializer
from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_none
from xmodule import block_metadata_utils
from xmodule.contentstore.content import StaticContent
from xmodule.contentstore.django import contentstore

from .data import (
CLIPBOARD_PURPOSE,
LIBRARY_SYNC_PURPOSE,
StagedContentData, StagedContentFileData, StagedContentStatus, UserClipboardData, UserLibrarySyncData,
StagedContentData,
StagedContentFileData,
StagedContentStatus,
UserClipboardData,
UserLibrarySyncData,
)
from .models import (
UserClipboard as _UserClipboard,
Expand All @@ -40,10 +43,7 @@


def _save_xblock_to_staged_content(
block: XBlock,
user_id: int,
purpose: str,
version_num: int | None = None
block: XBlock, user_id: int, purpose: str, version_num: int | None = None
) -> _StagedContent:
"""
Generic function to save an XBlock's OLX to staged content.
Expand Down Expand Up @@ -82,7 +82,7 @@ def _save_xblock_to_staged_content(
)

# Log an event so we can analyze how this feature is used:
log.info(f"Saved {usage_key.block_type} component \"{usage_key}\" to staged content for {purpose}.")
log.info(f'Saved {usage_key.block_type} component "{usage_key}" to staged content for {purpose}.')

# Try to copy the static files. If this fails, we still consider the overall save attempt to have succeeded,
# because intra-course operations will still work fine, and users can manually resolve file issues.
Expand Down Expand Up @@ -162,15 +162,14 @@ def save_xblock_to_user_clipboard(block: XBlock, user_id: int, version_num: int
defaults={
"content": staged_content,
"source_usage_key": usage_key,
}
},
)

return _user_clipboard_model_to_data(clipboard)


def save_xblock_to_user_library_sync(
block: XBlock,
user_id: int,
version_num: int | None = None
block: XBlock, user_id: int, version_num: int | None = None
) -> UserLibrarySyncData:
"""
Save an XBlock's OLX for library sync.
Expand All @@ -184,7 +183,7 @@ def save_xblock_to_user_library_sync(
defaults={
"content": staged_content,
"source_usage_key": usage_key,
}
},
)

return _user_library_sync_model_to_data(sync)
Expand Down Expand Up @@ -251,16 +250,11 @@ def get_user_library_sync_json(user_id: int, request: HttpRequest | None = None)
sync = _UserLibrarySync.objects.get(user_id=user_id)
except _UserLibrarySync.DoesNotExist:
# This user does not have any library sync content.
return {
"content": None,
"source_usage_key": "",
"source_context_title": "",
"source_edit_url": ""
}
return {"content": None, "source_usage_key": "", "source_context_title": "", "source_edit_url": ""}

serializer = _UserLibrarySyncSerializer(
_user_library_sync_model_to_data(sync),
context={'request': request},
context={"request": request},
)
return serializer.data

Expand Down Expand Up @@ -291,6 +285,7 @@ def _user_clipboard_model_to_data(clipboard: _UserClipboard) -> UserClipboardDat
source_context_title=clipboard.get_source_context_title(),
)


def _user_library_sync_model_to_data(sync: _UserLibrarySync) -> UserLibrarySyncData:
"""
Convert a UserLibrarySync model instance to an immutable data object.
Expand Down
8 changes: 5 additions & 3 deletions openedx/core/djangoapps/content_staging/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,16 @@ def source_context_key(self) -> LearningContextKey:
""" Get the context (course/library) that this was copied from """
return self.source_usage_key.context_key


@frozen
class UserLibrarySyncData:
""" Read-only data model for User Library Sync data """
"""Read-only data model for User Library Sync data"""

content: StagedContentData = field(validator=validators.instance_of(StagedContentData))
source_usage_key: UsageKey = field(validator=validators.instance_of(UsageKey))
source_usage_key: UsageKey = field(validator=validators.instance_of(UsageKey)) # type: ignore[type-abstract]
source_context_title: str

@property
def source_context_key(self) -> LearningContextKey:
""" Get the context (course/library) that this was copied from """
"""Get the context (course/library) that this was copied from"""
return self.source_usage_key.context_key
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,33 @@
class Migration(migrations.Migration):

dependencies = [
('auth', '0012_alter_user_first_name_max_length'),
('content_staging', '0005_stagedcontent_version_num'),
("auth", "0012_alter_user_first_name_max_length"),
("content_staging", "0005_stagedcontent_version_num"),
]

operations = [
migrations.CreateModel(
name='UserLibrarySync',
name="UserLibrarySync",
fields=[
('user', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, primary_key=True, serialize=False, to=settings.AUTH_USER_MODEL)),
('source_usage_key', opaque_keys.edx.django.models.UsageKeyField(help_text='Original usage key/ID of the thing that is being synced.', max_length=255)),
('content', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='content_staging.stagedcontent')),
(
"user",
models.OneToOneField(
on_delete=django.db.models.deletion.CASCADE,
primary_key=True,
serialize=False,
to=settings.AUTH_USER_MODEL,
),
),
(
"source_usage_key",
opaque_keys.edx.django.models.UsageKeyField(
help_text="Original usage key/ID of the thing that is being synced.", max_length=255
),
),
(
"content",
models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to="content_staging.stagedcontent"),
),
],
),
]
10 changes: 5 additions & 5 deletions openedx/core/djangoapps/content_staging/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,26 +151,26 @@ class UserLibrarySync(models.Model):
Each user can trigger a sync from a library component to that component in a course.
This model is used to facilitate that and to ease tracking.
"""

user = models.OneToOneField(User, on_delete=models.CASCADE, primary_key=True)
content = models.ForeignKey(StagedContent, on_delete=models.CASCADE)
source_usage_key = UsageKeyField(
max_length=255,
help_text=_("Original usage key/ID of the thing that is being synced."),
)


@property
def source_context_key(self) -> LearningContextKey:
""" Get the context (library) that this was copied from """
"""Get the context (library) that this was copied from"""
return self.source_usage_key.context_key

def get_source_context_title(self) -> str:
""" Get the title of the source context, if any """
"""Get the title of the source context, if any"""
# Just return the ID as the name, since it can only be a library
return str(self.source_context_key)

def clean(self):
""" Check that this model is being used correctly. """
"""Check that this model is being used correctly."""
# These could probably be replaced with constraints in Django 4.1+
if self.user.id != self.content.user.id:
raise ValidationError("User ID mismatch.")
Expand All @@ -180,7 +180,7 @@ def clean(self):
)

def save(self, *args, **kwargs):
""" Save this model instance """
"""Save this model instance"""
# Enforce checks on save:
self.full_clean()
return super().save(*args, **kwargs)
1 change: 1 addition & 0 deletions openedx/core/djangoapps/content_staging/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ class UserLibrarySyncSerializer(serializers.Serializer):
"""
Serializer for the status of the user's library sync (a UserLibrarySyncData instance)
"""

content = StagedContentSerializer(allow_null=True)
source_usage_key = serializers.CharField(allow_blank=True)
# The title of the course that the content came from originally, if relevant
Expand Down

0 comments on commit 3355545

Please sign in to comment.