diff --git a/.github/workflows/moodle-ci.yml b/.github/workflows/moodle-ci.yml index b66da86..9a8200e 100644 --- a/.github/workflows/moodle-ci.yml +++ b/.github/workflows/moodle-ci.yml @@ -10,7 +10,7 @@ jobs: services: postgres: - image: postgres:13 + image: postgres:14 env: POSTGRES_USER: 'postgres' POSTGRES_HOST_AUTH_METHOD: 'trust' @@ -48,6 +48,9 @@ jobs: - php: '8.1' moodle-branch: 'MOODLE_404_STABLE' database: 'mariadb' + - php: '8.1' + moodle-branch: 'MOODLE_405_STABLE' + database: 'mariadb' steps: - name: Check out repository code diff --git a/classes/assessment/activity.php b/classes/assessment/activity.php index 3f2fab0..0f2ac47 100644 --- a/classes/assessment/activity.php +++ b/classes/assessment/activity.php @@ -173,6 +173,28 @@ public function get_grade_items(): array { return !(empty($gradeitems)) ? $gradeitems : []; } + /** + * Delete all SORA overrides for the assessment. + * + * @return void + * @throws \coding_exception + * @throws \dml_exception + */ + public function delete_all_sora_overrides(): void { + global $CFG; + require_once($CFG->dirroot . '/group/lib.php'); + + // Find all the sora group overrides for the assessment. + $overrides = $this->get_assessment_sora_overrides(); + + // Delete all sora groups of that assessment from the course. + if (!empty($overrides)) { + foreach ($overrides as $override) { + groups_delete_group($override->groupid); + } + } + } + /** * Set the module instance. * @return void @@ -221,4 +243,37 @@ protected function get_gradeable_enrolled_users_with_capability(string $capabili return in_array($u->id, $gradeableids); }); } + + /** + * Remove user from previous SORA groups of the assessment. + * When a user is added to a new group, they should be removed from all other SORA groups of the assessment. + * + * @param int $userid User ID. + * @param int|null $excludedgroupid Group ID to exclude. This is the group that the user is being added to. + * @return void + */ + protected function remove_user_from_previous_sora_groups(int $userid, ?int $excludedgroupid = null): void { + // Get all the sora group overrides. + $overrides = $this->get_assessment_sora_overrides(); + + // No overrides to remove. + if (empty($overrides)) { + return; + } + + foreach ($overrides as $override) { + // Skip the excluded group. + if ($excludedgroupid && $override->groupid == $excludedgroupid) { + continue; + } + + // Remove the user from the previous SORA group. + groups_remove_member($override->groupid, $userid); + + // Delete the group if it is empty. + if (empty(groups_get_members($override->groupid))) { + groups_delete_group($override->groupid); + } + } + } } diff --git a/classes/assessment/assessment.php b/classes/assessment/assessment.php index 62cb975..7548db0 100644 --- a/classes/assessment/assessment.php +++ b/classes/assessment/assessment.php @@ -75,15 +75,17 @@ public function apply_extension(extension $extension): void { if ($extension instanceof ec) { $this->apply_ec_extension($extension); } else if ($extension instanceof sora) { - // Skip SORA overrides if the assessment is not an exam. - if (!$this->is_exam()) { - return; - } // Skip SORA overrides if the end date of the assessment is in the past. if ($this->get_end_date() < time()) { return; } + // Remove user from all SORA groups in this assessment. + if ($extension->get_time_extension() == 0) { + $this->remove_user_from_previous_sora_groups($extension->get_userid()); + return; + } + $this->apply_sora_extension($extension); } } @@ -208,15 +210,6 @@ public function get_module_name(): string { return ''; } - /** - * Check if the assessment is an exam. Override in child class if needed. - * - * @return bool - */ - public function is_exam(): bool { - return false; - } - /** * Check if the assessment is valid for marks transfer. * @@ -264,13 +257,13 @@ public function is_valid_for_extension(): \stdClass { } /** - * Delete SORA override for a Moodle assessment. + * Delete all SORA override for a Moodle assessment. + * It is used to delete all SORA overrides for an assessment when the mapping is removed. * - * @param array $groupids Default SORA overrides group ids in the course. * @return void * @throws \moodle_exception */ - public function delete_sora_overrides(array $groupids): void { + public function delete_all_sora_overrides(): void { // Default not supported. Override in child class if needed. throw new \moodle_exception('error:soraextensionnotsupported', 'local_sitsgradepush'); } diff --git a/classes/assessment/assign.php b/classes/assessment/assign.php index 779a43b..cefeb78 100644 --- a/classes/assessment/assign.php +++ b/classes/assessment/assign.php @@ -67,57 +67,6 @@ public function get_end_date(): ?int { return $this->sourceinstance->duedate; } - /** - * Check if this assignment is an exam. - * - * @return bool - */ - public function is_exam(): bool { - $start = $this->get_start_date(); - $end = $this->get_end_date(); - - if ($start && $end) { - $duration = $end - $start; - return $duration > 0 && $duration <= HOURSECS * 5; - } - - return false; - } - - /** - * Delete SORA override for the assignment. - * - * @param array $groupids Default SORA overrides group ids in the course. - * @return void - * @throws \coding_exception - * @throws \dml_exception - */ - public function delete_sora_overrides(array $groupids): void { - global $CFG, $DB; - require_once($CFG->dirroot . '/mod/assign/locallib.php'); - - // Skip if group ids are empty. - if (empty($groupids)) { - return; - } - - // Find all group overrides for the assignment having the default SORA overrides group ids. - [$insql, $params] = $DB->get_in_or_equal($groupids, SQL_PARAMS_NAMED); - $params['assignid'] = $this->sourceinstance->id; - $sql = "SELECT id FROM {assign_overrides} WHERE assignid = :assignid AND groupid $insql AND userid IS NULL"; - - $overrides = $DB->get_records_sql($sql, $params); - - if (empty($overrides)) { - return; - } - - $assign = new \assign($this->context, $this->get_course_module(), null); - foreach ($overrides as $override) { - $assign->delete_override($override->id); - } - } - /** * Apply EC extension to the assessment. * @@ -150,24 +99,51 @@ protected function apply_sora_extension(sora $sora): void { global $CFG; require_once($CFG->dirroot . '/group/lib.php'); - // Get time extension in seconds. - $timeextensionperhour = $sora->get_time_extension(); - // Calculate the new due date. - // Find the difference between the start and end date in hours. Multiply by the time extension per hour. - $actualextension = (($this->get_end_date() - $this->get_start_date()) / HOURSECS) * $timeextensionperhour; - $newduedate = $this->get_end_date() + round($actualextension); + $newduedate = $this->get_end_date() + $sora->get_time_extension(); + + // Total extension in minutes. + $totalminutes = round($sora->get_time_extension() / MINSECS); // Get the group id, create if it doesn't exist and add the user to the group. - $groupid = $sora->get_sora_group_id($this->get_course_id(), $sora->get_userid()); + $groupid = $sora->get_sora_group_id( + $this->get_course_id(), + $this->get_coursemodule_id(), + $sora->get_userid(), + $totalminutes + ); if (!$groupid) { throw new \moodle_exception('error:cannotgetsoragroupid', 'local_sitsgradepush'); } + // Remove the user from the previous SORA groups. + $this->remove_user_from_previous_sora_groups($sora->get_userid(), $groupid); $this->overrides_due_date($newduedate, $sora->get_userid(), $groupid); } + /** + * Get all SORA group overrides for the assignment. + * + * @return array + * @throws \dml_exception + */ + protected function get_assessment_sora_overrides(): array { + global $DB; + // Find all the group overrides for the assignment. + $sql = 'SELECT ao.* FROM {assign_overrides} ao + JOIN {groups} g ON ao.groupid = g.id + WHERE ao.assignid = :assignid AND ao.userid IS NULL AND g.name LIKE :name'; + + $params = [ + 'assignid' => $this->sourceinstance->id, + 'name' => sora::SORA_GROUP_PREFIX . '%', + ]; + + // Get all sora group overrides. + return $DB->get_records_sql($sql, $params); + } + /** * Overrides the due date for the user or group. * diff --git a/classes/assessment/quiz.php b/classes/assessment/quiz.php index 1fc94ff..2ebce2f 100644 --- a/classes/assessment/quiz.php +++ b/classes/assessment/quiz.php @@ -67,48 +67,6 @@ public function get_end_date(): ?int { return $this->get_source_instance()->timeclose; } - /** - * Check if this quiz is an exam. - * - * @return bool - */ - public function is_exam(): bool { - $originaltimelimit = $this->get_source_instance()->timelimit; - return $originaltimelimit > 0 && $originaltimelimit <= HOURMINS * 5; - } - - /** - * Delete SORA override for the quiz. - * - * @param array $groupids Default SORA overrides group ids in the course. - * @return void - * @throws \coding_exception - * @throws \dml_exception - */ - public function delete_sora_overrides(array $groupids): void { - global $CFG, $DB; - require_once($CFG->dirroot . '/mod/assign/locallib.php'); - - // Skip if group ids are empty. - if (empty($groupids)) { - return; - } - - // Find all group overrides for the quiz having the default SORA overrides group ids. - [$insql, $params] = $DB->get_in_or_equal($groupids, SQL_PARAMS_NAMED); - $params['quizid'] = $this->sourceinstance->id; - $sql = "SELECT * FROM {quiz_overrides} WHERE quiz = :quizid AND groupid $insql AND userid IS NULL"; - - $overrides = $DB->get_records_sql($sql, $params); - - if (empty($overrides)) { - return; - } - - // Delete the overrides. - $this->get_override_manager()->delete_overrides($overrides); - } - /** * Apply EC extension to the quiz. * @@ -136,15 +94,19 @@ protected function apply_ec_extension(ec $ec): void { protected function apply_sora_extension(sora $sora): void { global $DB; - // Get extra time from SORA. - $timeextension = $sora->get_time_extension(); + // Calculate the time close for this sora override. + $newtimeclose = $this->get_end_date() + $sora->get_time_extension(); - // Calculate the new time limit. - $originaltimelimit = $this->get_source_instance()->timelimit; - $newtimelimit = $originaltimelimit + (($originaltimelimit / HOURSECS) * $timeextension); + // Total extension in minutes. + $totalminutes = round($sora->get_time_extension() / MINSECS); // Get the group id. - $groupid = $sora->get_sora_group_id($this->get_course_id(), $sora->get_userid()); + $groupid = $sora->get_sora_group_id( + $this->get_course_id(), + $this->get_coursemodule_id(), + $sora->get_userid(), + $totalminutes + ); if (!$groupid) { throw new \moodle_exception('error:cannotgetsoragroupid', 'local_sitsgradepush'); @@ -153,7 +115,7 @@ protected function apply_sora_extension(sora $sora): void { $overridedata = [ 'quiz' => $this->get_source_instance()->id, 'groupid' => $groupid, - 'timelimit' => round($newtimelimit), + 'timeclose' => $newtimeclose, ]; // Get the override record if it exists. @@ -174,6 +136,28 @@ protected function apply_sora_extension(sora $sora): void { $this->get_override_manager()->save_override($overridedata); } + /** + * Get all SORA group overrides for the quiz. + * + * @return array + * @throws \dml_exception + */ + protected function get_assessment_sora_overrides() { + global $DB; + // Find all the group overrides for the quiz. + $sql = 'SELECT qo.* FROM {quiz_overrides} qo + JOIN {groups} g ON qo.groupid = g.id + WHERE qo.quiz = :quizid AND qo.userid IS NULL AND g.name LIKE :name'; + + $params = [ + 'quizid' => $this->sourceinstance->id, + 'name' => sora::SORA_GROUP_PREFIX . '%', + ]; + + // Get all the group overrides except the excluded group. + return $DB->get_records_sql($sql, $params); + } + /** * Get the quiz override manager. * diff --git a/classes/extension/aws_queue_processor.php b/classes/extension/aws_queue_processor.php index cfc02db..43d1caf 100644 --- a/classes/extension/aws_queue_processor.php +++ b/classes/extension/aws_queue_processor.php @@ -53,6 +53,9 @@ abstract class aws_queue_processor { /** @var int Maximum execution time in seconds */ const MAX_EXECUTION_TIME = 1800; // 30 minutes + /** @var int Maximum number of attempts */ + const MAX_ATTEMPTS = 2; + /** * Get the queue URL. * @@ -68,6 +71,13 @@ abstract protected function get_queue_url(): string; */ abstract protected function process_message(array $messagebody): void; + /** + * Get the queue name. + * + * @return string + */ + abstract protected function get_queue_name(): string; + /** * Fetch messages from the queue. * @@ -93,20 +103,28 @@ protected function fetch_messages( } /** - * Check if message is already processed. + * Check should we process the message. * * @param string $messageid AWS SQS Message ID * @return bool True if message is processed already, false otherwise * @throws \dml_exception */ - protected function is_processed_message(string $messageid): bool { + protected function should_not_process_message(string $messageid): bool { global $DB; try { - // Allow processing if message has not been processed successfully. - return $DB->record_exists( - 'local_sitsgradepush_aws_log', - ['messageid' => $messageid, 'status' => self::STATUS_PROCESSED] + $sql = 'SELECT id + FROM {local_sitsgradepush_aws_log} + WHERE messageid = :messageid + AND (status = :status OR attempts >= :attempts)'; + // Allow processing if message has not been processed successfully or exceeded maximum attempts. + return $DB->record_exists_sql( + $sql, + [ + 'messageid' => $messageid, + 'status' => self::STATUS_PROCESSED, + 'attempts' => self::MAX_ATTEMPTS, + ] ); } catch (\Exception $e) { logger::log($e->getMessage(), null, 'Failed to check message status'); @@ -158,8 +176,8 @@ public function execute(): void { foreach ($messages as $message) { try { - if ($this->is_processed_message($message['MessageId'])) { - mtrace("Skipping processed message: {$message['MessageId']}"); + if ($this->should_not_process_message($message['MessageId'])) { + mtrace("Skipping processed message or exceeded maximum attempts: {$message['MessageId']}"); continue; } $data = json_decode($message['Body'], true); @@ -167,12 +185,12 @@ public function execute(): void { throw new \Exception('Invalid JSON data: ' . json_last_error_msg()); } $this->process_message($data); - $this->save_message_record($message); + $this->save_message_record($message, $this->get_queue_name()); $this->delete_message($message['ReceiptHandle']); $processedcount++; } catch (\Exception $e) { logger::log($e->getMessage(), null, static::class . ' Processing Error'); - $this->save_message_record($message, self::STATUS_FAILED, $e->getMessage()); + $this->save_message_record($message, $this->get_queue_name(), self::STATUS_FAILED, $e->getMessage()); } } @@ -207,6 +225,7 @@ protected function delete_message(string $receipthandle): void { * Save message processing details to database * * @param array $message SQS message data + * @param string $type Queue type * @param string $status Processing status * @param string|null $error Error message if any * @return bool|int Returns record ID on success, false on failure @@ -214,23 +233,36 @@ protected function delete_message(string $receipthandle): void { */ protected function save_message_record( array $message, + string $type, string $status = self::STATUS_PROCESSED, ?string $error = null ): bool|int { - global $DB, $USER; + global $DB; try { - $record = new \stdClass(); - $record->messageid = $message['MessageId']; - $record->receipthandle = $message['ReceiptHandle']; - $record->queueurl = $this->get_queue_url(); - $record->status = $status; - $record->payload = $message['Body']; - $record->error_message = $error; - $record->timecreated = time(); - $record->usermodified = $USER->id; - - return $DB->insert_record('local_sitsgradepush_aws_log', $record); + // Check if message record already exists. + $record = $DB->get_record('local_sitsgradepush_aws_log', ['messageid' => $message['MessageId']]); + + // Prepare data to save. + $data = [ + 'messageid' => $message['MessageId'], + 'status' => $status, + 'error_message' => $error, + 'timemodified' => time(), + 'type' => $type, + 'payload' => $message['Body'], + 'attempts' => $record ? $record->attempts + 1 : 1, + ]; + + // Update record if exists. + if ($record) { + $data['id'] = $record->id; + $DB->update_record('local_sitsgradepush_aws_log', $data); + return $record->id; + } + + // Insert new record. + return $DB->insert_record('local_sitsgradepush_aws_log', $data); } catch (\Exception $e) { logger::log($e->getMessage(), null, 'Failed to save message record'); return false; diff --git a/classes/extension/extension.php b/classes/extension/extension.php index 8959641..7323dba 100644 --- a/classes/extension/extension.php +++ b/classes/extension/extension.php @@ -34,6 +34,9 @@ abstract class extension implements iextension { /** @var int User ID */ protected int $userid; + /** @var int Student code / ID number */ + protected int $studentcode; + /** @var bool Used to check if the extension data is set. */ protected bool $dataisset = false; @@ -153,7 +156,7 @@ public function get_mappings_by_userid(int $userid): array { $params = array_merge($courseinparam, $modinparams); // Find all mapped moodle assessments for the student. - $sql = "SELECT am.* + $sql = "SELECT am.*, mab.mapcode, mab.mabseq FROM {". manager::TABLE_ASSESSMENT_MAPPING ."} am JOIN {". manager::TABLE_COMPONENT_GRADE ."} mab ON am.componentgradeid = mab.id WHERE am.courseid $courseinsql AND am.moduletype $modinsql AND am.enableextension = 1"; diff --git a/classes/extension/sora.php b/classes/extension/sora.php index 8c73370..48085a1 100644 --- a/classes/extension/sora.php +++ b/classes/extension/sora.php @@ -18,6 +18,7 @@ use local_sitsgradepush\assessment\assessmentfactory; use local_sitsgradepush\logger; +use local_sitsgradepush\manager; /** * Class for Summary of Reasonable Adjustments (SORA). @@ -30,7 +31,16 @@ class sora extends extension { /** @var string Prefix used to create SORA groups */ - const SORA_GROUP_PREFIX = 'DEFAULT-SORA-'; + const SORA_GROUP_PREFIX = 'SORA-CM-'; + + /** @var string AWS datasource */ + const DATASOURCE_AWS = 'aws'; + + /** @var string API datasource */ + const DATASOURCE_API = 'api'; + + /** @var string SORA message type - EXAM */ + const SORA_MESSAGE_TYPE_EXAM = 'EXAM'; /** @var int Extra duration in minutes per hour */ protected int $extraduration; @@ -41,6 +51,12 @@ class sora extends extension { /** @var int Time extension in seconds, including extra and rest duration */ protected int $timeextension; + /** @var string Datasource */ + protected string $datasource; + + /** @var string|null SORA message type */ + protected ?string $soramessagetype; + /** * Return the whole time extension in seconds, including extra and rest duration. * @@ -72,26 +88,29 @@ public function get_rest_duration(): int { * Get the SORA group ID. Create the group if it does not exist. * Add the user to the group. Remove the user from other SORA groups. * - * @param int $courseid - * @param int $userid + * @param int $courseid The course ID. + * @param int $cmid The course module ID. + * @param int $userid The user ID. + * @param int $totalextension The total extension in minutes, including extra and rest duration. * * @return int * @throws \coding_exception * @throws \dml_exception * @throws \moodle_exception */ - public function get_sora_group_id(int $courseid, int $userid): int { + public function get_sora_group_id(int $courseid, int $cmid, int $userid, int $totalextension): int { global $CFG; require_once($CFG->dirroot . '/group/lib.php'); // Check group exists. - $groupid = groups_get_group_by_name($courseid, $this->get_extension_group_name()); + $groupname = self::get_extension_group_name($cmid, $totalextension); + $groupid = groups_get_group_by_name($courseid, $groupname); if (!$groupid) { // Create group. $newgroup = new \stdClass(); $newgroup->courseid = $courseid; - $newgroup->name = $this->get_extension_group_name(); + $newgroup->name = $groupname; $newgroup->description = ''; $newgroup->enrolmentkey = ''; $newgroup->picture = 0; @@ -107,23 +126,23 @@ public function get_sora_group_id(int $courseid, int $userid): int { throw new \moodle_exception('error:cannotaddusertogroup', 'local_sitsgradepush'); } - // Remove user from previous SORA groups. - $this->remove_user_from_previous_sora_groups($groupid, $courseid, $userid); - return $groupid; } /** * Get the SORA group name. * + * @param int $cmid The course module ID. + * @param int $totalextension The total extension in minutes, including extra and rest duration. * @return string */ - public function get_extension_group_name(): string { - return sprintf(self::SORA_GROUP_PREFIX . '%d', $this->get_extra_duration() + $this->get_rest_duration()); + public static function get_extension_group_name(int $cmid, int $totalextension): string { + return sprintf(self::SORA_GROUP_PREFIX . '%d-EX-%d', $cmid, $totalextension); } /** * Set properties from AWS SORA update message. + * This set the student code and user id for this SORA, the SORA extension information will be obtained from the API. * * @param string $messagebody * @return void @@ -131,32 +150,24 @@ public function get_extension_group_name(): string { * @throws \moodle_exception */ public function set_properties_from_aws_message(string $messagebody): void { - // Decode the JSON message body. $messagedata = $this->parse_event_json($messagebody); - // Check the message is valid. - if (empty($messagedata->entity->person_sora->sora[0])) { - throw new \moodle_exception('error:invalid_message', 'local_sitsgradepush', '', null, $messagebody); - } - - $soradata = $messagedata->entity->person_sora->sora[0]; + // Set datasource. + $this->datasource = self::DATASOURCE_AWS; - // Set properties. - $this->extraduration = (int) $soradata->extra_duration ?? 0; - $this->restduration = (int) $soradata->rest_duration ?? 0; + // Set SORA message type. + $this->soramessagetype = $messagedata->entity->person_sora->type->code ?? null; - // A SORA update message must have at least one of the durations. - if ($this->extraduration == 0 && $this->restduration == 0) { - throw new \moodle_exception('error:invalid_duration', 'local_sitsgradepush'); + // Check the message is valid and set student code. + $studentcode = $messagedata->entity->person_sora->person->student_code ?? null; + if (!empty($studentcode)) { + $this->studentcode = $studentcode; + $this->set_userid($studentcode); + } else { + throw new \moodle_exception('error:invalid_message', 'local_sitsgradepush', '', null, $messagebody); } - // Calculate and set the time extension in seconds. - $this->timeextension = $this->calculate_time_extension($this->extraduration, $this->restduration); - - // Set the user ID of the student. - $this->set_userid($soradata->person->student_code); - $this->dataisset = true; } @@ -167,8 +178,16 @@ public function set_properties_from_aws_message(string $messagebody): void { * @return void */ public function set_properties_from_get_students_api(array $student): void { + // Set student code. + $this->studentcode = $student['code']; + // Set the user ID. - $this->set_userid($student['code']); + if (!isset($this->userid)) { + $this->set_userid($student['code']); + } + + // Set datasource. + $this->datasource = self::DATASOURCE_API; // Set properties. $this->extraduration = (int) $student['assessment']['sora_assessment_duration']; @@ -198,16 +217,16 @@ public function process_extension(array $mappings): void { throw new \coding_exception('error:extensiondataisnotset', 'local_sitsgradepush'); } - // Exit if SORA extra assessment duration and rest duration are both 0. - if ($this->extraduration == 0 && $this->restduration == 0) { - return; - } - // Apply the extension to the assessments. foreach ($mappings as $mapping) { try { + // Update the SORA information from the API if the datasource is AWS. + if ($this->get_data_source() === self::DATASOURCE_AWS) { + $this->update_sora_info_from_api($mapping); + } + // Apply sora extension to the mapped moodle assessment. $assessment = assessmentfactory::get_assessment($mapping->sourcetype, $mapping->sourceid); - if ($assessment->is_user_a_participant($this->userid)) { + if ($assessment->is_user_a_participant($this->get_userid())) { $assessment->apply_extension($this); } } catch (\Exception $e) { @@ -217,33 +236,22 @@ public function process_extension(array $mappings): void { } /** - * Remove the user from previous SORA groups. + * Get the data source. + * To distinguish where the SORA data come from AWS or API. * - * @param int $newgroupid The new group ID or the group to keep the user in. - * @param int $courseid The course ID. - * @param int $userid The user ID. - * @return void - * @throws \dml_exception + * @return string */ - protected function remove_user_from_previous_sora_groups(int $newgroupid, int $courseid, int $userid): void { - global $DB; - - // Find all default SORA groups created by marks transfer. - $sql = 'SELECT g.id - FROM {groups} g - WHERE g.courseid = :courseid - AND g.name LIKE :name'; - $params = [ - 'courseid' => $courseid, - 'name' => self::SORA_GROUP_PREFIX . '%', - ]; - $soragroups = $DB->get_records_sql($sql, $params); - - foreach ($soragroups as $soragroup) { - if ($soragroup->id != $newgroupid) { - groups_remove_member($soragroup->id, $userid); - } - } + public function get_data_source(): string { + return $this->datasource; + } + + /** + * Get the SORA message type. + * + * @return string + */ + public function get_sora_message_type(): string { + return $this->soramessagetype; } /** @@ -256,4 +264,21 @@ protected function remove_user_from_previous_sora_groups(int $newgroupid, int $c private function calculate_time_extension(int $extraduration, int $restduration): int { return ($extraduration + $restduration) * MINSECS; } + + /** + * Update SORA information from the assessment API. + * Used when there is a SORA update message from AWS. + * + * @param \stdClass $mapping SITS assessment mapping + */ + private function update_sora_info_from_api(\stdClass $mapping): void { + // Call the assessment API to get the SORA data. + $students = manager::get_manager()->get_students_from_sits($mapping, true); + foreach ($students as $student) { + if ($student['code'] == $this->studentcode) { + $this->set_properties_from_get_students_api($student); + break; + } + } + } } diff --git a/classes/extension/sora_queue_processor.php b/classes/extension/sora_queue_processor.php index d725b31..727c75d 100644 --- a/classes/extension/sora_queue_processor.php +++ b/classes/extension/sora_queue_processor.php @@ -16,8 +16,6 @@ namespace local_sitsgradepush\extension; -use local_sitsgradepush\manager; - /** * SORA queue processor. * @@ -49,8 +47,23 @@ protected function get_queue_url(): string { protected function process_message(array $messagebody): void { $sora = new sora(); $sora->set_properties_from_aws_message($messagebody['Message']); + + // As the assessment api only returns exam type SORA, we only process exam type SORA update from AWS. + if ($sora->get_sora_message_type() !== sora::SORA_MESSAGE_TYPE_EXAM) { + return; + } + // Get all mappings for the student. $mappings = $sora->get_mappings_by_userid($sora->get_userid()); $sora->process_extension($mappings); } + + /** + * Get the queue name. + * + * @return string + */ + protected function get_queue_name(): string { + return 'sora'; + } } diff --git a/classes/extensionmanager.php b/classes/extensionmanager.php index 7ff3bfb..bc9f981 100644 --- a/classes/extensionmanager.php +++ b/classes/extensionmanager.php @@ -16,7 +16,6 @@ namespace local_sitsgradepush; -use local_sitsgradepush\assessment\assessment; use local_sitsgradepush\assessment\assessmentfactory; use local_sitsgradepush\extension\extension; use local_sitsgradepush\extension\sora; @@ -33,32 +32,34 @@ class extensionmanager { /** - * Update SORA extension for students in a mapping. + * Update SORA extension for students in a mapping using the SITS get students API as the data source. * * @param \stdClass $mapping Assessment component mapping ID. * @param array $students Students data from the SITS get students API. * @return void - * @throws \dml_exception + * @throws \dml_exception|\moodle_exception */ public static function update_sora_for_mapping(\stdClass $mapping, array $students): void { - try { - if ($mapping->enableextension !== '1') { - throw new \moodle_exception('error:extension_not_enabled_for_mapping', 'local_sitsgradepush', '', $mapping->id); - } + // Nothing to do if the extension is not enabled for the mapping. + if ($mapping->enableextension !== '1') { + return; + } - // If no students returned from SITS, nothing to do. - if (empty($students)) { - return; - } + // If no students returned from SITS, nothing to do. + if (empty($students)) { + return; + } - // Process SORA extension for each student or the specified student if user id is provided. - foreach ($students as $student) { + // Process SORA extension for each student or the specified student if user id is provided. + foreach ($students as $student) { + try { $sora = new sora(); $sora->set_properties_from_get_students_api($student); $sora->process_extension([$mapping]); + } catch (\Exception $e) { + $studentcode = $student["code"] ?? ''; + logger::log($e->getMessage(), null, "Mapping ID: $mapping->id, Student Idnumber: $studentcode"); } - } catch (\Exception $e) { - logger::log($e->getMessage(), null, "Mapping ID: $mapping->id"); } } @@ -126,28 +127,10 @@ public static function delete_sora_overrides(\stdClass $deletedmapping): void { return; } - $assessment->delete_sora_overrides(self::get_default_sora_groups_ids_in_course($deletedmapping->courseid)); + // Delete all SORA overrides for the assessment. + $assessment->delete_all_sora_overrides(); } catch (\Exception $e) { logger::log($e->getMessage(), null, "Deleted Mapping: " . json_encode($deletedmapping)); } } - - /** - * Get the default SORA groups IDs in a course. - * - * @param int $courseid - * @return array - * @throws \dml_exception - */ - public static function get_default_sora_groups_ids_in_course(int $courseid): array { - global $DB; - $like = $DB->sql_like('name', ':name', false); - $defaultsoragroups = $DB->get_records_select( - 'groups', - "courseid = :courseid AND $like", - ['courseid' => $courseid, 'name' => sora::SORA_GROUP_PREFIX . '%'], - fields: 'id', - ); - return !empty($defaultsoragroups) ? array_keys($defaultsoragroups) : []; - } } diff --git a/db/install.xml b/db/install.xml index 065bb11..7b926a2 100644 --- a/db/install.xml +++ b/db/install.xml @@ -115,13 +115,11 @@ - - + - - + @@ -129,7 +127,7 @@ - + diff --git a/db/upgrade.php b/db/upgrade.php index c5c8f4c..a846d92 100644 --- a/db/upgrade.php +++ b/db/upgrade.php @@ -557,13 +557,11 @@ function xmldb_local_sitsgradepush_upgrade($oldversion) { // Adding fields to table local_sitsgradepush_aws_log. $table->add_field('id', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, XMLDB_SEQUENCE, null); $table->add_field('messageid', XMLDB_TYPE_CHAR, '255', null, XMLDB_NOTNULL, null, null); - $table->add_field('receipthandle', XMLDB_TYPE_TEXT, null, null, XMLDB_NOTNULL, null, null); - $table->add_field('queueurl', XMLDB_TYPE_CHAR, '255', null, XMLDB_NOTNULL, null, null); $table->add_field('status', XMLDB_TYPE_CHAR, '20', null, XMLDB_NOTNULL, null, null); + $table->add_field('attempts', XMLDB_TYPE_INTEGER, '2', null, XMLDB_NOTNULL, null, null); $table->add_field('payload', XMLDB_TYPE_TEXT, null, null, XMLDB_NOTNULL, null, null); $table->add_field('error_message', XMLDB_TYPE_TEXT, null, null, null, null, null); - $table->add_field('timecreated', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null); - $table->add_field('usermodified', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null); + $table->add_field('timemodified', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null); // Adding keys to table local_sitsgradepush_aws_log. $table->add_key('primary', XMLDB_KEY_PRIMARY, ['id']); @@ -571,7 +569,7 @@ function xmldb_local_sitsgradepush_upgrade($oldversion) { // Adding indexes to table local_sitsgradepush_aws_log. $table->add_index('messageid', XMLDB_INDEX_NOTUNIQUE, ['messageid']); $table->add_index('status', XMLDB_INDEX_NOTUNIQUE, ['status']); - $table->add_index('timecreated', XMLDB_INDEX_NOTUNIQUE, ['timecreated']); + $table->add_index('timemodified', XMLDB_INDEX_NOTUNIQUE, ['timemodified']); // Conditionally launch create table for local_sitsgradepush_aws_log. if (!$dbman->table_exists($table)) { diff --git a/lang/en/local_sitsgradepush.php b/lang/en/local_sitsgradepush.php index 321a255..eaf89f1 100644 --- a/lang/en/local_sitsgradepush.php +++ b/lang/en/local_sitsgradepush.php @@ -111,6 +111,7 @@ $string['error:inserttask'] = 'Failed to insert task.'; $string['error:invalid_json_data'] = 'Invalid JSON data: {$a}'; $string['error:invalid_message'] = 'Invalid message received.'; +$string['error:invalid_sora_datasource'] = 'Invalid SORA data source.'; $string['error:invalid_source_type'] = 'Invalid source type. {$a}'; $string['error:lesson_practice'] = 'Practice lessons have no grades'; $string['error:lti_no_grades'] = 'LTI activity is set to not send grades to gradebook'; diff --git a/lib.php b/lib.php index 112a730..55bf708 100644 --- a/lib.php +++ b/lib.php @@ -24,6 +24,7 @@ */ use local_sitsgradepush\assessment\assessmentfactory; +use local_sitsgradepush\extension\extension; use local_sitsgradepush\manager; /** diff --git a/tests/extension/extension_common.php b/tests/extension/extension_common.php index 12821a4..981c9e8 100644 --- a/tests/extension/extension_common.php +++ b/tests/extension/extension_common.php @@ -99,7 +99,6 @@ protected function setUp(): void { 'course' => $this->course1->id, 'name' => 'Test Quiz 1', 'timeopen' => $assessmentstartdate, - 'timelimit' => 60, 'timeclose' => $assessmentenddate, ] ); diff --git a/tests/extension/sora_test.php b/tests/extension/sora_test.php index 8ea1fe7..4f82586 100644 --- a/tests/extension/sora_test.php +++ b/tests/extension/sora_test.php @@ -33,53 +33,6 @@ */ final class sora_test extends extension_common { - /** - * Test no SORA override for non-exam assessments. - * - * @covers \local_sitsgradepush\assessment\assign::is_exam - * @covers \local_sitsgradepush\assessment\quiz::is_exam - * @return void - * @throws \coding_exception - * @throws \dml_exception - * @throws \moodle_exception - */ - public function test_no_sora_override_for_non_exam_assessment(): void { - global $DB; - - // Set up the SORA overrides. - $this->setup_for_sora_testing(); - - $startdate = strtotime('+1 hour'); - $enddate = strtotime('+7 days'); - - // Modify assignment so that open duration more than 5 hours, i.e. not exam. - $DB->update_record('assign', (object) [ - 'id' => $this->assign1->id, - 'allowsubmissionsfromdate' => $startdate, - 'duedate' => $enddate, - ]); - - // Modify quiz so that time limit more than 5 hours, i.e. not exam. - $DB->update_record('quiz', (object) [ - 'id' => $this->quiz1->id, - 'timelimit' => 2880, // 48 hours. - ]); - - // Get mappings. - $mappings = manager::get_manager()->get_assessment_mappings_by_courseid($this->course1->id); - $sora = new sora(); - $sora->set_properties_from_get_students_api(tests_data_provider::get_sora_testing_student_data()); - $sora->process_extension($mappings); - - // Test no SORA override for the assignment. - $override = $DB->record_exists('assign_overrides', ['assignid' => $this->assign1->id]); - $this->assertFalse($override); - - // Test no SORA override for the quiz. - $override = $DB->record_exists('quiz_overrides', ['quiz' => $this->quiz1->id]); - $this->assertFalse($override); - } - /** * Test no SORA override for past assessments. * @@ -146,8 +99,8 @@ public function test_sora_process_extension_from_aws(): void { $sora->set_properties_from_aws_message(tests_data_provider::get_sora_event_data()); $sora->process_extension($sora->get_mappings_by_userid($sora->get_userid())); - // Test SORA override group exists. - $groupid = $DB->get_field('groups', 'id', ['name' => $sora->get_extension_group_name()]); + // Test SORA override group exists for assignment. + $groupid = $DB->get_field('groups', 'id', ['name' => $sora->get_extension_group_name($this->assign1->cmid, 25)]); $this->assertNotEmpty($groupid); // Test user is added to the SORA group. @@ -159,6 +112,14 @@ public function test_sora_process_extension_from_aws(): void { ->get_record('assign_overrides', ['assignid' => $this->assign1->id, 'userid' => null, 'groupid' => $groupid]); $this->assertEquals($override->groupid, $groupid); + // Test SORA override group exists for quiz. + $groupid = $DB->get_field('groups', 'id', ['name' => $sora->get_extension_group_name($this->quiz1->cmid, 25)]); + $this->assertNotEmpty($groupid); + + // Test user is added to the SORA group. + $groupmember = $DB->get_record('groups_members', ['groupid' => $groupid, 'userid' => $this->student1->id]); + $this->assertNotEmpty($groupmember); + // Test group override set in the quiz. $override = $DB->get_record('quiz_overrides', ['quiz' => $this->quiz1->id, 'userid' => null, 'groupid' => $groupid]); $this->assertEquals($override->groupid, $groupid); @@ -182,16 +143,19 @@ public function test_update_sora_for_mapping_with_extension_off(): void { // Get mappings. $mappings = manager::get_manager()->get_assessment_mappings_by_courseid($this->course1->id); - $mapping = reset($mappings); - // Process SORA extension for each mapping. - extensionmanager::update_sora_for_mapping($mapping, []); - - // Check error log. - $errormessage = get_string('error:extension_not_enabled_for_mapping', 'local_sitsgradepush', $mapping->id); - $sql = "SELECT * FROM {local_sitsgradepush_err_log} WHERE message = :message AND data = :data"; - $params = ['message' => $errormessage, 'data' => "Mapping ID: $mapping->id"]; - $log = $DB->get_record_sql($sql, $params); - $this->assertNotEmpty($log); + + // Process all mappings for SORA. + foreach ($mappings as $mapping) { + extensionmanager::update_sora_for_mapping($mapping, [tests_data_provider::get_sora_testing_student_data()]); + } + + // Test no SORA override for the assignment. + $override = $DB->record_exists('assign_overrides', ['assignid' => $this->assign1->id]); + $this->assertFalse($override); + + // Test no SORA override for the quiz. + $override = $DB->record_exists('quiz_overrides', ['quiz' => $this->quiz1->id]); + $this->assertFalse($override); } /** @@ -201,7 +165,6 @@ public function test_update_sora_for_mapping_with_extension_off(): void { * * @covers \local_sitsgradepush\extensionmanager::update_sora_for_mapping * @covers \local_sitsgradepush\extensionmanager::delete_sora_overrides - * @covers \local_sitsgradepush\extensionmanager::get_default_sora_groups_ids_in_course * @covers \local_sitsgradepush\manager::get_assessment_mappings_by_courseid * @return void * @throws \dml_exception|\coding_exception|\ReflectionException|\moodle_exception @@ -220,7 +183,7 @@ public function test_update_sora_for_mapping(): void { } // Test SORA override group exists. - $groupid = $DB->get_field('groups', 'id', ['name' => sora::SORA_GROUP_PREFIX . '25']); + $groupid = $DB->get_field('groups', 'id', ['name' => sora::get_extension_group_name($this->assign1->cmid, 25)]); $this->assertNotEmpty($groupid); // Test user is added to the SORA group. @@ -232,6 +195,14 @@ public function test_update_sora_for_mapping(): void { ->get_record('assign_overrides', ['assignid' => $this->assign1->id, 'userid' => null, 'groupid' => $groupid]); $this->assertEquals($override->groupid, $groupid); + // Test SORA override group exists. + $groupid = $DB->get_field('groups', 'id', ['name' => sora::get_extension_group_name($this->quiz1->cmid, 25)]); + $this->assertNotEmpty($groupid); + + // Test user is added to the SORA group. + $groupmember = $DB->get_record('groups_members', ['groupid' => $groupid, 'userid' => $this->student1->id]); + $this->assertNotEmpty($groupmember); + // Test group override set in the quiz. $override = $DB->get_record('quiz_overrides', ['quiz' => $this->quiz1->id, 'userid' => null, 'groupid' => $groupid]); $this->assertEquals($override->groupid, $groupid); @@ -257,10 +228,16 @@ public function test_update_sora_for_mapping(): void { */ protected function setup_for_sora_testing(): void { global $DB; - $mabid = $DB->get_field('local_sitsgradepush_mab', 'id', ['mapcode' => 'LAWS0024A6UF', 'mabseq' => '001']); - $this->insert_mapping($mabid, $this->course1->id, $this->assign1, 'assign'); + $mab1 = $DB->get_record('local_sitsgradepush_mab', ['mapcode' => 'LAWS0024A6UF', 'mabseq' => '001']); + $this->insert_mapping($mab1->id, $this->course1->id, $this->assign1, 'assign'); - $mabid = $DB->get_field('local_sitsgradepush_mab', 'id', ['mapcode' => 'LAWS0024A6UF', 'mabseq' => '002']); - $this->insert_mapping($mabid, $this->course1->id, $this->quiz1, 'quiz'); + $mab2 = $DB->get_record('local_sitsgradepush_mab', ['mapcode' => 'LAWS0024A6UF', 'mabseq' => '002']); + $this->insert_mapping($mab2->id, $this->course1->id, $this->quiz1, 'quiz'); + + $manager = manager::get_manager(); + $apiclient = $this->get_apiclient_for_testing(false, [tests_data_provider::get_sora_testing_student_data()]); + tests_data_provider::set_protected_property($manager, 'apiclient', $apiclient); + $manager->get_students_from_sits($mab1); + $manager->get_students_from_sits($mab2); } } diff --git a/tests/fixtures/sora_event_data.json b/tests/fixtures/sora_event_data.json index f45466c..c5951d7 100644 --- a/tests/fixtures/sora_event_data.json +++ b/tests/fixtures/sora_event_data.json @@ -15,29 +15,25 @@ ], "entity": { "person_sora": { - "sora": [ - { - "identifier": "AAAHM03-000001", - "type": { - "code": "EXAM", - "name": "Examinations" - }, - "arrangement_record_sequence_number": "000001", - "accessibility_assessment_arrangement_sequence_number": "000001", - "approved_indicator": null, - "extra_duration": 30, - "rest_duration": 5, - "note": null, - "post_deadline_details": null, - "expiry_date": null, - "last_updated": null, - "person": { - "identifier": "AAAHM03", - "student_code": "12345678", - "userid": null - } - } - ] + "identifier": "AAAHM03-000001", + "type": { + "code": "EXAM", + "name": "Examinations" + }, + "arrangement_record_sequence_number": "000001", + "accessibility_assessment_arrangement_sequence_number": "000001", + "approved_indicator": null, + "extra_duration": 30, + "rest_duration": 5, + "note": null, + "post_deadline_details": null, + "expiry_date": null, + "last_updated": null, + "person": { + "identifier": "AAAHM03", + "student_code": "12345678", + "userid": null + } } } }