Moodle
  1. Moodle
  2. MDL-33166

Add forum capability "mod/forum:allowforcesubscribe" or restore "mod/forum:initialsubscriptions"

    Details

    • Testing Instructions:
      Hide
      Test 1
      1. Create a course and don't enroll users
      2. Create forum with subscription mode = "Auto subscription"
      3. Enrol 1 teacher and 1 student to this course
      4. Make sure both are subscribed to forum (Settings -> Forum administration -> Show/edit current subscribers)
      5. Post a discussion and run cron, make sure both teacher and student get notification
      6. Enrol Manager in the same course and make sure forum subscribers doesn't have this Manager and no mail is sent to Manager
      Test 2
      1. Create a course and don't enroll users
      2. Create forum with subscription mode = "Force subscription"
      3. Enrol 1 teacher and 1 student to this course
      4. Make sure both are subscribed to forum (Settings -> Forum administration -> Show/edit current subscribers)
      5. Post a discussion and run cron, make sure both teacher and student get notification
      6. Enrol Manager in the same course and make sure forum subscribers doesn't have this Manager and no mail is sent to Manager
      Show
      Test 1 Create a course and don't enroll users Create forum with subscription mode = "Auto subscription" Enrol 1 teacher and 1 student to this course Make sure both are subscribed to forum (Settings -> Forum administration -> Show/edit current subscribers) Post a discussion and run cron, make sure both teacher and student get notification Enrol Manager in the same course and make sure forum subscribers doesn't have this Manager and no mail is sent to Manager Test 2 Create a course and don't enroll users Create forum with subscription mode = "Force subscription" Enrol 1 teacher and 1 student to this course Make sure both are subscribed to forum (Settings -> Forum administration -> Show/edit current subscribers) Post a discussion and run cron, make sure both teacher and student get notification Enrol Manager in the same course and make sure forum subscribers doesn't have this Manager and no mail is sent to Manager
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull Master Branch:
      wip-mdl-33166
    • Rank:
      41032

      Description

      In Moodle 2.2 the capability "mod/forum:initialsubscriptions" was removed to resolve this issue: MDL-30151

      However, that causes problems, because people relied on that capability to prevent certain roles from receiving emails from auto-subscribed forums (MDL-31481 and MDL-33050).

      Add this capability so user who don't have this capability should not be auto-subscribed to forum. (Both for force subscribe or auto subscribe.)

      original description.

      I resolve these problems I have created a new capability called mod/forum:receivemail. It acts similar to the other email specified capabilities such as:

      • mod/quiz:emailconfirmsubmission
      • mod/quiz:emailnotifysubmission
      • mod/feedback:receivemail

      If a user does not have a role with that capability set to Allow, then they will:

      • Not be sent email (either digest or per post) if they are associated with a forum that sends out to subscribers
      • Not have "Subscribe me to this forum" link in the nav block

      However, there is an issue in which that user will appear in the list of subscribed users for auto-subscribed forums, because there isn't a clean way to filter them out. But I hope that isn't a big deal or if someone else can figure out how to do that cleanly.

        Issue Links

          Activity

          Rex Lorenzo created issue -
          Rex Lorenzo made changes -
          Field Original Value New Value
          Link This issue will help resolve MDL-31481 [ MDL-31481 ]
          Rex Lorenzo made changes -
          Link This issue will help resolve MDL-33050 [ MDL-33050 ]
          Rex Lorenzo made changes -
          Labels patch
          Dan Poltawski made changes -
          Labels patch lost_functionality patch
          Hide
          Dan Poltawski added a comment - - edited

          Thanks for the patch, Rex! I agree we should be able to control this capabilities.

          One question in my head is that there is some sort of functionality for controlling this sort of thing with the messaging framework, it could be applicable here. Though I also have a vague memory that it only applies at system context.

          Added the lost_functionality tag and assigning this to stable backlog, because its really a regression.

          Show
          Dan Poltawski added a comment - - edited Thanks for the patch, Rex! I agree we should be able to control this capabilities. One question in my head is that there is some sort of functionality for controlling this sort of thing with the messaging framework, it could be applicable here. Though I also have a vague memory that it only applies at system context. Added the lost_functionality tag and assigning this to stable backlog, because its really a regression.
          Dan Poltawski made changes -
          Fix Version/s DEV backlog [ 10464 ]
          Labels lost_functionality patch lost_functionality patch triaged
          Dan Poltawski made changes -
          Fix Version/s STABLE backlog [ 10463 ]
          Fix Version/s DEV backlog [ 10464 ]
          Dan Poltawski made changes -
          Priority Minor [ 4 ] Critical [ 2 ]
          Rajesh Taneja made changes -
          Fix Version/s STABLE Sprint 23 [ 12358 ]
          Fix Version/s STABLE backlog [ 10463 ]
          Assignee moodle.com [ moodle.com ] Rajesh Taneja [ rajeshtaneja ]
          moodle.com made changes -
          Fix Version/s STABLE Sprint 23 Omega [ 12362 ]
          Fix Version/s STABLE Sprint 23 Alpha [ 12358 ]
          Rajesh Taneja made changes -
          Status Open [ 1 ] Development in progress [ 3 ]
          Rajesh Taneja made changes -
          Testing Instructions # Create forum as teacher
          # Add a new discussion
          # As student post to forum and make sure teacher receive email.
          # Remove mod/forum:receivemail for the teacher and as student add another post.
          # Check teacher should not receive email.
          Hide
          Rajesh Taneja added a comment -

          Thanks for the patch Rex, I have cleaned few things and now pushing it for peer-review.

          @Dan: In messaging framework you can control how you get message from forum, but it's not based on role.

          FYI:
          Removed 2.2 and 2.3 branches as, this is an enhancement. Not sure if this should go in old branches.

          Show
          Rajesh Taneja added a comment - Thanks for the patch Rex, I have cleaned few things and now pushing it for peer-review. @Dan: In messaging framework you can control how you get message from forum, but it's not based on role. FYI: Removed 2.2 and 2.3 branches as, this is an enhancement. Not sure if this should go in old branches.
          Rajesh Taneja made changes -
          Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
          Rajesh Taneja made changes -
          Pull 2.1 Branch MDL-31481_21-forum_receivemail
          Rajesh Taneja made changes -
          Labels lost_functionality patch triaged docs_required lost_functionality patch triaged
          Hide
          Dan Poltawski added a comment -

          Hi,

          IMO this is a regression and we should backport it to stable branches. We took away control from sites inadvertently, so we should consider giving it back with this capabilities. (Though I agree this needs some +1's to get all clear for backporting).

          Show
          Dan Poltawski added a comment - Hi, IMO this is a regression and we should backport it to stable branches. We took away control from sites inadvertently, so we should consider giving it back with this capabilities. (Though I agree this needs some +1's to get all clear for backporting).
          Hide
          Rajesh Taneja added a comment -

          Thanks Dan,

          I agree, this should be backported. We can consider this as special case and backport it.
          Will wait for more +1's and create 2.3 and 2.2 branches.

          Show
          Rajesh Taneja added a comment - Thanks Dan, I agree, this should be backported. We can consider this as special case and backport it. Will wait for more +1's and create 2.3 and 2.2 branches.
          Frédéric Massart made changes -
          Original Estimate 0 minutes [ 0 ]
          Remaining Estimate 0 minutes [ 0 ]
          Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
          Peer reviewer fred
          Hide
          Frédéric Massart added a comment -

          Thanks Rex and Raj for fixing this.

          First, I wonder if the name of the capability is well chosen. In the patch, we are disallowing the user from being 'subscribed' to the forum, but 'receiving email' is something different to me. If we intend to limit the ability to being subscribed, than 'allowsubscription' or something would probably be more verbose.

          The second thing is just about code. In mod/forum/lib.php:580~

          • I would move the capability check up where the list of subscribed users is retrieved (L511).
          • Also $modcontext which is used to check the capability, is not the proper context and could lead to some random behaviours. If the check if moved up to line 515, then the context will be correct.
          • Minor white spaces issue on line 590-591.
          • The logic also clearly skips subscribers if they can't receive emails. (cf. my first point)

          My +1 to backport this.

          Thanks guys!

          Show
          Frédéric Massart added a comment - Thanks Rex and Raj for fixing this. First, I wonder if the name of the capability is well chosen. In the patch, we are disallowing the user from being 'subscribed' to the forum, but 'receiving email' is something different to me. If we intend to limit the ability to being subscribed, than 'allowsubscription' or something would probably be more verbose. The second thing is just about code. In mod/forum/lib.php:580~ I would move the capability check up where the list of subscribed users is retrieved (L511). Also $modcontext which is used to check the capability, is not the proper context and could lead to some random behaviours. If the check if moved up to line 515, then the context will be correct. Minor white spaces issue on line 590-591. The logic also clearly skips subscribers if they can't receive emails. (cf. my first point) My +1 to backport this. Thanks guys!
          Frédéric Massart made changes -
          Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
          Hide
          Rajesh Taneja added a comment -

          Thanks For the inputs Fred,

          I have integrated your inputs, but not sure about capability name.
          I think initialsubscriptions or cansubscribe can be a good option. But not sure about it.

          Show
          Rajesh Taneja added a comment - Thanks For the inputs Fred, I have integrated your inputs, but not sure about capability name. I think initialsubscriptions or cansubscribe can be a good option. But not sure about it.
          Hide
          Dan Poltawski added a comment -

          Adding some watchers here. The more we think through this issue the less sense removing that old capability makes.

          If we add a capability only on receiving messages then why can such a user subscribe. If we prevent the user from subscribing then how will 'managers' be able to get an overview of a course and then choose to subscribe to a specific forum these are interested in.

          Show
          Dan Poltawski added a comment - Adding some watchers here. The more we think through this issue the less sense removing that old capability makes. If we add a capability only on receiving messages then why can such a user subscribe. If we prevent the user from subscribing then how will 'managers' be able to get an overview of a course and then choose to subscribe to a specific forum these are interested in.
          Hide
          Martin Dougiamas added a comment -

          I agree the old capability makes sense and we should restore mod/forum:initialsubscriptions

          Show
          Martin Dougiamas added a comment - I agree the old capability makes sense and we should restore mod/forum:initialsubscriptions
          Dan Poltawski made changes -
          Link This issue is a regression caused by MDL-30151 [ MDL-30151 ]
          Rajesh Taneja made changes -
          Testing Instructions # Create forum as teacher
          # Add a new discussion
          # As student post to forum and make sure teacher receive email.
          # Remove mod/forum:receivemail for the teacher and as student add another post.
          # Check teacher should not receive email.
          # Create forum as teacher
          # Add a new discussion
          # As student post to forum and make sure teacher receive email.
          # Remove mod/forum:getsubscriptions for the teacher and as student add another post.
          # Check teacher should not receive email.

          Test 2:
          # Make sure user should not see Subscribe to or unsubscribe from this forum link, if user doesn't have mod/forum:getsubscriptions capability. (should only see subscription modes in navigation)
          # Go to /mod/forum/index.php?id={course_id} and make sure you don't see "Subscribe to all forums" and "Unsubscribe from all forums" links and table should not have subscribed row.
          Rajesh Taneja made changes -
          Pull 2.3 Diff URL https://github.com/rajeshtaneja/moodle/compare/MOODLE_23_STABLE...wip-mdl-33166-m23
          Pull 2.2 Diff URL https://github.com/rajeshtaneja/moodle/compare/master...wip-mdl-33166-m22
          Pull 2.2 Branch wip-mdl-33166-m22
          Pull 2.3 Branch wip-mdl-33166-m23
          Hide
          Martin Dougiamas added a comment -

          Raj can you describe how this new capability is different from the one removed?

          Show
          Martin Dougiamas added a comment - Raj can you describe how this new capability is different from the one removed?
          Hide
          Rajesh Taneja added a comment - - edited

          mod/forum:getsubscriptions is similar to mod/forum:initialsubscriptions, but with few variations.

          mod/forum:initialsubscriptions mod/forum:getsubscriptions
          This allows a user to be subscribed initially to forums This doesn't do anything with subscription
          A user with this capability will receive notification emails from force-subscribed forums Same behaviour
          This capability is allowed for default teacher, non-editing teacher and student roles Same behaviour
          This capability is not set for default admin, course creator, Manager (2.0) roles so they don't receive unwanted forum subscription emails (i.e. emails from every forum that has forced-subscription). If this is set for any role that is used at system level (e.g. admin, course creators, Managers), assigning users to these roles will take a long time because users will be subscribed in forums in all courses. This capability is not set for admin, course creator and Manager roles. But notifications will be sent only, if these users are enrolled in course and have getsubscriptions capability
          Summary

          If user doesn't have mod/forum:getsubscriptions capability, then subscribe/unsubscribe links will not be visible. Also, user will not receive forum notification, if this capability is not set. By default only teacher, non-editing teacher and student have this capability.

          Show
          Rajesh Taneja added a comment - - edited mod/forum:getsubscriptions is similar to mod/forum:initialsubscriptions , but with few variations. mod/forum:initialsubscriptions mod/forum:getsubscriptions This allows a user to be subscribed initially to forums This doesn't do anything with subscription A user with this capability will receive notification emails from force-subscribed forums Same behaviour This capability is allowed for default teacher, non-editing teacher and student roles Same behaviour This capability is not set for default admin, course creator, Manager (2.0) roles so they don't receive unwanted forum subscription emails (i.e. emails from every forum that has forced-subscription). If this is set for any role that is used at system level (e.g. admin, course creators, Managers), assigning users to these roles will take a long time because users will be subscribed in forums in all courses. This capability is not set for admin, course creator and Manager roles. But notifications will be sent only, if these users are enrolled in course and have getsubscriptions capability Summary If user doesn't have mod/forum:getsubscriptions capability, then subscribe/unsubscribe links will not be visible. Also, user will not receive forum notification, if this capability is not set. By default only teacher, non-editing teacher and student have this capability.
          Rajesh Taneja made changes -
          Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
          Hide
          Martin Dougiamas added a comment -

          Thank you, that is a very useful comparison.

          So, are you proposing that this new capability actually replaces forum subscription or just duplicates it?

          If the former then that is a major change and you will need to replace all the subscription machinery.
          If the latter then it's quite confusing - what happens when the two settings conflict?

          Show
          Martin Dougiamas added a comment - Thank you, that is a very useful comparison. So, are you proposing that this new capability actually replaces forum subscription or just duplicates it? If the former then that is a major change and you will need to replace all the subscription machinery. If the latter then it's quite confusing - what happens when the two settings conflict?
          Hide
          Rajesh Taneja added a comment - - edited

          Hello Martin,

          This is not changing any forum subscription capability. initialsubscriptions was removed in 2.2, now we are adding getsubscriptions, which doesn't change any subscription mechanism.

          If user have this capability then he/she will receive forum notifications, else no notifications will be sent to user. Main reason for adding this capability was to avoid user from getting unwanted emails.

          Show
          Rajesh Taneja added a comment - - edited Hello Martin, This is not changing any forum subscription capability. initialsubscriptions was removed in 2.2, now we are adding getsubscriptions, which doesn't change any subscription mechanism. If user have this capability then he/she will receive forum notifications, else no notifications will be sent to user. Main reason for adding this capability was to avoid user from getting unwanted emails.
          Hide
          Martin Dougiamas added a comment -

          Just looked at the patch. I had misunderstood from your description about the extent this is controlling subscriptions. It doesn't seem to be touching any of the actual subscriptions that are listed in the forum_subscriptions table - it's just acting as an OVERRIDE for those, right?

          So with this patch, to get a notification from a forum the user needs to be in one of these situations:

          a) subscribed to forum AND mod/forum:getsubscriptions enabled
          b) enrolled in course AND forum set to force-subscribe AND mod/forum:getsubscriptions enabled

          And to see the subscription/unsubscription controls they also need mod/forum:getsubscriptions enabled.

          Right?

          So my questions are:

          1) What if a manager who is not part of a course does want to subscribe to a forum that they can obviously read directly? It seems with this patch that they are being prevented from doing so, because the same capability is being used for two things. I think actually using mod/forum:getsubscriptions for case (a) above is not needed, because if they DO have buttons they can subscribe/unsubscribe as they want to. The existing subscription mechanism is enough.

          2) If mod/forum:getsubscriptions is really only for Forcesubscribed forums (which already has the subscription buttons disabled IIRC) then perhaps it should be renamed something like mod/forum:subjecttoforcesubscribe or mod/forum:forcesubscribable

          Thanks! Hope I'm making sense! If not please explain!

          Show
          Martin Dougiamas added a comment - Just looked at the patch. I had misunderstood from your description about the extent this is controlling subscriptions. It doesn't seem to be touching any of the actual subscriptions that are listed in the forum_subscriptions table - it's just acting as an OVERRIDE for those, right? So with this patch, to get a notification from a forum the user needs to be in one of these situations: a) subscribed to forum AND mod/forum:getsubscriptions enabled b) enrolled in course AND forum set to force-subscribe AND mod/forum:getsubscriptions enabled And to see the subscription/unsubscription controls they also need mod/forum:getsubscriptions enabled. Right? So my questions are: 1) What if a manager who is not part of a course does want to subscribe to a forum that they can obviously read directly? It seems with this patch that they are being prevented from doing so, because the same capability is being used for two things. I think actually using mod/forum:getsubscriptions for case (a) above is not needed, because if they DO have buttons they can subscribe/unsubscribe as they want to. The existing subscription mechanism is enough. 2) If mod/forum:getsubscriptions is really only for Forcesubscribed forums (which already has the subscription buttons disabled IIRC) then perhaps it should be renamed something like mod/forum:subjecttoforcesubscribe or mod/forum:forcesubscribable Thanks! Hope I'm making sense! If not please explain!
          Hide
          Rajesh Taneja added a comment -

          If user is already subscribed to one/many forums, and he/she don't want forum notifications, then this capability can be removed and user won't get any forum notifications.
          It you think this should be only done for forcesubscription then, I can change patch to reflect this behaviour.

          Show
          Rajesh Taneja added a comment - If user is already subscribed to one/many forums, and he/she don't want forum notifications, then this capability can be removed and user won't get any forum notifications. It you think this should be only done for forcesubscription then, I can change patch to reflect this behaviour.
          Hide
          Martin Dougiamas added a comment - - edited

          I think that should be done, otherwise we have two very different ways to achieve the same goal, and it's confusing.

          If you make it explicitly about the forcesubscriptions then it's a lot clearer.

          How about mod/forum:canbeforcesubscribed ?

          Show
          Martin Dougiamas added a comment - - edited I think that should be done, otherwise we have two very different ways to achieve the same goal, and it's confusing. If you make it explicitly about the forcesubscriptions then it's a lot clearer. How about mod/forum:canbeforcesubscribed ?
          Hide
          Martin Dougiamas added a comment -

          And if we also make auto subscription mode part of this then the cap could be mod/forum:allowautosubscribe

          That could be used in the logic for both forcesubcribe and autosubscribe forums.

          Show
          Martin Dougiamas added a comment - And if we also make auto subscription mode part of this then the cap could be mod/forum:allowautosubscribe That could be used in the logic for both forcesubcribe and autosubscribe forums.
          Rajesh Taneja made changes -
          Status Waiting for peer review [ 10012 ] Development in progress [ 3 ]
          Hide
          Martin Dougiamas added a comment -

          There is also the setting $USER->autosubscribe. On reflection I think this should not be involved. Pity about the name though.

          For overall clarity I think the new cap should be mod/forum:allowforcesubscribe (sorry about the flipflop)

          Show
          Martin Dougiamas added a comment - There is also the setting $USER->autosubscribe. On reflection I think this should not be involved. Pity about the name though. For overall clarity I think the new cap should be mod/forum:allowforcesubscribe (sorry about the flipflop)
          Rajesh Taneja made changes -
          Testing Instructions # Create forum as teacher
          # Add a new discussion
          # As student post to forum and make sure teacher receive email.
          # Remove mod/forum:getsubscriptions for the teacher and as student add another post.
          # Check teacher should not receive email.

          Test 2:
          # Make sure user should not see Subscribe to or unsubscribe from this forum link, if user doesn't have mod/forum:getsubscriptions capability. (should only see subscription modes in navigation)
          # Go to /mod/forum/index.php?id={course_id} and make sure you don't see "Subscribe to all forums" and "Unsubscribe from all forums" links and table should not have subscribed row.
          h6. Test 1
          # Create a course and don't enroll users
          # Create forum with subscription mode = "Auto subscription"
          # Enrol 1 teacher and 1 student to this course
          # Make sure both are subscribed to forum (Settings -> Forum administration -> Show/edit current subscribers)
          # Post a discussion and run cron, make sure both teacher and student get notification
          # Enrol Manager in the same course and make sure forum subscribers doesn't have this Manager and no mail is sent to Manager

          h6. Test 2
          # Create a course and don't enroll users
          # Create forum with subscription mode = "Force subscription"
          # Enrol 1 teacher and 1 student to this course
          # Make sure both are subscribed to forum (Settings -> Forum administration -> Show/edit current subscribers)
          # Post a discussion and run cron, make sure both teacher and student get notification
          # Enrol Manager in the same course and make sure forum subscribers doesn't have this Manager and no mail is sent to Manager
          Hide
          Dan Poltawski added a comment -

          Me and Raj have had a lot of discussion about this, and just wonder if the solution to one of the problems is an event fired after user is enrolled AND role is assigned.

          Show
          Dan Poltawski added a comment - Me and Raj have had a lot of discussion about this, and just wonder if the solution to one of the problems is an event fired after user is enrolled AND role is assigned.
          Rajesh Taneja made changes -
          Hide
          Rajesh Taneja added a comment -

          As discussed with Dan:

          1. If user doesn't have any role in course, then user should not be auto subscribed to forum.
          2. Having a new event is a good option, but it will not take care of any change in role and user will be always subscribed to forum.
            1. In case user has no role and is enrolled in course. At this point he is not subscribed to any forum.
            2. Change user role to student and user expects to be enrolled ? If yes, then firing subscription check at role_assigned make sense. But it's expensive (Current patch is doing it fine)

          Still trying to figure out the best possible solution.

          Show
          Rajesh Taneja added a comment - As discussed with Dan: If user doesn't have any role in course, then user should not be auto subscribed to forum. Having a new event is a good option, but it will not take care of any change in role and user will be always subscribed to forum. In case user has no role and is enrolled in course. At this point he is not subscribed to any forum. Change user role to student and user expects to be enrolled ? If yes, then firing subscription check at role_assigned make sense. But it's expensive (Current patch is doing it fine) Still trying to figure out the best possible solution.
          Hide
          Rajesh Taneja added a comment -

          Added Petr, to get some more feedback.

          Show
          Rajesh Taneja added a comment - Added Petr, to get some more feedback.
          Hide
          Rajesh Taneja added a comment -

          I think subscribing user while assigning role make sense. Pushing it for peer-review to get more feedback.

          Show
          Rajesh Taneja added a comment - I think subscribing user while assigning role make sense. Pushing it for peer-review to get more feedback.
          Rajesh Taneja made changes -
          Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
          Rajesh Taneja made changes -
          Description In Moodle 2.2 the capability "mod/forum:initialsubscriptions" was removed to resolve this issue: MDL-30151

          However, that causes problems, because people relied on that capability to prevent certain roles from receiving emails from auto-subscribed forums (MDL-31481 and MDL-33050).

          I resolve these problems I have created a new capability called mod/forum:receivemail. It acts similar to the other email specified capabilities such as:

          * mod/quiz:emailconfirmsubmission
          * mod/quiz:emailnotifysubmission
          * mod/feedback:receivemail

          If a user does not have a role with that capability set to Allow, then they will:
          * Not be sent email (either digest or per post) if they are associated with a forum that sends out to subscribers
          * Not have "Subscribe me to this forum" link in the nav block

          However, there is an issue in which that user will appear in the list of subscribed users for auto-subscribed forums, because there isn't a clean way to filter them out. But I hope that isn't a big deal or if someone else can figure out how to do that cleanly.
          In Moodle 2.2 the capability "mod/forum:initialsubscriptions" was removed to resolve this issue: MDL-30151

          However, that causes problems, because people relied on that capability to prevent certain roles from receiving emails from auto-subscribed forums (MDL-31481 and MDL-33050).

          Add this capability so user who don't have this capability should not be auto-subscribed to forum. (Both for force subscribe or auto subscribe.)

          h2. original description.
          I resolve these problems I have created a new capability called mod/forum:receivemail. It acts similar to the other email specified capabilities such as:

          * mod/quiz:emailconfirmsubmission
          * mod/quiz:emailnotifysubmission
          * mod/feedback:receivemail

          If a user does not have a role with that capability set to Allow, then they will:
          * Not be sent email (either digest or per post) if they are associated with a forum that sends out to subscribers
          * Not have "Subscribe me to this forum" link in the nav block

          However, there is an issue in which that user will appear in the list of subscribed users for auto-subscribed forums, because there isn't a clean way to filter them out. But I hope that isn't a big deal or if someone else can figure out how to do that cleanly.
          Hide
          Rajesh Taneja added a comment -
          mod/forum:initialsubscriptions mod/forum:allowforcesubscribe
          This allows a user to be subscribed initially to forums Same behaviour
          A user with this capability will receive notification emails from force-subscribed forums Same behaviour
          This capability is allowed for default teacher, non-editing teacher and student roles Same behaviour
          This capability is not set for default admin, course creator, Manager (2.0) roles so they don't receive unwanted forum subscription emails (i.e. emails from every forum that has forced-subscription). If this is set for any role that is used at system level (e.g. admin, course creators, Managers), assigning users to these roles will take a long time because users will be subscribed in forums in all courses. This capability will only subscribe users if they are enrolled in course and assigned role. Changing role between having capability and not having capability will subscribe and unsubscribe users
          Summary

          If user role doesn't have mod/forum:allowforcesubscribe capability, then user will not be subscribed to force or auto subscribe forums and will not receive any notifications. Although for auto-subscribe forums user can manually subscribe, but for force subscribe forums they should have mod/forum:allowforcesubscribe capability.

          Show
          Rajesh Taneja added a comment - mod/forum:initialsubscriptions mod/forum:allowforcesubscribe This allows a user to be subscribed initially to forums Same behaviour A user with this capability will receive notification emails from force-subscribed forums Same behaviour This capability is allowed for default teacher, non-editing teacher and student roles Same behaviour This capability is not set for default admin, course creator, Manager (2.0) roles so they don't receive unwanted forum subscription emails (i.e. emails from every forum that has forced-subscription). If this is set for any role that is used at system level (e.g. admin, course creators, Managers), assigning users to these roles will take a long time because users will be subscribed in forums in all courses. This capability will only subscribe users if they are enrolled in course and assigned role. Changing role between having capability and not having capability will subscribe and unsubscribe users Summary If user role doesn't have mod/forum:allowforcesubscribe capability, then user will not be subscribed to force or auto subscribe forums and will not receive any notifications. Although for auto-subscribe forums user can manually subscribe, but for force subscribe forums they should have mod/forum:allowforcesubscribe capability.
          Rajesh Taneja made changes -
          Summary Adding forum capability mod/forum:receivemail Add forum capability "mod/forum:allowforcesubscribe" or restore "mod/forum:initialsubscriptions"
          Hide
          Steve Bond added a comment -

          Hi all,

          Raj asked me over in the forums to comment on this, so here goes:

          I suggest that mod/forum:allowforcesubscribe is preferable to mod/forum:initialsubscriptions, because I think there is another difference that has not been noted above.

          In 1.9 I had a bunch of teachers who were enrolled in multiple courses for oversight purposes, and I got a complaint about unwanted email from forced-subscribe forums. So I changed their role so that mod/forum:initialsubscriptions was disabled. However, this didn't stop the emails for all the teachers. It seemed to me that those teachers who had already received emails from forced-subscription forums had been added to the mdl_forum_subscriptions table, and so would continue to receive emails even though mod/forum:initialsubscriptions was now disabled. I guess that's why it's named "initial" - because it controls the initial state rather than the ongoing state?

          Maybe I misunderstood what was going on, but if not, then I think this permission should be a straightforward on/off switch for forced-subscription emails, rather than just controlling the initial state.

          Steve

          Show
          Steve Bond added a comment - Hi all, Raj asked me over in the forums to comment on this, so here goes: I suggest that mod/forum:allowforcesubscribe is preferable to mod/forum:initialsubscriptions, because I think there is another difference that has not been noted above. In 1.9 I had a bunch of teachers who were enrolled in multiple courses for oversight purposes, and I got a complaint about unwanted email from forced-subscribe forums. So I changed their role so that mod/forum:initialsubscriptions was disabled. However, this didn't stop the emails for all the teachers. It seemed to me that those teachers who had already received emails from forced-subscription forums had been added to the mdl_forum_subscriptions table, and so would continue to receive emails even though mod/forum:initialsubscriptions was now disabled. I guess that's why it's named "initial" - because it controls the initial state rather than the ongoing state? Maybe I misunderstood what was going on, but if not, then I think this permission should be a straightforward on/off switch for forced-subscription emails, rather than just controlling the initial state. Steve
          Hide
          Rajesh Taneja added a comment - - edited

          Thanks for the inputs Steve,

          With current patch we are trying to achieve the following:

          1. For force subscription forum: If user is enroled in a course with certain role (say Manager), and manager doesn't have mod/forum:allowforcesubscribe capability, then user will not receive forum notifications. But if same user gets a teacher role in same course (which has mod/forum:allowforcesubscribe capability), then user will get notifications.
          2. For auto subscription forum: (This is tricky) When user is enroled in a course with teacher/student role (having mod/forum:allowforcesubscribe capability) then user will be subscribed to forum, if user role is changed to manager then user will not be unsubscribed from forum, but can unsubscribe manually.

          Just want to make sure if this should happen on course role assignment or on site level (If user is assigned Manager at site level, then should we subscribe user) as well ? IMO, it should not be done on site-wide, as it is very costly operation and on large sites may not be acceptable.

          Show
          Rajesh Taneja added a comment - - edited Thanks for the inputs Steve, With current patch we are trying to achieve the following: For force subscription forum : If user is enroled in a course with certain role (say Manager), and manager doesn't have mod/forum:allowforcesubscribe capability, then user will not receive forum notifications. But if same user gets a teacher role in same course (which has mod/forum:allowforcesubscribe capability), then user will get notifications. For auto subscription forum : (This is tricky) When user is enroled in a course with teacher/student role (having mod/forum:allowforcesubscribe capability) then user will be subscribed to forum, if user role is changed to manager then user will not be unsubscribed from forum, but can unsubscribe manually. Just want to make sure if this should happen on course role assignment or on site level (If user is assigned Manager at site level, then should we subscribe user) as well ? IMO, it should not be done on site-wide, as it is very costly operation and on large sites may not be acceptable.
          Michael de Raadt made changes -
          Link This issue has been marked as being related by MDL-35027 [ MDL-35027 ]
          Frédéric Massart made changes -
          Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
          Hide
          Frédéric Massart added a comment -

          Hi Raj,

          the patch looks good to me, and I agree with you to limit to a course level. Even if it could confuse some administrators playing with roles and capabilities on a site level.

          I also think that it's a good thing that auto subscriptions are not removed when the role changes, but only set if missing.

          Show
          Frédéric Massart added a comment - Hi Raj, the patch looks good to me, and I agree with you to limit to a course level. Even if it could confuse some administrators playing with roles and capabilities on a site level. I also think that it's a good thing that auto subscriptions are not removed when the role changes, but only set if missing.
          Frédéric Massart made changes -
          Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
          Hide
          Rajesh Taneja added a comment -

          Thanks for the feedback Fred,

          pushing it for integration review.

          Show
          Rajesh Taneja added a comment - Thanks for the feedback Fred, pushing it for integration review.
          Rajesh Taneja made changes -
          Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
          Pull 2.3 Diff URL https://github.com/rajeshtaneja/moodle/compare/MOODLE_23_STABLE...wip-mdl-33166-m23
          Pull 2.2 Diff URL https://github.com/rajeshtaneja/moodle/compare/MOODLE_22_STABLE...wip-mdl-33166-m22
          Pull 2.2 Branch wip-mdl-33166-m22
          Pull 2.3 Branch wip-mdl-33166-m23
          Hide
          Steve Bond added a comment -

          Hi Raj,

          The patch description sounds fine to me. I'm not sure I understand the site-wide question. Certainly if I assigned a system role of Manager to a user, I would not want that user to receive emails from every single forced-sub forum on the site. I assume that would not happen under your proposals.

          In any case, I no longer understand system roles (as evidenced here http://tracker.moodle.org/browse/MDL-30931) so perhaps I should keep quiet on that issue.

          Steve

          Show
          Steve Bond added a comment - Hi Raj, The patch description sounds fine to me. I'm not sure I understand the site-wide question. Certainly if I assigned a system role of Manager to a user, I would not want that user to receive emails from every single forced-sub forum on the site. I assume that would not happen under your proposals. In any case, I no longer understand system roles (as evidenced here http://tracker.moodle.org/browse/MDL-30931 ) so perhaps I should keep quiet on that issue. Steve
          Hide
          Rajesh Taneja added a comment -

          Thanks Steve,

          With system role means site-wide manager or course-creator role.

          Show
          Rajesh Taneja added a comment - Thanks Steve, With system role means site-wide manager or course-creator role.
          Aparup Banerjee made changes -
          Currently in integration Yes [ 10041 ]
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          Well, I've one existential problem with this capability as it's pretty simple:

          • Capabilities are about the rights/permissions one user have against the system (aka: what the user can/cannot do WITH Moodle). And...
          • Capabilities are NOT about what the system must/must not do TO users.

          And IMO, we are clearly using one capability here 100% in the wrong way, to decide what the system must/must not do TO users (aka, force/soft-subscribe them to forums).

          To have one capability to allow/deny users to subscribe (themselves) to forums is ok. And to have one capability to allow/deny users to receive mails from forums is ok. Bot define what users can do WITH Moodle.

          But to have one capability to decide if the system must/must not subscribe users to forums is wrong. It defines what the system must do TO users, not what users can do WITH Moodle at all.

          I know it's a subtle difference, but in my mind (crazy and tired one) it's pretty clear and real. Capabilities are user permissions, not system todo tasks.

          In fact you cannot say "user X can/cannot initialsubscriptions in mod/forum" or "user X can/cannot allowforcesubscribe in mod/forum". Perhaps the simpler way to express my rationale.

          Said that, 2 points:

          0) Can the goals be implemented via permissions (capabilities)? Surely yes, code proves it. But it's abusing of permissions (in my mind).

          1) Can we implement it without capabilitites? Surely yes, too. By defining which roles are part of the learning experience (does the concept "participant" sounds to us and applies here?) and which ones are not. And then use that list to make the system to perform any action. Site-wide, course-wide, module-wide or whatever. But not using user permissions.

          So, said that, I'm going to reopen this, just to give it one turn of thinking. Note it's only my opinion, and I'll be happy with whatever is finally decided.

          But please, don't try to follow the shortest way only because there is already one patch ready, that would be an incorrect way to confront the problem. Let's define what a simple and pristine capability is (that said 6 years after roles and caps were introduced sounds like I joke, I know, but...)

          I'll do whatever it's decided, np. But I could not let this land before exposing my thoughts.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - - edited Well, I've one existential problem with this capability as it's pretty simple: Capabilities are about the rights/permissions one user have against the system (aka: what the user can/cannot do WITH Moodle). And... Capabilities are NOT about what the system must/must not do TO users. And IMO, we are clearly using one capability here 100% in the wrong way, to decide what the system must/must not do TO users (aka, force/soft-subscribe them to forums). To have one capability to allow/deny users to subscribe (themselves) to forums is ok. And to have one capability to allow/deny users to receive mails from forums is ok. Bot define what users can do WITH Moodle. But to have one capability to decide if the system must/must not subscribe users to forums is wrong. It defines what the system must do TO users, not what users can do WITH Moodle at all. I know it's a subtle difference, but in my mind (crazy and tired one) it's pretty clear and real. Capabilities are user permissions, not system todo tasks. In fact you cannot say "user X can/cannot initialsubscriptions in mod/forum" or "user X can/cannot allowforcesubscribe in mod/forum". Perhaps the simpler way to express my rationale. Said that, 2 points: 0) Can the goals be implemented via permissions (capabilities)? Surely yes, code proves it. But it's abusing of permissions (in my mind). 1) Can we implement it without capabilitites? Surely yes, too. By defining which roles are part of the learning experience (does the concept "participant" sounds to us and applies here?) and which ones are not. And then use that list to make the system to perform any action. Site-wide, course-wide, module-wide or whatever. But not using user permissions. So, said that, I'm going to reopen this, just to give it one turn of thinking. Note it's only my opinion, and I'll be happy with whatever is finally decided. But please, don't try to follow the shortest way only because there is already one patch ready, that would be an incorrect way to confront the problem. Let's define what a simple and pristine capability is (that said 6 years after roles and caps were introduced sounds like I joke, I know, but...) I'll do whatever it's decided, np. But I could not let this land before exposing my thoughts. Ciao
          Eloy Lafuente (stronk7) made changes -
          Status Waiting for integration review [ 10010 ] Reopened [ 4 ]
          Hide
          Rajesh Taneja added a comment -

          Aha, I never had this view about capabilities. Thanks Eloy.

          Initial patch was to add receive forum notifications capability, so if we plan to add forum:receivenotifications capability then it's a user capability and not system todo task. Am I right in saying so?

          Participant is a good option, but user can be enrolled in course as a co-teacher/manager who is not interested in getting forum notifications etc. Not sure, but will look further and see if this is possible.

          Show
          Rajesh Taneja added a comment - Aha, I never had this view about capabilities. Thanks Eloy. Initial patch was to add receive forum notifications capability, so if we plan to add forum:receivenotifications capability then it's a user capability and not system todo task. Am I right in saying so? Participant is a good option, but user can be enrolled in course as a co-teacher/manager who is not interested in getting forum notifications etc. Not sure, but will look further and see if this is possible.
          CiBoT made changes -
          Status Reopened [ 4 ] Reopened [ 4 ]
          Currently in integration Yes [ 10041 ]
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Tim Hunt added a comment -

          I will just note that there are already other places where we use permissions to control what Moodle does with/to users. For example mod/quiz:emailconfirmsubmission and mod/quiz:emailnotifysubmission. Those have been there for a long time. Those have been there for a long time, and they work well.

          Show
          Tim Hunt added a comment - I will just note that there are already other places where we use permissions to control what Moodle does with/to users. For example mod/quiz:emailconfirmsubmission and mod/quiz:emailnotifysubmission. Those have been there for a long time. Those have been there for a long time, and they work well.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Note I'm not negating they (can) work. I'm just raising the fact that they are not user permissions at all.

          Show
          Eloy Lafuente (stronk7) added a comment - Note I'm not negating they (can) work. I'm just raising the fact that they are not user permissions at all.
          Rajesh Taneja made changes -
          Status Reopened [ 4 ] Development in progress [ 3 ]
          Hide
          Martin Dougiamas added a comment -

          Eloy, good catch. I do agree theoretically with you but unless anyone can see any practical downsides of this I'd still be OK to go ahead with it as defined now, as a permission.

          The alternative implementation would be some sort of course setting with a multi-select list of "roles that should cause their user to be automatically subscribed to forums in this course whenever they are added" and that is also pretty messy (plugin dependency by core, no way for plugins to have course-wide settings).

          By comparison, the current solution is a lot neater.

          Show
          Martin Dougiamas added a comment - Eloy, good catch. I do agree theoretically with you but unless anyone can see any practical downsides of this I'd still be OK to go ahead with it as defined now, as a permission. The alternative implementation would be some sort of course setting with a multi-select list of "roles that should cause their user to be automatically subscribed to forums in this course whenever they are added" and that is also pretty messy (plugin dependency by core, no way for plugins to have course-wide settings). By comparison, the current solution is a lot neater.
          Hide
          Rajesh Taneja added a comment -

          Just had a word with Eloy and he thinks the capability name should be mod/forum:avoidinitialsuscriptions (user can/cannot avoid initial subscriptions in mod/forum).

          let me know your thoughts, so I can take it forward.

          Show
          Rajesh Taneja added a comment - Just had a word with Eloy and he thinks the capability name should be mod/forum:avoidinitialsuscriptions (user can/cannot avoid initial subscriptions in mod/forum). let me know your thoughts, so I can take it forward.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          (or mod/forum:avoidforcedsuscriptions, if forced ones are the affected)

          Anything, but in the form of a user permission: "User can/cannot xxxx on yyy"

          Also note that such change is not only a syntactical one but it also changes the accept/prevent meaning of the previous candidate).

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - (or mod/forum:avoidforcedsuscriptions, if forced ones are the affected) Anything, but in the form of a user permission: "User can/cannot xxxx on yyy" Also note that such change is not only a syntactical one but it also changes the accept/prevent meaning of the previous candidate). Ciao
          Hide
          John White added a comment -

          I agree with Eloy that it is truly anomalous - and always has been - that a capability should ever have been used to determine what the system does to you rather than what you can do in the system; both these capabilities turned the notion on its head! But nonetheless that's the way they have been used, and without one or both we are plunged back into trouble with 'observer-type' enrolments resulting in unwanted (and unwarranted) email traffic.

          I have just completed writing an 'Observer' enrolment plugin in 2.2+ which has features that are a hybrid of more common enrolments processes; in particular the ability to enrol in a course is conditional on hidden profile fields OR cohort membership, but the enrolment is not done for you. Its analogous to conditional self enrolment. But I have immediately hit against the forum capability issue where there is now no control over who is forced into forum subscription save that they are enrolled on the course in the first place. This is seriously retrograde so I have voted for this issue.

          Regards and thanks,
          John

          Show
          John White added a comment - I agree with Eloy that it is truly anomalous - and always has been - that a capability should ever have been used to determine what the system does to you rather than what you can do in the system; both these capabilities turned the notion on its head! But nonetheless that's the way they have been used, and without one or both we are plunged back into trouble with 'observer-type' enrolments resulting in unwanted (and unwarranted) email traffic. I have just completed writing an 'Observer' enrolment plugin in 2.2+ which has features that are a hybrid of more common enrolments processes; in particular the ability to enrol in a course is conditional on hidden profile fields OR cohort membership, but the enrolment is not done for you. Its analogous to conditional self enrolment. But I have immediately hit against the forum capability issue where there is now no control over who is forced into forum subscription save that they are enrolled on the course in the first place. This is seriously retrograde so I have voted for this issue. Regards and thanks, John
          Hide
          Martin Dougiamas added a comment -

          One of the rules we have managed to stick with in capabilities is to avoid negative capability names. Same with config settings. They need to be positive.

          Otherwise you get confusing situations where you turn a permission/setting ON and the behaviour turns off.

          Show
          Martin Dougiamas added a comment - One of the rules we have managed to stick with in capabilities is to avoid negative capability names. Same with config settings. They need to be positive. Otherwise you get confusing situations where you turn a permission/setting ON and the behaviour turns off.
          Hide
          Steve Bond added a comment -

          I'm not sure if this is what Raj & Eloy mean but if the permission were mod/forum:avoidforcedsuscriptions (or perhaps mod/forum:overrideforcedsuscriptions), then the capability, if allowed, would confer the ability to "unsubscribe" from forced-subscription forums.

          That would be a positive capability, and a genuine one, insofar as it affects what the user can do, rather than what the system can do. If implemented this way, it would mean that users who had this permission would see an "unsubscribe from this forum" link even for forced-subscription forums.

          That might be a lot more complicated to code, of course.

          Show
          Steve Bond added a comment - I'm not sure if this is what Raj & Eloy mean but if the permission were mod/forum:avoidforcedsuscriptions (or perhaps mod/forum:overrideforcedsuscriptions), then the capability, if allowed, would confer the ability to "unsubscribe" from forced-subscription forums. That would be a positive capability, and a genuine one, insofar as it affects what the user can do, rather than what the system can do. If implemented this way, it would mean that users who had this permission would see an "unsubscribe from this forum" link even for forced-subscription forums. That might be a lot more complicated to code, of course.
          Hide
          John White added a comment -

          So as to be clear:

          1. I was not advocating inverted capability sense, just agreeing that HISTORICALLY that's what these two capabilities represented.
          They always were an oddity - at least since 1.9.

          2. That whilst it was desirable to get rid of them, it was less desirable not to replace them with a positive alternative.
          I expect to be hit with howls of complaint from users with receiving unwanted email traffic from New forums when term reopens.

          3. So I need a fix for this rather quickly, but taking up Steve's suggestion of 'mod/forum:avoidforcedsubscriptions' I have the following mod to function forum_get_potential_subscribers()...

          mod/forum/lib.php
          // Some comments here
          function forum_get_potential_subscribers($forumcontext, $groupid, $fields, $sort) {
              global $DB;
          
              // only active enrolled users or everybody on the frontpage
              list($esql, $params) = get_enrolled_sql($forumcontext, '', $groupid, true);
          
              $sql = "SELECT $fields
                        FROM {user} u
                        JOIN ($esql) je ON je.id = u.id";
              if ($sort) {
                  $sql = "$sql ORDER BY $sort";
              } else {
                  $sql = "$sql ORDER BY u.lastname ASC, u.firstname ASC";
              }
              
              $users = $DB->get_records_sql($sql, $params);
              
              // find those to keep out of list
              list($esql, $params) = get_enrolled_sql($forumcontext, 'mod/forum:avoidforcedsubscriptions', $groupid, true);
          
              $sql = "SELECT $fields
                        FROM {user} u
                        JOIN ($esql) je ON je.id = u.id";
              
              $keepouts = $DB->get_records_sql($sql, $params);
          
              foreach ($keepouts as $key=>$keepout) {
              	unset($users[$key]);
              }
          
              return $users;
          }
          
          

          Its also necessary to put the new capability into the forum's access.php and kick that off with a minor upgrade.

          It works, so far as I have tested, although it does remove users even from the potential of being manually added to a subscription list whilst they have this capability set.

          The extra load of the second call is probably relatively small compared with the load of the first. Any view?

          John

          Show
          John White added a comment - So as to be clear: 1. I was not advocating inverted capability sense, just agreeing that HISTORICALLY that's what these two capabilities represented. They always were an oddity - at least since 1.9. 2. That whilst it was desirable to get rid of them, it was less desirable not to replace them with a positive alternative. I expect to be hit with howls of complaint from users with receiving unwanted email traffic from New forums when term reopens. 3. So I need a fix for this rather quickly, but taking up Steve's suggestion of 'mod/forum:avoidforcedsubscriptions' I have the following mod to function forum_get_potential_subscribers()... mod/forum/lib.php // Some comments here function forum_get_potential_subscribers($forumcontext, $groupid, $fields, $sort) { global $DB; // only active enrolled users or everybody on the frontpage list($esql, $params) = get_enrolled_sql($forumcontext, '', $groupid, true ); $sql = "SELECT $fields FROM {user} u JOIN ($esql) je ON je.id = u.id"; if ($sort) { $sql = "$sql ORDER BY $sort" ; } else { $sql = "$sql ORDER BY u.lastname ASC, u.firstname ASC" ; } $users = $DB->get_records_sql($sql, $params); // find those to keep out of list list($esql, $params) = get_enrolled_sql($forumcontext, 'mod/forum:avoidforcedsubscriptions', $groupid, true ); $sql = "SELECT $fields FROM {user} u JOIN ($esql) je ON je.id = u.id"; $keepouts = $DB->get_records_sql($sql, $params); foreach ($keepouts as $key=>$keepout) { unset($users[$key]); } return $users; } Its also necessary to put the new capability into the forum's access.php and kick that off with a minor upgrade. It works, so far as I have tested, although it does remove users even from the potential of being manually added to a subscription list whilst they have this capability set. The extra load of the second call is probably relatively small compared with the load of the first. Any view? John
          Rajesh Taneja made changes -
          Fix Version/s STABLE Sprint 24 Omega [ 12364 ]
          Fix Version/s STABLE Sprint 23 Omega [ 12362 ]
          Hide
          Rajesh Taneja added a comment -

          Pushing it back for integration review, to see if something needs to be done on this.

          Show
          Rajesh Taneja added a comment - Pushing it back for integration review, to see if something needs to be done on this.
          Rajesh Taneja made changes -
          Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Eloy Lafuente (stronk7) made changes -
          Currently in integration Yes [ 10041 ]
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Appart from suggesting to rename the capability to "mod/forum:theonethatexistsandshouldt"...

          I'm going to review and, if ok, integrate this now. I hope at least all this serves to double-think always we introduce a new cap in the future.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Appart from suggesting to rename the capability to "mod/forum:theonethatexistsandshouldt"... I'm going to review and, if ok, integrate this now. I hope at least all this serves to double-think always we introduce a new cap in the future. Ciao
          Eloy Lafuente (stronk7) made changes -
          Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
          Integrator stronk7
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (22, 23 & master), thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (22, 23 & master), thanks!
          Eloy Lafuente (stronk7) made changes -
          Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
          Affects Version/s 2.4 [ 12255 ]
          Fix Version/s 2.2.6 [ 12372 ]
          Fix Version/s 2.3.3 [ 12373 ]
          Tim Barker made changes -
          Tester andyjdavis
          Andrew Davis made changes -
          Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
          Hide
          Andrew Davis added a comment - - edited

          Im not sure if this is related but Im getting an error when I try to enrol users at step 3 of test 1.

          Can not find data record in database table course_modules.
          URL: /
          Debug info: SELECT id,course FROM {course_modules} WHERE id = ? [array ( 0 => '6', )] Error code: invalidrecord
          Stack trace:
          
          * line 1335 of /lib/dml/moodle_database.php: dml_missing_record_exception thrown
          * line 1311 of /lib/dml/moodle_database.php: call to moodle_database->get_record_select()
          * line 6783 of /lib/accesslib.php: call to moodle_database->get_record()
          * line 6187 of /mod/forum/lib.php: call to context_module::instance()
          * line ? of unknownfile: call to forum_user_role_assigned()
          * line 299 of /lib/eventslib.php: call to call_user_func()
          * line 519 of /lib/eventslib.php: call to events_dispatch()
          * line 1664 of /lib/accesslib.php: call to events_trigger()
          * line 1263 of /lib/enrollib.php: call to role_assign()
          * line 124 of /enrol/manual/ajax.php: call to enrol_plugin->enrol_user()
          

          If I refresh the enrolment screen the users have been enrolled. However they have not been subscribed to the forum ie step 4 fails.

          Show
          Andrew Davis added a comment - - edited Im not sure if this is related but Im getting an error when I try to enrol users at step 3 of test 1. Can not find data record in database table course_modules. URL: / Debug info: SELECT id,course FROM {course_modules} WHERE id = ? [array ( 0 => '6', )] Error code: invalidrecord Stack trace: * line 1335 of /lib/dml/moodle_database.php: dml_missing_record_exception thrown * line 1311 of /lib/dml/moodle_database.php: call to moodle_database->get_record_select() * line 6783 of /lib/accesslib.php: call to moodle_database->get_record() * line 6187 of /mod/forum/lib.php: call to context_module::instance() * line ? of unknownfile: call to forum_user_role_assigned() * line 299 of /lib/eventslib.php: call to call_user_func() * line 519 of /lib/eventslib.php: call to events_dispatch() * line 1664 of /lib/accesslib.php: call to events_trigger() * line 1263 of /lib/enrollib.php: call to role_assign() * line 124 of /enrol/manual/ajax.php: call to enrol_plugin->enrol_user() If I refresh the enrolment screen the users have been enrolled. However they have not been subscribed to the forum ie step 4 fails.
          Andrew Davis made changes -
          Status Testing in progress [ 10011 ] Problem during testing [ 10007 ]
          Andrew Davis made changes -
          Link This issue testing discovered MDL-35517 [ MDL-35517 ]
          Hide
          Andrew Davis added a comment -

          Ive also linked MDL-35517 which I discovered while testing this.

          Show
          Andrew Davis added a comment - Ive also linked MDL-35517 which I discovered while testing this.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          ops, yup, must not be activity->id but cm->id...

          Show
          Eloy Lafuente (stronk7) added a comment - ops, yup, must not be activity->id but cm->id...
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Added this commit:

          http://git.moodle.org/gw?p=integration.git;a=commitdiff;h=b09764da0739b2e02ce4e12a9a26fca81d14105d

          to the 3 branches to fix the 2 incorrect uses of forum->id in capability checks.

          Sending this back to testing... ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Added this commit: http://git.moodle.org/gw?p=integration.git;a=commitdiff;h=b09764da0739b2e02ce4e12a9a26fca81d14105d to the 3 branches to fix the 2 incorrect uses of forum->id in capability checks. Sending this back to testing... ciao
          Eloy Lafuente (stronk7) made changes -
          Status Problem during testing [ 10007 ] Waiting for testing [ 10005 ]
          Tim Barker made changes -
          Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
          Hide
          Tim Barker added a comment -

          Tested very thoroughly and passed at every stage. Good work!

          Show
          Tim Barker added a comment - Tested very thoroughly and passed at every stage. Good work!
          Tim Barker made changes -
          Status Testing in progress [ 10011 ] Tested [ 10006 ]
          Hide
          Dan Poltawski added a comment -

          Congratulations, you've done it!

          Thanks, this change is now in the latest weekly release!

          Join the crowds of people tomorrow from 8am and download this Moodle release from your local apple store!

          Show
          Dan Poltawski added a comment - Congratulations, you've done it! Thanks, this change is now in the latest weekly release! Join the crowds of people tomorrow from 8am and download this Moodle release from your local apple store!
          Dan Poltawski made changes -
          Status Tested [ 10006 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          Currently in integration Yes [ 10041 ]
          Integration date 20/Sep/12
          Michael de Raadt made changes -
          Link This issue caused a regression MDL-35607 [ MDL-35607 ]
          Michael de Raadt made changes -
          Labels docs_required lost_functionality patch triaged docs_required lost_functionality patch triaged ui_change
          Mary Cooch made changes -
          Labels docs_required lost_functionality patch triaged ui_change lost_functionality patch triaged ui_change
          Hide
          Mary Cooch added a comment -

          Removing docs_required label as this is documented here http://docs.moodle.org/24/en/Capabilities/mod/forum:allowforcesubscribe and also in 2.3

          Show
          Mary Cooch added a comment - Removing docs_required label as this is documented here http://docs.moodle.org/24/en/Capabilities/mod/forum:allowforcesubscribe and also in 2.3
          Rex Lorenzo made changes -
          Link This issue testing discovered MDL-37118 [ MDL-37118 ]
          Rex Lorenzo made changes -
          Link This issue caused a regression MDL-37633 [ MDL-37633 ]
          Eloy Lafuente (stronk7) made changes -
          Fix Version/s STABLE Sprint 24 Omega [ 12364 ]

            People

            • Votes:
              4 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: