Skip to content

Commit

Permalink
Fix recent codechecker failures
Browse files Browse the repository at this point in the history
  • Loading branch information
timhunt committed May 2, 2024
1 parent 3283f96 commit e9bd531
Show file tree
Hide file tree
Showing 9 changed files with 124 additions and 65 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ jobs:

- name: Moodle Code Checker
if: ${{ always() }}
run: moodle-plugin-ci phpcs --max-warnings 0
run: moodle-plugin-ci phpcs # --max-warnings 0 # Add back once #[\Override] works.

- name: Moodle PHPDoc Checker
continue-on-error: true # This step will show errors but will not fail.
Expand Down
1 change: 1 addition & 0 deletions backup/moodle2/backup_qtype_oumatrix_plugin.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ protected function define_question_plugin_structure(): backup_plugin_element {
return $plugin;
}

#[\Override]
public static function get_qtype_fileareas() {
return [
'feedback' => 'qtype_oumatrix_rows',
Expand Down
30 changes: 14 additions & 16 deletions edit_oumatrix_form.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,22 @@
class qtype_oumatrix_edit_form extends question_edit_form {

/** The default starting number of columns (answers). */
private const COL_NUM_START = 3;
protected const COL_NUM_START = 3;

/** The number of columns (answers) that get added at a time. */
private const COL_NUM_ADD = 2;
protected const COL_NUM_ADD = 2;

/** The default starting number of rows (question row). */
private const ROW_NUM_START = 4;
protected const ROW_NUM_START = 4;

/** The number of rows (question row) that get added at a time.*/
private const ROW_NUM_ADD = 2;
protected const ROW_NUM_ADD = 2;

/** @var int Number of columns. */
private $numcolumns;
protected int $numcolumns;

/** @var string inputtype of rows. */
private $inputtype;
protected string $inputtype;

/** @var array of HTML tags allowed in column and row names. */
protected $allowedhtmltags = [
Expand All @@ -54,10 +54,10 @@ class qtype_oumatrix_edit_form extends question_edit_form {
];

/** @var string regex to match HTML open tags. */
private $htmltstarttagsandattributes = '~<\s*\w+\b[^>]*>~';
protected string $htmltstarttagsandattributes = '~<\s*\w+\b[^>]*>~';

/** @var string regex to match HTML close tags or br. */
private $htmltclosetags = '~<\s*/\s*\w+\b[^>]*>~';
protected string $htmltclosetags = '~<\s*/\s*\w+\b[^>]*>~';

/**
* Set the inputtype and number of rows and columns method.
Expand All @@ -81,6 +81,7 @@ protected function set_current_settings(): void {
$this->numcolumns = $columns ? count($columns) : $this->numcolumns;
}

#[\Override]
protected function definition_inner($mform) {

// Set the number of columns and rows.
Expand Down Expand Up @@ -210,6 +211,7 @@ protected function get_per_row_fields(MoodleQuickForm $mform, string $label, arr
return $repeated;
}

#[\Override]
public function data_preprocessing($question) {
$question = parent::data_preprocessing($question);
$question = $this->data_preprocessing_options($question);
Expand Down Expand Up @@ -310,6 +312,7 @@ private function data_preprocessing_rows(stdClass $question): object {
return $question;
}

#[\Override]
public function validation($data, $files) {
$errors = parent::validation($data, $files);

Expand Down Expand Up @@ -448,8 +451,8 @@ public function validation($data, $files) {
}

/**
* Vaidate some input to make sure it does not contain any tags other than
* $this->allowedhtmltags.
* Validate some input to make sure it does not contain any tags other than $this->allowedhtmltags.
*
* @param string $text the input to validate.
* @return string any validation errors.
*/
Expand Down Expand Up @@ -506,12 +509,7 @@ private function get_list_of_printable_allowed_tags(array $allowedhtmltags): str
return implode(', ', $allowedtaglist);
}


/**
* Returns the question type name.
*
* @return string The question type name.
*/
#[\Override]
public function qtype(): string {
return 'oumatrix';
}
Expand Down
65 changes: 54 additions & 11 deletions question.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,26 +34,38 @@
* @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
abstract class qtype_oumatrix_base extends question_graded_automatically {
public $shuffleanswers;
/** @var bool whether the rows should be shuffled. */
public bool $shuffleanswers;

/** @var string 'All or none' or 'partial' grading method for multiple response. */
public $grademethod;
public string $grademethod;

public $correctfeedback;
public $correctfeedbackformat;
public $partiallycorrectfeedback;
public $partiallycorrectfeedbackformat;
public $incorrectfeedback;
public $incorrectfeedbackformat;
/** @var string feedback for any correct response. */
public string $correctfeedback;

/** @var int format of $correctfeedback. */
public int $correctfeedbackformat;

/** @var string feedback for any partially correct response. */
public string $partiallycorrectfeedback;

/** @var int format of $partiallycorrectfeedback. */
public int $partiallycorrectfeedbackformat;

/** @var string feedback for any incorrect response. */
public string $incorrectfeedback;

/** @var int format of $incorrectfeedback. */
public int $incorrectfeedbackformat;

/** @var column[] The columns (answers) object, indexed by number. */
public $columns;
public array $columns;

/** @var row[] The rows (subquestions) object, indexed by number. */
public $rows;
public array $rows;

/** @var array The order of the rows, key => row number. */
protected $roworder = null;
protected ?array $roworder = null;

/**
* Returns true if the response has been selected for that row and column.
Expand All @@ -65,10 +77,13 @@ abstract class qtype_oumatrix_base extends question_graded_automatically {
*/
abstract public function is_choice_selected(array $response, int $rowkey, int $colnumber): bool;

#[\Override]
abstract public function is_same_response(array $prevresponse, array $newresponse);

#[\Override]
abstract public function grade_response(array $response);

#[\Override]
public function start_attempt(question_attempt_step $step, $variant) {
$this->roworder = array_keys($this->rows);
if ($this->shuffleanswers) {
Expand All @@ -77,6 +92,7 @@ public function start_attempt(question_attempt_step $step, $variant) {
$step->set_qt_var('_roworder', implode(',', $this->roworder));
}

#[\Override]
public function apply_attempt_state(question_attempt_step $step) {
$this->roworder = explode(',', $step->get_qt_var('_roworder'));
}
Expand All @@ -103,10 +119,12 @@ protected function init_roworder(question_attempt $qa) {
}
}

#[\Override]
public function is_gradable_response(array $response) {
return $this->is_complete_response($response);
}

#[\Override]
public function check_file_access($qa, $options, $component, $filearea, $args, $forcedownload) {
if ($component == 'question' && in_array($filearea,
['correctfeedback', 'partiallycorrectfeedback', 'incorrectfeedback'])) {
Expand All @@ -124,13 +142,15 @@ public function check_file_access($qa, $options, $component, $filearea, $args, $
}
}

#[\Override]
public function get_validation_error(array $response): string {
if ($this->is_complete_response($response)) {
return '';
}
return get_string('pleaseananswerallparts', 'qtype_oumatrix');
}

#[\Override]
public function validate_can_regrade_with_other_version(question_definition $otherversion): ?string {
$basemessage = parent::validate_can_regrade_with_other_version($otherversion);
if ($basemessage) {
Expand Down Expand Up @@ -168,10 +188,12 @@ public function has_specific_feedback(): bool {
*/
class qtype_oumatrix_single extends qtype_oumatrix_base {

#[\Override]
public function get_renderer(moodle_page $page) {
return $page->get_renderer('qtype_oumatrix', 'single');
}

#[\Override]
public function get_expected_data(): array {
$expected = [];
foreach ($this->rows as $row) {
Expand All @@ -180,6 +202,7 @@ public function get_expected_data(): array {
return $expected;
}

#[\Override]
public function is_choice_selected(array $response, int $rowkey, int $colnumber): bool {
$responsekey = $this->field($rowkey);
if (array_key_exists($responsekey, $response)) {
Expand All @@ -188,6 +211,7 @@ public function is_choice_selected(array $response, int $rowkey, int $colnumber)
return false;
}

#[\Override]
public function is_same_response(array $prevresponse, array $newresponse): bool {
foreach ($this->roworder as $key => $notused) {
$fieldname = $this->field($key);
Expand All @@ -208,6 +232,7 @@ protected function field(int $rowkey): string {
return 'rowanswers' . $rowkey;
}

#[\Override]
public function get_correct_response(): array {
$response = [];
foreach ($this->roworder as $key => $rownumber) {
Expand All @@ -220,6 +245,7 @@ public function get_correct_response(): array {
return $response;
}

#[\Override]
public function summarise_response(array $response): ?string {
$responsewords = [];
foreach ($this->roworder as $key => $rownumber) {
Expand All @@ -238,6 +264,7 @@ public function summarise_response(array $response): ?string {
return implode('; ', $responsewords);
}

#[\Override]
public function is_complete_response(array $response): bool {
foreach ($this->roworder as $key => $rownumber) {
$fieldname = $this->field($key);
Expand All @@ -248,6 +275,7 @@ public function is_complete_response(array $response): bool {
return true;
}

#[\Override]
public function classify_response(array $response): array {
$classifiedresponse = [];
foreach ($this->roworder as $key => $rownumber) {
Expand All @@ -269,6 +297,7 @@ public function classify_response(array $response): array {
return $classifiedresponse;
}

#[\Override]
public function prepare_simulated_post_data($simulatedresponse): array {
// Expected structure of $simulatedresponse is Row field name => Col name.
// Each row must be present, in order.
Expand All @@ -293,13 +322,15 @@ public function prepare_simulated_post_data($simulatedresponse): array {
return $postdata;
}

#[\Override]
public function grade_response(array $response): array {
// Retrieve the number of right responses and the total number of responses.
[$numrightparts, $total] = $this->get_num_parts_right($response);
$fraction = $numrightparts / $total;
return [$fraction, question_state::graded_state_for_fraction($fraction)];
}

#[\Override]
public function get_num_parts_right(array $response): array {
$numright = 0;
foreach ($this->roworder as $key => $rownumber) {
Expand All @@ -318,10 +349,12 @@ public function get_num_parts_right(array $response): array {
*/
class qtype_oumatrix_multiple extends qtype_oumatrix_base {

#[\Override]
public function get_renderer(moodle_page $page) {
return $page->get_renderer('qtype_oumatrix', 'multiple');
}

#[\Override]
public function get_expected_data(): array {
$expected = [];
foreach ($this->rows as $row) {
Expand All @@ -332,6 +365,7 @@ public function get_expected_data(): array {
return $expected;
}

#[\Override]
public function is_choice_selected(array $response, int $rowkey, int $colnumber): bool {
$responsekey = $this->field($rowkey, $colnumber);
if (array_key_exists($responsekey, $response)) {
Expand All @@ -351,6 +385,7 @@ protected function field(int $rowkey, int $columnnumber): string {
return 'rowanswers' . $rowkey . '_' . $columnnumber;
}

#[\Override]
public function is_same_response(array $prevresponse, array $newresponse): bool {
foreach ($this->roworder as $key => $notused) {
foreach ($this->columns as $column) {
Expand All @@ -363,6 +398,7 @@ public function is_same_response(array $prevresponse, array $newresponse): bool
return true;
}

#[\Override]
public function get_correct_response(): ?array {
$response = [];
foreach ($this->roworder as $key => $rownumber) {
Expand All @@ -375,6 +411,7 @@ public function get_correct_response(): ?array {
return $response;
}

#[\Override]
public function summarise_response(array $response): ?string {
$responsewords = [];
foreach ($this->roworder as $key => $rownumber) {
Expand All @@ -396,6 +433,7 @@ public function summarise_response(array $response): ?string {
return implode('; ', $responsewords);
}

#[\Override]
public function is_complete_response(array $response): bool {
foreach ($this->roworder as $key => $rownumber) {
$inputresponse = false;
Expand All @@ -412,6 +450,7 @@ public function is_complete_response(array $response): bool {
return true;
}

#[\Override]
public function classify_response(array $response) {
$classifiedresponse = [];
foreach ($this->roworder as $rowkey => $rownumber) {
Expand All @@ -433,6 +472,7 @@ public function classify_response(array $response) {
return $classifiedresponse;
}

#[\Override]
public function prepare_simulated_post_data($simulatedresponse): array {
$postdata = [];
$subquestions = array_keys($simulatedresponse);
Expand All @@ -457,6 +497,7 @@ public function prepare_simulated_post_data($simulatedresponse): array {
return $postdata;
}

#[\Override]
public function grade_response(array $response): array {
// Retrieve the number of right responses and the total number of responses.
if ($this->grademethod == 'allnone') {
Expand Down Expand Up @@ -527,6 +568,8 @@ public function get_num_parts_grade_partial(array $response): array {
}

/**
* Get the number of selected choices in a response.
*
* @param array $response responses, as returned by
* {@link question_attempt_step::get_qt_data()}.
* @return int the number of choices that were selected in this response.
Expand Down
Loading

1 comment on commit e9bd531

@mkassaei
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @timhunt, These changes look good to me, thank you for sorting this out and making it works with php8.3. I have not seen any changes to the test_classify_response() and get_possible_responses() methods, Am I missing something?

Please sign in to comment.