PoetOS / moodle-mod_questionnaire

Questionnaire module for Moodle - Contributed module
GNU General Public License v3.0
79 stars 179 forks source link

[3.2.X] [3.3.1] Question with choices (e.g. Rate) - Form saves empty line as question choice (Not a bug, but possibly unnecessary) #106

Closed danowar2k closed 4 years ago

danowar2k commented 6 years ago

In the question form where you have to enter stuff with the syntax 1=Text 2=Other Text

and so on, when you have an empty line at the end, that line is still saved as a choice of this question.

After it is saved, you can't even delete the choice by removing the empty choice when editing the question.

This leads to other bad behaviour, see separate ticket.

mchurchward commented 6 years ago

What version of questionnaire are you seeing this on? I am looking at the latest version (release 3.3.1 Build - 2017051800), and I cannot duplicate this problem. For example, I entered the following in a rate question:

4=Very easy to use 3=Easy to use 2=Somewhat difficult to use 1=Difficult to use Formatting Your Course Laying out Your Course Number of Clicks to Access Needed Content Adding Content Ability to Add/Change Themes/Appearance Overall Navigation of Moodle [blank line]

where [blank line] is an actual blank line. When I look at the questionnaire question, it is fine. When I edit the question, the blank line has been removed as expected.

danowar2k commented 6 years ago

EDIT: See below, I've understood more.

We're on Moodle 3.2.5 using the Questionnaire plugin 3.2.4. Looking at the differences between 3.2.4 and 3.2.5, the bug hasn't been fixed in the latest version. (Source: https://github.com/PoetOS/moodle-mod_questionnaire/compare/6c7180954249564f18018981c756ce0809a4e3f7...b4c2b0bd804e8fa8609b21237e969e5fbbad5bdb)

I think one of the problem could be that system dependent line endings are not respected. What if the user uses a Windows browser? Then the line ending is "\r\n". When you use a Unix browser, your line ending is "\n". All "\r\n" are transformed to "\n". But what if a Mac user submits the form? ("\r" only)

The problems in order:

  1. When I edit a question, I have to use the format "1=XXX\n" etc. . Then the rate question class does the following (Source: rate.php):
/**
 * Preprocess choice data.
 */
protected function form_preprocess_choicedata($formdata) {
    if (empty($formdata->allchoices)) {
        // Add dummy blank space character for empty value.
        $formdata->allchoices = " ";
    } else {
        $ispossibleanswer = false;
        $nbnameddegrees = 0;
        $nbvalues = 0;
        foreach ($allchoices as $choice) {
            if ($choice) {
                // Check for number from 1 to 3 digits, followed by the equal sign =.
                if (preg_match("/^[0-9]{1,3}=/", $choice)) {
                    $nbnameddegrees++;
                } else {
                    $nbvalues++;
                    $ispossibleanswer = true;
                }
            }
        }
        // Add carriage return and dummy blank space character for empty value.
        if (!$ispossibleanswer) {
            $formdata->allchoices .= "\n ";
        }

So when there is no "possible answer" (whatever that is), "\n " is added at the end of the choices. Why is an empty option necessary at all? When I save a rate question with no options, a space is added then, too. I don't understand. A space shouldn't be necessary, should it? (Well if the rest expects it, it would, but there's no reason to)

Then when choice records are updated (base.php)

        public function form_update($formdata, $questionnaire) {

The system does not prevent an empty choice to be stored in the database. Instead a RegEx checks if the option is a value option. If it isn't, it just sets "NULL" as the value. $choicerecord->content = trim($newchoices[$nidx]); $r = preg_match_all("/^(\d{1,2})(=.*)$/", $newchoices[$nidx], $matches); // This choice has been attributed a "score value" OR this is a rate question type. if ($r) { $newscore = $matches[1][0]; $choicerecord->value = $newscore; } else { // No score value for this choice. $choicerecord->value = null; }

danowar2k commented 6 years ago

EDIT: I understand now, see below.

Quick question: Where can I read about the syntax rules I have to use when filling out the allchoices form field? Above, you have four valued options:

4=Very easy to use 3=Easy to use 2=Somewhat difficult to use 1=Difficult to use

and six options(plus the blank line) that I can't quite place how and why they would be used in the scale:

Formatting Your Course Laying out Your Course Number of Clicks to Access Needed Content Adding Content Ability to Add/Change Themes/Appearance Overall Navigation of Moodle [blank line]

Are the lower options required?

danowar2k commented 6 years ago

Argh, I just understood (probably) why you always add a line with a space. You only add it if there is no "possible answer". What irritated me that the it seems the lines are called "possible answers", but they don't seem to be answers, but some kind of "subquestions" (Question: "What is your experience with the following OS?") (Subquestion: "What is your experience with the OS A?") And the "possible answers" would be options on the scale that you can choose from. (1,2,3,4) And then there would be a "chosen answer" from the "possible answers" to the "subquestion" of the "question".

danowar2k commented 6 years ago

Which leads me back to the original bug report... Obviously the space is inserted so that even when there is no "Subquestion", there is always one in the database. Why is that necessary? Couldn't the code just check if there are any defined subquestions? And if there are none, just add an empty label to the front of the scale? Or is the empty subquestion label necessary for anything?

mchurchward commented 4 years ago

There has been a lot of structural changes to the code around these areas. I am going to close this issue. If you still see a problem on one of the latest releases, feel free to reopen this issue or create a new one.