Moodle
  1. Moodle
  2. MDL-19263

Site-level users with grading capability receive Assignment emails

    Details

    • Testing Instructions:
      Hide

      1. Create User A with a valid email address and assign the Manager system role.
      2. Create User B with a valid email address. Do not assign a system role.
      3. Create User C. Do not assign a system role.
      4. Create a course and enrol User B as a teacher and User C as a student.
      3. Create an assignment. Make sure "Email alerts to teachers" is set to Yes.
      4. As User C, submit an assignment.
      5. Verify that User A does not receive an email notification.
      6. Verify that User B receives an email notification.

      Show
      1. Create User A with a valid email address and assign the Manager system role. 2. Create User B with a valid email address. Do not assign a system role. 3. Create User C. Do not assign a system role. 4. Create a course and enrol User B as a teacher and User C as a student. 3. Create an assignment. Make sure "Email alerts to teachers" is set to Yes. 4. As User C, submit an assignment. 5. Verify that User A does not receive an email notification. 6. Verify that User B receives an email notification.
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-19263-master

      Description

      We have a custom role for our User Support Center that is similar to administrator, but lacks most of the system-wide privileges. These users need to be able to behave like teachers in any course without actually being enrolled in courses.

      However, these users are considered members of the course for certain instances (choices, groups/groupings, participants lists, locally assigned roles for activities, and the recent activity block). The User Support Center role is a hidden assignment to minimize these appearances.

      Whether or not the role assignment is hidden, when a teacher enables "Send e-mail notifications to teachers" in an assignment, anyone with the User Support Center role (and any other site-level course:view role) receives notification e-mails.

      The only thing remotely like a fix that we've found is to disable the e-mail address of anyone with a User Support Center role, but we would like to enroll these users in a course and have them receive e-mails.

      These site-level users do not receive other types of email from within the course (e.g. Forum posts).

      EDITED: We did further testing and discovered that this issue is not limited to hidden role assignments after all. It is true for all site-level users with the capability to grade assignments.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Caroline Moore added a comment -

            Here is a forum discussion about this: http://moodle.org/mod/forum/discuss.php?d=123917

            Show
            Caroline Moore added a comment - Here is a forum discussion about this: http://moodle.org/mod/forum/discuss.php?d=123917
            Hide
            Yaju Mahida added a comment -

            Hi we have the same role named General Support at site level as defined in the above description.

            In our case General Support role receives Feedback emails and they have capability for the followings

            mod/feedback:viewreports
            mod/feedback:view

            Moodle 1.9.5

            Show
            Yaju Mahida added a comment - Hi we have the same role named General Support at site level as defined in the above description. In our case General Support role receives Feedback emails and they have capability for the followings mod/feedback:viewreports mod/feedback:view Moodle 1.9.5
            Hide
            Anthony Borrow added a comment -

            Mike - I hope you don't mind that I added you as a watcher to this issue. Feel free to un-watch if you do not have time. Based on your comment in the forum (http://moodle.org/mod/forum/discuss.php?d=123917#p543152) I figured you might be able to come up with an updated patch file to attach to this issue. If not, just let me know and I'll try to do so.

            I do agree that just getting the folks that have the mod/assignment:grade capability is a little too broad as it includes site admins. Perhaps we could add an option to email all graders (including admin) or only course graders. For those looking for an immediate work around, site admins could disable their email but then they would not receive any emails from Moodle. Alternatively, the teacher could create a group with all of their students and themselves in it and since the site admins are not part of that group and if the assignment is setup for separate groups that should also exclude the admins.

            Peace - Anthony

            Show
            Anthony Borrow added a comment - Mike - I hope you don't mind that I added you as a watcher to this issue. Feel free to un-watch if you do not have time. Based on your comment in the forum ( http://moodle.org/mod/forum/discuss.php?d=123917#p543152 ) I figured you might be able to come up with an updated patch file to attach to this issue. If not, just let me know and I'll try to do so. I do agree that just getting the folks that have the mod/assignment:grade capability is a little too broad as it includes site admins. Perhaps we could add an option to email all graders (including admin) or only course graders. For those looking for an immediate work around, site admins could disable their email but then they would not receive any emails from Moodle. Alternatively, the teacher could create a group with all of their students and themselves in it and since the site admins are not part of that group and if the assignment is setup for separate groups that should also exclude the admins. Peace - Anthony
            Hide
            Anthony Borrow added a comment -

            Would anyone watching this issue be able to check if this is an issue in Moodle 2.0? Peace - Anthony

            Show
            Anthony Borrow added a comment - Would anyone watching this issue be able to check if this is an issue in Moodle 2.0? Peace - Anthony
            Hide
            Craig Drayton added a comment -

            Hi there,

            This has just come up for us when trying to create an "Academic leader" role to allow academic management to have the same rights as tutors, throughout a faculty, without receiving all the email notifications.

            It seems to me that it would work well if "get email notifications of assignment submission" was a separate permission to mod/assignment:grade.

            This would be consistent with Quizzes, where there is a seperate "Get email notification of submissions" permission and a "Grade quizzes manually" permission.

            Cheers,
            Craig

            Show
            Craig Drayton added a comment - Hi there, This has just come up for us when trying to create an "Academic leader" role to allow academic management to have the same rights as tutors, throughout a faculty, without receiving all the email notifications. It seems to me that it would work well if "get email notifications of assignment submission" was a separate permission to mod/assignment:grade. This would be consistent with Quizzes, where there is a seperate "Get email notification of submissions" permission and a "Grade quizzes manually" permission. Cheers, Craig
            Hide
            Matthew Cannings added a comment - - edited

            Following Craigs comment I decided to have a go at adding an extra role which was relatively straightforward to do with changes to just one line and additions to two others. This was done on Moodle 2.0+
            This is not recommended for production servers and is for information purposes only.

            In moodle/mod/assignment/db/access.php are the list of capabilities that assignment module supports. In here I added

            'mod/assignment:receivemail' => array(

            'captype' => 'read',
            'contextlevel' => CONTEXT_MODULE,
            'archetypes' => array(
            'teacher' => CAP_ALLOW,
            'editingteacher' => CAP_ALLOW
            )
            ),

            Which creates a new capability called receivemail for the assignment module for teacher and editingteacher roles.

            Then in moodle/mod/assignment/lib.php within the function get_graders I changed the capability that is required to get sent an email.

            To do this change
            $potgraders = get_users_by_capability($this->context, 'mod/assignment:grade', '', '', '', '', '', '', false, false);

            to

            $potgraders = get_users_by_capability($this->context, 'mod/assignment:receivemail', '', '', '', '', '', '', false, false);

            That is all that is required. The other addition that needs to be made is to the language file to add strings for the new capability.

            In moodle/mod/assignment/lang/en/assignment.php add the line
            $string['assignment:receivemail'] = 'Receive Submission Notification';

            Show
            Matthew Cannings added a comment - - edited Following Craigs comment I decided to have a go at adding an extra role which was relatively straightforward to do with changes to just one line and additions to two others. This was done on Moodle 2.0+ This is not recommended for production servers and is for information purposes only. In moodle/mod/assignment/db/access.php are the list of capabilities that assignment module supports. In here I added 'mod/assignment:receivemail' => array( 'captype' => 'read', 'contextlevel' => CONTEXT_MODULE, 'archetypes' => array( 'teacher' => CAP_ALLOW, 'editingteacher' => CAP_ALLOW ) ), Which creates a new capability called receivemail for the assignment module for teacher and editingteacher roles. Then in moodle/mod/assignment/lib.php within the function get_graders I changed the capability that is required to get sent an email. To do this change $potgraders = get_users_by_capability($this->context, 'mod/assignment:grade', '', '', '', '', '', '', false, false); to $potgraders = get_users_by_capability($this->context, 'mod/assignment:receivemail', '', '', '', '', '', '', false, false); That is all that is required. The other addition that needs to be made is to the language file to add strings for the new capability. In moodle/mod/assignment/lang/en/assignment.php add the line $string ['assignment:receivemail'] = 'Receive Submission Notification';
            Hide
            Matthew Cannings added a comment -

            Could someone change the list of versions that this affects as it should also include 2.0+ which is what I am using and also appears to still affect the latest 2.2+

            Show
            Matthew Cannings added a comment - Could someone change the list of versions that this affects as it should also include 2.0+ which is what I am using and also appears to still affect the latest 2.2+
            Hide
            Jason Bennett added a comment -

            I am experiencing the same thing on Moodle 2.2. I've tested this thoroughly and it is clearly the mod/assignment:grade permission which is causing site level roles to be sent email notifications of submissions.

            Show
            Jason Bennett added a comment - I am experiencing the same thing on Moodle 2.2. I've tested this thoroughly and it is clearly the mod/assignment:grade permission which is causing site level roles to be sent email notifications of submissions.
            Hide
            Charles Fulton added a comment -

            I've written up Matthew Cannings' patch (with some tweaks) against 2.3 master and bumped the affected versions. Near as I can tell get_graders() is used for this purpose only, so it might be a good idea to rename the function completely. For now I altered the inline docs to make it clear what it's doing and why.

            Show
            Charles Fulton added a comment - I've written up Matthew Cannings' patch (with some tweaks) against 2.3 master and bumped the affected versions. Near as I can tell get_graders() is used for this purpose only, so it might be a good idea to rename the function completely. For now I altered the inline docs to make it clear what it's doing and why.
            Hide
            Dan Poltawski added a comment -

            Hi Charles,

            Thanks a lot for your work. The patch looks great.

            There are two things about this issue though:

            1) We don't tend to add new capabilities on stable branches without special approval as it isrupts the stable nature of the branch and can change peoples setup (in this case it could mean custom grader roles are broken with assignment notificates, for example).

            So getting this into the stable branch is going a debate for discussion. Getting it into master is also going to be problematic as to complicate matters a new assignment module is being developed for 2.3 making this perhaps not relevant for master..

            But..

            2) I think in this particular case of the described issue, one way to solve it which might in fact be more correct could is to use the function 'get_enrolled_sql()'. This isn't very well documented i'm afraid but it is new in 2.x allowing you to retrieve only users enrolled on a course with a capability. This would mean that only users enrolled on the course with the grade capability would get the email. See forum_get_potential_subscribers() for one use of this.

            Show
            Dan Poltawski added a comment - Hi Charles, Thanks a lot for your work. The patch looks great. There are two things about this issue though: 1) We don't tend to add new capabilities on stable branches without special approval as it isrupts the stable nature of the branch and can change peoples setup (in this case it could mean custom grader roles are broken with assignment notificates, for example). So getting this into the stable branch is going a debate for discussion. Getting it into master is also going to be problematic as to complicate matters a new assignment module is being developed for 2.3 making this perhaps not relevant for master.. But.. 2) I think in this particular case of the described issue, one way to solve it which might in fact be more correct could is to use the function 'get_enrolled_sql()'. This isn't very well documented i'm afraid but it is new in 2.x allowing you to retrieve only users enrolled on a course with a capability. This would mean that only users enrolled on the course with the grade capability would get the email. See forum_get_potential_subscribers() for one use of this.
            Hide
            Michael de Raadt added a comment -

            Hi, Charles.

            I've just triaged this issue, but please don't stop working on it.

            Show
            Michael de Raadt added a comment - Hi, Charles. I've just triaged this issue, but please don't stop working on it.
            Hide
            Charles Fulton added a comment -

            Sorry, I'm only now getting back to this. get_enrolled_sql makes perfect sense and (I think) preferable to a new capability. Patch rewritten.

            Show
            Charles Fulton added a comment - Sorry, I'm only now getting back to this. get_enrolled_sql makes perfect sense and (I think) preferable to a new capability. Patch rewritten.
            Hide
            Dan Poltawski added a comment -

            This makes sense to me.

            If you create branches for 21_STABLE, 22_STABLE and also fill out more extensive testing instructions I will push this for integration.

            Show
            Dan Poltawski added a comment - This makes sense to me. If you create branches for 21_STABLE, 22_STABLE and also fill out more extensive testing instructions I will push this for integration.
            Hide
            Charles Fulton added a comment -

            Rebased against current master and additional branches created. Sorry for the delay.

            Show
            Charles Fulton added a comment - Rebased against current master and additional branches created. Sorry for the delay.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Integrated (21, 22 & master), thanks!

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

            Works as described in master

            Show
            Andrew Davis added a comment - Works as described in master
            Hide
            Andrew Davis added a comment -

            Also works as described in 2.2 and 2.1 stable. Passing.

            Show
            Andrew Davis added a comment - Also works as described in 2.2 and 2.1 stable. Passing.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            This has been near becoming rejected, because it's not the best code you are able to produce.

            But, luckily, at the end, it has landed and has been spread to all repos out there.

            Many thanks and, don't forget it, keep improving your skills, you can!

            Closing, ciao

            Show
            Eloy Lafuente (stronk7) added a comment - This has been near becoming rejected, because it's not the best code you are able to produce. But, luckily, at the end, it has landed and has been spread to all repos out there. Many thanks and, don't forget it, keep improving your skills, you can! Closing, ciao

              People

              • Votes:
                18 Vote for this issue
                Watchers:
                17 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: