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

      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

          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: