Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-32358

Validation error does not always show correct messages for numerical and calculateds question types

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1, 2.2.2
    • Fix Version/s: 2.1.6, 2.2.3
    • Component/s: Questions
    • Labels:
    • Testing Instructions:
      Hide

      To test, create a numerical question with units shown on select or radio element.
      On preview with adaptative mode verify that you have the correct error message

      1. use a , in your number if your are set in english , select a unit

      the error :Please enter your answer without using the thousand separator (,).

      2. use a , in your number if your are set in english , DO NOT select a unit

      the error : You must select a unit.

      This confirms that the error function have the same codeflow that the is_complete_response()

      3. put a unit in the number field and select or not a unit

      the error : You must enter a valid number. Do not include a unit in your response.

      and so on

      As calculated and calculatedsimple use the numerical renderer, there in no need to test them.

      Show
      To test, create a numerical question with units shown on select or radio element. On preview with adaptative mode verify that you have the correct error message 1. use a , in your number if your are set in english , select a unit the error :Please enter your answer without using the thousand separator (,). 2. use a , in your number if your are set in english , DO NOT select a unit the error : You must select a unit. This confirms that the error function have the same codeflow that the is_complete_response() 3. put a unit in the number field and select or not a unit the error : You must enter a valid number. Do not include a unit in your response. and so on As calculated and calculatedsimple use the numerical renderer, there in no need to test them.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      I was using my french keyboard so the , was used for the numbers.
      I received the message that the unit was not selected.
      This message comes from numerical/question.php
      public function get_validation_error(array $response) {
      which code reproduces the steps of public function is_complete_response(array $response)

      { to select the correct message. This will work as long as the two functions receive the same data. In numerical this imply answer and unit, the actual call in numerical/renderer.php is $question->get_validation_error(array('answer' => $currentanswer) the $selectedunit was forgotten so I received the no unit selected message. Adding the unitseletected {noformat}

      if ($qa->get_state() == question_state::$invalid)

      { $result .= html_writer::nonempty_tag('div', $question->get_validation_error(array('answer' => $currentanswer,'unit' => $selectedunit)), array('class' => 'validationerror')); }

       

      solves the problem.

        Gliffy Diagrams

          Activity

          Hide
          ppichet Pierre Pichet added a comment -

          Another simple bug in a learning process

          Show
          ppichet Pierre Pichet added a comment - Another simple bug in a learning process
          Hide
          ppichet Pierre Pichet added a comment - - edited

          To test, create a numerical question with units shown on select or radio element.
          On preview with adaptative mode verify that you have the correct error message
          1. use a , in your number if your are set in english , select a unit

          the error :Please enter your answer without using the thousand separator (,).

          2. use a , in your number if your are set in english , DO NOT select a unit

          the error : You must select a unit.

          This confirms that the error function have the same codeflow that the is_complete_response()

          3. put a unit in the number field and select or not a unit

          the error : You must enter a valid number. Do not include a unit in your response.

          and so on

          As calculated and calculatedsimple use the numerical renderer, there in no need to test them.

          Show
          ppichet Pierre Pichet added a comment - - edited To test, create a numerical question with units shown on select or radio element. On preview with adaptative mode verify that you have the correct error message 1. use a , in your number if your are set in english , select a unit the error :Please enter your answer without using the thousand separator (,). 2. use a , in your number if your are set in english , DO NOT select a unit the error : You must select a unit. This confirms that the error function have the same codeflow that the is_complete_response() 3. put a unit in the number field and select or not a unit the error : You must enter a valid number. Do not include a unit in your response. and so on As calculated and calculatedsimple use the numerical renderer, there in no need to test them.
          Hide
          timhunt Tim Hunt added a comment -

          Thanks Pierre. This is a good fix. Submitting for integration now.

          I will just note that the separate unit field is only used if $question->has_separate_unit_field() returns true. However, when that returns false, we set $selectedunit to null, and it will cause no harm to pass the extra 'unit' => null to get_validation_error.

          Two minor comments:
          1. Moodle coding style likes a space after a ,
          2. We don't use the "Signed-off-by" thing in Moodle. I suppose it does no harm to include it, but it would be more consistent with other people to leave it out.

          Show
          timhunt Tim Hunt added a comment - Thanks Pierre. This is a good fix. Submitting for integration now. I will just note that the separate unit field is only used if $question->has_separate_unit_field() returns true. However, when that returns false, we set $selectedunit to null, and it will cause no harm to pass the extra 'unit' => null to get_validation_error. Two minor comments: 1. Moodle coding style likes a space after a , 2. We don't use the "Signed-off-by" thing in Moodle. I suppose it does no harm to include it, but it would be more consistent with other people to leave it out.
          Hide
          timhunt Tim Hunt added a comment -

          One other thing Pierre:

          3. You should put the testing instructions in the special 'Testing instructions' field. You may have to use the edit issue button to do this.

          I have just done the necessary copy-and-paste.

          Show
          timhunt Tim Hunt added a comment - One other thing Pierre: 3. You should put the testing instructions in the special 'Testing instructions' field. You may have to use the edit issue button to do this. I have just done the necessary copy-and-paste.
          Hide
          ppichet Pierre Pichet added a comment -

          I take note of your comments. I did not know how to edit Testing instructions.

          I have noted the lines in the renderer

             if ($question->has_separate_unit_field()) {
                      $selectedunit = $qa->get_last_qt_var('unit');
                  } else {
                      $selectedunit = null;
                  }
           

          and I did tests with questions without units or with units in a single input field

          Show
          ppichet Pierre Pichet added a comment - I take note of your comments. I did not know how to edit Testing instructions. I have noted the lines in the renderer if ($question->has_separate_unit_field()) { $selectedunit = $qa->get_last_qt_var('unit'); } else { $selectedunit = null; } and I did tests with questions without units or with units in a single input field
          Hide
          poltawski Dan Poltawski added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          poltawski Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          ppichet Pierre Pichet added a comment -

          Tim,
          "Moodle coding style likes a space after a ,"
          As I need to rebase everything, I could correct the space in the process.
          One way to do it could be to amend the commit ( as this is the last commit) in each version before
          doing the rebase for each of them.

          What do you suggest ?

          Show
          ppichet Pierre Pichet added a comment - Tim, "Moodle coding style likes a space after a ," As I need to rebase everything, I could correct the space in the process. One way to do it could be to amend the commit ( as this is the last commit) in each version before doing the rebase for each of them. What do you suggest ?
          Hide
          timhunt Tim Hunt added a comment -

          Yes, please fix the code style with an amend before rebasing if you have the time. Thanks.

          Show
          timhunt Tim Hunt added a comment - Yes, please fix the code style with an amend before rebasing if you have the time. Thanks.
          Hide
          ppichet Pierre Pichet added a comment -

          Amend and rebasing done .

          Show
          ppichet Pierre Pichet added a comment - Amend and rebasing done .
          Hide
          poltawski Dan Poltawski added a comment -

          Thanks Pierre,

          This has been integrated now

          Show
          poltawski Dan Poltawski added a comment - Thanks Pierre, This has been integrated now
          Hide
          ankit_frenz Ankit Agarwal added a comment -

          works as expected!
          Thanks

          Show
          ankit_frenz Ankit Agarwal added a comment - works as expected! Thanks
          Hide
          poltawski Dan Poltawski added a comment -

          Bonza mate!

          Your changes have made it into the Moodle release! Its time to celebrate! Put a shrimp on the barbie and grab a stubby.

          Hooroo

          Show
          poltawski Dan Poltawski added a comment - Bonza mate! Your changes have made it into the Moodle release! Its time to celebrate! Put a shrimp on the barbie and grab a stubby. Hooroo

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                14/May/12