Moodle
  1. Moodle
  2. MDL-30559

Conditional availablility does not restrict min and max grades to be numbers.

    Details

    • Testing Instructions:
      Hide

      Turn on conditional availability and activity completion for your Moodle.

      Edit an activity and make sure that all the following trigger the right validation messages:

      1. Min not a number
      2. Max not a number
      3. Min > Max
      4. Min and/or max set, but no grade item selected.
      5. Grade item selected, but neither min nor max given.

      Check that the following valid combinations are accepted

      A. grade item, and both min < max input
      B. grade item with only min input.
      C. grade item with only max input.

      Show
      Turn on conditional availability and activity completion for your Moodle. Edit an activity and make sure that all the following trigger the right validation messages: 1. Min not a number 2. Max not a number 3. Min > Max 4. Min and/or max set, but no grade item selected. 5. Grade item selected, but neither min nor max given. Check that the following valid combinations are accepted A. grade item, and both min < max input B. grade item with only min input. C. grade item with only max input.
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      33337

      Description

      Try to make one activity only available when the grade on another activity is between 'frog' and 'toad'. You will get an error like

      Error writing to database
      
      More information about this error
      Debug info: ERROR: invalid input syntax for type numeric: "frog"
      INSERT INTO mdl_course_modules_availability (coursemoduleid,gradeitemid,grademin,grademax) VALUES
      [array (
      'coursemoduleid' => '20828',
      'gradeitemid' => '27',
      'grademin' => 'frog',
      'grademax' => 'toad',
      )]
      Stack trace:
      
      line 397 of /lib/dml/moodle_database.php: dml_write_exception thrown
          line 232 of /lib/dml/pgsql_native_moodle_database.php: call to moodle_database->query_end()
          line 792 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->query_end()
          line 844 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->insert_record_raw()
          line 249 of /lib/conditionlib.php: call to pgsql_native_moodle_database->insert_record()
          line 653 of /lib/conditionlib.php: call to condition_info->add_grade_condition()
          line 319 of /course/modedit.php: call to condition_info::update_cm_from_form()
      

        Activity

        Hide
        Tim Hunt added a comment -

        Sam, please review. If it is OK, I will backport to all relevant branches.

        Show
        Tim Hunt added a comment - Sam, please review. If it is OK, I will backport to all relevant branches.
        Hide
        Sam Marshall added a comment -

        Looks great to me.

        Show
        Sam Marshall added a comment - Looks great to me.
        Hide
        Aparup Banerjee added a comment -

        Tim, did you want to look @ this ? (seems like you haven't seen this after SamM)

        Show
        Aparup Banerjee added a comment - Tim, did you want to look @ this ? (seems like you haven't seen this after SamM)
        Hide
        Tim Hunt added a comment -

        Oh, I see. Sam just submitted this for integration after peer-reviewing, without giving me a chance to back-port. I'll back-port it shortly.

        Show
        Tim Hunt added a comment - Oh, I see. Sam just submitted this for integration after peer-reviewing, without giving me a chance to back-port. I'll back-port it shortly.
        Hide
        Tim Hunt added a comment -

        Mass branchification done. This can be integrated now.

        Show
        Tim Hunt added a comment - Mass branchification done. This can be integrated now.
        Hide
        Eloy Lafuente (stronk7) 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
        Eloy Lafuente (stronk7) 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
        Aparup Banerjee added a comment -

        lol @ branchification

        Show
        Aparup Banerjee added a comment - lol @ branchification
        Hide
        Aparup Banerjee added a comment -

        Thanks, i've integrated this into master and 2.x branches barring 2.0.x which has recently gone into security support only (although theres np picking into 2.0.x).

        Show
        Aparup Banerjee added a comment - Thanks, i've integrated this into master and 2.x branches barring 2.0.x which has recently gone into security support only (although theres np picking into 2.0.x).
        Hide
        Tim Hunt added a comment -

        My policy is that is a bug-fix cleanly cherry-picks into 2.0, we should apply it there. I know we are no longer officially supporting that branch, but if the same fix works there, I think we should apply it.

        Show
        Tim Hunt added a comment - My policy is that is a bug-fix cleanly cherry-picks into 2.0, we should apply it there. I know we are no longer officially supporting that branch, but if the same fix works there, I think we should apply it.
        Hide
        Aparup Banerjee added a comment -

        fair enough. integrated into 2.0.x as well with your +1

        Show
        Aparup Banerjee added a comment - fair enough. integrated into 2.0.x as well with your +1
        Hide
        Michael de Raadt added a comment -

        Test result: success!

        Show
        Michael de Raadt added a comment - Test result: success!
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks!

        Now... disconnect, relax and enjoy the next days, yay!

        Closing...ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks! Now... disconnect, relax and enjoy the next days, yay! Closing...ciao

          People

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

            Dates

            • Created:
              Updated:
              Resolved: