Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.2
    • Component/s: Forum
    • Labels:
    • Testing Instructions:
      Hide

      1. Ensure that RSS is turned on at site level (there are two settings; the overall one and the yes/no dropdown for forums; you can find both if you search for 'rss' in admin menu).

      2. Ensure that RSS is turned on for a test forum in the forum settings. (also check that the forum settings RSS are on and not 0 being displayed)

      3. Return to the main page of the test forum where discussions are listed.

      Verify that the orange RSS icon is visible somewhere on the forum page (as part of the actual page content, regardless of which web browser is in use).

      Show
      1. Ensure that RSS is turned on at site level (there are two settings; the overall one and the yes/no dropdown for forums; you can find both if you search for 'rss' in admin menu). 2. Ensure that RSS is turned on for a test forum in the forum settings. (also check that the forum settings RSS are on and not 0 being displayed) 3. Return to the main page of the test forum where discussions are listed. Verify that the orange RSS icon is visible somewhere on the forum page (as part of the actual page content, regardless of which web browser is in use).
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      26127

      Description

      If you enable RSS for a forum, the RSS icon does not display anywhere on the forum page. According to the docs page there should be an orange icon but this does not display.

      The RSS feed URL is correctly visible if you view source code and look for the alternate url in header (you can just search for 'rss' in source). It is just that the icon isn't displayed, so users may not realise that RSS is available.

        Issue Links

          Activity

          Hide
          Martin Dougiamas added a comment -

          I notice that there is a link in 2.1 stable under "Forum adminstration" in Settings block, but it's not in 2.2. Looks like a regression.

          We definitely need a little orange icon somewhere too.

          Show
          Martin Dougiamas added a comment - I notice that there is a link in 2.1 stable under "Forum adminstration" in Settings block, but it's not in 2.2. Looks like a regression. We definitely need a little orange icon somewhere too.
          Hide
          Helen Foster added a comment -

          Just checking on moodle.org (running 2.1) it seems that the forum RSS icon is not displayed when logged in as a guest.

          Show
          Helen Foster added a comment - Just checking on moodle.org (running 2.1) it seems that the forum RSS icon is not displayed when logged in as a guest.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Just to confirm that logged both as admin and as moodler (@ moodle.org) I can see the orange icon in the Settings menu of forums.

          So perhaps this is, as stated by Helen, a problem for guests only, clearly behaving inconsistent because the feed is available for them if they have access to the forum (in fact the "alternate" is created and detected by - some - browsers), but the icon is missing.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Just to confirm that logged both as admin and as moodler (@ moodle.org) I can see the orange icon in the Settings menu of forums. So perhaps this is, as stated by Helen, a problem for guests only, clearly behaving inconsistent because the feed is available for them if they have access to the forum (in fact the "alternate" is created and detected by - some - browsers), but the icon is missing. Ciao
          Hide
          Michael de Raadt added a comment -

          This is blocking a QA test.

          Show
          Michael de Raadt added a comment - This is blocking a QA test.
          Hide
          Nicolas Martignoni added a comment -

          Confirming that RSS icon is present for authenticated users.

          For guest user though, the icon is not present. See MDL-30385 (duplicate ?).

          Show
          Nicolas Martignoni added a comment - Confirming that RSS icon is present for authenticated users. For guest user though, the icon is not present. See MDL-30385 (duplicate ?).
          Hide
          Rossiani Wijaya added a comment -

          This issue occurs due to an additional checking for user enrollment in displaying rss feed (http://git.moodle.org/gw?p=integration.git;a=commitdiff;h=7bbdd17d6e0a2480d448eadc226164e228d22e0e).

          Removing the enrolled check fixed the issue.

          Creating patch to fix the code regression.

          Show
          Rossiani Wijaya added a comment - This issue occurs due to an additional checking for user enrollment in displaying rss feed ( http://git.moodle.org/gw?p=integration.git;a=commitdiff;h=7bbdd17d6e0a2480d448eadc226164e228d22e0e ). Removing the enrolled check fixed the issue. Creating patch to fix the code regression.
          Hide
          Michael de Raadt added a comment -

          Authenticated users can see Forum feed links and access the feeds. So the issue is that guests and admins cannot see the feed link, and if they can, they are getting the feed with errors.

          Two options:

          1. add is_guest() and is_admin() checks
          2. add a capability for viewing rss feeds so this can be controlled by admins and controlled at different levels (MDL-30482)

          I'm in favour of the second option.

          Some points that need to be considered:

          • This needs to be consistent across all RSS feeds (for blogs, glossaries and forums), otherwise there should be a capability for each.
          • The checks made when the link is shown and when the feed is accessed should be consistent, in other words, the checks should be made when the link is shown and again when the feed is accessed so that students enrolled and later unenrolled are dealt with.
          Show
          Michael de Raadt added a comment - Authenticated users can see Forum feed links and access the feeds. So the issue is that guests and admins cannot see the feed link, and if they can, they are getting the feed with errors. Two options: add is_guest() and is_admin() checks add a capability for viewing rss feeds so this can be controlled by admins and controlled at different levels ( MDL-30482 ) I'm in favour of the second option. Some points that need to be considered: This needs to be consistent across all RSS feeds (for blogs, glossaries and forums), otherwise there should be a capability for each. The checks made when the link is shown and when the feed is accessed should be consistent, in other words, the checks should be made when the link is shown and again when the feed is accessed so that students enrolled and later unenrolled are dealt with.
          Hide
          Michael de Raadt added a comment -

          As Martin pointed out, we don't need a new capability, we can use the forum entry view capability.

          Show
          Michael de Raadt added a comment - As Martin pointed out, we don't need a new capability, we can use the forum entry view capability.
          Hide
          Rossiani Wijaya added a comment -

          Updating the patch to use can_access_course() instead of $enrolled.

          For frontpage forum, non-login user will be assigned guest token for rss link.

          Show
          Rossiani Wijaya added a comment - Updating the patch to use can_access_course() instead of $enrolled. For frontpage forum, non-login user will be assigned guest token for rss link.
          Hide
          Sam Hemelryk added a comment -

          Hi Rossi,

          The changes look good to me. There is one thing that I noticed however, in lib.php the call to can_access_course should be done only if everything else is acceptable. can_access_course entails DB queries which ideally should be avoided unless needed.
          Because of this that call (along with the || SITEID check) should be moved into the if statement and put at the end. OR should be made conditional on everything else.
          Otherwise spot on!

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Rossi, The changes look good to me. There is one thing that I noticed however, in lib.php the call to can_access_course should be done only if everything else is acceptable. can_access_course entails DB queries which ideally should be avoided unless needed. Because of this that call (along with the || SITEID check) should be moved into the if statement and put at the end. OR should be made conditional on everything else. Otherwise spot on! Cheers Sam
          Hide
          Rossiani Wijaya added a comment -

          Update patch.

          Thanks Sam for reviewing.

          Submitting for integration review.

          Show
          Rossiani Wijaya added a comment - Update patch. Thanks Sam for reviewing. Submitting for integration review.
          Hide
          Aparup Banerjee added a comment -

          Thanks all, this has been integrated into master.

          Show
          Aparup Banerjee added a comment - Thanks all, this has been integrated into master.
          Hide
          Aparup Banerjee added a comment - - edited

          updated step 2 of the test with "(also check that the forum settings RSS are on and not 0 being displayed)" - would this apply to the QA test too? - scratch that (i need glasses)

          oh, tested and it works for me.

          Show
          Aparup Banerjee added a comment - - edited updated step 2 of the test with "(also check that the forum settings RSS are on and not 0 being displayed)" - would this apply to the QA test too? - scratch that (i need glasses) oh, tested and it works for me.
          Hide
          Aparup Banerjee added a comment -

          passing!

          Show
          Aparup Banerjee added a comment - passing!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Sent upstream! Just in time for Moodle 2.2rc1 (if related), yay!

          Closing and big thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Sent upstream! Just in time for Moodle 2.2rc1 (if related), yay! Closing and big thanks!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: