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

            Jérôme Mouneyrac created issue -
            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.
            Jérôme Mouneyrac made changes -
            Field Original Value New Value
            Description Tested on qa.moodle.net

            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.
            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.
            Jérôme Mouneyrac made changes -
            Link This issue will help resolve MDLQA-553 [ MDLQA-553 ]
            Petr Skoda made changes -
            Fix Version/s STABLE backlog [ 10463 ]
            Affects Version/s 2.0.1 [ 10420 ]
            Affects Version/s STABLE Sprint 2 [ 10482 ]
            Petr Skoda made changes -
            Assignee Petr Škoda (skodak) [ skodak ] moodle.com [ moodle.com ]
            Component/s Forum [ 10051 ]
            Component/s IMS resource type [ 10093 ]
            Helen Foster made changes -
            Labels triaged
            Priority Minor [ 4 ] Blocker [ 1 ]
            Martin Dougiamas made changes -
            Fix Version/s STABLE Sprint 6 [ 10535 ]
            Fix Version/s STABLE backlog [ 10463 ]
            Martin Dougiamas made changes -
            Assignee moodle.com [ moodle.com ] Andrew Davis [ andyjdavis ]
            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.
            Andrew Davis made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            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.
            Andrew Davis made changes -
            Status In Progress [ 3 ] Ready for review [ 10010 ]
            Resolution Fixed [ 1 ]
            Andrew Davis made changes -
            Link This issue will be resolved by PULL-340 [ PULL-340 ]
            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.
            Eloy Lafuente (stronk7) made changes -
            Fix Version/s 2.0.3 [ 10537 ]
            Difficulty Easy
            Eloy Lafuente (stronk7) made changes -
            Summary Forum: rating average aggregate doesn't work Forum: error (out of limits) changing one activity scale to shorter one
            Eloy Lafuente (stronk7) made changes -
            Summary Forum: error (out of limits) changing one activity scale to shorter one Error (out of limits) changing one activity scale to shorter one
            Component/s Ratings [ 10640 ]
            Component/s Forum [ 10051 ]
            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!
            Helen Foster made changes -
            Status Ready for review [ 10010 ] Closed [ 6 ]
            QA Assignee tsala
            Martin Dougiamas made changes -
            Workflow MDL Workflow [ 68028 ] MDL Full Workflow [ 96153 ]
            Dan Poltawski made changes -
            Link This issue caused a regression MDL-30955 [ MDL-30955 ]
            Dan Poltawski made changes -
            Link This issue caused a regression MDLSITE-1753 [ MDLSITE-1753 ]
            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.
            Eloy Lafuente (stronk7) made changes -
            Fix Version/s STABLE Sprint 6 [ 10535 ]

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: