Moodle
  1. Moodle
  2. MDL-29534

Use email address in reply can not be deactivated

    Details

    • Database:
      MySQL
    • Testing Instructions:
      Hide

      Pre-requisite

      1. Course with atleast one forum activity with active discussion and two students.
      2. Enroled Students should have "Email digest type" = "No digest" in there profile.
      3. unset forum_replytouser (Site administration ► Plugins ► Activity modules ► Forum)
      4. In site policies reduce max editing time to 1 min. (Home ► Site administration ► Security ► Site policies)

      Test 1

      1. Log in as student and post something to discussion.
      2. Wait for 1 min and run cron and check email or both students. It should come from noreply@ {YourMOODLEInstall}

        with the name of student (who created post)

      Test 2

      1. Log in as admin and set forum_replytouser (Site administration ► Plugins ► Activity modules ► Forum)
      2. Log in as student and post something to discussion.
      3. Run cron and check email or both students. It should come from student email (who created post)

      Test 3

      1. Set "Hour to send digest emails" so that digest can be mailed in next cron (Site administration ► Plugins ► Activity modules ► Forum)
      2. Set "Email digest type" = "complete" in enroled user profile.
      3. Post in forum with different student login.
      4. Wait for hour (as set in Hour to send digest emails) and run cron and make sure you get the digest email, with noreply email and site short name : forum digest.
      Show
      Pre-requisite Course with atleast one forum activity with active discussion and two students. Enroled Students should have "Email digest type" = "No digest" in there profile. unset forum_replytouser (Site administration ► Plugins ► Activity modules ► Forum) In site policies reduce max editing time to 1 min. (Home ► Site administration ► Security ► Site policies) Test 1 Log in as student and post something to discussion. Wait for 1 min and run cron and check email or both students. It should come from noreply@ {YourMOODLEInstall} with the name of student (who created post) Test 2 Log in as admin and set forum_replytouser (Site administration ► Plugins ► Activity modules ► Forum) Log in as student and post something to discussion. Run cron and check email or both students. It should come from student email (who created post) Test 3 Set "Hour to send digest emails" so that digest can be mailed in next cron (Site administration ► Plugins ► Activity modules ► Forum) Set "Email digest type" = "complete" in enroled user profile. Post in forum with different student login. Wait for hour (as set in Hour to send digest emails) and run cron and make sure you get the digest email, with noreply email and site short name : forum digest.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      wip-mdl-29534-m24
    • Pull Master Branch:
      wip-mdl-29534
    • Rank:
      19012

      Description

      Deactivation of 'Use email address in reply' in the forum module settings (Site administration ► Plugins ► Activity modules ► Forum -> forum_replytouser) does not work, Moodle continues to send out forum mails with the users email address as sender. Setting users emaildisplay to 'Hide my email address from everyone' seems to be the only way to avoid this

      1. patch_MDL-29534.txt
        0.6 kB
        Isaac Marco Blancas
      2. patch01_MDL-29534.txt
        0.8 kB
        Isaac Marco Blancas

        Activity

        Hide
        Bente Olsen added a comment -

        I have changed priority to 'Major' while the 'workaround' can't be used without changing peoples individual profile settings.

        Show
        Bente Olsen added a comment - I have changed priority to 'Major' while the 'workaround' can't be used without changing peoples individual profile settings.
        Hide
        Adam Nave added a comment -

        Also affects version 2.2.3.

        Show
        Adam Nave added a comment - Also affects version 2.2.3.
        Hide
        Tomasz Muras added a comment -

        This is a problem with forum_replytouser not used correctly in the forum module.
        From mod/forum/lib.php:
        $mailresult = email_to_user($userto, $site->shortname, $postsubject, $posttext, $posthtml, $attachment, $attachname, $usetrueaddress, $CFG->forum_replytouser);

        The last argument here: $CFG->forum_replytouser (9th argument for email_to_user) will be either 0 or 1, depending on the settings as per description of this bug. email_to_user function however, expects $replyto email address here.

        This means that "reply to" will never be set correctly and that "Use email address in reply" setting is not functional. A real user's email will always be used in "from" field unless user blocks it in his profile. I've confirmed it's still the case in latest master 2.4dev (Build: 20120706).

        Show
        Tomasz Muras added a comment - This is a problem with forum_replytouser not used correctly in the forum module. From mod/forum/lib.php: $mailresult = email_to_user($userto, $site->shortname, $postsubject, $posttext, $posthtml, $attachment, $attachname, $usetrueaddress, $CFG->forum_replytouser); The last argument here: $CFG->forum_replytouser (9th argument for email_to_user) will be either 0 or 1, depending on the settings as per description of this bug. email_to_user function however, expects $replyto email address here. This means that "reply to" will never be set correctly and that "Use email address in reply" setting is not functional. A real user's email will always be used in "from" field unless user blocks it in his profile. I've confirmed it's still the case in latest master 2.4dev (Build: 20120706).
        Hide
        Isaac Marco Blancas added a comment -

        Some body should review this patch I wrote.

        I think is important to force the students to participate on forums and if we put the mail of our teachers on the mail notification they used to reply directly to the teacher.

        The patch is tested on Moodle 2.2.4.

        Show
        Isaac Marco Blancas added a comment - Some body should review this patch I wrote. I think is important to force the students to participate on forums and if we put the mail of our teachers on the mail notification they used to reply directly to the teacher. The patch is tested on Moodle 2.2.4.
        Hide
        David Paniagua Ruano added a comment -

        The problem is solved by Isaac's Patch (add lines in mod/forum/lib.php).

        I tested on moodle 2.2.5 and 2.2.5+ (last week revision).

        David

        Show
        David Paniagua Ruano added a comment - The problem is solved by Isaac's Patch (add lines in mod/forum/lib.php). I tested on moodle 2.2.5 and 2.2.5+ (last week revision). David
        Hide
        Isaac Marco Blancas added a comment -

        My first patch had some problems. The mail was not send to the user who send the post.

        I have rewritted a new version patch01_MDL-29534.txt

        patch01_MDL-29534.txt
        
        --- mod/forum/lib.php_original	2012-09-27 19:20:34.000000000 +0200
        +++ mod/forum/lib.php	2012-10-11 16:39:29.000000000 +0200
        @@ -628,6 +628,13 @@
                         $eventdata->component        = 'mod_forum';
                         $eventdata->name             = 'posts';
                         $eventdata->userfrom         = $userfrom;
        +		
        +		//Patch for MDL-29534. If forum_replytouser param in forum module is unchecked send mail using the noreplyaddress
        +		//overriding the maildisplay user settig to 0 = "Hide my email address from everyone"
        +	        //by: marcoblancas 11/10/2012
        +	        //tested on: 2.2.4
        +	        if (!$CFG->forum_replytouser) $userto->maildisplay=0;
        +
                         $eventdata->userto           = $userto;
                         $eventdata->subject          = $postsubject;
                         $eventdata->fullmessage      = $posttext;
        
        Show
        Isaac Marco Blancas added a comment - My first patch had some problems. The mail was not send to the user who send the post. I have rewritted a new version patch01_ MDL-29534 .txt patch01_ MDL-29534 .txt --- mod/forum/lib.php_original 2012-09-27 19:20:34.000000000 +0200 +++ mod/forum/lib.php 2012-10-11 16:39:29.000000000 +0200 @@ -628,6 +628,13 @@ $eventdata->component = 'mod_forum'; $eventdata->name = 'posts'; $eventdata->userfrom = $userfrom; + + //Patch for MDL-29534. If forum_replytouser param in forum module is unchecked send mail using the noreplyaddress + //overriding the maildisplay user settig to 0 = "Hide my email address from everyone" + //by: marcoblancas 11/10/2012 + //tested on: 2.2.4 + if (!$CFG->forum_replytouser) $userto->maildisplay=0; + $eventdata->userto = $userto; $eventdata->subject = $postsubject; $eventdata->fullmessage = $posttext;
        Hide
        Isaac Marco Blancas added a comment -

        A new patch revision... the last one was on error too. Sorry.

        Now is tested on 2.2.5 with and without digest mode.

        
        diff --git a/mod/forum/lib.php b/mod/forum/lib.php
        index 8cbe330..35d7a3f 100644
        --- a/mod/forum/lib.php
        +++ b/mod/forum/lib.php
        @@ -628,6 +628,11 @@ function forum_cron() {
                         $eventdata->component        = 'mod_forum';
                         $eventdata->name             = 'posts';
                         $eventdata->userfrom         = $userfrom;
        +
        +               //Patch for MDL-29534. If forum_replytouser param in forum module is unchecked send mail using the noreplyaddress
        +               //overriding the maildisplay user settig to 0 = "Hide my email address from everyone" over userfrom
        +               if (!$CFG->forum_replytouser) $eventdata->userfrom->maildisplay='0';
        +
                         $eventdata->userto           = $userto;
                         $eventdata->subject          = $postsubject;
                         $eventdata->fullmessage      = $posttext;
        
        Show
        Isaac Marco Blancas added a comment - A new patch revision... the last one was on error too. Sorry. Now is tested on 2.2.5 with and without digest mode. diff --git a/mod/forum/lib.php b/mod/forum/lib.php index 8cbe330..35d7a3f 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -628,6 +628,11 @@ function forum_cron() { $eventdata->component = 'mod_forum'; $eventdata->name = 'posts'; $eventdata->userfrom = $userfrom; + + //Patch for MDL-29534. If forum_replytouser param in forum module is unchecked send mail using the noreplyaddress + //overriding the maildisplay user settig to 0 = "Hide my email address from everyone" over userfrom + if (!$CFG->forum_replytouser) $eventdata->userfrom->maildisplay='0'; + $eventdata->userto = $userto; $eventdata->subject = $postsubject; $eventdata->fullmessage = $posttext;
        Hide
        Henriette Stewart added a comment -

        Chipping in to say Isaac's new patch from 24/10/12 fixes the bug for me too, and we're using Moodle 2.2.1+ (Build: 20120202). Thank you Isaac!

        Show
        Henriette Stewart added a comment - Chipping in to say Isaac's new patch from 24/10/12 fixes the bug for me too, and we're using Moodle 2.2.1+ (Build: 20120202). Thank you Isaac!
        Hide
        Rajesh Taneja added a comment -

        Thanks for working on this,

        I have put this in next sprint, to have a look.

        Show
        Rajesh Taneja added a comment - Thanks for working on this, I have put this in next sprint, to have a look.
        Hide
        Rajesh Taneja added a comment -

        Thanks for the patch Isaac,

        IMO maildisplay = 0, we should set email to noreplyaddress. I have created branch with your patch and then added another commit to change $userfrom->email = $CFG-->noreplyaddress.

        Pushing it for peer-review.

        Show
        Rajesh Taneja added a comment - Thanks for the patch Isaac, IMO maildisplay = 0, we should set email to noreplyaddress. I have created branch with your patch and then added another commit to change $userfrom->email = $CFG-->noreplyaddress. Pushing it for peer-review.
        Hide
        Damyon Wiese added a comment -

        Hi Raj,

        The $usetrueaddress variable is never used by the email_to_user function (because the from address is a string). I think it would be better to remove it entirely.

        Everything else looks great.

        Thanks, Damyon

        Show
        Damyon Wiese added a comment - Hi Raj, The $usetrueaddress variable is never used by the email_to_user function (because the from address is a string). I think it would be better to remove it entirely. Everything else looks great. Thanks, Damyon
        Hide
        Damyon Wiese added a comment -

        Checklist:

        [Y] Syntax
        [-] Output
        [Y] Whitespace
        [-] Language
        [-] Databases
        [Y] Testing
        [-] Security
        [-] Documentation
        [Y] Git
        [Y] Sanity check - (Once $usetrueaddress is removed).

        Thanks Raj - feel free to submit this for integration once that variable is removed.

        Show
        Damyon Wiese added a comment - Checklist: [Y] Syntax [-] Output [Y] Whitespace [-] Language [-] Databases [Y] Testing [-] Security [-] Documentation [Y] Git [Y] Sanity check - (Once $usetrueaddress is removed). Thanks Raj - feel free to submit this for integration once that variable is removed.
        Hide
        Rajesh Taneja added a comment -

        Thanks Damyon,

        It make sense to remove usetrueaddress. I have removed it and now pushing it for integration.

        Show
        Rajesh Taneja added a comment - Thanks Damyon, It make sense to remove usetrueaddress. I have removed it and now pushing it for integration.
        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
        Hide
        Dan Poltawski added a comment -

        Integrated, thanks Raj.

        Show
        Dan Poltawski added a comment - Integrated, thanks Raj.
        Hide
        Mark Nelson added a comment -

        Raj, not sure how a student is supposed to select 'mailnow' since the capability 'moodle/course:manageactivities' is required for that option to be available. Can you clarify in the testing instructions before I continue and end up testing it incorrectly. I also realised that this capability is only checked when displaying the 'mailnow' option, but not when the form is submitted, so someone could easily just create their own form with this option present and it would be set.

        Show
        Mark Nelson added a comment - Raj, not sure how a student is supposed to select 'mailnow' since the capability 'moodle/course:manageactivities' is required for that option to be available. Can you clarify in the testing instructions before I continue and end up testing it incorrectly. I also realised that this capability is only checked when displaying the 'mailnow' option, but not when the form is submitted, so someone could easily just create their own form with this option present and it would be set.
        Hide
        Mark Nelson added a comment -

        Actually, the last point may not be an issue as the form object is recreated when the data is posted, so this element will not be present in this object regardless if it was in the data they submitted. So the check !empty($fromform->mailnow) should be sufficient.

        Show
        Mark Nelson added a comment - Actually, the last point may not be an issue as the form object is recreated when the data is posted, so this element will not be present in this object regardless if it was in the data they submitted. So the check !empty($fromform->mailnow) should be sufficient.
        Hide
        Rajesh Taneja added a comment - - edited

        Sorry Mark, my bad. Student don't see that checkbox.
        In site policies set Maximum time to edit posts (maxeditingtime) to 1 min. (Home ► Site administration ► Security ► Site policies) and run cron after 1 min.

        Show
        Rajesh Taneja added a comment - - edited Sorry Mark, my bad. Student don't see that checkbox. In site policies set Maximum time to edit posts (maxeditingtime) to 1 min. (Home ► Site administration ► Security ► Site policies) and run cron after 1 min.
        Hide
        Mark Nelson added a comment -

        I discovered an issue where the email was not being sent to one of the users as it was attempting to send to noreply@localhost instead of their email. The patch for this issue only changed the userto field so I didn't think it was related, however, after much debugging with Raj it was discovered that it was a reference issue. Raj is working on a patch to fix this.

        Show
        Mark Nelson added a comment - I discovered an issue where the email was not being sent to one of the users as it was attempting to send to noreply@localhost instead of their email. The patch for this issue only changed the userto field so I didn't think it was related, however, after much debugging with Raj it was discovered that it was a reference issue. Raj is working on a patch to fix this.
        Hide
        Rajesh Taneja added a comment -

        Grrrr references.
        Thanks again for spotting this Mark.

        I have added another commit, can you please pick this Dan.

        Show
        Rajesh Taneja added a comment - Grrrr references. Thanks again for spotting this Mark. I have added another commit, can you please pick this Dan.
        Hide
        Dan Poltawski added a comment -

        Pulled that in, thanks Raj.

        Show
        Dan Poltawski added a comment - Pulled that in, thanks Raj.
        Hide
        Mark Nelson added a comment -

        Works now, passing! Woohoo.

        Show
        Mark Nelson added a comment - Works now, passing! Woohoo.
        Hide
        Dan Poltawski added a comment -

        Hurray! We did it! Thanks to all the reporters, testers, user and watchers for a bumper week of Moodling!

        Show
        Dan Poltawski added a comment - Hurray! We did it! Thanks to all the reporters, testers, user and watchers for a bumper week of Moodling!

          People

          • Votes:
            14 Vote for this issue
            Watchers:
            16 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: