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

When returning from modedit.php to course/view.php, go to the appropriate section in more cases

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.2
    • Fix Version/s: 2.0.3
    • Component/s: Course
    • Labels:
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE

      Description

      In a few places, when you get sent back from course/modedit.php to course/view.php, it adds "#section-".$cw->section to send you back to the appropriate part of the course page. There are a number of other places where it does not bother to do that.

      This patch adds the missing "#section-".$cw->section bits.

      Note that I chose to do a minimal patch, rather than changing everything to use moodle_url. Let me know if you would rather I did this differently.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            timhunt Tim Hunt added a comment -

            Suggested fix: https://github.com/timhunt/moodle/commit/fd5a5884dbf9985e82f76017aefcd74a4fc3dbf0

            Note the way it also fixes some silly 'view.php?id=$course->id' code.

            Show
            timhunt Tim Hunt added a comment - Suggested fix: https://github.com/timhunt/moodle/commit/fd5a5884dbf9985e82f76017aefcd74a4fc3dbf0 Note the way it also fixes some silly 'view.php?id=$course->id' code.
            Hide
            timhunt Tim Hunt added a comment -

            Pull request done.

            Most of these changes only make a difference when an error occurs, which is why no-one had noticed that the existing code was broken. So, I don't have any great suggestions for how to test this.

            Show
            timhunt Tim Hunt added a comment - Pull request done. Most of these changes only make a difference when an error occurs, which is why no-one had noticed that the existing code was broken. So, I don't have any great suggestions for how to test this.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Looks ok IMO.

            I think it's ok to do it as string urls (like the rest of redirects in the file). All them, someday, can be switched together.

            Crazy the single quoted uses. I guess they doesn't happen often, lol.

            Thanks, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Looks ok IMO. I think it's ok to do it as string urls (like the rest of redirects in the file). All them, someday, can be switched together. Crazy the single quoted uses. I guess they doesn't happen often, lol. Thanks, ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Uhm, Tim... looking for similar bugs I found MDL-25272 with this patch proposed:

            https://git.luns.net.uk/?p=moodle.git;a=commitdiff;h=ddc15b6bc9a617d9f50be321c309226336c3ebee

            Just guessing when it's better to check if section was there instead of adding it always.

            FYC, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Uhm, Tim... looking for similar bugs I found MDL-25272 with this patch proposed: https://git.luns.net.uk/?p=moodle.git;a=commitdiff;h=ddc15b6bc9a617d9f50be321c309226336c3ebee Just guessing when it's better to check if section was there instead of adding it always. FYC, ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Probably we should be backporting this too, as far as MDL-25272 was reported for 19_STABLE.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Probably we should be backporting this too, as far as MDL-25272 was reported for 19_STABLE.
            Hide
            timhunt Tim Hunt added a comment -

            Oh, sorry for not finding the existing bug first. I'm sure I did search before creating a new issue.

            I was just given the job of taking some of the changes we had made in our copy of Moodle 1.9, and getting them into 2.0 official release. If you want to do more work than that, it would be great, but I don't think I can spend more time on this. Sorry.

            Show
            timhunt Tim Hunt added a comment - Oh, sorry for not finding the existing bug first. I'm sure I did search before creating a new issue. I was just given the job of taking some of the changes we had made in our copy of Moodle 1.9, and getting them into 2.0 official release. If you want to do more work than that, it would be great, but I don't think I can spend more time on this. Sorry.
            Hide
            tsala Helen Foster added a comment -

            Again, thanks Tim.

            Show
            tsala Helen Foster added a comment - Again, thanks Tim.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  5/May/11