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

          Issue Links

            Activity

            jerome Jérôme Mouneyrac created issue -
            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.
            jerome 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.
            jerome Jérôme Mouneyrac made changes -
            Link This issue will help resolve MDLQA-553 [ MDLQA-553 ]
            skodak Petr Skoda made changes -
            Fix Version/s STABLE backlog [ 10463 ]
            Affects Version/s 2.0.1 [ 10420 ]
            Affects Version/s STABLE Sprint 2 [ 10482 ]
            skodak Petr Skoda made changes -
            Assignee Petr Škoda (skodak) [ skodak ] moodle.com [ moodle.com ]
            Component/s Forum [ 10051 ]
            Component/s IMS resource type [ 10093 ]
            tsala Helen Foster made changes -
            Labels triaged
            Priority Minor [ 4 ] Blocker [ 1 ]
            dougiamas Martin Dougiamas made changes -
            Fix Version/s STABLE Sprint 6 [ 10535 ]
            Fix Version/s STABLE backlog [ 10463 ]
            dougiamas Martin Dougiamas made changes -
            Assignee moodle.com [ moodle.com ] Andrew Davis [ andyjdavis ]
            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.
            andyjdavis Andrew Davis made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            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.
            andyjdavis Andrew Davis made changes -
            Status In Progress [ 3 ] Ready for review [ 10010 ]
            Resolution Fixed [ 1 ]
            andyjdavis Andrew Davis made changes -
            Link This issue will be resolved by PULL-340 [ PULL-340 ]
            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.
            stronk7 Eloy Lafuente (stronk7) made changes -
            Fix Version/s 2.0.3 [ 10537 ]
            Difficulty Easy
            stronk7 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
            stronk7 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
            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!
            tsala Helen Foster made changes -
            Status Ready for review [ 10010 ] Closed [ 6 ]
            QA Assignee tsala
            dougiamas Martin Dougiamas made changes -
            Workflow MDL Workflow [ 68028 ] MDL Full Workflow [ 96153 ]
            poltawski Dan Poltawski made changes -
            Link This issue caused a regression MDL-30955 [ MDL-30955 ]
            poltawski Dan Poltawski made changes -
            Link This issue caused a regression MDLSITE-1753 [ MDLSITE-1753 ]
            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.
            stronk7 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:
                  Fix Release Date:
                  5/May/11