[FEATURE] [MER-4106] Check certification worker #5438
Conversation
…ay trigger a certification check worker
…lated to course section certification
| if certificate.requires_instructor_approval do | ||
| {:ok, :send_email_to_instructor} | ||
| else |
There was a problem hiding this comment.
Should we also create the granted certificate (with a :pending state)? or do you think we should create the GC later?
There was a problem hiding this comment.
Good catch. I’ve changed the logic here. Now, we create the certificate and determine which entity the email should be sent to.
| @impl Oban.Worker | ||
| def perform(%Oban.Job{args: %{"user_id" => user_id, "section_id" => section_id}}) do | ||
| GrantedCertificates.has_qualified(user_id, section_id) | ||
|
|
There was a problem hiding this comment.
we might add a TODO here to explain that:
- if we get a
{:ok, :send_email_to_instructor}we want to trigger that process. - if an
:earnedGC was created/updated (I believe is the{:ok, granted_cert}case) we need to trigger a student email notification (this case corresponds to courses that do not require instructor approval)
If we already have a mer ticket, we can add it's number as part of the "TODO" message.
UPDATE:
Re-reading the ticket's pseudo-code, I understand we are in this worker:
CHECK_CERT (student, section, cert_details)
case TRANSITION_CERT(student, section, cert_details) do
{:ok, :no_change} -> do nothing
{:ok, :notify_instructor} -> Send email to instructor for approval (I believe all instructors must be notified)
{:ok, :notify_student} -> Spawn Async BUILD_CERT_FOR_STUDENT job
end
end
The third clause should be handled by this one:
BUILD_CERT_FOR_STUDENT routine (student_id, cert_id):
case GENERATE_AND_STORE_CERT(student_id, cert_id) do
{:ok, granted_cert_details} -> Send email to student
{:error, e} -> {:error}
end
end
And the GENERATE_AND_STORE_CERT is already built (lib/oli/delivery/sections/certificates/workers/generate_pdf.ex)
There was a problem hiding this comment.
I made some modifications to the code to address the notification part. I also added references in the description to indicate where this work should be connected to the emailing functionality
| current_percentage = | ||
| from(ra in ResourceAccess, | ||
| where: ra.section_id == ^section_id, | ||
| where: ra.user_id == ^user_id, | ||
| where: ra.resource_id in ^required_assignment_ids, | ||
| select: | ||
| fragment( | ||
| "COALESCE(SUM(? / ? * 100) / ?, 0.0)", | ||
| ra.score, | ||
| ra.out_of, | ||
| ^total_required_assignments | ||
| ) | ||
| ) | ||
| |> Oli.Repo.one() | ||
|
|
||
| distinction_result = | ||
| if current_percentage >= min_percentage_for_distinction, | ||
| do: :passed_min_percentage_for_distinction, | ||
| else: :failed_min_percentage_for_distinction |
There was a problem hiding this comment.
This logic checks if the average score of the required assignments exceeds the required threshold. However, according to the Figma design we should check that all the pages (individually) exceed that required value.
| current_percentage = | ||
| from(ra in ResourceAccess, | ||
| where: ra.section_id == ^section_id, | ||
| where: ra.user_id == ^user_id, | ||
| where: ra.resource_id in ^required_assignment_ids, | ||
| select: | ||
| fragment( | ||
| "COALESCE(SUM(? / ? * 100) / ?, 0.0)", | ||
| ra.score, | ||
| ra.out_of, | ||
| ^total_required_assignments | ||
| ) | ||
| ) | ||
| |> Oli.Repo.one() | ||
|
|
||
| completion_result = | ||
| if current_percentage >= min_percentage_for_completion, | ||
| do: :passed_min_percentage_for_completion, | ||
| else: :failed_min_percentage_for_completion | ||
|
|
||
| distinction_result = | ||
| if current_percentage >= min_percentage_for_distinction, | ||
| do: :passed_min_percentage_for_distinction, | ||
| else: :failed_min_percentage_for_distinction |
Ticket: MER-4106
Parent ticket (description is here): MER-3796
Things to consider when reviewing this PR:
Commit 5579bac indicates the places where a student may trigger the threshold check:
graded.ex,discussions_live.ex, andlesson_live.ex.The conditions that may trigger the threshold checks are indicated in
certification_eligibility.ex(see commit 6c00c0d)The LV plug
set_require_certification_check.exprevents theCheckCertificationworker from running if either of the following conditions are met:The section has certificates disabled (false).
The student has already earned a certificate with distinction.
In these cases, a flag is added to stop the checks. (see here)
The workflow described in the ticket can be followed using the
has_qualifiedfunction indelivery/granted_certificates.ex. This function returns tuples that indicate the appropriate action: do nothing, notify the instructor, notify the student, or create a certificate.NOTE:
Two atoms are used in the following functions:
Functions:
create_granted_cert_maybe_spawn_builder– riggered when a student earns either a completion or a distinction certificate.update_granted_certificate_with_distinction– Triggered when a student earns a distinction certificate.These atoms serve as references for where the email-sending logic should be implemented.
The sending email work is part of MER-4107. @manelli