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:

      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.

        Gliffy Diagrams

          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: