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

feat: emit verified event for attempt #186

Merged

Conversation

alangsto
Copy link
Member

JIRA: MST-2111

Description: If an attempt's status is updated to verified, emit a verified event.

Merge checklist:

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

Post merge:

  • Delete working branch (if not needed anymore)

@alangsto alangsto force-pushed the alangsto/exam_verified_producer branch 2 times, most recently from dbc11a0 to 5145643 Compare October 2, 2023 17:10
@MichaelRoytman MichaelRoytman force-pushed the michaelroytman/MST-2114-exam-completion-event branch 2 times, most recently from cac99f7 to d324b9d Compare October 2, 2023 18:37
@alangsto alangsto force-pushed the alangsto/exam_verified_producer branch 2 times, most recently from c06ca19 to 844af81 Compare October 2, 2023 18:39
@MichaelRoytman MichaelRoytman force-pushed the michaelroytman/MST-2114-exam-completion-event branch 3 times, most recently from 6a86447 to 267dc5f Compare October 2, 2023 19:24
@alangsto alangsto force-pushed the alangsto/exam_verified_producer branch 4 times, most recently from 8656325 to 4ce91b9 Compare October 2, 2023 20:41
@MichaelRoytman MichaelRoytman force-pushed the michaelroytman/MST-2114-exam-completion-event branch from 267dc5f to 980ba38 Compare October 2, 2023 20:42
@alangsto alangsto force-pushed the alangsto/exam_verified_producer branch from 4ce91b9 to 83e4fa2 Compare October 2, 2023 20:47
@alangsto alangsto force-pushed the alangsto/exam_verified_producer branch from 83e4fa2 to d648410 Compare October 2, 2023 20:48
@alangsto alangsto marked this pull request as ready for review October 2, 2023 20:54
Base automatically changed from michaelroytman/MST-2114-exam-completion-event to MST-1789-downstream-triggers October 2, 2023 21:49
Copy link
Contributor

@zacharis278 zacharis278 left a comment

Choose a reason for hiding this comment

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

surprised the tests are already hitting code inside signals.py somehow. I would have thought we'd need to #nocover that 🤔

@alangsto alangsto merged commit a2dc5a7 into MST-1789-downstream-triggers Oct 3, 2023
7 checks passed
@alangsto alangsto deleted the alangsto/exam_verified_producer branch October 3, 2023 12:48
alangsto added a commit that referenced this pull request Oct 3, 2023
ilee2u pushed a commit that referenced this pull request Oct 6, 2023
* feat: add openedx_events and edx_event_bus_kafka to INSTALLED_APPS

* feat: install confluent_kafka[avro,schema-registry] for use with edx-event-bus-kafka

* feat: add event bus and Kafka Django settings

* feat: make User.full_name field non-nullable

This commit makes the User.full_name field non-nullable to ensure that there is only one representation of the empty value - the empty string. Currently, empty values for this field can be either None or the empty string. Because we're transitioning from a nullable to non-nullable state, a default is required to handle existing rows with a null value.

This is best practice; Django discourages setting both null=True and blank=True on CharField model fields. Furthermore, this was required by our event bus work. If an empty value is represented by None, this causes issues with the event bus, because None is not JSON serializable. Instead of converting a None value to the empty string in the event producer, correcting the the model definition is a better approach.

* feat: upgrade openedx-events to 8.8.0

* feat: emit the EXAM_ATTEMPT_SUBMITTED Open edX event when an exam is submitted

* feat: emit verified event for attempt (#186)

* feat: emit rejected exam signal (#181)

* feat: add errored event producer (#189)

* chore: fix upgrades

* chore: remove confluent-kafka import

* docs: local event bus development

* fix: remove local Kafka settings

This commit removes Django settings from the local settings file, used by the local application server, related to setting up the Kafka implementation of the event bus. This is because the event bus does not work outside of a Docker container. This is because the event bus is run through the devstack networking layer, which is inaccessible by the local application server.

* feat: update topic names for events (#193)

* feat: exam reset producer (#196)

send event to the event bus when an exam attempt is reset

---------

Co-authored-by: michaelroytman <[email protected]>
Co-authored-by: Michael Roytman <[email protected]>
Co-authored-by: Zachary Hancock <[email protected]>
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.

2 participants