Moodle
  1. Moodle
  2. MDL-40344

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

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical 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 2.4 Branch:
      wip-MDL-40344-m24
    • Pull 2.5 Branch:
      wip-MDL-40344-m25
    • Pull Master Branch:
      wip-MDL-40344-m26
    • Rank:
      51123

      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.

        Issue Links

          Activity

          Hide
          Sam Hemelryk added a comment -

          Putting this straight up for integration.

          The fix is simple and the same on all branches.

          Show
          Sam Hemelryk added a comment - Putting this straight up for integration. The fix is simple and the same on all branches.
          Hide
          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 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
          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
          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
          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 Glancy added a comment - I like your fix more because it eliminates the problem when course and course category have the same id
          Hide
          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
          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
          Dan Poltawski added a comment -

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

          Show
          Dan Poltawski added a comment - Integrated to master, 25, 24 and 23 - thanks Sam
          Hide
          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 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
          Ankit Agarwal added a comment -

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

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

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

          Cheers, Damyon

          Show
          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: