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

Section highlighting does not stick with topic section with AJAX off

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Course
    • Labels:
    • Testing Instructions:
      Hide
      1. Run phpunit tests
      1. Log in as admin
      2. Navigate to Site admin > Appearance > AJAX and JavaScript
      3. Uncheck "Enable AJAX" and save settings
      4. Navigate to a course or create a new one
      5. Edit the course settings and set the course format to Topics
      6. Add some content to sections so you can track their movement
      7. Highlight a section somewhere in the middle
      8. Click the up icon to shift the highlighted section up
      9. VERIFY: highlighted section is moved up
      10. Click the down icon to shift the highlighted section down
      11. VERIFY: highlighted section is moved down
      12. Navigate to Site admin > Appearance > AJAX and JavaScript
      13. Check "Enable AJAX" and save settings
      14. Navigate to the same course
      15. Drag the highlighted course section up
      16. VERIFY: turn editing off and verify that the highlighted section has moved
      17. Drag the highlighted course section down
      18. VERIFY: turn editing off and verify that the highlighted section has moved
      19. Drag the a non-highlighted section from below the highlighted section to a place above it
      20. VERIFY: turn editing off and verify that the highlighted section has now changed, it is the same section.
      21. Drag the a non-highlighted section from above the highlighted section to a place below it
      22. VERIFY: turn editing off and verify that the highlighted section has now changed, it is the same section.
      Show
      Run phpunit tests Log in as admin Navigate to Site admin > Appearance > AJAX and JavaScript Uncheck "Enable AJAX" and save settings Navigate to a course or create a new one Edit the course settings and set the course format to Topics Add some content to sections so you can track their movement Highlight a section somewhere in the middle Click the up icon to shift the highlighted section up VERIFY: highlighted section is moved up Click the down icon to shift the highlighted section down VERIFY: highlighted section is moved down Navigate to Site admin > Appearance > AJAX and JavaScript Check "Enable AJAX" and save settings Navigate to the same course Drag the highlighted course section up VERIFY: turn editing off and verify that the highlighted section has moved Drag the highlighted course section down VERIFY: turn editing off and verify that the highlighted section has moved Drag the a non-highlighted section from below the highlighted section to a place above it VERIFY: turn editing off and verify that the highlighted section has now changed, it is the same section. Drag the a non-highlighted section from above the highlighted section to a place below it VERIFY: turn editing off and verify that the highlighted section has now changed, it is the same section.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:

      Description

      I discovered a peculiar error when shifting sections while testing. When I highlighted a section and shifted that section up, the highlighting shifted to the section that took its place (formerly above, now below). When I shifted a highlighted section down, the highlighting stayed with the topic section. This seems to happen only with AJAX turned off.

      This reminded me of a case that Raj mentioned recently where unit tests were only testing section moving in one direction, but not the other. It might be worth checking the unit tests for this as well.

      Replication steps:

      1. Log in as admin
      2. Navigate to Site admin > Appearance > AJAX and JavaScript
      3. Uncheck "Enable AJAX" and save settings
      4. Navigate to a course or create a new one
      5. Edit the course settings and set the course format to Topics
      6. Add some content to sections so you can track their movement
      7. Highlight a section somewhere in the middle
      8. Click the up icon to shift the highlighted section up
      9. Click the down icon to shift the highlighted section down
      10. Navigate to Site admin > Appearance > AJAX and JavaScript
      11. Check "Enable AJAX" and save settings
      12. Navigate to the same course
      13. Drag the highlighted course section up
      14. Drag the highlighted course section down

      Expected result: The highlighting stays with the same topic section

      Actual result: When moving the highlighted section upwards, with AJAX off, highlighting shifts to another section

        Gliffy Diagrams

          Issue Links

            Activity

            salvetore Michael de Raadt created issue -
            salvetore Michael de Raadt made changes -
            Field Original Value New Value
            Fix Version/s STABLE backlog [ 10463 ]
            Labels triaged
            Assignee moodle.com [ moodle.com ] Dan Poltawski [ poltawski ]
            salvetore Michael de Raadt made changes -
            Link This issue discovered while testing MDLQA-2260 [ MDLQA-2260 ]
            salvetore Michael de Raadt made changes -
            Summary Section highlighting does not stick with topic section Section highlighting does not stick with topic section with AJAX off
            Description I discovered a peculiar error when shifting sections while testing. When I highlighted a section and shifted that section up, the highlighting shifted to the section that took its place (formerly above, now below). When I shifted a highlighted section down, the highlighting stayed with the topic section. This seems to happen with AJAX turned on or off.

            This reminded me of a case that Raj mentioned recently where unit tests were only testing section moving in one direction, but not the other. It might be worth checking the unit tests for this as well.

            *Replication steps:*
            # Log in as admin
            # Navigate to Site admin > Appearance > AJAX and JavaScript
            # Uncheck "Enable AJAX" and save settings
            # Navigate to a course or create a new one
            # Edit the course settings and set the course format to Topics
            # Add some content to sections so you can track their movement
            # Highlight a section somewhere in the middle
            # Click the up icon to shift the highlighted section up
            # Click the down icon to shift the highlighted section down
            # Navigate to Site admin > Appearance > AJAX and JavaScript
            # Check "Enable AJAX" and save settings
            # Navigate to the same course
            # Drag the highlighted course section up
            # Drag the highlighted course section down

            Expected result: The highlighting stays with the same topic section

            Actual result: When moving the highlighted section upwards, highlighting shifts to another section
            I discovered a peculiar error when shifting sections while testing. When I highlighted a section and shifted that section up, the highlighting shifted to the section that took its place (formerly above, now below). When I shifted a highlighted section down, the highlighting stayed with the topic section. This seems to happen only with AJAX turned off.

            This reminded me of a case that Raj mentioned recently where unit tests were only testing section moving in one direction, but not the other. It might be worth checking the unit tests for this as well.

            *Replication steps:*
            # Log in as admin
            # Navigate to Site admin > Appearance > AJAX and JavaScript
            # Uncheck "Enable AJAX" and save settings
            # Navigate to a course or create a new one
            # Edit the course settings and set the course format to Topics
            # Add some content to sections so you can track their movement
            # Highlight a section somewhere in the middle
            # Click the up icon to shift the highlighted section up
            # Click the down icon to shift the highlighted section down
            # Navigate to Site admin > Appearance > AJAX and JavaScript
            # Check "Enable AJAX" and save settings
            # Navigate to the same course
            # Drag the highlighted course section up
            # Drag the highlighted course section down

            Expected result: The highlighting stays with the same topic section

            Actual result: When moving the highlighted section upwards, with AJAX off, highlighting shifts to another section
            Hide
            poltawski Dan Poltawski added a comment -

            Hmm. the move section function depends on a decent course record being passed to it.

            Show
            poltawski Dan Poltawski added a comment - Hmm. the move section function depends on a decent course record being passed to it.
            Hide
            poltawski Dan Poltawski added a comment -

            Hmm, well this is very weird because it seemed to be a line of code doing exactly the wrong thing. I've pondered for a while if i'm missing something, but I don't think I am. I expect it was made incorrect by the changes in MDL-33212.

            I have added unit tests to prove my theories in moving sections and ensuring the marker is kept correctly.

            Show
            poltawski Dan Poltawski added a comment - Hmm, well this is very weird because it seemed to be a line of code doing exactly the wrong thing. I've pondered for a while if i'm missing something, but I don't think I am. I expect it was made incorrect by the changes in MDL-33212 . I have added unit tests to prove my theories in moving sections and ensuring the marker is kept correctly.
            poltawski Dan Poltawski made changes -
            Status Open [ 1 ] Waiting for peer review [ 10012 ]
            Pull Master Diff URL https://github.com/danpoltawski/moodle/compare/MDL-33623
            Pull Master Branch MDL-33623
            Pull from Repository git://github.com/danpoltawski/moodle.git
            salvetore Michael de Raadt made changes -
            Tester salvetore
            rajeshtaneja Rajesh Taneja made changes -
            Original Estimate 0 minutes [ 0 ]
            Remaining Estimate 0 minutes [ 0 ]
            Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
            Peer reviewer rajeshtaneja
            Hide
            poltawski Dan Poltawski added a comment -

            Adding Ruslan here as the code came from him in MDL-31216

            Show
            poltawski Dan Poltawski added a comment - Adding Ruslan here as the code came from him in MDL-31216
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Code looks Good Dan. Few things you might want to consider before pushing it for integration:

            1. We don't need $moveup status, so probably can remove

                  $moveup = false;
                  if ($section > $destination) {
                      $moveup = true;
                  }

              and it's use.

            2. Add test instructions
            Show
            rajeshtaneja Rajesh Taneja added a comment - Code looks Good Dan. Few things you might want to consider before pushing it for integration: We don't need $moveup status, so probably can remove $moveup = false; if ($section > $destination) { $moveup = true; } and it's use. Add test instructions
            rajeshtaneja Rajesh Taneja made changes -
            Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
            Hide
            poltawski Dan Poltawski added a comment -

            Hi Raj, moveup is used just a few lines below it?

            Show
            poltawski Dan Poltawski added a comment - Hi Raj, moveup is used just a few lines below it?
            Hide
            poltawski Dan Poltawski added a comment -

            Raj explained to me that in fact there is no need for moveup in the following lines because of the other checks going on.

            Show
            poltawski Dan Poltawski added a comment - Raj explained to me that in fact there is no need for moveup in the following lines because of the other checks going on.
            Hide
            poltawski Dan Poltawski added a comment -

            Thanks Raj for the comments, i've updated testing instructions and submitted

            Show
            poltawski Dan Poltawski added a comment - Thanks Raj for the comments, i've updated testing instructions and submitted
            poltawski Dan Poltawski made changes -
            Testing Instructions # Log in as admin
            # Navigate to Site admin > Appearance > AJAX and JavaScript
            # Uncheck "Enable AJAX" and save settings
            # Navigate to a course or create a new one
            # Edit the course settings and set the course format to Topics
            # Add some content to sections so you can track their movement
            # Highlight a section somewhere in the middle
            # Click the up icon to shift the highlighted section up
            # VERIFY: highlighted section is moved up
            # Click the down icon to shift the highlighted section down
            # VERIFY: highlighted section is moved down
            # Navigate to Site admin > Appearance > AJAX and JavaScript
            # Check "Enable AJAX" and save settings
            # Navigate to the same course
            # Drag the highlighted course section up
            # VERIFY: turn editing off and verify that the highlighted section has moved
            # Drag the highlighted course section down
            # VERIFY: turn editing off and verify that the highlighted section has moved
            # Drag the a non-highlighted section from below the highlighted section to a place above it
            # VERIFY: turn editing off and verify that the highlighted section has now changed, it is the same section.
            # Drag the a non-highlighted section from above the highlighted section to a place below it
            # VERIFY: turn editing off and verify that the highlighted section has now changed, it is the same section.
            poltawski Dan Poltawski made changes -
            Testing Instructions # Log in as admin
            # Navigate to Site admin > Appearance > AJAX and JavaScript
            # Uncheck "Enable AJAX" and save settings
            # Navigate to a course or create a new one
            # Edit the course settings and set the course format to Topics
            # Add some content to sections so you can track their movement
            # Highlight a section somewhere in the middle
            # Click the up icon to shift the highlighted section up
            # VERIFY: highlighted section is moved up
            # Click the down icon to shift the highlighted section down
            # VERIFY: highlighted section is moved down
            # Navigate to Site admin > Appearance > AJAX and JavaScript
            # Check "Enable AJAX" and save settings
            # Navigate to the same course
            # Drag the highlighted course section up
            # VERIFY: turn editing off and verify that the highlighted section has moved
            # Drag the highlighted course section down
            # VERIFY: turn editing off and verify that the highlighted section has moved
            # Drag the a non-highlighted section from below the highlighted section to a place above it
            # VERIFY: turn editing off and verify that the highlighted section has now changed, it is the same section.
            # Drag the a non-highlighted section from above the highlighted section to a place below it
            # VERIFY: turn editing off and verify that the highlighted section has now changed, it is the same section.
            # Run phpunit tests

            # Log in as admin
            # Navigate to Site admin > Appearance > AJAX and JavaScript
            # Uncheck "Enable AJAX" and save settings
            # Navigate to a course or create a new one
            # Edit the course settings and set the course format to Topics
            # Add some content to sections so you can track their movement
            # Highlight a section somewhere in the middle
            # Click the up icon to shift the highlighted section up
            # VERIFY: highlighted section is moved up
            # Click the down icon to shift the highlighted section down
            # VERIFY: highlighted section is moved down
            # Navigate to Site admin > Appearance > AJAX and JavaScript
            # Check "Enable AJAX" and save settings
            # Navigate to the same course
            # Drag the highlighted course section up
            # VERIFY: turn editing off and verify that the highlighted section has moved
            # Drag the highlighted course section down
            # VERIFY: turn editing off and verify that the highlighted section has moved
            # Drag the a non-highlighted section from below the highlighted section to a place above it
            # VERIFY: turn editing off and verify that the highlighted section has now changed, it is the same section.
            # Drag the a non-highlighted section from above the highlighted section to a place below it
            # VERIFY: turn editing off and verify that the highlighted section has now changed, it is the same section.
            Hide
            poltawski Dan Poltawski added a comment -

            (Updated code and testing instructions)

            Show
            poltawski Dan Poltawski added a comment - (Updated code and testing instructions)
            poltawski Dan Poltawski made changes -
            Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            Integrator stronk7
            Currently in integration Yes [ 10041 ]
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Integrated, thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
            Fix Version/s 2.3 [ 10657 ]
            Fix Version/s STABLE backlog [ 10463 ]
            Hide
            poltawski Dan Poltawski added a comment -

            Pinging Michael!

            Show
            poltawski Dan Poltawski added a comment - Pinging Michael!
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            getting onto this for rc1 release...

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - getting onto this for rc1 release...
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            Tester salvetore stronk7
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Passing.

            I've run the unit tests against mysql and postgresql and also followed the steps (all good). Thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Passing. I've run the unit tests against mysql and postgresql and also followed the steps (all good). Thanks!
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Testing in progress [ 10011 ] Tested [ 10006 ]
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            And this has been incorporated to all the weekly builds and also, to Moodle 2.3 Release Candidate 1, yay!

            Many, many thanks for your hard work!

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - And this has been incorporated to all the weekly builds and also, to Moodle 2.3 Release Candidate 1, yay! Many, many thanks for your hard work! Ciao
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Tested [ 10006 ] Closed [ 6 ]
            Resolution Fixed [ 1 ]
            Currently in integration Yes [ 10041 ]
            Integration date 24/Jun/12
            salvetore Michael de Raadt made changes -
            Link This issue has been marked as being related by MDL-34798 [ MDL-34798 ]

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  25/Jun/12