[MDL-33767] Moving resource in a course section page redirects to wrong page after action is completed/cancelled - Moodle Tracker

Details

Type: Bug
Status: Closed
Priority: Minor
Resolution: Fixed
Affects Version/s: 2.3.1, 2.4
Fix Version/s: 2.3.2
Component/s: Course
Labels:
- triaged

Testing Instructions:
Hide

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
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:
git://github.com/FMCorz/moodle.git
Pull Master Branch:
~~MDL-33767~~-master
Pull Master Diff URL:
https://github.com/FMCorz/moodle/compare/master...MDL-33767-master

Description

Goto any section page
Turn Ajax off at site level
Try moving a resource/activity from the default section (when in the section page)
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

has a non-specific relationship to

MDLQA-3896 Course

Open

Testing discovered

MDLQA-2285 CLONE - In Firefox, on the index page, course controls work correctly for weekly format, in paged mode, when ajax is turned off.

Passed

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Ankit Agarwal added a comment - 15/Jun/12 2:42 PM

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 - 15/Jun/12 2:42 PM 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

Permalink

Tim Barker added a comment - 19/Jun/12 11:56 AM

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.

Show

Tim Barker added a comment - 19/Jun/12 11:56 AM 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

Permalink

Dan Poltawski added a comment - 19/Jun/12 9:20 PM

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 - 19/Jun/12 9:20 PM 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

Permalink

Frédéric Massart added a comment - 20/Jun/12 9:16 AM

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 - 20/Jun/12 9:16 AM 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

Permalink

Frédéric Massart added a comment - 27/Jun/12 9:57 AM

Repushing for peer review.

Show

Frédéric Massart added a comment - 27/Jun/12 9:57 AM Repushing for peer review.

Hide

Permalink

Dan Poltawski added a comment - 27/Jun/12 2:47 PM

Ah, understood +1

Show

Dan Poltawski added a comment - 27/Jun/12 2:47 PM Ah, understood +1

Hide

Permalink

Frédéric Massart added a comment - 27/Jun/12 3:50 PM

Thanks Dan! Pushing for integration.

Show

Frédéric Massart added a comment - 27/Jun/12 3:50 PM Thanks Dan! Pushing for integration.

Hide

Permalink

Frédéric Massart added a comment - 29/Jun/12 1:56 PM

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 - 29/Jun/12 1:56 PM 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

Permalink

Dan Poltawski added a comment - 12/Jul/12 6:03 PM

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 - 12/Jul/12 6:03 PM 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

Aparup Banerjee added a comment - 17/Jul/12 2:40 PM

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 - 17/Jul/12 2:40 PM 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

Permalink

Frédéric Massart added a comment - 17/Jul/12 3:46 PM

Thanks Apu, the branches are updated and all yours!

Show

Frédéric Massart added a comment - 17/Jul/12 3:46 PM Thanks Apu, the branches are updated and all yours!

Hide

Permalink

Aparup Banerjee added a comment - 17/Jul/12 10:34 PM

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

Show

Aparup Banerjee added a comment - 17/Jul/12 10:34 PM Thanks for the changes Fred, the patch was pulled into master and picked into MOODLE_23_STABLE.

Hide

Permalink

Michael de Raadt added a comment - 18/Jul/12 1:14 PM

Test result: Success!

Tested in 2.3 and Master.

Show

Michael de Raadt added a comment - 18/Jul/12 1:14 PM Test result: Success! Tested in 2.3 and Master.

Hide

Permalink

Dan Poltawski added a comment - 19/Jul/12 2:43 PM

*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 - 19/Jul/12 2:43 PM * 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

Assignee:

Frédéric Massart

Reporter:

Ankit Agarwal

Peer reviewer:

Dan Poltawski

Integrator:

Aparup Banerjee

Tester:

Michael de Raadt

Participants:

Ankit Agarwal, Aparup Banerjee, Dan Poltawski, Frédéric Massart, Michael de Raadt, Tim Barker

Votes:

1 Vote for this issue

Watchers:

3 Start watching this issue

Dates

Created:

15/Jun/12 2:41 PM

Updated:

30/Sep/13 10:20 AM

Resolved:

19/Jul/12 2:43 PM

Agile

View on Board