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 Improvement
    • Status: Closed
    • Priority: Major 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
    • Rank:
      16634

      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.

        Issue Links

          Activity

          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          Eloy Lafuente (stronk7) added a comment -

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

          Show
          Eloy Lafuente (stronk7) added a comment - Probably we should be backporting this too, as far as MDL-25272 was reported for 19_STABLE.
          Hide
          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
          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
          Helen Foster added a comment -

          Again, thanks Tim.

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