Moodle
  1. Moodle
  2. MDL-27741

Regression: forum_get_subscribe_link

    Details

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

      1. create a social format course
      2. allow guests to gain access
      3. login as a guest
      4. view the course and the word "No" should not appear in the bottom right of section 0

      Show
      1. create a social format course 2. allow guests to gain access 3. login as a guest 4. view the course and the word "No" should not appear in the bottom right of section 0
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      17390

      Description

      the forum_get_subscribe_link function is used by the social course format to display a link if required.

      MDL-276833 changed it's behavior and now it displays the text "No" if the user isn't enrolled - so now when a user visits a social format course that they aren't enrolled in it displays the text "No" in a really strange place.

        Issue Links

          Activity

          Hide
          Dan Marsden added a comment -

          IMO this function should return an empty string - it's also used in the socialplus course format.

          Show
          Dan Marsden added a comment - IMO this function should return an empty string - it's also used in the socialplus course format.
          Hide
          Michael de Raadt added a comment -

          Thanks for reporting this.

          I've put it on our backlog and we'll try to get to it as soon as we can.

          Show
          Michael de Raadt added a comment - Thanks for reporting this. I've put it on our backlog and we'll try to get to it as soon as we can.
          Hide
          Michael de Raadt added a comment -

          Raising the priority on this so we can be sure to give it some love, post-2.1.

          Michael;

          Show
          Michael de Raadt added a comment - Raising the priority on this so we can be sure to give it some love, post-2.1. Michael;
          Hide
          Dan Marsden added a comment -

          Hi Rosie,

          my vote would be to remove this code that Skodak added:
          if (!is_enrolled($context, $USER, '', true))

          { return get_string('no'); }

          the function forum_get_subscribe_link is also used in other 3rd party contrib course formats - so this bug appears in other code that expects the function to return a link or empty if no link is available.

          Show
          Dan Marsden added a comment - Hi Rosie, my vote would be to remove this code that Skodak added: if (!is_enrolled($context, $USER, '', true)) { return get_string('no'); } the function forum_get_subscribe_link is also used in other 3rd party contrib course formats - so this bug appears in other code that expects the function to return a link or empty if no link is available.
          Hide
          Rossiani Wijaya added a comment -

          Thanks Dan for the suggestion.

          I created a patch to remove the 'no' string.

          Assigning to Raj for peer review.

          Show
          Rossiani Wijaya added a comment - Thanks Dan for the suggestion. I created a patch to remove the 'no' string. Assigning to Raj for peer review.
          Hide
          Rajesh Taneja added a comment -

          Hello Rossie,

          Can you please confirm the behavior of forum_get_subscribe_link, for guest users.
          It seems like previously we don't allow guest to get a link and now removing this check changes that behavior.

          Probably, we can leave if statement and return empty string (as expected.)

            if (!is_enrolled($context, $USER, '', true)) {
                return '';
            }  
          
          Show
          Rajesh Taneja added a comment - Hello Rossie, Can you please confirm the behavior of forum_get_subscribe_link, for guest users. It seems like previously we don't allow guest to get a link and now removing this check changes that behavior. Probably, we can leave if statement and return empty string (as expected.) if (!is_enrolled($context, $USER, '', true )) { return ''; }
          Hide
          Dan Marsden added a comment -

          good point Raj - +1 to keep is_enrolled but return ''

          Show
          Dan Marsden added a comment - good point Raj - +1 to keep is_enrolled but return ''
          Hide
          Rajesh Taneja added a comment -

          Thanks Dan and Rossie,
          It looks Good to me

          Show
          Rajesh Taneja added a comment - Thanks Dan and Rossie, It looks Good to me
          Hide
          Rossiani Wijaya added a comment -

          Raj,

          Yup,user needs to enroll to a course in order to be able to subscribe to a forum.

          Show
          Rossiani Wijaya added a comment - Raj, Yup,user needs to enroll to a course in order to be able to subscribe to a forum.
          Hide
          Rossiani Wijaya added a comment -

          Thanks Raj for reviewing.

          Submitting for integration.

          Show
          Rossiani Wijaya added a comment - Thanks Raj for reviewing. Submitting for integration.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
          Hide
          Andrew Davis added a comment -

          Looks fine.

          Show
          Andrew Davis added a comment - Looks fine.
          Hide
          Sam Hemelryk added a comment -

          Congratulations - this fix has just been released in the weeklies.

          Show
          Sam Hemelryk added a comment - Congratulations - this fix has just been released in the weeklies.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: