Skip to content

fix: contract course asset URLs to /static/ before saving xblock data (LP-704)#377

Open
djoseph-apphelix wants to merge 1 commit into
release-ulmofrom
djoseph/LP-704
Open

fix: contract course asset URLs to /static/ before saving xblock data (LP-704)#377
djoseph-apphelix wants to merge 1 commit into
release-ulmofrom
djoseph/LP-704

Conversation

@djoseph-apphelix

@djoseph-apphelix djoseph-apphelix commented Jul 7, 2026

Copy link
Copy Markdown
Member

Description

Fixes LP-704: Studio bakes course-pinned absolute asset URLs into stored xblock content, breaking JSInput problems and images after a course re-run or export/import.

Jira Ticket

LP-704

Root cause

The authoring read/write round-trip is asymmetric:

  • get_block_info runs replace_static_urls, expanding portable /static/<name> references into absolute asset-v1:<Org>+<Course>+<Run>+... URLs pinned to the current course (cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py).
  • _save_xblock persists the POSTed data verbatim — nothing contracts those URLs back.

The client editor is the only safety net, and not every editor path contracts every attribute (e.g. the course-authoring MFE misses JSInput's html_file). Any save through a non-contracting path permanently bakes the current course's id into storage. After a re-run, those URLs still point at the original course: assets 404 for learners (or silently serve stale content from the old course while it happens to be public).

The fix

  • New contract_static_urls(text, course_id) in common/djangoapps/static_replace/__init__.py — the reverse of replace_static_urls for a course's own assets. It contracts all forms back to /static/<name>:
    • …+type@asset+block@<name> and …+type@asset+block/<name>
    • host-qualified (https://host/asset-v1:…) and protocol-relative
    • versioned (/assets/courseware/v1/<digest>/asset-v1:…)
    • legacy /c4x/<org>/<course>/asset/<name>
    • preserves query strings; leaves other courses' asset URLs and genuine external URLs untouched.
  • _save_xblock now runs string data through contract_static_urls before persisting, so course-pinned URLs are never stored regardless of which editor performed the save.

rewrite_nonportable_content_links (store_utilities.py) was considered for reuse but never matches split-mongo course ids — its pattern is not re.escaped, so the + separators act as regex quantifiers. That is a separate pre-existing bug.

Testing

  • 8 new unit tests in common/djangoapps/static_replace/test/test_static_replace.py covering all URL forms, query-string preservation, foreign-course/external URLs left untouched, and non-string passthrough.
  • 2 new integration tests in cms/djangoapps/contentstore/views/tests/test_block.py: saving data with a baked same-course URL stores /static/<name>; another course's asset URL is preserved verbatim.
  • Verified end to end on devstack: a JSInput problem with a baked html_file URL heals on the next save, and re-saving portable content no longer bakes it.

Screenshots

Before

Screenshot 2026-07-08 at 1 38 19 PM

After the fix

Screenshot 2026-07-08 at 1 43 05 PM

Companion frontend fix: edx/frontend-app-authoring#106

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR prevents Studio from persisting course-pinned (non-portable) absolute asset URLs inside stored XBlock data, by contracting same-course asset URLs back to portable /static/<name> before saving.

Changes:

  • Added contract_static_urls(text, course_id) to reverse replace_static_urls for same-course asset URLs (including versioned, host-qualified, and legacy /c4x/ forms).
  • Updated _save_xblock to contract URLs in string data prior to persisting xblock.data.
  • Added unit and integration tests to verify contraction behavior and that foreign-course URLs are preserved.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
common/djangoapps/static_replace/init.py Adds contract_static_urls implementation for contracting same-course asset URLs back to /static/.
common/djangoapps/static_replace/test/test_static_replace.py Adds unit tests covering contraction across multiple URL forms and passthrough behavior.
cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py Contracts same-course asset URLs in string data before saving XBlock content.
cms/djangoapps/contentstore/views/tests/test_block.py Adds integration tests ensuring saved data is portable and foreign-course URLs remain unchanged.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +263 to +265
if not text or not course_id:
return text

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread common/djangoapps/static_replace/__init__.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants