Events for exams IDA #268
Events for exams IDA #268
Conversation
ilee2u
force-pushed
the
exams-ida-hackathon
branch
from
423b066
to
6424a40
Compare
ilee2u
changed the title
openedx_events/learning/data.py
Outdated
| credit_requirement_status (str): status to change the student's CreditRequirement to | ||
| """ | ||
|
|
||
| user_id = attr.ib(type=int) |
There was a problem hiding this comment.
I noticed @MichaelRoytman's PR uses the UserData object here. Do you think we should use that? The LMS can look it up but it might be nice to match how other events are sending user id / email / etc
There was a problem hiding this comment.
Looking at the parameters of the downstream functions, it looks like most of them take user_id as a parameter anyways, so I think user_id is the way to go here.
Also I believe Michael's PR just has placeholder data for now.
There was a problem hiding this comment.
I think it would make sense to use the UserData. It seems to be more common to use that than a user_id. But I do see uses of both, so there's precedence to use both.
I don't think we have to model our events off the way they're being consumed now. Ideally, they shouldn't be so context aware. I think, for me, it's less, "the Instructor Service currently only expects a user_id so that's what we should put in the event" and more "how can I best and most generically represent the concept of a user in my event"?
Is sending PII when we don't need to a consideration?
There was a problem hiding this comment.
I think there's a good chance future downstream logic would use the pii. This probably gets hooked up to name affirmation at some point.
openedx_events/learning/signals.py
Outdated
|
|
||
| # .. event_type: org.openedx.learning.exam.attempt.submitted.v1 | ||
| # .. event_name: EXAM_ATTEMPT_SUBMITTED | ||
| # .. event_description: Emitted when an exam attempt is submitted by a learner. |
There was a problem hiding this comment.
Some high-level questions:
- Does the platform have other types of "exams" that would not trigger these events? If so, how do we differentiate? Note that an event is defined by its event trigger + event data.
- For the exams covered by these events, are there multiple exam types? Would it be useful to know that as part of the event data?
- If there are different exam types, do some of these events only apply to certain types of exam types? If so, would that be useful to note?
There was a problem hiding this comment.
- kind of. The existing exam system (edx-proctoring) does not trigger these events, the service that emits these is intended to replacement. There are no other types of exams outside that.
- Good point. There are multiple exam types and edx-platform is mostly unaware of this since all the state/ui flows are manage by the exams/proctoring service. That said, I think there is value including this. Consumers may likely care if an exam is proctored or not. We might have actually missed a need for this. I think Credits depend on if an exam is proctored.
- Yup some of these events are impossible depending on the exam type. A timed exam cannot be rejected since there is no video review to reject for example. We should add those details to the event descriptions
There was a problem hiding this comment.
Are there non-timed exams, and are those included in this as well? Or are these just timed-exams, proctored and not-proctored? Just curious.
There was a problem hiding this comment.
🤔 I guess this doesn't include 'all' exams just 'special exams' which have a different UX and workflow. Right now, all special exam types are a subset of timed. This wouldn't include normal sections instructors are using as or are graded as exams however. We could possibly reintroduce the term 'special exam' for these events which could clear this up.
There was a problem hiding this comment.
If you think of this more as an API discussion around exams and special-exams, who do you think should be a part of this discussion? Maybe https://openedx.atlassian.net/wiki/spaces/COMM/pages/3554214289/Arch+Coordination+-+How+to+announce+and+tune-in will help you get the right discussion going?
There was a problem hiding this comment.
^ I think this is a good idea to have before we merge. I could see people getting confused on two points
- these events only pertain to 'special' exams
- these events are not published by the existing exams system
There was a problem hiding this comment.
I've made the following changes to address this:
- Changed the data name to "SpecialExamAttemptData"
- Added a comment about how events using this data are only emitted for special exams from the newer exams backend, edx-exams
There was a problem hiding this comment.
That's better! Thank you folks.
There was a problem hiding this comment.
I don't want to push hard on this, because it's not my domain of expertise, but the naming difference between EXAM_ATTEMPT_SUBMITTED and SpecialExamAttemptData just catches my attention. The obvious other option is to rename to SPECIAL_EXAM_ATTEMPT_SUBMITTED , and I guess the question I have is around the pros/cons of going with the more specific name.
There was a problem hiding this comment.
🤔 🤔 It's a good question, I'd almost lean towards not using 'special' in the name at all and just commenting to the meaning? Otherwise, we need to consistently use the term across Open edX. I don't think 'special exam' has any additional meaning to anyone working outside this domain unless it's described. There was a actually discussion a while back that we stop using this term because it's just confusing, trying to dig that up.
I'm not sure how to best describe the fact that this intends to include all content that is treated with an exam user experience, but not content that might be titled (on the section or grading policy) by course teams as 'exam'. The latter is ultimately no different from other content, the platform doesn't do anything functionally different. (hence not special)
ilee2u
force-pushed
the
exams-ida-hackathon
branch
from
37dd373
to
7e5cabd
Compare
|
Hi, we've been informed that v8.6.0, which this PR is based off, is having issues, so please do not merge this until we find a solution/workaround for those issues. |
mariajgrimaldi
added
the
blocked by other work
ilee2u
force-pushed
the
exams-ida-hackathon
branch
2 times, most recently
from
a360649
to
ca3ff5e
Compare
| Decision | ||
| ******** | ||
|
|
||
| * Since `edx-exams` is independent, we will want to prevent circular dependencies between `edx-platform` and `edx-exams`. |
There was a problem hiding this comment.
I don't think this is a decision that impacts this events library and is better explained in detail by the ADR you already linked above.
|
|
||
| * Since `edx-exams` is independent, we will want to prevent circular dependencies between `edx-platform` and `edx-exams`. | ||
| * We have defined the events that we plan to define in `this ADR <https://github.com/edx/edx-exams/blob/main/docs/decisions/0004-downstream-effect-events.rst>`_ in `edx-exams` to use the event bus, as the new exams backend is independent in this ADR | ||
| * We plan for these events to be produced in `edx-exams`, and consumed in various `edx-platform` services (e.g. certificates, credit, instructor, grades). |
There was a problem hiding this comment.
I think we can flip our focus here to be less about edx-exams and more how these events impact edx-platform. Maybe
- exam events will be consumed by
edx-platformto handle all behavior as it pertains to the state of an exam subsection. - the edx-exams service will produce these events
- there is no plan to have edx-proctoring produce these events
|
|
||
| **Draft** 2023-09-29 | ||
|
|
||
| Context |
There was a problem hiding this comment.
I think the context section might be a good place to explain the meaning of exam / special exam.
Rough points to hit
- course subsections with an
exam_typehave additional logic that governs completion, grading, credit requirements, and more based on the type of exam. - these sections are sometimes referred to as special exams
- course subsections without an exam type configured may still have a grading policy named 'Exam'. This type of content does not have the exam user experience and is not governed by any exam specific logic.
- Also changed user_id to student_user
- Specified that the events are only emitted when special exams go through edx-exams. - Re-ordered params in data docstring
- I literally forgot a period oi
ilee2u
force-pushed
the
exams-ida-hackathon
branch
from
273f27a
to
9e4744c
Compare
docs/decisions/0013-special-exam-submission-and-review-events.rst
Outdated
Show resolved
Hide resolved
mariajgrimaldi
removed
the
blocked by other work
|
Hi @ilee2u! This PR introduced an ADR with the status draft. Do you think we can change the ADR's status to accepted by now? |
Hi again, yes we should be able to! Thanks :) |
Description: We are developing a new backend for exams called edx-exams. This backend will be utilized by instructor dashboards to interact with student exam attempts, and will store data regarding those attempts.
We are currently working to implement the downstream effects triggered whenever an exam attempt is submitted or reviewed. For example, when an exam attempt is submitted, we will want to make sure edx-platform knows to mark the exam subsection.
We have added several event definitions based on different actions that can be performed on exam attempts:
These events will be consumed by various services in edx-platform in order to perform tasks such as creating grade overrides, invalidating certificates, changing credit requirement statuses, and more.
ISSUE: MST-2083 (Internal)
Dependencies: None.
Merge deadline: List merge deadline (if any)
Installation instructions: List any non-trivial installation
instructions.
Testing instructions:
Reviewers:
Merge checklist:
Post merge:
finished.
Author concerns: List any concerns about this PR - inelegant
solutions, hacks, quick-and-dirty implementations, concerns about
migrations, etc.