Moodle
  1. Moodle
  2. MDL-19171

Make course fullname link to the course in upcoming events block

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.4, 2.6
    • Fix Version/s: 2.6
    • Component/s: Calendar
    • Labels:
    • Testing Instructions:
      Hide

      Prerequisites:

      • A course, C1.
      • A user, U1 (can be the admin user), enrolled in C1.
      • A group, G1, in C1 with U1 as a member.
      • A course event in C1 in the near future.
      • A group event in G1 in the near future.
      • An assignment activity in C1 with deadline in the near future.
      • Upcoming events block is displayed on both the C1 page and the site front page.

      N.B.: "Near future" means within the upcoming events period, a week will suffice.

      Test script:

      • Log in as U1.
      • Go to the front page.
      • In the upcoming events block, verify that assignment deadline, course event, and group event are all shown, with course name of C1 displayed in each instance, and with the text linked to the C1 page.
      • Go to the course page of C1.
      • In the upcoming events block, verify that assignment deadline, course event, and group event are all shown, but with no course name displayed.
      • Go to the Moodle calendar of the user.
      • Switch to daily view.
      • Verify that assignment deadline, course event, and group event are all shown (possibly on different days), with course name of C1 displayed in each instance, and with the text linked to the C1 page.
      Show
      Prerequisites: A course, C1. A user, U1 (can be the admin user), enrolled in C1. A group, G1, in C1 with U1 as a member. A course event in C1 in the near future. A group event in G1 in the near future. An assignment activity in C1 with deadline in the near future. Upcoming events block is displayed on both the C1 page and the site front page. N.B.: "Near future" means within the upcoming events period, a week will suffice. Test script: Log in as U1. Go to the front page. In the upcoming events block, verify that assignment deadline, course event, and group event are all shown, with course name of C1 displayed in each instance, and with the text linked to the C1 page. Go to the course page of C1. In the upcoming events block, verify that assignment deadline, course event, and group event are all shown, but with no course name displayed. Go to the Moodle calendar of the user. Switch to daily view. Verify that assignment deadline, course event, and group event are all shown (possibly on different days), with course name of C1 displayed in each instance, and with the text linked to the C1 page.
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_26_STABLE
    • Pull Master Branch:

      Description

      If the upcoming events block is displayed on the front page, then every course-related event (module event, group event, course event) in the block should contain the name of the relevant course, and a link to the course page.

      Also, if these events are displayed in the Moodle calendar, at least in the daily view, the name of the course and a link to it should be visible. (This is currently not the case for group events.)

      This is useful since events often have very generic names ("Lecture", "Seminar", "Assignment 3") and when they are displayed out of the context of the course they're in, it's helpful to see the course name.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Henning Bostelmann added a comment -

            Taking up this old issue from Moodle 1.9. This is, at least similarly, still relevant in the current version.

            We have been using local code modifications for a long time (since Moodle 1.9 at least) in order to provide this extremely useful feature. I will post a cleaned-up version of the relevant patch soon.

            Show
            Henning Bostelmann added a comment - Taking up this old issue from Moodle 1.9. This is, at least similarly, still relevant in the current version. We have been using local code modifications for a long time (since Moodle 1.9 at least) in order to provide this extremely useful feature. I will post a cleaned-up version of the relevant patch soon.
            Hide
            Henning Bostelmann added a comment -

            Submitting a patch against master for peer review.

            I'm not sure whether this improvement can be integrated to 2.5 according to the processes, but this would certainly be technically possible and I would be willing to provide a backport of the patch.

            Note that the patch does not include displaying the course name in the monthly calendar (MDL-1322). While I think that this should be addressed as well, the code appears to be quite different at that point and I'd therefore like to handle that separately.

            Show
            Henning Bostelmann added a comment - Submitting a patch against master for peer review. I'm not sure whether this improvement can be integrated to 2.5 according to the processes, but this would certainly be technically possible and I would be willing to provide a backport of the patch. Note that the patch does not include displaying the course name in the monthly calendar ( MDL-1322 ). While I think that this should be addressed as well, the code appears to be quite different at that point and I'd therefore like to handle that separately.
            Hide
            Ankit Agarwal added a comment -

            Hi Henning,
            Thanks for working on this issue. The patch looks great, here are a few minor suggestions:-

            1. There are few issues with coding guidelines (http://docs.moodle.org/dev/Coding_style):-
            • if (!$courseid) return ''; please add brackets
            • spaces around =>
            1. I would recommend using html_writer for printing the div in calendar_get_block_upcoming()
            2. Would it make more sense to $showcourselink default to false instead, to prevent any backward compatibility issues?
            3. I might be a bit picky on this one, but personally I would prefer the course name to be displayed in () besides the event name. However this might be just a personal preference, feel free to ignore that.
            4. It is recommended to have unit tests for new apis.

            I am adding myself as watcher to this issue, I will push it forward once, I get feedback from you.
            Thanks for the hard work.

            Show
            Ankit Agarwal added a comment - Hi Henning, Thanks for working on this issue. The patch looks great, here are a few minor suggestions:- There are few issues with coding guidelines ( http://docs.moodle.org/dev/Coding_style):- if (!$courseid) return ''; please add brackets spaces around => I would recommend using html_writer for printing the div in calendar_get_block_upcoming() Would it make more sense to $showcourselink default to false instead, to prevent any backward compatibility issues? I might be a bit picky on this one, but personally I would prefer the course name to be displayed in () besides the event name. However this might be just a personal preference, feel free to ignore that. It is recommended to have unit tests for new apis. I am adding myself as watcher to this issue, I will push it forward once, I get feedback from you. Thanks for the hard work.
            Hide
            Henning Bostelmann added a comment -

            Hi Ankit,

            thanks for your comments. I hope to have fixed the problems with coding style (seems I wasn't quite up to date here).

            1. html_writer for div, now fixed (without however refactoring the surrounding code)
            2. Good point, I have changed the default value to false.
            3. Since the course name is already on a separate line, I decided to ignore this.
            4. Don't you think this is a bit disproportionate in the present case? The entire calendar/lib.php is essentially without unit tests, there's hardly any "infrastructure code" for generating test data etc.

            Best wishes
            Henning

            Show
            Henning Bostelmann added a comment - Hi Ankit, thanks for your comments. I hope to have fixed the problems with coding style (seems I wasn't quite up to date here). 1. html_writer for div, now fixed (without however refactoring the surrounding code) 2. Good point, I have changed the default value to false. 3. Since the course name is already on a separate line, I decided to ignore this. 4. Don't you think this is a bit disproportionate in the present case? The entire calendar/lib.php is essentially without unit tests, there's hardly any "infrastructure code" for generating test data etc. Best wishes Henning
            Hide
            Ankit Agarwal added a comment - - edited

            Hi Henning,
            Thanks for the changes. Your comments makes sense.

            Pushing forward for integration review.

            Thanks

            Show
            Ankit Agarwal added a comment - - edited Hi Henning, Thanks for the changes. Your comments makes sense. Pushing forward for integration review. Thanks
            Hide
            Dan Poltawski added a comment -

            Hi Henning,

            Thanks a lot for the patch, i've integrated it into master.

            I would've liked to have seen some unit tests with the new function, but i decided to let that slide for today.

            Show
            Dan Poltawski added a comment - Hi Henning, Thanks a lot for the patch, i've integrated it into master. I would've liked to have seen some unit tests with the new function, but i decided to let that slide for today.
            Hide
            Damyon Wiese added a comment -

            The test instructions are good - thanks.

            All passing for me!

            Show
            Damyon Wiese added a comment - The test instructions are good - thanks. All passing for me!
            Hide
            Dan Poltawski added a comment -

            Cảm ơn!

            Your changes have now been merged upstream in git and will be available on the Moodle download sites shortly!

            Một hai ba, yo

            Show
            Dan Poltawski added a comment - Cảm ơn! Your changes have now been merged upstream in git and will be available on the Moodle download sites shortly! Một hai ba, yo

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: