From a1bad61185c582762b963ea9925b2b1101635d7d Mon Sep 17 00:00:00 2001 From: Jun Pataleta Date: Mon, 17 Jun 2024 10:53:19 +0800 Subject: [PATCH] Validate rated responses before saving them --- classes/api.php | 44 ++++++++++++++++++++++-- lang/en/threesixo.php | 2 ++ tests/api_test.php | 78 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 121 insertions(+), 3 deletions(-) diff --git a/classes/api.php b/classes/api.php index 008865c..6d67077 100644 --- a/classes/api.php +++ b/classes/api.php @@ -82,6 +82,15 @@ class api { /** Activity close event type. */ const THREESIXO_EVENT_TYPE_CLOSE = 'close'; + /** @var int Default minimum rating (Strongly disagree). */ + const RATING_MIN = 1; + + /** @var int Default maximum rating (Strongly agree). */ + const RATING_MAX = 6; + + /** @var int Not applicable. */ + const RATING_NA = 0; + /** * Fetches the 360-degree feedback instance. * @@ -828,7 +837,7 @@ public static function get_submission($id, $fromuser = 0, $fields = '*') { * @param int $threesixtyid The 360-degree feedback instance ID. * @param int $fromuser The respondent's user ID. * @param int $touser The feedback recipient's user ID. - * @return stdClass + * @return false|stdClass * @throws dml_exception */ public static function get_submission_by_params($threesixtyid, $fromuser, $touser) { @@ -898,6 +907,11 @@ public static function get_scales() { public static function save_responses($threesixty, $touser, $responses) { global $DB, $USER; + $error = self::validate_responses($threesixty, $responses); + if ($error !== '') { + throw new moodle_exception($error); + } + $fromuser = $USER->id; $savedresponses = $DB->get_records('threesixo_response', [ 'threesixo' => $threesixty, @@ -918,22 +932,46 @@ public static function save_responses($threesixty, $touser, $responses) { } } + $response->value = $value; if (empty($response->id)) { $response->threesixo = $threesixty; $response->item = $key; $response->touser = $touser; $response->fromuser = $fromuser; - $response->value = $value; $id = $DB->insert_record('threesixo_response', $response); $result &= !empty($id); } else { - $response->value = $value; $result &= $DB->update_record('threesixo_response', $response); } } return $result; } + /** + * Validate user responses to the 360-degree feedback activity, especially the values for the rated questions. + * + * @param int $threesixty The 360-degree feedback activity identifier. + * @param array $responses The array of responses. + * @return string The error message if validation errors are found. Returns an empty string if validation passes. + * @throws coding_exception + * @throws dml_exception + */ + public static function validate_responses(int $threesixty, array $responses): string { + $items = self::get_items($threesixty); + foreach ($responses as $itemid => $value) { + $item = $items[$itemid] ?? false; + if ($item === false) { + return get_string('errorinvaliditem', 'mod_threesixo'); + } + if ($item->type == self::QTYPE_RATED) { + if ((float)$value < self::RATING_NA || (float)$value > self::RATING_MAX) { + return get_string('errorinvalidratingvalue', 'mod_threesixo', $value); + } + } + } + return ''; + } + /** * Anonymises the responses for a feedback submission. This is simply done by setting the fromuser field to 0. * diff --git a/lang/en/threesixo.php b/lang/en/threesixo.php index 02926ce..6885d37 100644 --- a/lang/en/threesixo.php +++ b/lang/en/threesixo.php @@ -58,6 +58,8 @@ $string['errorcannotupdateitem'] = 'Cannot update the 360° feedback item.'; $string['erroritemnotfound'] = 'The 360° feedback item was not found.'; $string['errorinvalidstatus'] = 'Invalid status'; +$string['errorinvaliditem'] = 'Invalid item'; +$string['errorinvalidratingvalue'] = 'Invalid rating response value: {$a}'; $string['errornocaptoedititems'] = 'Sorry, but you don\'t have the capability to edit 360° feedback items.'; $string['errornotenrolled'] = 'You need to be enrolled in this course in order to be able to participate in this 360° feedback activity.'; $string['errornotingroup'] = 'You need to be in a group in order to be able to participate in this 360° feedback activity. Please contact your course administrator.'; diff --git a/tests/api_test.php b/tests/api_test.php index fd21471..f7746a9 100644 --- a/tests/api_test.php +++ b/tests/api_test.php @@ -163,4 +163,82 @@ public function test_is_open($open, $close, $messagewhenclosed, $expected) { $this->assertEquals($expected, $result); } } + + /** + * Data provider for response validation. + * + * @return array[] + */ + public function validate_responses_provider(): array { + return [ + 'Valid responses' => [ + true, [1, 6, 0], '', + ], + 'Responses with null values' => [ + true, [6, null, 0], '', + ], + 'Invalid item' => [ + false, [], get_string('errorinvaliditem', 'mod_threesixo'), + ], + 'Negative value response' => [ + true, [1, -6, 0], get_string('errorinvalidratingvalue', 'mod_threesixo', -6), + ], + 'Over expected int value response' => [ + true, [1, 6, (api::RATING_MAX + 1)], get_string('errorinvalidratingvalue', 'mod_threesixo', 7), + ], + 'Over expected float value response' => [ + true, [1, 6, 6.1], get_string('errorinvalidratingvalue', 'mod_threesixo', 6.1), + ], + ]; + } + + /** + * Test for {@see api::validate_responses()} + * + * @dataProvider validate_responses_provider + * @covers ::validate_responses + * @param bool $validitem If false, we'll pass an invalid item ID that does not belong in the feedback activity. + * @param array $responsedata The response data. + * @param string $message The expected error message. + * @return void + */ + public function test_validate_responses(bool $validitem, array $responsedata, string $message): void { + $this->resetAfterTest(); + $this->setAdminUser(); + $generator = $this->getDataGenerator(); + + // Create a course. + $course = $generator->create_course(); + + // Create the instance. + $params = [ + 'course' => $course->id, + ]; + $options = [ + 'ratedquestions' => [ + 'R1', + 'R2', + 'R3', + ], + 'commentquestions' => [], + ]; + $threesixo = $this->getDataGenerator()->create_module('threesixo', $params, $options); + + $items = api::get_items($threesixo->id); + + $responses = []; + $index = 0; + $maxitemid = 0; + foreach ($items as $item) { + $responses[$item->id] = $responsedata[$index] ?? api::RATING_NA; + $maxitemid = max($item->id, $maxitemid); + $index++; + } + if (!$validitem) { + $maxitemid++; + $responses[$maxitemid] = api::RATING_MAX; + } + $result = api::validate_responses($threesixo->id, $responses); + $this->assertEquals($message, $result); + } }