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

Ratings pop up sort links broken

          Details

          • Type: Bug
          • Status: Closed
          • Priority: Minor
          • Resolution: Fixed
          • Affects Version/s: 2.1.6, 2.2, 2.3
          • Fix Version/s: 2.1.7, 2.2.4
          • Component/s: Ratings
          • Labels:
          • Testing Instructions:
            Hide

            Create a forum and enable ratings in the forum settings. Set it to average with a max grade of 5 or similar.

            I'd recommend posting as a student then rating that post as admin and as teacher so you have 2 ratings to sort. Posting as student then having the teacher and admin rate avoids having to alter the default capabilities for this test.

            Log in as admin and next to the rating drop down is a number that is the aggregate of all ratings the student has received.

            Click it and a popup will appear that lists the submitted ratings.

            Click the column headings (name, rating, time) and the ratings should be sorted. Note that it is not currently possible to reverse the sort order so clicking a single heading multiple times doesn't do anything. You'll only notice a change when you click a different column heading. Even then the order won't change if they are already in the correct order.

            Show
            Create a forum and enable ratings in the forum settings. Set it to average with a max grade of 5 or similar. I'd recommend posting as a student then rating that post as admin and as teacher so you have 2 ratings to sort. Posting as student then having the teacher and admin rate avoids having to alter the default capabilities for this test. Log in as admin and next to the rating drop down is a number that is the aggregate of all ratings the student has received. Click it and a popup will appear that lists the submitted ratings. Click the column headings (name, rating, time) and the ratings should be sorted. Note that it is not currently possible to reverse the sort order so clicking a single heading multiple times doesn't do anything. You'll only notice a change when you click a different column heading. Even then the order won't change if they are already in the correct order.
          • Affected Branches:
            MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
          • Fixed Branches:
            MOODLE_21_STABLE, MOODLE_22_STABLE
          • Pull from Repository:
          • Pull Master Branch:
            MDL-32649_ratings_sort
          • Pull Master Diff URL:

            Description

            When viewing items in the ratings pop-up, clicking on 'name', 'rating' or 'time' to change the sort order takes you instead to a different screen. Looks like a minor error on line 87 of rating/index.php (but shouldn't it also have 'component' as a param?).

              Gliffy Diagrams

                Attachments

                  Activity

                  Ascending order - Click to sort in descending order
                  Hide
                  Permalink
                  salvetore Michael de Raadt added a comment -

                  Thanks for reporting this.

                  I've put it on our backlog.

                  In the meantime adding more information, such as screenshots, replication instructions, fix test instructions, a workaround or even a code solution, will help us and other users. If you are able to provide a patch, please add a patch label so we will spot it.

                  Show
                  salvetore Michael de Raadt added a comment - Thanks for reporting this. I've put it on our backlog. In the meantime adding more information, such as screenshots, replication instructions, fix test instructions, a workaround or even a code solution, will help us and other users. If you are able to provide a patch, please add a patch label so we will spot it.
                  Hide
                  Permalink
                  andyjdavis Andrew Davis added a comment -

                  Adding a branch and testing instructions. Putting this up for peer review.

                  Show
                  andyjdavis Andrew Davis added a comment - Adding a branch and testing instructions. Putting this up for peer review.
                  Hide
                  Permalink
                  samhemelryk Sam Hemelryk added a comment -

                  The changes look spot on thanks Andrew, the only thing I did wonder about was whether using !empty was the best option and whether it may be better to explicitly check for the default values of the params. Of course no apparent harm at the moment so entirely up to you.

                  Cheers
                  Sam

                  Show
                  samhemelryk Sam Hemelryk added a comment - The changes look spot on thanks Andrew, the only thing I did wonder about was whether using !empty was the best option and whether it may be better to explicitly check for the default values of the params. Of course no apparent harm at the moment so entirely up to you. Cheers Sam
                  Hide
                  Permalink
                  andyjdavis Andrew Davis added a comment -

                  My current thinking is that Ill leave it using !empty() as the default values are kind of all over the place. One param uses null, one an empty string and one uses 0 !empty() catches all of them and will continue to work if they are ever made uniform.

                  Show
                  andyjdavis Andrew Davis added a comment - My current thinking is that Ill leave it using !empty() as the default values are kind of all over the place. One param uses null, one an empty string and one uses 0 !empty() catches all of them and will continue to work if they are ever made uniform.
                  Hide
                  Permalink
                  andyjdavis Andrew Davis added a comment -

                  Added branches. Putting this up for integration.

                  Show
                  andyjdavis Andrew Davis added a comment - Added branches. Putting this up for integration.
                  Hide
                  Permalink
                  poltawski Dan Poltawski added a comment -

                  The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

                  TIA and ciao

                  Show
                  poltawski Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
                  Hide
                  Permalink
                  stronk7 Eloy Lafuente (stronk7) added a comment -

                  Integrated, thanks!

                  Show
                  stronk7 Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
                  Hide
                  Permalink
                  rajeshtaneja Rajesh Taneja added a comment -

                  Works Great

                  Thanks for fixing this Andrew.

                  Show
                  rajeshtaneja Rajesh Taneja added a comment - Works Great Thanks for fixing this Andrew.
                  Hide
                  Permalink
                  stronk7 Eloy Lafuente (stronk7) added a comment -

                  We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3.

                  Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish.

                  Many thanks for your collaboration!

                  Ciao

                  Show
                  stronk7 Eloy Lafuente (stronk7) added a comment - We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3. Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish. Many thanks for your collaboration! Ciao

                    People

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

                      Dates

                      • Created:
                        Updated:
                        Resolved:
                        Fix Release Date:
                        9/Jul/12