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.

Sign up for GitHub

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

Jump to bottom

feat: add xapi transformer for exam attempts events #349

Merged
merged 1 commit into from
Sep 19, 2023
Merged

feat: add xapi transformer for exam attempts events #349

merged 1 commit into from
Sep 19, 2023

Conversation

Ian2012
Copy link
Contributor

@Ian2012 Ian2012 commented Sep 8, 2023 •
edited
Loading

Description: This PR add xAPI transformers for the following events:

'edx.special_exam.timed.attempt.created',
'edx.special_exam.timed.attempt.submitted',
'edx.special_exam.practice.attempt.created',
'edx.special_exam.practice.attempt.submitted',
'edx.special_exam.proctored.attempt.created',
'edx.special_exam.proctored.attempt.submitted',

Testing instructions:

  1. Enable timed exams: https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/en/latest/course_features/timed_exams.html#enable-timed-exams
  2. Create a subsection with a problem in it.
  3. Set a subsection to be timed: https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/en/latest/course_features/timed_exams.html#set-a-subsection-to-be-timed
  4. Access as a student to the Subsection and start the attempt.
  5. Verify an xAPI event is produced.
  6. Finish and submit the attempt.
  7. Verify an xAPI event is produced.

Merge checklist:

  • All reviewers approved
  • CI build is green
  • Version bumped
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Commits are squashed

Post merge:

  • Create a tag
  • Check new version is pushed to PyPI after tag-triggered build is
    finished.

Author concerns: List any concerns about this PR - inelegant
solutions, hacks, quick-and-dirty implementations, concerns about
migrations, etc.

@openedx-webhooks
Copy link

openedx-webhooks commented Sep 8, 2023 •
edited
Loading

Thanks for the pull request, @Ian2012! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Sep 8, 2023
@Ian2012 Ian2012 force-pushed the cag/add-exam-events branch 4 times, most recently from 46dd84b to 162ec8d Compare September 8, 2023 20:53
@Ian2012 Ian2012 force-pushed the cag/add-exam-events branch 3 times, most recently from d6c6570 to fe1cebf Compare September 8, 2023 21:37
@Ian2012 Ian2012 marked this pull request as ready for review September 8, 2023 21:37
@Ian2012 Ian2012 force-pushed the cag/add-exam-events branch 4 times, most recently from f8c5424 to ec0051b Compare September 12, 2023 20:23
@Ian2012 Ian2012 force-pushed the cag/add-exam-events branch 4 times, most recently from 63ddbf5 to 2e53f4f Compare September 13, 2023 18:18
bmtcril
bmtcril approved these changes Sep 14, 2023
View reviewed changes
Copy link
Contributor

@bmtcril bmtcril left a comment

Choose a reason for hiding this comment

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

Couple of comments, otherwise lgtm. @ziafazal this make some changes to testing to make debugging easier, do you want to take a look?

docs/event-mapping/Supported_events.rst Outdated Show resolved Hide resolved
event_routing_backends/processors/xapi/event_transformers/exam_events.py Outdated Show resolved Hide resolved
@ziafazal
Copy link
Contributor

@Ian2012 Thanks for adding those events. Overall looks good, However in order to test these could you please provide testing instructions?

@Ian2012
Copy link
Contributor Author

Ian2012 commented Sep 18, 2023

@ziafazal I've updated the PR cover letter to include testing instructions. @bmtcril I've refactored the implementation

@bmtcril
Copy link
Contributor

bmtcril commented Sep 18, 2023

I think this is good to go once the build is green

@Ian2012 Ian2012 force-pushed the cag/add-exam-events branch 4 times, most recently from 70369b0 to 2ca16b1 Compare September 18, 2023 18:52
@Ian2012
Copy link
Contributor Author

Ian2012 commented Sep 18, 2023

@bmtcril ready

@Ian2012
feat: add xapi transformer for exam attempts events
7c23fb4 
fix: add attempt type for exam attempt events

fix: add context data to event attempt events

test: add fixtures for special exams

refactor: use exam object for event attempts

fix: add missing xapi concepts for exam events

fix: use scorm pattern for exam events

test: add fixtures for special exams

fix: remove proctored and timed extensions

fix: add duration extension

fix: do not override get context extensions

feat: include practice and proctored exam events

fix: use attempt-id for attempt events

test: add fixtures for proctored and practice exam events

chore: refactor exam events
@bmtcril bmtcril changed the title feat: add xapi transformer for exam attempts events feat: add xapi transformer for exam attempts events Sep 19, 2023
@bmtcril bmtcril merged commit b1869a0 into master Sep 19, 2023
8 checks passed
@bmtcril bmtcril deleted the cag/add-exam-events branch September 19, 2023 13:05
@openedx-webhooks
Copy link

@Ian2012 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

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

@bmtcril bmtcril bmtcril approved these changes

@ziafazal ziafazal Awaiting requested review from ziafazal

Assignees
No one assigned
Labels
open-source-contribution PR author is not from Axim or 2U
Projects
Contributions
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Ian2012 @openedx-webhooks @ziafazal @bmtcril