Moodle
  1. Moodle
  2. MDL-33767

Moving resource in a course section page redirects to wrong page after action is completed/cancelled

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.1, 2.4
    • Fix Version/s: 2.3.2
    • Component/s: Course
    • Labels:
    • Testing Instructions:
      Hide

      Prerequisites

      1. Set up a course with 'one section per page'
      2. This test has to be done with enableajax setting on AND off
      3. This test has to be done from a section page, as well as from the index page of a course
      4. When doing the 'within a section' test, do not pick the default nor the first section
      5. In total, the steps will be repeated 4 times

      Test steps
      1. Go to the index page or the section page (depending what test you're doing)
      2. Edit the section, update the name, and save. Make sure you're redirected back on the section page.
      3. Duplicate an activity, make sure you're redirected back on the section page.
      4. Move an activity, make sure you're redirected back on the section page.
      5. Add an activity to the section, and make sure you're redirected on the section page.

      Second test

      1. Edit the name of the default and another section
      2. Check that it is displayed correctly in One section per page, and Multiple section per page mode
      3. Edit the name of those sections again and set "Use default section name"
      4. Make sure that now the default section does not display a name
      Show
      Prerequisites Set up a course with 'one section per page' This test has to be done with enableajax setting on AND off This test has to be done from a section page, as well as from the index page of a course When doing the 'within a section' test, do not pick the default nor the first section In total, the steps will be repeated 4 times Test steps 1. Go to the index page or the section page (depending what test you're doing) 2. Edit the section, update the name, and save. Make sure you're redirected back on the section page. 3. Duplicate an activity, make sure you're redirected back on the section page. 4. Move an activity, make sure you're redirected back on the section page. 5. Add an activity to the section, and make sure you're redirected on the section page. Second test Edit the name of the default and another section Check that it is displayed correctly in One section per page, and Multiple section per page mode Edit the name of those sections again and set "Use default section name" Make sure that now the default section does not display a name
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-33767-master

      Description

      1. Goto any section page
      2. Turn Ajax off at site level
      3. Try moving a resource/activity from the default section (when in the section page)
      4. press cancel in there resulting page or select a place to move the resource

      Expected result:-
      You are taken to the section page where you started

      Actual result:-
      You are taken to a page with all sections

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Ankit Agarwal added a comment -

            This issue intentionally doesn't have mdlqa label as it is not blocking the linked QA. It is only this issue was discovered while testing the linked QA.

            Show
            Ankit Agarwal added a comment - This issue intentionally doesn't have mdlqa label as it is not blocking the linked QA. It is only this issue was discovered while testing the linked QA.
            Hide
            Tim Barker added a comment -

            I have found that when the user edits the news section on a section page that the user is returned to the course page by mistake.

            To recreate:
            Pre-requisites:
            Course is configured to show one section per page.

            Test Steps:

            1. As Teacher, navigate to any section page that is not the news section.
            2. Turn editing on and edit the news section.
            3. Save changes or cancel.

            Expected results:
            User is returned to section page.

            Actual Results:
            User is returned to course page.

            Show
            Tim Barker added a comment - I have found that when the user edits the news section on a section page that the user is returned to the course page by mistake. To recreate: Pre-requisites: Course is configured to show one section per page. Test Steps: As Teacher, navigate to any section page that is not the news section. Turn editing on and edit the news section. Save changes or cancel. Expected results: User is returned to section page. Actual Results: User is returned to course page.
            Hide
            Dan Poltawski added a comment -

            These problems just keep coming huh..

            Could you explain about this change more and why its not covered by the other issues?

            The change to the renderer section_header() is not correct IMO. if $onsectionpage is true than you can use $section->section to work out where to return to.

            Show
            Dan Poltawski added a comment - These problems just keep coming huh.. Could you explain about this change more and why its not covered by the other issues? The change to the renderer section_header() is not correct IMO. if $onsectionpage is true than you can use $section->section to work out where to return to.
            Hide
            Frédéric Massart added a comment -

            Dan, this change has actually only one purpose, getting the default section actions to redirect to the currently displayed section instead of the currently worked-on section. As the default section has an identifier of 0, using $section->section would create a redirect to the index page.

            Show
            Frédéric Massart added a comment - Dan, this change has actually only one purpose, getting the default section actions to redirect to the currently displayed section instead of the currently worked-on section. As the default section has an identifier of 0, using $section->section would create a redirect to the index page.
            Hide
            Frédéric Massart added a comment -

            Repushing for peer review.

            Show
            Frédéric Massart added a comment - Repushing for peer review.
            Hide
            Dan Poltawski added a comment -

            Ah, understood +1

            Show
            Dan Poltawski added a comment - Ah, understood +1
            Hide
            Frédéric Massart added a comment -

            Thanks Dan! Pushing for integration.

            Show
            Frédéric Massart added a comment - Thanks Dan! Pushing for integration.
            Hide
            Frédéric Massart added a comment -

            Small update following http://moodle.org/mod/forum/discuss.php?d=205724 which you can easily review as it is part of a second commit. I have rebased the branch on stable and created the branch for 2.3.

            Show
            Frédéric Massart added a comment - Small update following http://moodle.org/mod/forum/discuss.php?d=205724 which you can easily review as it is part of a second commit. I have rebased the branch on stable and created the branch for 2.3.
            Hide
            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
            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
            Aparup Banerjee added a comment -

            Fred,

            • please use the 'good' style in the course/format/renderer.php near the !$onsectionpage change with the long control structure. it will help towards readability.
            • print_section(), print_section_add_menus() in course/lib.php really needs full and complete phpdocs, fortunately you're patching those function signatures.

            patch looks alright otherwise.

            ps :noting that the arguments on print_section() are growing to an interesting length where due to php's optional parameters, you've to specify 2 default params, kind of not so default lol. somewhere along this trend it would be cool to consolidate these params into an object or so perhaps but that's for the future.

            Show
            Aparup Banerjee added a comment - Fred, please use the 'good' style in the course/format/renderer.php near the !$onsectionpage change with the long control structure. it will help towards readability. print_section(), print_section_add_menus() in course/lib.php really needs full and complete phpdocs, fortunately you're patching those function signatures. patch looks alright otherwise. ps :noting that the arguments on print_section() are growing to an interesting length where due to php's optional parameters, you've to specify 2 default params, kind of not so default lol. somewhere along this trend it would be cool to consolidate these params into an object or so perhaps but that's for the future.
            Hide
            Frédéric Massart added a comment -

            Thanks Apu, the branches are updated and all yours!

            Show
            Frédéric Massart added a comment - Thanks Apu, the branches are updated and all yours!
            Hide
            Aparup Banerjee added a comment -

            Thanks for the changes Fred, the patch was pulled into master and picked into MOODLE_23_STABLE.

            Show
            Aparup Banerjee added a comment - Thanks for the changes Fred, the patch was pulled into master and picked into MOODLE_23_STABLE.
            Hide
            Michael de Raadt added a comment -

            Test result: Success!

            Tested in 2.3 and Master.

            Show
            Michael de Raadt added a comment - Test result: Success! Tested in 2.3 and Master.
            Hide
            Dan Poltawski added a comment -

            *Notice*: Undefined variable: friendlyintegrator in /Users/danp/git/tokenintegrationthanks.php on line 26

            Congratulations

            {tracker.user.name}

            !

            You've made into Moodle

            {tracker.fixversion-1}

            +

            I would like to personally thank you for this contribution on behalf of all Moodle users throughout the world.

            cheers!

            {tracker.friendlyintegrator}
            Show
            Dan Poltawski added a comment - * Notice *: Undefined variable: friendlyintegrator in /Users/danp/git/tokenintegrationthanks.php on line 26 Congratulations {tracker.user.name} ! You've made into Moodle {tracker.fixversion-1} + I would like to personally thank you for this contribution on behalf of all Moodle users throughout the world. cheers! {tracker.friendlyintegrator}

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: