Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi question xapi #352

Merged
merged 9 commits into from
Sep 26, 2023
Merged

Multi question xapi #352

merged 9 commits into from
Sep 26, 2023

Conversation

bmtcril
Copy link
Contributor

@bmtcril bmtcril commented Sep 25, 2023

Note: This is continuing on the work @pomegranited did in #325 . More context is there, this is mostly just a rebase.

Description:

Updates the base event processor and tests to support transforming a single event into multiple events, and
modifies the server-side problem_check xAPI event transformer to transform multi-problem submissions into:

  • 1 event for the problem submission, plus
  • 1 event for each sub-problem

Resolves #219

JIRA: FAL-3430 (OpenCraft internal link)

Testing instructions:

This change should be covered by tests, but to test it manually:

  • Install this branch on your Tutor LMS
  • Login, and submit various responses to the Multiple Choice Questions block in the Demo course, or create your own.
  • Watch multiple xAPI events flow through into Aspects from a single submission.

Author concerns:

  • Suggest reviewing this commit by commit to understand the reasoning.
  • This change only affects xAPI, not Caliper events.
  • Some minor changes to single question problem_check events occurred: added object.definition.name and interactionType.

Expected fixtures record the current behavior
Test was failing locally because test4.com is a real URL that responded
with a 200 when we posted events to it.

Not sure why it was succeeding in CI?
Updates the event processor change to allow for the possibility that
some event transformers may generate multiple events from a single
event, and these events need to be carried down the processor chain.
…mation

This reduces the side-effects of event transformation, which will allow
us to re-use base event transformation methods when generating multiple
events from a single source event.

* Removes the BaseTransformerMixin.transformed_event instance variable
  in favor of passing an event through to base_transform() to be
  modified and returned.
* Adds BaseTransformerMixin.get_object() so that caliper events don't
  need to reference self.transformed_event when updating the object data.
* Adds BaseTransformerMixin.get_extensions() so that caliper events can
  don't have to hack in their transformerVersion during
  BaseTransformerMixin.transform()
Single-question problem_check events still only produce one xAPI event.

Changes to the top-level multi-question problem_check event data:

* object.type changed from Activity to GroupActivity
* object.id shows the base problem usage_key
* object.definition.interaction_type is "other"

New events emitted for each child problem are identical the top-level event,
except for:

* object.type is Activity
* object.id shows the base problem usage_key including the child usage string
* object.definition.interaction_type is determined by the child problem response_type
* result.score is omitted -- only relevant to the parent problem
* result.response is provided, pulled from the child question submission
* result.success is provided, pulled from the child question submission

Related fixes to all problem_check events:

* object.definition.name now shows the problem display_name
* object.id now uses shows the problem usage_key
* result.score max and raw are now always provided if present in the
  source event (bug fix)
* fixes "AttributeError: 'list' object has no attribute 'to_json'"
  during event processing for one-to-many problem_check events
* adds test for ^
* reverts unneeded change to test_caliper to reinstate 100% test coverage
* stops mocking uuid5 during tests
  uuid5 generates the same UUID when provided with the same namespace + name,
  and so we can rely on this remaining the same in the expected fixture files.
* updates test data to use the actual uuid5s generated for the given input events.
* pulls event ID generation into get_event_id(),
  taking care not to modify how parent event IDs are generated.
* overrides get_event_id() for child events by using the child_id as
  part of the UUID namespace_key.
* updates tests to check that child events and their parent event use
  different event IDs, and the affected problem_check multiple-question
  test data
@bmtcril
Copy link
Contributor Author

bmtcril commented Sep 25, 2023

@ziafazal last chance if you want to review this beast of a PR. :)

@ziafazal
Copy link
Contributor

@bmtcril tested locally LGTM.

@bmtcril bmtcril merged commit 99ede4e into master Sep 26, 2023
8 checks passed
@bmtcril bmtcril deleted the multi-question-xapi branch September 26, 2023 12:58
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.

Multi-question submission emits only a single xAPI statement
4 participants