Skip to content

Commit

Permalink
Fix codechecker issues
Browse files Browse the repository at this point in the history
  • Loading branch information
timhunt committed Nov 10, 2023
1 parent ce572dc commit 43c10eb
Show file tree
Hide file tree
Showing 18 changed files with 223 additions and 149 deletions.
32 changes: 19 additions & 13 deletions classes/attempt.php
Original file line number Diff line number Diff line change
Expand Up @@ -396,17 +396,17 @@ public function process_submitted_actions(array $simulatedpostdata = null) {
attempt_storage::instance()->update_timemodified($this->quba->get_id());

// Log the submit.
\filter_embedquestion\event\question_attempted::create(['context' => $this->embedlocation->context,
'objectid' => $this->current_question()->id])->trigger();
\filter_embedquestion\event\question_attempted::create(
['context' => $this->embedlocation->context, 'objectid' => $this->current_question()->id])->trigger();
$transaction->allow_commit();
}

/**
* Log that the user is viewing the question.
*/
public function log_view() {
\filter_embedquestion\event\question_viewed::create(['context' => $this->embedlocation->context,
'objectid' => $this->current_question()->id])->trigger();
\filter_embedquestion\event\question_viewed::create(
['context' => $this->embedlocation->context, 'objectid' => $this->current_question()->id])->trigger();
}

