There is a singleton SplitMongoModuleStore instance that is returned
whenever we call the ubiquitous modulestore() function (wrapped in a
MixedModuleStore). During initialization, SplitMongoModuleStore sets
up a small handful of XBlock runtime services that are intended to be
shared globally: i18n, fs, cache.
When we get an individual block back from the store using get_item(),
SplitMongoModuleStore creates a SplitModuleStoreRuntime using
SplitMongoModuleStore.create_runtime(). These runtimes are intended to
be modified on a per-item, and later per-user basis (using
prepare_runtime_for_user()).
Prior to this commit, the create_runtime() method was assigning the
globally shared SplitMongoModuleStore.services dict directly to the
newly instantiated SplitModuleStoreRuntime. This meant that even though
each block had its own _services dict, they were all in fact pointing
to the same underlying object. This exposed us to a risk of multiple
threads contaminating each other's SplitModuleStoreRuntime services
when deployed under load in multithreaded mode. We believe this led to
a race condition that caused student submissions to be mis-scored in
some cases.
This commit makes a copy of the SplitMongoModuleStore.services dict for
each SplitModuleStoreRuntime. The baseline global services are still
shared, but other per-item and per-user services are now better
isolated from each other.
This commit also includes a small modification to the PartitionService,
which up until this point had relied on the (incorrect) shared instance
behavior. The details are provided in the comments in the
PartitionService __init__().
It's worth noting that the historical rationale for having a singleton
ModuleStore instance is that the ModuleStore used to be extremely
expensive to initialize. This was because at one point, the init
process required reading entire XML-based courses into memory, or
pre-computing complex field inheritance caches. This is no longer the
case, and SplitMongoModuleStore initialization is in the 1-2 ms range,
with most of that being for PyMongo's connection setup. We should try
to fully remove the global singleton in the Verawood release cycle in
order to make this kind of bug less likely.
refactor: move editor_saved to VideoConfigService (#37829)
* This moves edx-platform-specific logic out of the VideoBlock,
in preparation for the VideoBlock extraction
This is to fix an issue in the following common migration situation:
1. An existing course references content in a legacy content library.
2. The legacy content library is migrated to the new library system.
3. The user clicks on "Update reference" from the Randomized Content
Block in the course.
This action is supposed to update the children of the
LibraryContentBlock (usually ProblemBlocks) so that the "upstream"
attribute is set to point at the UsageKeys of the content in the new
libraries they were migrated to. What was happening instead was that the
upstream entries for these child blocks were left blank, breaking the
upstream/sync connection and making it so that the courses did not
receive any updates from the migrated libraries.
There were two issues:
1. get_forwarding_for_blocks() was being called with the child UsageKeys
in the course, when it should have been called with the v1 library
usage keys instead (since those are the things being forwarded).
2. We were checking that the target_key was a v2 Library key, but really
the upstream target_key is supposed to be a LibraryUsageLocatorV2,
i.e. the key of the specific piece of content, not the library it
ended up in.
Note on testing:
Although there were unit tests for the migration of legacy content
libraries, there were not any unit tests for the migration of legacy
library *blocks*.
This commit adds a minimal test, which would have caught the bug we're
fixing. It would be good to add more comprehensive testing unit testing
for this part of the migration flow.
---------
Co-authored-by: Kyle McCormick <kyle@axim.org>
Adds API to fetch all legacy library content blocks that are ready to be updated to use library v2 and convert to item banks.
Also adds API to update all the references via a user celery task and to fetch its status.
For legacy library_content references in courses, this PR:
- **Removes the spurious sync after updating a reference to a migrated
library**, so that users don't need to "update" their content _after_
updating their reference, _unless_ there were real content edits that
happened since they last synced. We do this by correctly associating a
DraftChangeLogRecord with the ModulestoreBlockSource migration artifact,
and then comparing that version information before offering a sync.
(related issue:
https://github.com/openedx/frontend-app-authoring/issues/2626).
- **Prompts users to update a reference to a migrated library with higher
priority than prompting them to sync legacy content updates for that
reference**, so that users don't end up needing to accept legacy content
updates in order to get a to a point where they can update to V2 content.
- **Ensures the library references in courses always follow the correct
migration,** as defined by the data `forwarded` fields in the data model,
which are populated based on the REST API spec and the stated product UI
requirements.
* For the migration itself, this PR:
- **Allows non-admins to migrate libraries**, fixing:
https://github.com/openedx/edx-platform/issues/37774
- **When triggered via the UI, ensures the migration uses nice title-based
target slugs instead of ugly source-hash-based slugs.** We've had this as an
option for a long time, but preserve_url_slugs defaulted to True instead of
False in the REST API serializer, so we weren't taking advantage of it.
- **Unifies logic between single-source and bulk migration**. These were
implement as two separate code paths, with drift in their implementations. In
particular, the collection update-vs-create-new logic was completely
different for single-souce vs. bulk.
- **When using the Skip or Update strategies for repeats, it consistently
follows mappings established by the latest successful migration** rather than
following mappings across arbitrary previous migrations.
- **We log unexpected exceptions more often**, although there is so much more
room for improvement here.
- **Adds more validation to the REST API** so that client mistakes more often
become 400s with validation messages rather than 500s.
For developers, this PR:
- Adds unit tests to the REST API
- Ensures that all migration business logic now goes through a general-purpose
Python API.
- Ensures that the data model (specifically `forwarded`, and
`change_log_record`) is now populated and respected.
- Adds more type annotations.
This commit implements a comprehensive solution for test score integration in the
enhancement system along with improvements to the score rendering mechanism. Key
changes include:
- Add event handler for rendering blocks with edx-submissions scores
- Implement event-based mechanism to render XBlocks with scoring data
- Create signal handlers in handlers.py to process external grader scores
- Develop specialized XBlock loader for rendering without HTTP requests
- Add queue_key propagation across the submission pipeline
- Register submission URLs in LMS routing configuration
- Add complete docstrings to score render module for better code maintainability
- Add ADR for XBlock rendering with external grader integration
- Add openedx-events fork branch as a dependency in testing.in
- Upgrade edx submission dependency
These changes support the migration from traditional XQueue callback HTTP requests
to a more robust event-based architecture, improving performance and reliability
when processing submission scores. The included ADR documents the architectural
decision and implementation approach for this significant improvement to the
external grading workflow.
This commit fixes accessibility issue for video transcripts as when a
video component's SRT file has an empty line, the transcript has an
empty link that is still interactive.
Empty links should not be interactive and should be hidden from
keyboard users as it takes extra click for them when an empty line
occurs in the transcript.
Co-authored-by: Muhammad Faraz Maqsood <faraz.maqsood@A006-01130.local>
BREAKING CHANGE: All references to the hardcoded 'proctortrack' string have
been removed from the codebase, as well as the `studio.show_review_rules`
waffle flag. These were used to determine whether an escalation email is
required and whether review rules should be shown. These decisions are now made
based on the value of 'requires_escalation_email' (default False) and 'show_review_rules'
(default True) config items in the PROCTORING_BACKENDS entry.
Additionally:
* The proctoring info api will now return the list of providers which require an escalation
email so that frontend-app-learning does not need to use a hardcoded check agaist
the provider name 'proctortrack'.
* Removed translation commands, mock variables and user facing strings that contained
'proctortrack'.
* Updated all test cases that were using proctortrack to use fake providers names.
Part of: https://github.com/openedx/edx-platform/issues/36329
As part of the ongoing effort to deprecate and eventually remove xmodule,
we’ve started gradually migrating the necessary code files from xmodule
to more appropriate locations within the codebase.
Ticket: https://github.com/openedx/public-engineering/issues/445
Also: this tweaks importlinter ignores & add follow-up issue links
Co-authored-by: Kyle McCormick <kyle@axim.org>
* chore: remove branding related code from video_block class as it is stop being used, we have plan to remove the specialized VIDEO_CDN_URL in the next phase as after removing URL nothing will reference the branding code so we're removing it first.
The child.save() call while migrating legacy library block to library version 2 fails with `InvalidScopeError` due to presence of `LmsFieldData` in` _bound_field_data` field. Removing this call does not seem to have any adverse affect.
* Consolidates and renames the runtime used as a base for all the others:
* Before: `xmodule.x_module:DescriptorSystem` and
`xmodule.mako_block:MakoDescriptorSystem`.
* After: `xmodule.x_module:ModuleStoreRuntime`.
* Co-locates and renames the runtimes for importing course OLX:
* Before: `xmodule.x_module:XMLParsingSystem` and
`xmodule.modulestore.xml:ImportSystem`.
* After: `xmodule.modulestore.xml:XMLParsingModuleStoreRuntime` and
`xmodule.modulestore.xml:XMLImportingModuleStoreRuntime`.
* Note: I would have liked to consolidate these, but it would have
involved nontrivial test refactoring.
* Renames the stub Old Mongo runtime:
* Before: `xmodule.modulestore.mongo.base:CachingDescriptorSystem`.
* After: `xmodule.modulestore.mongo.base:OldModuleStoreRuntime`.
* Renames the Split Mongo runtime, the which is what runs courses in LMS and CMS:
* Before: `xmodule.modulestore.split_mongo.caching_descriptor_system:CachingDescriptorSystem`.
* After: `xmodule.modulestore.split_mongo.runtime:SplitModuleStoreRuntime`.
* Renames some of the dummy runtimes used only in unit tests.
This commit migrates the data calculation logic for the GradeSummary
table, which was previously in the frontend-app-learning.
This commit also introduces a new visibility option for assignment
scores: “Never show individual assessment results, but show overall
assessment results after the due date.”
With this option, learners cannot see question-level correctness or
scores at any time. However, once the due date has passed, they can
view their overall score in the total grades section on the Progress
page.
These two changes are coupled with each other because it compromises
the integrity of this data to do the score hiding logic on the front
end.
The corresponding frontend PR is: openedx/frontend-app-learning#1797
- `handle_update_xblock_upstream_link` is called asynchronously with celery. In `update_upstream_downstream_link_handler`, the xblock has the updated version, but when calling `handle_update_xblock_upstream_link` inside Celery, the xblock is outdated in a previous version, which is why the error occurs. This happens because `on_commit_changes_to` is executed before the MySQL transaction ends.
- Added `ImmediateOnCommitMixin` to be used in tests that need to call `on_commit_changes_to`. See https://github.com/openedx/edx-platform/pull/37485#issuecomment-3412979170 for more info
* feat: show item bank ui for migrated legacy library content
* feat: migrate legacy content block to item bank block on view in studio
* fix: duplicate and copy issues
* refactor: migration location and add tests
* fix: lint issues
* fix: item bank and library content children view add button functionality
Newly added blocks from library in children view page of item bank block
and migrated library content block were not displayed automatically.
* fix: lint issues
* fix: lint issues
* feat: only migrate if same version of library is migrated
* refactor: migrate block on request
* fix: component reload on migration
* fix: tests
* refactor: comments and message wordings
* refactor: update alert text
* docs: add context
* fix: component links not being created on migrating legacy blocks
* fix: api docs and types
* refactor: use inheritance and specific parent method call
* fix: imports
* fix: api typing
* fix: upstream_version check
* refactor: rename variables
* refactor: parsing entity keys to usage_keys
Python-evaluated problems were failing to render because they were trying to
invoke `course_id.make_asset_key` in order to obtain the asset key of the
custom Python ZIP file. Learning Core assets work differently (no asset keys,
etc.), and, furthermore, we haven't really thought though how and whether we
want to support custom Python ZIPs in libraries.
So, this fix punts on supporting Python ZIP files in libraries for now, but
enables rendering of advanced CAPA problems which don't rely on a ZIP. This
brings us to parity with Legacy Libraries, which didn't support assets at all
and thus didn't support Python ZIPs either.
Fixes: https://github.com/openedx/edx-platform/issues/37447
And update the studio-view function to raise a NotImplementedError since
we now rely on the authoring environment to provide the edit view of
this block.
Unfortunately, a copy of the template lives in the lms and relies on the
the underlying JS and CSS to render a tiny-mce editor in the instructor
dashboard. This means we can't remove the CSS and JS until we've dropped
the usage of it in the instructor dash. The good news is, that the work
to replace the instructor dash is in progress so those assets should get
cleaned up soon.
The places where the JS and CSS is still being used are:
* lms/templates/instructor/instructor_dashboard_2/instructor_dashboard_2.html
* lms/djangoapps/instructor/views/instructor_dashboard.py
