feat: adds xApiTransforms for completion aggregator events by pomegranited · Pull Request #205 · open-craft/openedx-completion-aggregator

pomegranited · 2024-06-11T07:53:38Z

Description: Describe in a couple of sentence what this PR adds

Defines xAPI event transformers and tests for the aggregator events using the base event-routing-backends transformers.

Based heavily on openedx/event-routing-backends#377 -- thank you @andrey-canon!

Part of openedx/openedx-aspects#222

JIRA: FAL-3766 (private link)

Dependencies:

Installation instructions:

Install the https://github.com/openedx/tutor-contrib-aspects plugin
Add this repo+PR branch to OPENEDX_EXTRA_PIP_REQUIREMENTS.
Add the repo+PR branch for Allow event-routing-backends to be used when transforming non-openedx events openedx/event-routing-backends#431 to OPENEDX_EXTRA_PIP_REQUIREMENTS too -- have to do this otherwise Aspects will install its own version.
Create a course with sections, and subsections, units, and blocks.

Testing instructions:

In the LMS: complete some blocks, units, subsections and sections.
In Superset: login as a superuser, and visit the SQL tab.
You should be able to see progressed and completed events corresponding to chapter, sequential, vertical, and unit objects in the xapi.xapi_events_all_parsed_mv table.

Reviewers:

Merge checklist:

Revert the TEMP commit and update requirements after Allow event-routing-backends to be used when transforming non-openedx events openedx/event-routing-backends#431 merges.
All reviewers approved
CI build is green
Version bumped
Changelog record added
Documentation updated (not only docstrings)
Commits are squashed
PR author is listed in AUTHORS

Post merge:

Create a tag
Check new version is pushed to PyPI after tag-triggered build is
finished.
Delete working branch (if not needed anymore)

completion_aggregator/settings/common.py

use branch from openedx/event-routing-backends#431

* adds dependency on edx-event-routing-backends * adds transformers to xapi.completion and xapi.progress * adds the completion_aggregator events to the event tracking whitelist

and adds CHANGELOG entry.

test settings and plugin settings. The previous commit resulted in the COMPLETION_AGGREGATOR_ASYNC_AGGREGATION flag being flipped for some tests -- it was True in test_settings, but False in plugin_settings() So for these tests, we're now overriding that setting as needed.

…_completion_events

and rearrange INSTALLED_APPS to fix tests.

Uses transforms fixture test utilities from ERB to ensure that raw aggregator events are transformed to xAPI as expected. * test_output/ -- used by ERB to write failed event transforms for debugging * adds 'factory' as a test requirement because ERB tests needs it.

test_settings.py

completion_aggregator/settings/common.py

bmtcril

