feat: exam reset producer #196
Conversation
| """ | ||
| get_producer().send( | ||
| signal=EXAM_ATTEMPT_RESET, | ||
| topic='exam-attempt-reset', |
There was a problem hiding this comment.
I think we want to use the learning-exam-attempt-lifecycle topic that Alie created in https://github.com/edx/terraform/pull/6144. Maybe we can merge in #193 first and you can rebase?
There was a problem hiding this comment.
yeah I can wait on this and grab that value after it's merged
There was a problem hiding this comment.
Sounds good. Alie's pull request was merged, so I can re-review once you've rebased.
| self.assertFalse(ExamAttempt.objects.filter(id=self.exam_attempt.id).exists()) | ||
|
|
||
| @patch('edx_exams.apps.core.signals.signals.EXAM_ATTEMPT_RESET.send_event') | ||
| def test_event_emitted(self, mock_event_send): |
There was a problem hiding this comment.
Can this be worked into test_attempt_event_emitted in this file? Here it is on the feature branch.
There was a problem hiding this comment.
I think this one is still separate because a reset doesn't go through the useual 'update_attempt_status' flow
edx_exams/apps/core/api.py
Outdated
| @@ -142,6 +143,23 @@ def update_attempt_status(attempt_id, to_status): | |||
| return attempt_id | |||
|
|
|||
|
|
|||
| def delete_exam_attempt(attempt, requesting_user): | |||
| """ | |||
| Delete or 'reset' an exam attempt | |||
There was a problem hiding this comment.
[question] Is there a reason why both delete and reset are used? Might be missing some context here, but wondering if this could be confusing. But if there is a reason to keep both delete and reset, let me know! Thanks.
There was a problem hiding this comment.
We often refer to a delete as 'reset' so that's why I included it. That said, I think you have a point here. This doesn't just delete and it also triggers other things on platform. It would be better to just rename this to reset_exam_attempt
There was a problem hiding this comment.
Thanks for clarifying! And sounds good.
zacharis278
force-pushed
the
zhancock/reset-producer
branch
from
c883f85 to
00891d6
Compare
| """ | ||
| course_key = CourseKey.from_string(attempt.exam.course_id) | ||
| usage_key = UsageKey.from_string(attempt.exam.content_id) | ||
|
|
There was a problem hiding this comment.
Should we add a log here, similar to the one we have in edx-proctoring?
There was a problem hiding this comment.
I think we have a few things to track this that we didn't have when that was written. There's a distinct API request for deletes and we also keep track of changes with the simple history library.
edit: actually I'm just going to add this anyway cuz we do a log on update
* 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]>
JIRA: MST-2113
Description: Send event to the event bus when an exam attempt is reset