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

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

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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

      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.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              jerome 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
              jerome 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
              stronk7 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
              stronk7 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
              andyjdavis 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
              andyjdavis 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
              stronk7 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
              stronk7 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
              andyjdavis Andrew Davis added a comment -

              Ive raised MDL-26575

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

              Thanks guys!

              Show
              tsala Helen Foster added a comment - Thanks guys!
              Hide
              poltawski 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
              poltawski 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
              andyjdavis 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
              andyjdavis 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:
                    Fix Release Date:
                    5/May/11