I am not sure if the changes I've suggested are the correct ones, and I have a suspicion that the version of ERB you're using here is before the settings refactor (but also didn't have time to look) but those changes did get tests to pass for me locally.

…_completion_events

not Score.scaled

Ordinary completion events are simply "progress" events with a recorded progress %, so we should do the same here -- no need for "completed" events.

…_completion_events

Agrendalath

@pomegranited, is there a simple configuration to check the sent events without the tutor plugin? I'm getting the edx.devstack-palm.lms | 2024-06-17 15:50:18,680 INFO 3682 [eventtracking.backends.async_routing] [user 3] [ip 172.29.0.1] async_routing.py:41 - [EventEmissionExit] skipping event openedx.completion_aggregator.completion.vertical every time the event should be sent.

requirements/base.in

requirements/constraints.txt

test_settings.py

completion_aggregator/settings/common.py

completion_aggregator/transformers.py

…tings Initialize EVENT_TRACKING_BACKENDS_ALLOWED_XAPI_EVENTS and EVENT_TRACKING_BACKENDS_ALLOWED_CALIPER_EVENTS if they aren't already, and append our events to them. ERB will do the same to preserve our settings if they're loaded first.

Agrendalath

@pomegranited, the changes look good to me. Could you please expand the readme section from #206 to include a note about the xAPI events and update the changelog (I think we should bump the minor version once again).

👍

I tested this: I can see the xapi_tracking INFO logs with the Successfully processed event "openedx.completion_aggregator.progress.{block_type}" message without any additional configuration on the devstack.
I read through the code
I checked for accessibility issues: n/a
Includes documentation: ⚠️ not yet

pomegranited · 2024-06-21T00:38:59Z

@Agrendalath I've fixed the event-routing-backends requirement (812188c), bumped the package version to 4.2.0 (a4858e5) and added a note about xAPI transforms to the README (3850789).

Thank you so much for your thorough reviews! I'll merge this now.

andrey-canon and others added 5 commits December 4, 2023 17:14

feat: add tracking logs

ead8f61

feat: addressing pr comments

a82984e

Merge branch 'master' into jill/nelc/and/add_tracking_logs

4c791f9

chore: bump edx-completion

d71cffe

fix: use "openedx" as the event prefix instead of "edx"

407a1d1

pomegranited force-pushed the jill/nelc/and/add_completion_events branch 2 times, most recently from 1e6e047 to f886219 Compare June 11, 2024 08:15

pomegranited commented Jun 11, 2024

View reviewed changes

completion_aggregator/settings/common.py Show resolved Hide resolved

pomegranited added 2 commits June 12, 2024 15:26

temp: adds edx-event-routing-backends WIP requirement

4a229fd

use branch from openedx/event-routing-backends#431

feat: adds xApiTransforms for completion aggregator events

7652429

* adds dependency on edx-event-routing-backends * adds transformers to xapi.completion and xapi.progress * adds the completion_aggregator events to the event tracking whitelist

pomegranited force-pushed the jill/nelc/and/add_completion_events branch from f886219 to 7652429 Compare June 12, 2024 06:32

pomegranited added 7 commits June 13, 2024 15:54

chore: bumps version to 4.0.4

bde2bdf

and adds CHANGELOG entry.

test: add call to plugin_settings to fix coverage

abfdb2a

Merge branch 'jill/nelc/and/add_tracking_logs' into jill/nelc/and/add…

bb2f8e6

…_completion_events

fix: call enabled_aggregator_events with a settings arg

e82c49d

and rearrange INSTALLED_APPS to fix tests.

revert: no need to change production settings

a1ea151

bmtcril reviewed Jun 13, 2024

View reviewed changes

test_settings.py Show resolved Hide resolved

bmtcril reviewed Jun 13, 2024

View reviewed changes

test_settings.py Show resolved Hide resolved

bmtcril reviewed Jun 13, 2024

View reviewed changes

completion_aggregator/settings/common.py Show resolved Hide resolved

bmtcril reviewed Jun 13, 2024

View reviewed changes

pomegranited added 8 commits June 14, 2024 08:45

fix: address PR review

4d7d097

fix: CHANGELOG use minor version bump

b862bbc

test: adds test for plugin settings

ab96203

Merge branch 'jill/nelc/and/add_tracking_logs' into jill/nelc/and/add…

fd1cc92

…_completion_events

refactor: moves the xAPI transforms to their own module

e694f16

fix: use Result.extension to send progress %

1204f91

not Score.scaled

refactor: simplify emitted events

29f6b89

Ordinary completion events are simply "progress" events with a recorded progress %, so we should do the same here -- no need for "completed" events.

Merge branch 'jill/nelc/and/add_tracking_logs' into jill/nelc/and/add…

13ea455

…_completion_events

refactor: only need to transform progress events

a142ce2

pomegranited requested review from Agrendalath and bmtcril and removed request for bmtcril June 14, 2024 06:34

pomegranited mentioned this pull request Jun 14, 2024

Allow event-routing-backends to be used when transforming non-openedx events openedx/event-routing-backends#431

Merged

8 tasks

Agrendalath requested changes Jun 17, 2024

View reviewed changes

Base automatically changed from jill/nelc/and/add_tracking_logs to master June 20, 2024 05:52

pomegranited added 5 commits June 20, 2024 15:26

Merge branch 'master' into jill/nelc/and/add_completion_events

f1e6511

chore: bumps edx-event-routing-backends to 9.3.0

812188c

chore: bumps version to 4.2.0

a4858e5

docs: adds note about xAPI to README

3850789

Agrendalath approved these changes Jun 20, 2024

View reviewed changes

pomegranited merged commit 2dfb6e2 into master Jun 21, 2024

pomegranited deleted the jill/nelc/and/add_completion_events branch June 21, 2024 00:42

This was referenced Jun 21, 2024

feat: transforms aggregated completion "progress" events into a new fact open-craft/dbt-completion-aggregator#1

Merged

Plugin: add aggregated completion events openedx/openedx-aspects#222

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: adds xApiTransforms for completion aggregator events#205

feat: adds xApiTransforms for completion aggregator events#205
pomegranited merged 28 commits intomasterfrom
jill/nelc/and/add_completion_events

pomegranited commented Jun 11, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bmtcril left a comment

Uh oh!

Agrendalath left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Agrendalath left a comment

Uh oh!

pomegranited commented Jun 21, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

pomegranited commented Jun 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bmtcril left a comment

Choose a reason for hiding this comment

Uh oh!

Agrendalath left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Agrendalath left a comment

Choose a reason for hiding this comment

Uh oh!

pomegranited commented Jun 21, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pomegranited commented Jun 11, 2024 •

edited

Loading