Skip to content

Conversation

@irtazaakram
Copy link
Member

Summary

This PR fixes the Python and JavaScript (Karma) tests for the newly extracted Problem (CAPA) XBlock.

Context

What Changed

  • Python Tests: Fixed import paths and module references in tests/ to align with the new directory structure.
  • Javascript Tests: Added Karma configuration and Webpack config.
  • Setup: Changed how xblocks-contribute is installed by requiring npm build.

Notes

  • Pylint cleanup and documentation updates are out of scope for this PR and will be addressed in a follow-up contribution.
  • The primary goal of this PR is to get the test suite green to unblock integration.

@irtazaakram irtazaakram changed the title fix: python & javascript tests Fix Python and JavaScript (Karma) tests for Problem Xblock Feb 3, 2026
@irtazaakram irtazaakram changed the title Fix Python and JavaScript (Karma) tests for Problem Xblock Fix Python and JavaScript (Karma) tests for Problem XBlock Feb 3, 2026
@irtazaakram irtazaakram self-assigned this Feb 3, 2026
@irtazaakram irtazaakram moved this to 👀 In review in Aximprovements Team Feb 3, 2026
@irtazaakram irtazaakram force-pushed the fix-problemblock branch 2 times, most recently from 3c433b8 to e0497fd Compare February 3, 2026 14:33
@kdmccormick kdmccormick self-requested a review February 4, 2026 14:16

- name: Build JavaScript bundles
run: |
npm install
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I added the npm run build step because the package requires the bundled JavaScript and CSS files in the public folder for the problem XBlock. These assets aren’t stored in the repo (public is gitignored) and are only supposed to be included in the published PyPi package.

Copy link
Member

@kdmccormick kdmccormick left a comment

Choose a reason for hiding this comment

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

I'm still working through the review, here are my comments so far

Comment on lines +31 to +32
try:
from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag
Copy link
Member

Choose a reason for hiding this comment

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

Instead of bringing this flag in here, I think we should add this function the XQueueService, and then ensure that the service is available in LMS. The implementation in openedx-platform would check the waffle flag, and on the xblocks-contrib side, we would just do something like self.service(self, 'xqueue').use_edx_submissions_for_queue().

If you don't want to do that refactor as part of this PR, that makes sense--it would be good to get this PR merged in soon since it's so big. But I would like to remove this import before we enable the block in openedx-platform.

cc'ing @farhan, as has experience moving methods into XBlock services from the Video extraction work.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just studied XQueueService, it seems like we are going to create new XBlock service in the edx-platform naming 'xqueue` for this waffle flag, is it correct? @kdmccormick

FEATURES = getattr(settings, "FEATURES", {})
except ImproperlyConfigured:
FEATURES = {}
class InheritanceKeyValueStore(KeyValueStore):
Copy link
Contributor

@farhan farhan Feb 9, 2026

Choose a reason for hiding this comment

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

Can we try avoid its definition and use xblock/runtime.py:DictKeyValueStore directly from xblock package as we are not using inherited_settings of InheritanceKeyValueStore in its usage.


return xblock

@property
Copy link
Contributor

@farhan farhan Feb 9, 2026

Choose a reason for hiding this comment

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

We can create a separate mixin like we did in video block here; also avoid usage of name xmodule

f"Unable to create xml for block {self.location}. "
f"Context: '{lines[line - 1][offset - 40 : offset + 40]}'"
)
raise ValueError(msg) from err
Copy link
Contributor

Choose a reason for hiding this comment

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

I think its better to define SerializationError keeping it in sync with source code.

return block

@classmethod
def parse_xml(cls, node, runtime, keys):
Copy link
Contributor

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 👀 In review

Development

Successfully merging this pull request may close these issues.

4 participants