/**
Expand All @@ -429,18 +429,21 @@ public function render_question(\filter_embedquestion\output\renderer $renderer)
// If the question is finished, add a Start again button.
if ($this->is_question_finished()) {
$this->options->extrainfocontent = \html_writer::div(
\html_writer::empty_tag('input', ['type' => 'submit', 'name' => 'restart',
'value' => get_string('restart', 'filter_embedquestion'),
'class' => 'btn btn-secondary', 'data-formchangechecker-non-submit' => 1])
\html_writer::empty_tag('input', [
'type' => 'submit',
'name' => 'restart',
'value' => get_string('restart', 'filter_embedquestion'),
'class' => 'btn btn-secondary',
'data-formchangechecker-non-submit' => 1,
])
);
}

// Show an 'Edit question' action to those with permissions.
$relevantcourseid = utils::get_relevant_courseid($this->embedlocation->context);
if (question_has_capability_on($this->current_question(), 'edit')) {
$this->options->editquestionparams = ['returnurl' => $this->embedlocation->pageurl,
'courseid' => $relevantcourseid];

$this->options->editquestionparams =
['returnurl' => $this->embedlocation->pageurl, 'courseid' => $relevantcourseid];
}

// Show an 'Question bank' action to those with permissions.
Expand All @@ -456,9 +459,12 @@ public function render_question(\filter_embedquestion\output\renderer $renderer)

// Start the question form.
$output = '';
$output .= \html_writer::start_tag('form',
['method' => 'post', 'action' => $this->get_action_url(),
'enctype' => 'multipart/form-data', 'id' => 'responseform']);
$output .= \html_writer::start_tag('form', [
'method' => 'post',
'action' => $this->get_action_url(),
'enctype' => 'multipart/form-data',
'id' => 'responseform',
]);
$output .= \html_writer::start_tag('div');
$output .= \html_writer::empty_tag('input',
['type' => 'hidden', 'name' => 'sesskey', 'value' => sesskey()]);
Expand Down
2 changes: 1 addition & 1 deletion classes/event/category_token_created.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,6 @@ public function get_description() {
}

public static function get_objectid_mapping() {
return array('db' => 'question_categories', 'restore' => 'question_categories');
return ['db' => 'question_categories', 'restore' => 'question_categories'];
}
}
2 changes: 1 addition & 1 deletion classes/event/question_attempted.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,6 @@ public function get_description() {
}

public static function get_objectid_mapping() {
return array('db' => 'question', 'restore' => 'question');
return ['db' => 'question', 'restore' => 'question'];
}
}
2 changes: 1 addition & 1 deletion classes/event/question_started.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,6 @@ public function get_description() {
}

public static function get_objectid_mapping() {
return array('db' => 'question', 'restore' => 'question');
return ['db' => 'question', 'restore' => 'question'];
}
}
2 changes: 1 addition & 1 deletion classes/event/question_viewed.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,6 @@ public function get_description() {
}

public static function get_objectid_mapping() {
return array('db' => 'question', 'restore' => 'question');
return ['db' => 'question', 'restore' => 'question'];
}
}
2 changes: 1 addition & 1 deletion classes/event/token_created.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,6 @@ public function get_description() {
}

public static function get_objectid_mapping() {
return array('db' => 'question', 'restore' => 'question');
return ['db' => 'question', 'restore' => 'question'];
}
}
29 changes: 21 additions & 8 deletions classes/external.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public static function get_sharable_question_choices(int $courseid, string $cate
global $USER;

self::validate_parameters(self::get_sharable_question_choices_parameters(),
array('courseid' => $courseid, 'categoryidnumber' => $categoryidnumber));
['courseid' => $courseid, 'categoryidnumber' => $categoryidnumber]);

$context = \context_course::instance($courseid);
self::validate_context($context);
Expand Down Expand Up @@ -190,13 +190,26 @@ public static function get_embed_code(int $courseid, string $categoryidnumber, s
string $forcedlanguage): string {
global $CFG;

self::validate_parameters(self::get_embed_code_parameters(),
array('courseid' => $courseid, 'categoryidnumber' => $categoryidnumber, 'questionidnumber' => $questionidnumber,
'iframedescription' => $iframedescription,
'behaviour' => $behaviour, 'maxmark' => $maxmark, 'variant' => $variant, 'correctness' => $correctness,
'marks' => $marks, 'markdp' => $markdp, 'feedback' => $feedback, 'generalfeedback' => $generalfeedback,
'rightanswer' => $rightanswer, 'history' => $history, 'forcedlanguage' => $forcedlanguage,
));
self::validate_parameters(
self::get_embed_code_parameters(),
[
'courseid' => $courseid,
'categoryidnumber' => $categoryidnumber,
'questionidnumber' => $questionidnumber,
'iframedescription' => $iframedescription,
'behaviour' => $behaviour,
'maxmark' => $maxmark,
'variant' => $variant,
'correctness' => $correctness,
'marks' => $marks,
'markdp' => $markdp,
'feedback' => $feedback,
'generalfeedback' => $generalfeedback,
'rightanswer' => $rightanswer,
'history' => $history,
'forcedlanguage' => $forcedlanguage,
]
);

$context = \context_course::instance($courseid);
self::validate_context($context);
Expand Down
6 changes: 4 additions & 2 deletions classes/form/embed_options_form.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,10 @@ public function definition() {

$mform->addElement('header', 'attemptheader', get_string('attemptoptions', 'filter_embedquestion'));

$behaviours = ['' => get_string('defaultx', 'filter_embedquestion',
\question_engine::get_behaviour_name($defaultoptions->behaviour))] + utils::behaviour_choices();
$behaviours = [
'' => get_string('defaultx', 'filter_embedquestion',
\question_engine::get_behaviour_name($defaultoptions->behaviour)),
] + utils::behaviour_choices();
$mform->addElement('select', 'behaviour', get_string('howquestionbehaves', 'filter_embedquestion'),
$behaviours);

Expand Down
19 changes: 13 additions & 6 deletions classes/output/renderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,19 @@ protected function add_fill_with_correct_link(string $questionhtml): string {
* @return string HTML string
*/
public function render_fill_with_correct(): string {
return \html_writer::div(\html_writer::tag('button',
$this->pix_icon('e/tick', '', 'moodle', ['class' => 'iconsmall']) .
\html_writer::span(get_string('fillcorrect', 'mod_quiz')),
['type' => 'submit', 'name' => 'fillwithcorrect', 'value' => 1,
'class' => 'btn btn-link']),
'filter_embedquestion-fill-link');
return \html_writer::div(
\html_writer::tag(
'button',
$this->pix_icon('e/tick', '', 'moodle', ['class' => 'iconsmall']) .
\html_writer::span(get_string('fillcorrect', 'mod_quiz')),
[
'type' => 'submit',
'name' => 'fillwithcorrect', 'value' => 1,
'class' => 'btn btn-link',
],
),
'filter_embedquestion-fill-link',
);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions classes/question_options.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public function __construct() {
* @return array names and param types of the options we read from the request.
*/
public static function get_field_types(): array {
return array(
return [
'iframedescription' => PARAM_TEXT,
'behaviour' => PARAM_ALPHA,
'maxmark' => PARAM_FLOAT,
Expand All @@ -101,7 +101,7 @@ public static function get_field_types(): array {
'rightanswer' => PARAM_BOOL,
'history' => PARAM_BOOL,
'forcedlanguage' => PARAM_LANG,
);
];
}

/**
Expand Down
35 changes: 21 additions & 14 deletions classes/task/cleanup_task.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,21 +45,28 @@ public function execute() {
$lastmodifiedcutoff = time() - self::MAX_AGE;

mtrace("\n Cleaning up old embedded question attempts...", '');
$oldattempts = new \qubaid_join('{question_usages} quba', 'quba.id',
'quba.component = :qubacomponent
AND NOT EXISTS (
SELECT 1
FROM {question_attempts} subq_qa
JOIN {question_attempt_steps} subq_qas ON subq_qas.questionattemptid = subq_qa.id
JOIN {question_usages} subq_qu ON subq_qu.id = subq_qa.questionusageid
WHERE subq_qa.questionusageid = quba.id
AND subq_qu.component = :qubacomponent2
AND (subq_qa.timemodified > :qamodifiedcutoff
OR subq_qas.timecreated > :stepcreatedcutoff)
)
$oldattempts = new \qubaid_join(
'{question_usages} quba',
'quba.id',
'quba.component = :qubacomponent
AND NOT EXISTS (
SELECT 1
FROM {question_attempts} subq_qa
JOIN {question_attempt_steps} subq_qas ON subq_qas.questionattemptid = subq_qa.id
JOIN {question_usages} subq_qu ON subq_qu.id = subq_qa.questionusageid
WHERE subq_qa.questionusageid = quba.id
AND subq_qu.component = :qubacomponent2
AND (subq_qa.timemodified > :qamodifiedcutoff
OR subq_qas.timecreated > :stepcreatedcutoff)
)
',
array('qubacomponent' => 'filter_embedquestion', 'qubacomponent2' => 'filter_embedquestion',
'qamodifiedcutoff' => $lastmodifiedcutoff, 'stepcreatedcutoff' => $lastmodifiedcutoff));
[
'qubacomponent' => 'filter_embedquestion',
'qubacomponent2' => 'filter_embedquestion',
'qamodifiedcutoff' => $lastmodifiedcutoff,
'stepcreatedcutoff' => $lastmodifiedcutoff,
],
);

\question_engine::delete_questions_usage_by_activities($oldattempts);
mtrace('done.');
Expand Down
38 changes: 23 additions & 15 deletions classes/utils.php
Original file line number Diff line number Diff line change
Expand Up @@ -168,18 +168,21 @@ public static function get_question_by_idnumber(int $categoryid, string $idnumbe

if (self::has_question_versionning()) {
$question = $DB->get_record_sql('
SELECT q.*, qbe.idnumber, qbe.questioncategoryid AS category,
qv.id AS versionid, qv.version, qv.questionbankentryid
FROM {question_bank_entries} qbe
JOIN {question_versions} qv ON qv.questionbankentryid = qbe.id AND qv.version = (
SELECT MAX(version)
FROM {question_versions}
WHERE questionbankentryid = qbe.id AND status = :ready
)
JOIN {question} q ON q.id = qv.questionid
WHERE qbe.questioncategoryid = :category AND qbe.idnumber = :idnumber',
['ready' => question_version_status::QUESTION_STATUS_READY,
'category' => $categoryid, 'idnumber' => $idnumber]);
SELECT q.*, qbe.idnumber, qbe.questioncategoryid AS category,
qv.id AS versionid, qv.version, qv.questionbankentryid
FROM {question_bank_entries} qbe
JOIN {question_versions} qv ON qv.questionbankentryid = qbe.id AND qv.version = (
SELECT MAX(version)
FROM {question_versions}
WHERE questionbankentryid = qbe.id AND status = :ready
)
JOIN {question} q ON q.id = qv.questionid
WHERE qbe.questioncategoryid = :category AND qbe.idnumber = :idnumber',
[
'ready' => question_version_status::QUESTION_STATUS_READY,
'category' => $categoryid, 'idnumber' => $idnumber,
],
);
} else {
$question = $DB->get_record_select('question',
"category = ? AND idnumber = ? AND hidden = 0 AND parent = 0",
Expand All @@ -204,9 +207,14 @@ public static function get_question_by_idnumber(int $categoryid, string $idnumbe
public static function is_latest_version(\question_definition $question): bool {
global $DB;

$latestversion = $DB->get_field('question_versions', 'MAX(version)',
['questionbankentryid' => $question->questionbankentryid,
'status' => question_version_status::QUESTION_STATUS_READY]);
$latestversion = $DB->get_field(
'question_versions',
'MAX(version)',
[
'questionbankentryid' => $question->questionbankentryid,
'status' => question_version_status::QUESTION_STATUS_READY,
]
);

return $question->version == $latestversion;
}
Expand Down
12 changes: 6 additions & 6 deletions tests/behat/behat_filter_embedquestion.php
Original file line number Diff line number Diff line change
Expand Up @@ -131,18 +131,18 @@ public function user_has_attempted_with_responses($username, $contextlevel, $con

if (!array_key_exists($questioninfo['pagename'], $datas)) {
$datas[$questioninfo['pagename']] = [
'context' => $attemptcontext,
'user' => $user,
'slots' => []
'context' => $attemptcontext,
'user' => $user,
'slots' => [],
];
}

$question = $generator->get_question_from_embed_id($questioninfo['question']);

$datas[$questioninfo['pagename']]['slots'][] = [
'no' => $questioninfo['slot'],
'question' => $question,
'response' => $questioninfo['response']
'no' => $questioninfo['slot'],
'question' => $question,
'response' => $questioninfo['response'],
];
}

Expand Down
Loading

0 comments on commit 43c10eb

Please sign in to comment.