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

Navigation AJAX mixes up branches and tries to load the wrong content in some situations.

          Details

          • Type: Bug
          • Status: Closed
          • Priority: Critical
          • Resolution: Fixed
          • Affects Version/s: 2.3.7, 2.4.4, 2.5
          • Fix Version/s: 2.3.8, 2.4.5, 2.5.1
          • Component/s: Navigation
          • Labels:
          • Testing Instructions:
            Hide

            The actual situation is hard to repdroduce here we need to test the AJAX loading of the navigation to make sure it still works.

            Using a site with a categories, sub categories, lots of courses, and students with enrolments.

            1. Log in as a user with no enrolments and check the "Courses" branch still loads correctly as do sub categories and courses through the navigation.
            2. Log in as a user with enrolment, check the My Courses branch expands as you would expect.
            3. Log in as admin and toggle My courses to show categories (check navigation settings)
            4. Check the My courses branch again.
            Show
            The actual situation is hard to repdroduce here we need to test the AJAX loading of the navigation to make sure it still works. Using a site with a categories, sub categories, lots of courses, and students with enrolments. Log in as a user with no enrolments and check the "Courses" branch still loads correctly as do sub categories and courses through the navigation. Log in as a user with enrolment, check the My Courses branch expands as you would expect. Log in as admin and toggle My courses to show categories (check navigation settings) Check the My courses branch again.
          • Affected Branches:
            MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
          • Fixed Branches:
            MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
          • Pull from Repository:
          • Pull Master Branch:
            wip-MDL-40344-m26
          • Pull Master Diff URL:

            Description

            This is an annoying issue to reproduce, you need to have the following situation:

            1. Two categories A and B
            2. One course C which has the same ID as Category B. This course should be in Category A.
            3. One course D which has a random ID and is in Category B.
            4. A user enrolled in both courses.

            I hit this with my generated site that contains 25+ categories (3 levels deep) and 100 courses randomly assigned to categories.

            With the above situation produced:

            1. proceed to enrol a student in all courses.
            2. Log in as the student
            3. Using the navigation block expand the My courses branch, and then expand the first category.
            4. Now try to expand the sibling category.
            5. You'll likely get the wrong content.

              Gliffy Diagrams

                Attachments

                  Issue Links

                  duplicates

                  Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-39982 Course category menus not expanding after creation of 3rd sub-category

                  • Critical - Crashes server, loss of data, severe memory leak
                  • Closed
                  Testing discovered

                  Improvement - An enhancement to an existing Moodle feature. MDL-40297 Empty Categories in My Courses are always shown.

                  • Minor - Minor loss of function where workaround is possible
                  • Closed

                    Activity

                    Ascending order - Click to sort in descending order
                    Hide
                    Permalink
                    samhemelryk Sam Hemelryk added a comment -

                    Putting this straight up for integration.

                    The fix is simple and the same on all branches.

                    Show
                    samhemelryk Sam Hemelryk added a comment - Putting this straight up for integration. The fix is simple and the same on all branches.
                    Hide
                    Permalink
                    marina Marina Glancy added a comment -

                    Great that you fixed it Sam. BTW this issue already existed and I even pinged you about it MDL-39982

                    Show
                    marina Marina Glancy added a comment - Great that you fixed it Sam. BTW this issue already existed and I even pinged you about it MDL-39982
                    Hide
                    Permalink
                    samhemelryk Sam Hemelryk added a comment -

                    Ah I wondered about that issue. I flicked it open the other day but as it didn't look obvious and I was busy I moved onto other things.
                    I had a search for it today but didn't find it.
                    Looks like his fix is nearly the same, I'll link the two issues and close both when this gets integrated.

                    Show
                    samhemelryk Sam Hemelryk added a comment - Ah I wondered about that issue. I flicked it open the other day but as it didn't look obvious and I was busy I moved onto other things. I had a search for it today but didn't find it. Looks like his fix is nearly the same, I'll link the two issues and close both when this gets integrated.
                    Hide
                    Permalink
                    marina Marina Glancy added a comment -

                    I like your fix more because it eliminates the problem when course and course category have the same id

                    Show
                    marina Marina Glancy added a comment - I like your fix more because it eliminates the problem when course and course category have the same id
                    Hide
                    Permalink
                    stronk7 Eloy Lafuente (stronk7) 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
                    stronk7 Eloy Lafuente (stronk7) 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
                    poltawski Dan Poltawski added a comment -

                    Integrated to master, 25, 24 and 23 - thanks Sam

                    Show
                    poltawski Dan Poltawski added a comment - Integrated to master, 25, 24 and 23 - thanks Sam
                    Hide
                    Permalink
                    ankit_frenz Ankit Agarwal added a comment -

                    Hi Sam,
                    Doesn't look related but, when am on /course/index.php, after realoading the page, the icon next to the courses shows in expanded form, but the tree is not shown.
                    The tree is visible as usual after clicking on the icon twice.
                    Let me know if you want a sep issue for that.
                    Thanks

                    Show
                    ankit_frenz Ankit Agarwal added a comment - Hi Sam, Doesn't look related but, when am on /course/index.php, after realoading the page, the icon next to the courses shows in expanded form, but the tree is not shown. The tree is visible as usual after clicking on the icon twice. Let me know if you want a sep issue for that. Thanks
                    Hide
                    Permalink
                    ankit_frenz Ankit Agarwal added a comment -

                    tested on 24 25 and master. Passing as the issue I found is not related.
                    Thanks

                    Show
                    ankit_frenz Ankit Agarwal added a comment - tested on 24 25 and master. Passing as the issue I found is not related. Thanks
                    Hide
                    Permalink
                    damyon Damyon Wiese added a comment -

                    This issue is fixed! Hurray! Hurray!
                    Your issue is fixed, what a wonderful day!

                    Cheers, Damyon

                    Show
                    damyon Damyon Wiese added a comment - This issue is fixed! Hurray! Hurray! Your issue is fixed, what a wonderful day! Cheers, Damyon

                      People

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

                        Dates

                        • Created:
                          Updated:
                          Resolved:
                          Fix Release Date:
                          8/Jul/13