Details

    • Testing Instructions:
      Hide

      Test 1

      1. Log in as Course Creator
      2. Select an existing category
      3. Click Site administration -> Courses Add/edit courses"
      4. Click "Add a new course"
      5. After saving user is redirected to newly created course.

      Test 2

      1. Repeat Steps 1-4 from Test 1
      2. Click cancel and you should be redirected to the category page where you came from.

      Test 3

      1. Log in as Course Creator
      2. Select an existing course
      3. Click Site administration -> Courses Add/edit courses"
      4. Click "Add a new course"
      5. After saving user is redirected to newly created course.

      Test 4

      1. Repeat Steps 1-4 from Test 3
      2. Click cancel and you should be redirected to top level category page where you came from.

      Test5

      1. Log in as admin
      2. Repeat Steps 2-4 from test 1 and test 3
      3. Make sure you get redirected to enrol user page.

      Test6

      1. Log in as admin
      2. Repeat Steps 2-4 from test 2 and test 4
      3. Make sure you get redirected to category page you came from.
      Show
      Test 1 Log in as Course Creator Select an existing category Click Site administration -> Courses Add/edit courses" Click "Add a new course" After saving user is redirected to newly created course. Test 2 Repeat Steps 1-4 from Test 1 Click cancel and you should be redirected to the category page where you came from. Test 3 Log in as Course Creator Select an existing course Click Site administration -> Courses Add/edit courses" Click "Add a new course" After saving user is redirected to newly created course. Test 4 Repeat Steps 1-4 from Test 3 Click cancel and you should be redirected to top level category page where you came from. Test5 Log in as admin Repeat Steps 2-4 from test 1 and test 3 Make sure you get redirected to enrol user page. Test6 Log in as admin Repeat Steps 2-4 from test 2 and test 4 Make sure you get redirected to category page you came from.
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      wip-mdl-32526-m24
    • Pull Master Branch:
      wip-mdl-32526
    • Rank:
      39419

      Description

      Login as user with CC role.
      Specify course settings
      Save changes

      Use is taken to home page.
      Previously user was take to:
      1-Enrol users, or more recently
      2-Course page of course just created

      One of these needs to be reinstated as throwing the user out to the site is hugely confusing especialy on busy sites.

      2 is my preference

        Issue Links

          Activity

          Hide
          Rajesh Taneja added a comment -

          Thanks for reporting this, Ray,

          I just tested this on master and it seems it has been fixed.

          On saving a new course as CC, user is redirected to recently created course page and not home page.
          Can you please try it on recent build and let us know if this is resolved for you.

          Show
          Rajesh Taneja added a comment - Thanks for reporting this, Ray, I just tested this on master and it seems it has been fixed. On saving a new course as CC, user is redirected to recently created course page and not home page. Can you please try it on recent build and let us know if this is resolved for you.
          Hide
          Ray Lawrence added a comment -

          Hi,

          This is still broken. Moodle 2.4 (Build: 20121203)

          Show
          Ray Lawrence added a comment - Hi, This is still broken. Moodle 2.4 (Build: 20121203)
          Hide
          Rajesh Taneja added a comment -

          Thanks Ray,

          Not sure what is wrong. I can't reproduce this issue.
          If I login as CC and save changes. It redirects me to newly created course page.

          Have you tried this with fresh copy of Moodle? If not can you please try it.

          Show
          Rajesh Taneja added a comment - Thanks Ray, Not sure what is wrong. I can't reproduce this issue. If I login as CC and save changes. It redirects me to newly created course page. Have you tried this with fresh copy of Moodle? If not can you please try it.
          Hide
          Ray Lawrence added a comment -

          http://screencast.com/t/se1fOr7k

          Clean site.

          User is a CC in Misc category.

          User is thrown out at site level. Ideally they need to be directed to course page (or enrolled users as before).

          Taking user to system level confusing.

          Show
          Ray Lawrence added a comment - http://screencast.com/t/se1fOr7k Clean site. User is a CC in Misc category. User is thrown out at site level. Ideally they need to be directed to course page (or enrolled users as before). Taking user to system level confusing.
          Hide
          Rajesh Taneja added a comment -

          Thanks Ray,

          I can reproduce it now. It has different behaviour when user click on "Add/edit courses" when he/she is in a course.

          1. Log in as CC
          2. Select an existing course
          3. Click Site administration -> Courses Add/edit courses"
          4. Click "Add a new course"
          5. After saving user is redirected to newly created course.

          I realized that we are adding "returnto=category" when user click "Add a new course" after selecting a category. This was done intentionally in MDL-23144.

          Adding Aparup to this as he added this code and might want to add his 2 cents here.

          Show
          Rajesh Taneja added a comment - Thanks Ray, I can reproduce it now. It has different behaviour when user click on "Add/edit courses" when he/she is in a course. Log in as CC Select an existing course Click Site administration -> Courses Add/edit courses" Click "Add a new course" After saving user is redirected to newly created course. I realized that we are adding "returnto=category" when user click "Add a new course" after selecting a category. This was done intentionally in MDL-23144 . Adding Aparup to this as he added this code and might want to add his 2 cents here.
          Hide
          Ray Lawrence added a comment -

          OK, I read the other issue.

          It depends on your view of what a course creator wants to do:

          1. Create a course and do something in it.
          2. Create a course and be taken back to where they came from 9perhaps to create another course).

          I'd favour 1 as this made sense and was the default in 1.9 and early 2.x.

          Neither approach will be universally correct.

          Whichever approach is adopted it needs to be consistent for both methods.

          Show
          Ray Lawrence added a comment - OK, I read the other issue. It depends on your view of what a course creator wants to do: 1. Create a course and do something in it. 2. Create a course and be taken back to where they came from 9perhaps to create another course). I'd favour 1 as this made sense and was the default in 1.9 and early 2.x. Neither approach will be universally correct. Whichever approach is adopted it needs to be consistent for both methods.
          Hide
          Rajesh Taneja added a comment -

          Hi! Ray,

          I agree that it should show newly created course to user if user doesn't have capability to enrol users.
          Current patch will take user to enrol page (if user has enrol capability), else will show newly created course.

          IMO this is more consistent approach, no matter where user clicked "Add a course" (category or course).

          Show
          Rajesh Taneja added a comment - Hi! Ray, I agree that it should show newly created course to user if user doesn't have capability to enrol users. Current patch will take user to enrol page (if user has enrol capability), else will show newly created course. IMO this is more consistent approach, no matter where user clicked "Add a course" (category or course).
          Hide
          Frédéric Massart added a comment -

          Your patch looks good Raj, here are just a few things:

          1. Looking at the current behaviour and MDL-23144, cancelling the form brings you back on the correct category page. The proposed patch changes this behaviour by bringing back the user to the main category page.
          2. Could you add some testing instructions checking the 'Cancel' action?
          3. Also some testing instructions with a role which can enrol users, who should be redirected to the enrol page.

          Many thanks!
          Fred

          Show
          Frédéric Massart added a comment - Your patch looks good Raj, here are just a few things: Looking at the current behaviour and MDL-23144 , cancelling the form brings you back on the correct category page. The proposed patch changes this behaviour by bringing back the user to the main category page. Could you add some testing instructions checking the 'Cancel' action? Also some testing instructions with a role which can enrol users, who should be redirected to the enrol page. Many thanks! Fred
          Hide
          Rajesh Taneja added a comment -

          Thanks Fred.... I totally missed that...
          Let me revisit this.

          Show
          Rajesh Taneja added a comment - Thanks Fred.... I totally missed that... Let me revisit this.
          Hide
          Rajesh Taneja added a comment -

          Thanks Fred,

          I have done required changes, back for your review.

          Show
          Rajesh Taneja added a comment - Thanks Fred, I have done required changes, back for your review.
          Hide
          Frédéric Massart added a comment -

          As we discussed it IRL, I think that removing the returnto entirely to redirect to the course page will make some people happy, but some other not. Also considering that there are 2 different behaviours regarding that you are in course/index ou course/category, I think the best option would be to add a button on the course edit page which would display "Save and return". Reading the returnto value we redirect the user, and assume they were on the main category page when this value is empty. This patch would not be backported, but I don't think this is a problem.

          Show
          Frédéric Massart added a comment - As we discussed it IRL, I think that removing the returnto entirely to redirect to the course page will make some people happy, but some other not. Also considering that there are 2 different behaviours regarding that you are in course/index ou course/category, I think the best option would be to add a button on the course edit page which would display "Save and return". Reading the returnto value we redirect the user, and assume they were on the main category page when this value is empty. This patch would not be backported, but I don't think this is a problem.
          Hide
          Rajesh Taneja added a comment - - edited

          Thanks Fred, that makes sense.

          Will wait for Ray's feedback, before changing the patch.

          Show
          Rajesh Taneja added a comment - - edited Thanks Fred, that makes sense. Will wait for Ray's feedback, before changing the patch.
          Hide
          Rajesh Taneja added a comment -

          I was looking at this again, and it seems there is no nice solution which can make everyone happy.

          Case1: New site with no course/category.
          After creating a new course and clicking save changes, (IMO) user will expects to see the newly created course and not category paqe.

          Case2: User is in course and click on Add/edit course (from site administration which is not expanded)
          In this case also, user should see newly created course and not category with courses.

          Case 3: User is in category and click add a new course (Case you provided)
          In this case user should return to category list if he/she can't enrol user. This seems to be correct case for CC who is just creating empty courses and probably not good for CC who is creating new course and want to create some activity within.

          IMO: This bug should not be fixed as there is no good solution which can satisfy everyone. As different people will have different expectation, any solution will cause regression and some unhappy users.
          Also, Adding an extra button will be confusing for existing user base and will have no meaning when user is updating a course.

          Ray Lawrence If you think otherwise then let us know, else I will close this as "Won't fix".

          FYI: Patch is attached, so user will be navigated to newly created course page. Feel free to use it, if you need that behaviour.

          Show
          Rajesh Taneja added a comment - I was looking at this again, and it seems there is no nice solution which can make everyone happy. Case1: New site with no course/category. After creating a new course and clicking save changes, (IMO) user will expects to see the newly created course and not category paqe. Case2: User is in course and click on Add/edit course (from site administration which is not expanded) In this case also, user should see newly created course and not category with courses. Case 3: User is in category and click add a new course (Case you provided) In this case user should return to category list if he/she can't enrol user. This seems to be correct case for CC who is just creating empty courses and probably not good for CC who is creating new course and want to create some activity within. IMO: This bug should not be fixed as there is no good solution which can satisfy everyone. As different people will have different expectation, any solution will cause regression and some unhappy users. Also, Adding an extra button will be confusing for existing user base and will have no meaning when user is updating a course. Ray Lawrence If you think otherwise then let us know, else I will close this as "Won't fix". FYI: Patch is attached, so user will be navigated to newly created course page. Feel free to use it, if you need that behaviour.
          Hide
          Ray Lawrence added a comment -

          Sorry, I don't agree.

          This needs fixing as behaviour should be predictable and not vary without any clue to the underlying logic.

          With the current state of functionality the behaviour should be to proceed for the course for the reason outlined by me on 11/Dec/12 5:35 PM.

          The proposal for an extra button makes sense as it leaves the next step with the user rather than us trying to predict their needs.

          So "Save and view course" and "Save and return to category" would work.

          I agree this would be confusing for teachers editing course settings so it need to be shown the first time the settings page is accessed only.

          Show
          Ray Lawrence added a comment - Sorry, I don't agree. This needs fixing as behaviour should be predictable and not vary without any clue to the underlying logic. With the current state of functionality the behaviour should be to proceed for the course for the reason outlined by me on 11/Dec/12 5:35 PM. The proposal for an extra button makes sense as it leaves the next step with the user rather than us trying to predict their needs. So "Save and view course" and "Save and return to category" would work. I agree this would be confusing for teachers editing course settings so it need to be shown the first time the settings page is accessed only.
          Hide
          Rajesh Taneja added a comment -

          Thanks Ray,

          I will put this discussion on Dev Chat and try get more feedback.
          I am not sure, if adding an extra button for edge-case (CC adding course from category), is a good way to go about it.

          CC gets enroled in new course as teacher, so IMHO he/she should be redirected to new course.
          I am not sure how many users create empty courses and are enroled as teacher.

          Will wait for more feedback.

          Show
          Rajesh Taneja added a comment - Thanks Ray, I will put this discussion on Dev Chat and try get more feedback. I am not sure, if adding an extra button for edge-case (CC adding course from category), is a good way to go about it. CC gets enroled in new course as teacher, so IMHO he/she should be redirected to new course. I am not sure how many users create empty courses and are enroled as teacher. Will wait for more feedback.
          Hide
          Ray Lawrence added a comment -

          OK, just trying to be flexible on this...

          "CC gets enroled in new course as teacher, so IMHO he/she should be redirected to new course. I am not sure how many users create empty courses and are enroled as teacher."

          So you agree with me...

          Show
          Ray Lawrence added a comment - OK, just trying to be flexible on this... "CC gets enroled in new course as teacher, so IMHO he/she should be redirected to new course. I am not sure how many users create empty courses and are enroled as teacher." So you agree with me...
          Hide
          Eloy Lafuente (stronk7) added a comment -

          How I see it (constant behavior):

          • Anybody with create course perms click on a "add course" link from page xxx_source_xxx (xxx being any page).
          • The edit (new) course page is shown, with some bits filled (for example the category when coming from a given category and so on). Out from the scope of this issue.
          • Pressing cancel ALWAYS return the user to xxx_source_xxx.
          • Saving always does => course enrollments (if the user has perms to) => show course. Always. It never returns to xxx_source_xxx, front page nor any other page.

          That's the usual behavior I've get when creating courses with Moodle. It should not depend of being CC/Admin/Whatever. The only point where a check is necessary is to decide if, after saving, the course enrollments page is shown or the user goes straight to the course page.

          So, "graphically":

          xxx_source_xxx ---> edit (new) course ---> save >---> enrollments ---> view course
               ^                       |                    (optional, perm-based)
               |                       |
               |-------< cancel <------|
          

          Constant (independent of roles), predictable (in my mind) behavior.

          IMO that would make happy everybody BUT those course creators creating hundred of courses in a row. But that's another issue, let's sort it out separately if needed (after all they are just 1/2-clicks away from creating a new course via admin or we can provide them extra "create cours" actions in navigation everywhere).

          Just my opinion based in my expectations/experience, for sure better than going back to the site page or category page.

          Hope it helps, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - How I see it (constant behavior): Anybody with create course perms click on a "add course" link from page xxx_source_xxx (xxx being any page). The edit (new) course page is shown, with some bits filled (for example the category when coming from a given category and so on). Out from the scope of this issue. Pressing cancel ALWAYS return the user to xxx_source_xxx. Saving always does => course enrollments (if the user has perms to) => show course. Always. It never returns to xxx_source_xxx, front page nor any other page. That's the usual behavior I've get when creating courses with Moodle. It should not depend of being CC/Admin/Whatever. The only point where a check is necessary is to decide if, after saving, the course enrollments page is shown or the user goes straight to the course page. So, "graphically": xxx_source_xxx ---> edit ( new ) course ---> save >---> enrollments ---> view course ^ | (optional, perm-based) | | |-------< cancel <------| Constant (independent of roles), predictable (in my mind) behavior. IMO that would make happy everybody BUT those course creators creating hundred of courses in a row. But that's another issue, let's sort it out separately if needed (after all they are just 1/2-clicks away from creating a new course via admin or we can provide them extra "create cours" actions in navigation everywhere). Just my opinion based in my expectations/experience, for sure better than going back to the site page or category page. Hope it helps, ciao
          Hide
          Ray Lawrence added a comment -

          Thanks Eloy... but it doesn't always go to course enrolments. See http://screencast.com/t/se1fOr7k This user is a CC.

          "Saving always does => course enrollments (if the user has perms to) => show course. Always. It never returns to xxx_source_xxx, front page nor any other page."

          Isn't that always the case with default config as Use who is CC is auto assigned as Teacher when course is created?

          Show
          Ray Lawrence added a comment - Thanks Eloy... but it doesn't always go to course enrolments. See http://screencast.com/t/se1fOr7k This user is a CC. "Saving always does => course enrollments (if the user has perms to) => show course. Always. It never returns to xxx_source_xxx, front page nor any other page." Isn't that always the case with default config as Use who is CC is auto assigned as Teacher when course is created?
          Hide
          Rajesh Taneja added a comment -

          Looking at the graph there, it seems we should redirect user to view course after creating new course.
          Hence, current patch is fine. Will push it for peer-review again, so get another opinion.

          Show
          Rajesh Taneja added a comment - Looking at the graph there, it seems we should redirect user to view course after creating new course. Hence, current patch is fine. Will push it for peer-review again, so get another opinion.
          Hide
          Frédéric Massart added a comment -

          This looks good to me Raj, pushing forward. (FYI, I don't think you need $CFG->wwwroot in moodle_url)

          Show
          Frédéric Massart added a comment - This looks good to me Raj, pushing forward. (FYI, I don't think you need $CFG->wwwroot in moodle_url)
          Hide
          Rajesh Taneja added a comment -

          Grrr copy/paste.
          Thanks Fred, have update branches.

          Show
          Rajesh Taneja added a comment - Grrr copy/paste. Thanks Fred, have update branches.
          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
          Rajesh Taneja added a comment -

          Added Marina as watcher.

          Show
          Rajesh Taneja added a comment - Added Marina as watcher.
          Hide
          Dan Poltawski added a comment -

          Did any of you look where this crazy behaviour came from? Please always try and look at git blame to find out the history, because it often is useful in working out why we ended up like we did and preventing us doing the same thing again. When we didn't write the code ourselves this is often the best way to work out why we ended up here.

          Apparently it came from MDL-23144. Raj, could you check that this is going to be reintroducing a problem identified by that bug..

          Show
          Dan Poltawski added a comment - Did any of you look where this crazy behaviour came from? Please always try and look at git blame to find out the history, because it often is useful in working out why we ended up like we did and preventing us doing the same thing again. When we didn't write the code ourselves this is often the best way to work out why we ended up here. Apparently it came from MDL-23144 . Raj, could you check that this is going to be reintroducing a problem identified by that bug..
          Hide
          Rajesh Taneja added a comment -

          Hello Dan,

          This has been identified before and we are taking care of the original problem (reported in MDL-23144)
          Now behaviour is:

          1. On clicking cancel, user will be redirected to where he/she came from
          2. On clicking Save, user will be redirected to enrol page (if he/she has capability) or view newly created course. (As shown in graph by Eloy https://tracker.moodle.org/browse/MDL-32526?focusedCommentId=197486&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-197486)
          Show
          Rajesh Taneja added a comment - Hello Dan, This has been identified before and we are taking care of the original problem (reported in MDL-23144 ) Now behaviour is: On clicking cancel, user will be redirected to where he/she came from On clicking Save, user will be redirected to enrol page (if he/she has capability) or view newly created course. (As shown in graph by Eloy https://tracker.moodle.org/browse/MDL-32526?focusedCommentId=197486&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-197486 )
          Hide
          Dan Poltawski added a comment -

          Thanks for clarifying (point it out to me) Raj.

          Show
          Dan Poltawski added a comment - Thanks for clarifying (point it out to me) Raj.
          Hide
          Dan Poltawski added a comment -

          Integrated to 24, 23 and master, thanks everyone - this was an interesting issue!

          Show
          Dan Poltawski added a comment - Integrated to 24, 23 and master, thanks everyone - this was an interesting issue!
          Hide
          Mark Nelson added a comment -

          Works as expected, passing.

          Show
          Mark Nelson added a comment - Works as expected, passing.
          Hide
          Ray Lawrence added a comment -

          Thanks everyone for sticking with this issue. Looking forward to adding this logical navigation to the Moodle 2.4 manual...

          Show
          Ray Lawrence added a comment - Thanks everyone for sticking with this issue. Looking forward to adding this logical navigation to the Moodle 2.4 manual...
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks!

          Closing as fixed, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks! Closing as fixed, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: