Moodle
  1. Moodle
  2. MDL-26461

Error (out of limits) changing one activity scale to shorter one

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.0.1
    • Fix Version/s: 2.0.3
    • Component/s: Ratings
    • Labels:
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Rank:
      16177

      Description

      Tested on qa.moodle.net (MDLQA-553)

      Select average aggregate = 100
      With a teacher and a manager vote 100 for a student post
      The change average aggregate = 10
      Go back to see the settings, and click on 10 (2).
      The popup display an exception.

        Issue Links

          Activity

          Hide
          Jérôme Mouneyrac added a comment -

          note that the first screenshot appears but I cannot remember the steps to reproduce (I was playing a lot with the forum aggregate settings and voting everywhere).

          All+submitted+ratings-1.jpg is the screenshot related to the described steps in the issue description.

          Show
          Jérôme Mouneyrac added a comment - note that the first screenshot appears but I cannot remember the steps to reproduce (I was playing a lot with the forum aggregate settings and voting everywhere). All+submitted+ratings-1.jpg is the screenshot related to the described steps in the issue description.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Note that, really, the problem isn't in ratings but in the inability of the settings of the modules of being locked once the activity has ratings IMO.

          Show
          Eloy Lafuente (stronk7) added a comment - Note that, really, the problem isn't in ratings but in the inability of the settings of the modules of being locked once the activity has ratings IMO.
          Hide
          Andrew Davis added a comment -

          Ive committed code to prevent this. There was an existing safety check for this situation but it had an off by 1 bug in it

          repo: git://github.com/andyjdavis/moodle.git
          branch: MDL-26461_change_rating_scale
          diff: https://github.com/andyjdavis/moodle/compare/master...MDL-26461_change_rating_scale

          Ultimately what we need is either to prevent the scale being changed after ratings have been submitted as Eloy suggested or to rescale submitted ratings when the forum scale is changed ie 60/100 becomes 6/10.

          Show
          Andrew Davis added a comment - Ive committed code to prevent this. There was an existing safety check for this situation but it had an off by 1 bug in it repo: git://github.com/andyjdavis/moodle.git branch: MDL-26461 _change_rating_scale diff: https://github.com/andyjdavis/moodle/compare/master...MDL-26461_change_rating_scale Ultimately what we need is either to prevent the scale being changed after ratings have been submitted as Eloy suggested or to rescale submitted ratings when the forum scale is changed ie 60/100 becomes 6/10.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          From PULL-340: This has been integrated, thanks!

          WARN: I've decided to add it in order to avoid the error ASAP, but please, create one new MDL issue (blocker/critical) about the need of:

          1) Block changes in scale if there are already ratings using them (modedit).
          2) Allow to delete existing / re-escale existing if the teacher wants to unlock the setting (based on some cap/s).

          I think this can be achieved in a similar way than activity completion is doing right now, take a look to if as reference.

          Show
          Eloy Lafuente (stronk7) added a comment - From PULL-340: This has been integrated, thanks! WARN: I've decided to add it in order to avoid the error ASAP, but please, create one new MDL issue (blocker/critical) about the need of: 1) Block changes in scale if there are already ratings using them (modedit). 2) Allow to delete existing / re-escale existing if the teacher wants to unlock the setting (based on some cap/s). I think this can be achieved in a similar way than activity completion is doing right now, take a look to if as reference.
          Hide
          Andrew Davis added a comment -

          Ive raised MDL-26575

          Show
          Andrew Davis added a comment - Ive raised MDL-26575
          Hide
          Helen Foster added a comment -

          Thanks guys!

          Show
          Helen Foster added a comment - Thanks guys!
          Hide
          Dan Poltawski added a comment -

          Hi Andrew/ others.

          I do not understand why we are setting the max to count - 1 as introduced in this issue. Please can you explain?

          It is causing ratings to be rounded down by one as reported in MDL-30955 and MDLSITE-1753

          Show
          Dan Poltawski added a comment - Hi Andrew/ others. I do not understand why we are setting the max to count - 1 as introduced in this issue. Please can you explain? It is causing ratings to be rounded down by one as reported in MDL-30955 and MDLSITE-1753
          Hide
          Andrew Davis added a comment -

          The minus 1 is there because we're dealing with an array index. For example an array containing a numeric scale 0-100 will have 101 elements in it. If you just do "count($scalemenu)" you will get a $maxrating of 101 which is 1 too many. I'll comment further in MDL-30955. This logic may not be quite right for a custom scale rather than a numeric scale.

          Show
          Andrew Davis added a comment - The minus 1 is there because we're dealing with an array index. For example an array containing a numeric scale 0-100 will have 101 elements in it. If you just do "count($scalemenu)" you will get a $maxrating of 101 which is 1 too many. I'll comment further in MDL-30955 . This logic may not be quite right for a custom scale rather than a numeric scale.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: