Skip to content

Commit

Permalink
Merge branch 'MDL-81039' of https://github.com/timhunt/moodle
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewnicols committed Mar 12, 2024
2 parents 9cdc585 + 02de9a1 commit 9e09693
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 39 deletions.
70 changes: 40 additions & 30 deletions question/engine/lib.php
Original file line number Diff line number Diff line change
Expand Up @@ -477,21 +477,21 @@ public static function initialise_js() {
*/
class question_display_options {
/**#@+
* @var integer named constants for the values that most of the options take.
* @var int named constants for the values that most of the options take.
*/
const SHOW_ALL = -1;
const HIDDEN = 0;
const VISIBLE = 1;
const EDITABLE = 2;
/**#@-*/

/**#@+ @var integer named constants for the {@link $marks} option. */
/**#@+ @var int named constants for the {@see $marks} option. */
const MAX_ONLY = 1;
const MARK_AND_MAX = 2;
/**#@-*/

/**
* @var integer maximum value for the {@link $markpd} option. This is
* @var int maximum value for the {@see $markpd} option. This is
* effectively set by the database structure, which uses NUMBER(12,7) columns
* for question marks/fractions.
*/
Expand All @@ -514,65 +514,76 @@ class question_display_options {
* This includes the green/red hilighting of the bits of their response,
* whether the one-line summary of the current state of the question says
* correct/incorrect or just answered.
* @var integer {@link question_display_options::HIDDEN} or
* {@link question_display_options::VISIBLE}
* @var int {@see question_display_options::HIDDEN} or
* {@see question_display_options::VISIBLE}
*/
public $correctness = self::VISIBLE;

/**
* The the mark and/or the maximum available mark for this question be visible?
* @var integer {@link question_display_options::HIDDEN},
* {@link question_display_options::MAX_ONLY} or {@link question_display_options::MARK_AND_MAX}
* @var int {@see question_display_options::HIDDEN},
* {@see question_display_options::MAX_ONLY} or {@see question_display_options::MARK_AND_MAX}
*/
public $marks = self::MARK_AND_MAX;

/** @var number of decimal places to use when formatting marks for output. */
/** @var int of decimal places to use when formatting marks for output. */
public $markdp = 2;

/**
* Should the flag this question UI element be visible, and if so, should the
* flag state be changable?
* @var integer {@link question_display_options::HIDDEN},
* {@link question_display_options::VISIBLE} or {@link question_display_options::EDITABLE}
* flag state be changeable?
*
* @var int {@see question_display_options::HIDDEN},
* {@see question_display_options::VISIBLE} or {@see question_display_options::EDITABLE}
*/
public $flags = self::VISIBLE;

/**
* Should the specific feedback be visible.
* @var integer {@link question_display_options::HIDDEN} or
* {@link question_display_options::VISIBLE}
*
* Specific feedback is typically the part of the feedback that changes based on the
* answer that the student gave. For example the feedback shown if a particular choice
* has been chosen in a multi-choice question. It also includes the combined feedback
* that a lost of question types have (e.g. feedback for any correct/incorrect response.)
*
* @var int {@see question_display_options::HIDDEN} or
* {@see question_display_options::VISIBLE}
*/
public $feedback = self::VISIBLE;

/**
* For questions with a number of sub-parts (like matching, or
* multiple-choice, multiple-reponse) display the number of sub-parts that
* were correct.
* @var integer {@link question_display_options::HIDDEN} or
* {@link question_display_options::VISIBLE}
* @var int {@see question_display_options::HIDDEN} or
* {@see question_display_options::VISIBLE}
*/
public $numpartscorrect = self::VISIBLE;

/**
* Should the general feedback be visible?
* @var integer {@link question_display_options::HIDDEN} or
* {@link question_display_options::VISIBLE}
*
* This is typically feedback shown to all students after the question
* is finished, irrespective of which answer they gave.
*
* @var int {@see question_display_options::HIDDEN} or
* {@see question_display_options::VISIBLE}
*/
public $generalfeedback = self::VISIBLE;

/**
* Should the automatically generated display of what the correct answer is
* be visible?
* @var integer {@link question_display_options::HIDDEN} or
* {@link question_display_options::VISIBLE}
* Should the automatically generated display of what the correct answer be visible?
*
* @var int {@see question_display_options::HIDDEN} or
* {@see question_display_options::VISIBLE}
*/
public $rightanswer = self::VISIBLE;

/**
* Should the manually added marker's comment be visible. Should the link for
* adding/editing the comment be there.
* @var integer {@link question_display_options::HIDDEN},
* {@link question_display_options::VISIBLE}, or {@link question_display_options::EDITABLE}.
* @var int {@see question_display_options::HIDDEN},
* {@see question_display_options::VISIBLE}, or {@see question_display_options::EDITABLE}.
* Editable means that form fields are displayed inline.
*/
public $manualcomment = self::VISIBLE;
Expand All @@ -593,8 +604,8 @@ class question_display_options {

/**
* Should the history of previous question states table be visible?
* @var integer {@link question_display_options::HIDDEN} or
* {@link question_display_options::VISIBLE}
* @var int {@see question_display_options::HIDDEN} or
* {@see question_display_options::VISIBLE}
*/
public $history = self::HIDDEN;

Expand Down Expand Up @@ -659,9 +670,8 @@ class question_display_options {
public ?bool $versioninfo = null;

/**
* Set all the feedback-related fields {@link $feedback}, {@link generalfeedback},
* {@link rightanswer} and {@link manualcomment} to
* {@link question_display_options::HIDDEN}.
* Set all the feedback-related fields, feedback, numpartscorrect, generalfeedback,
* rightanswer, manualcomment} and correctness to {@see question_display_options::HIDDEN}.
*/
public function hide_all_feedback() {
$this->feedback = self::HIDDEN;
Expand All @@ -676,10 +686,10 @@ public function hide_all_feedback() {
* Returns the valid choices for the number of decimal places for showing
* question marks. For use in the user interface.
*
* Calling code should probably use {@link question_engine::get_dp_options()}
* Calling code should probably use {@see question_engine::get_dp_options()}
* rather than calling this method directly.
*
* @return array suitable for passing to {@link html_writer::select()} or similar.
* @return array suitable for passing to {@see html_writer::select()} or similar.
*/
public static function get_dp_options() {
$options = array();
Expand Down
7 changes: 3 additions & 4 deletions question/type/multichoice/question.php
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,9 @@ public function check_file_access($qa, $options, $component, $filearea, $args, $
break;
}
}
// Param $options->suppresschoicefeedback is a hack specific to the
// oumultiresponse question type. It would be good to refactor to
// avoid refering to it here.
return $options->feedback && empty($options->suppresschoicefeedback) &&
qtype_multichoice::support_legacy_review_options_hack($options);
return $options->feedback &&
$options->feedback !== qtype_multichoice::COMBINED_BUT_NOT_CHOICE_FEEDBACK &&
$isselected;

} else if ($component == 'question' && $filearea == 'hint') {
Expand Down
29 changes: 29 additions & 0 deletions question/type/multichoice/questiontype.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,35 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class qtype_multichoice extends question_type {
/**
* @var int a special value that can be set for {@see question_display_options::$feedback}.
*
* This is not used by the core question type, but is used by some variants of this question
* types in the plugins database, including qtype_oumultiresponse and qtype_answersselect.
*
* If ->feedback is set to this value, then the renderer will display the combined feebdack,
* but not the feedback for each specific choice.
*/
const COMBINED_BUT_NOT_CHOICE_FEEDBACK = 0x100;

/**
* Helper to catch and update if a plugin is using the old version of the COMBINED_BUT_NOT_CHOICE_FEEDBACK thing.
*
* @param question_display_options $options to be updated before being used.
*/
public static function support_legacy_review_options_hack(question_display_options $options): void {
if (empty($options->suppresschoicefeedback)) {
return; // Nothing to do.
}

debugging('$options->suppresschoicefeedback should no longer be used. To get a similar effect, ' .
'instead set $options->feedback = $options->feedback && qtype_multichoice::COMBINED_BUT_NOT_CHOICE_FEEDBACK.');
if ($options->feedback) {
$options->feedback = self::COMBINED_BUT_NOT_CHOICE_FEEDBACK;
}
unset($options->suppresschoicefeedback);
}

public function get_question_options($question) {
global $DB, $OUTPUT;

Expand Down
8 changes: 3 additions & 5 deletions question/type/multichoice/renderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ abstract protected function get_input_id(question_attempt $qa, $value);
/**
* Whether a choice should be considered right, wrong or partially right.
* @param question_answer $ans representing one of the choices.
* @return fload 1.0, 0.0 or something in between, respectively.
* @return float 1.0, 0.0 or something in between, respectively.
*/
abstract protected function is_right(question_answer $ans);

Expand Down Expand Up @@ -118,10 +118,8 @@ public function formulation_and_controls(question_attempt $qa,
'data-region' => 'answer-label',
]);

// Param $options->suppresschoicefeedback is a hack specific to the
// oumultiresponse question type. It would be good to refactor to
// avoid refering to it here.
if ($options->feedback && empty($options->suppresschoicefeedback) &&
qtype_multichoice::support_legacy_review_options_hack($options);
if ($options->feedback && $options->feedback !== qtype_multichoice::COMBINED_BUT_NOT_CHOICE_FEEDBACK &&
$isselected && trim($ans->feedback)) {
$feedback[] = html_writer::tag('div',
$question->make_html_inline($question->format_text(
Expand Down

0 comments on commit 9e09693

Please sign in to comment.