Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-27741

Regression: forum_get_subscribe_link

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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

          Attachments

            Issue Links

              Activity

              Hide
              danmarsden Dan Marsden added a comment -

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

              Show
              danmarsden Dan Marsden added a comment - IMO this function should return an empty string - it's also used in the socialplus course format.
              Hide
              salvetore 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
              salvetore 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
              salvetore 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
              salvetore 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
              danmarsden 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
              danmarsden 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
              rwijaya 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
              rwijaya 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
              rajeshtaneja 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
              rajeshtaneja 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
              danmarsden Dan Marsden added a comment -

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

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

              Thanks Dan and Rossie,
              It looks Good to me

              Show
              rajeshtaneja Rajesh Taneja added a comment - Thanks Dan and Rossie, It looks Good to me
              Hide
              rwijaya 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
              rwijaya 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
              rwijaya Rossiani Wijaya added a comment -

              Thanks Raj for reviewing.

              Submitting for integration.

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

              Integrated, thanks!

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

              Looks fine.

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

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

              Show
              samhemelryk 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:
                    Fix Release Date:
                    1/Aug/11