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

Controlling Different eMail addresses in Moodle

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.13, 2.1.1, 2.3
    • Fix Version/s: 2.3
    • Component/s: Messages
    • Database:
      MySQL
    • Testing Instructions:
      Hide

      This issue introduces a new system setting. As such it requires a version increase to get that setting picked up. The version I've got in version.php may not be the version the integrators ultimately use. It just needs to be a version above whatever you currently have in your version.php to get an upgrade to run.

      After the upgrade has run to go My profile settings > Messaging and check that the email address box is available. Its towards the bottom of the page.

      As an admin search for the new setting in the Settings block. It is messagingallowemailoverride. Untick it and save.

      Go back to your messaging settings and check that the email address area is gone.

      Retick the settings and save. Reload your messaging settings and check that it has returned.

      Show
      This issue introduces a new system setting. As such it requires a version increase to get that setting picked up. The version I've got in version.php may not be the version the integrators ultimately use. It just needs to be a version above whatever you currently have in your version.php to get an upgrade to run. After the upgrade has run to go My profile settings > Messaging and check that the email address box is available. Its towards the bottom of the page. As an admin search for the new setting in the Settings block. It is messagingallowemailoverride. Untick it and save. Go back to your messaging settings and check that the email address area is gone. Retick the settings and save. Reload your messaging settings and check that it has returned.
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_21_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      MDL-29615_control_override_email2

      Description

      We had a lot of confusion due to the fact that moodle sends forumposts and messages to different eMail addresses.
      Reason is the possibility to define eMail address in two different places

      • user profile
      • settings of messages

      I don't understand the reason for this so I assume this is a bug

      It would be very nice if:
      1) there would be only one place to define the EMail address
      or
      2) there would be a flag in the rights management where could be specified if these addresses should be synchronized or not.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            andyjdavis Andrew Davis added a comment -

            There are indeed currently two places you can specify your email address. The primary one is your profile. You can optionally add a second email address on the messaging settings screen. By default this is empty and your primary email address is used. The ability to override it in your messaging settings exists for people who have multiple email addresses and want their messages delivered to one of their non-primary email addresses.

            Currently the text around the email box in the messaging settings says the following
            Send email notifications to: [ the text box ](Default: person@example.com)

            I could change this to say something like this to make the relationship more obvious.
            Send email notifications to: [ the text box ](If left empty notifications will be sent to person@example.com)

            I could also do something like add a site settings that allows/disallows users overriding the email address in their profile. If set we could simply not display the email section on the messaging preferences page.

            Show
            andyjdavis Andrew Davis added a comment - There are indeed currently two places you can specify your email address. The primary one is your profile. You can optionally add a second email address on the messaging settings screen. By default this is empty and your primary email address is used. The ability to override it in your messaging settings exists for people who have multiple email addresses and want their messages delivered to one of their non-primary email addresses. Currently the text around the email box in the messaging settings says the following Send email notifications to: [ the text box ](Default: person@example.com) I could change this to say something like this to make the relationship more obvious. Send email notifications to: [ the text box ](If left empty notifications will be sent to person@example.com) I could also do something like add a site settings that allows/disallows users overriding the email address in their profile. If set we could simply not display the email section on the messaging preferences page.
            Hide
            salvetore Michael de Raadt added a comment -

            Perhaps adding a permission would be better than adding a setting.

            Show
            salvetore Michael de Raadt added a comment - Perhaps adding a permission would be better than adding a setting.
            Hide
            salvetore Michael de Raadt added a comment -

            I'm changing this from a bug to an improvement, as that more accurately reflects the issue's nature.

            Show
            salvetore Michael de Raadt added a comment - I'm changing this from a bug to an improvement, as that more accurately reflects the issue's nature.
            Hide
            klaus.mueller Klaus Müller added a comment -

            Dear Andrew,
            thank you very much for your kind explanation.

            A permission which allows the field to be used would be great.

            Only one question:
            Do you think there is any chance to move the definition of an alternative email-address for the messages to the user profile? It took us a long time to understand why moodle sends mail to an email-address different to the one defined in the profile. In my point of view the problem is not the existence of the second address (indeed I think its a great feature); the problem was to find out that moodle can use two different addresses.

            Best regards
            Klaus

            Show
            klaus.mueller Klaus Müller added a comment - Dear Andrew, thank you very much for your kind explanation. A permission which allows the field to be used would be great. Only one question: Do you think there is any chance to move the definition of an alternative email-address for the messages to the user profile? It took us a long time to understand why moodle sends mail to an email-address different to the one defined in the profile . In my point of view the problem is not the existence of the second address (indeed I think its a great feature); the problem was to find out that moodle can use two different addresses. Best regards Klaus
            Hide
            monash-vle Monash University VLE team added a comment -

            As a university who wants all mail delivered to the users university account, we would definitely like to see the site setting that allows/disallows users overriding the email address in their Messaging settings.

            Show
            monash-vle Monash University VLE team added a comment - As a university who wants all mail delivered to the users university account, we would definitely like to see the site setting that allows/disallows users overriding the email address in their Messaging settings.
            Hide
            andyjdavis Andrew Davis added a comment -

            I've added a master branch version and testing instructions.

            I wasted a bunch of time fiddling around with capabilities before realizing that wasn't going to work. Messaging is outside of any course so course related capabilities like "student" don't really make sense. duh.

            So now I've introduced a new system setting.

            Show
            andyjdavis Andrew Davis added a comment - I've added a master branch version and testing instructions. I wasted a bunch of time fiddling around with capabilities before realizing that wasn't going to work. Messaging is outside of any course so course related capabilities like "student" don't really make sense. duh. So now I've introduced a new system setting.
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Hello Andrew,

            you might want to consider using empty for

            if (!$CFG->messagingallowemailoverride) {

            to avoid any warnings during upgrade.

            Rest looks great

            Show
            rajeshtaneja Rajesh Taneja added a comment - Hello Andrew, you might want to consider using empty for if (!$CFG->messagingallowemailoverride) { to avoid any warnings during upgrade. Rest looks great
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Hi guys,
            Just wondering what exactly will happen when :-

            • Email override is allowed
            • A user overrides his email id
            • Admin Disables Email override

            will the user continue to get emails sent to the over-ridden email or default email will be used?

            Show
            ankit_frenz Ankit Agarwal added a comment - Hi guys, Just wondering what exactly will happen when :- Email override is allowed A user overrides his email id Admin Disables Email override will the user continue to get emails sent to the over-ridden email or default email will be used?
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Good point, Ankit.
            Overridden email id will be used in this case. But, not sure if we should use overridden or default email.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Good point, Ankit. Overridden email id will be used in this case. But, not sure if we should use overridden or default email.
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            +1 to use default in such cases

            Show
            ankit_frenz Ankit Agarwal added a comment - +1 to use default in such cases
            Hide
            andyjdavis Andrew Davis added a comment -

            Hi guys. Good spotting. I've taken your suggestions. Im having some trouble pushing changes to github right now but will put up the new version asap.

            Show
            andyjdavis Andrew Davis added a comment - Hi guys. Good spotting. I've taken your suggestions. Im having some trouble pushing changes to github right now but will put up the new version asap.
            Hide
            andyjdavis Andrew Davis added a comment - - edited

            Adding various branches. Assuming there are no more issues Rajesh feel free to hit the submit for integration button after you've had another look.

            Show
            andyjdavis Andrew Davis added a comment - - edited Adding various branches. Assuming there are no more issues Rajesh feel free to hit the submit for integration button after you've had another look.
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Looks Good Andrew

            Show
            rajeshtaneja Rajesh Taneja added a comment - Looks Good Andrew
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            @integrators:
            Not sure if 2nd commit should go in, as it might be confusing for users. Leaving it for integrators to decide.

            Show
            rajeshtaneja Rajesh Taneja added a comment - @integrators: Not sure if 2nd commit should go in, as it might be confusing for users. Leaving it for integrators to decide.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            differences in version between 2.2 and 2.3dev are not allowed at this stage, sorry!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - differences in version between 2.2 and 2.3dev are not allowed at this stage, sorry!
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Added the integration_held label to this, it will be looked at once we split MOODLE_22_STABLE and master.

            Show
            samhemelryk Sam Hemelryk added a comment - Added the integration_held label to this, it will be looked at once we split MOODLE_22_STABLE and master.
            Hide
            stronk7 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
            stronk7 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
            stronk7 Eloy Lafuente (stronk7) added a comment -

            The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday.

            This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody.

            This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P

            Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday. This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody. This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao
            Hide
            stronk7 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
            stronk7 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
            stronk7 Eloy Lafuente (stronk7) added a comment -

            I'm moving out from these as integrator for now... will retake them only if I'm able to review properly all the "enrol" pending ones that have max priority for this week.

            So any other integrator, feel free to pick them if pending... TIA and ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - I'm moving out from these as integrator for now... will retake them only if I'm able to review properly all the "enrol" pending ones that have max priority for this week. So any other integrator, feel free to pick them if pending... TIA and ciao
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi Andrew,

            Getting several conflicts when trying to merge this on master.
            Can you please rebase your branches and put this back up for integration review when you get a chance.
            As a heads up in admin/settings/*.php use `new lang_string` rather than `get_string` from now on.

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi Andrew, Getting several conflicts when trying to merge this on master. Can you please rebase your branches and put this back up for integration review when you get a chance. As a heads up in admin/settings/*.php use `new lang_string` rather than `get_string` from now on. Cheers Sam
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Also noting it conflicts on 21 and 22 as well.

            Show
            samhemelryk Sam Hemelryk added a comment - Also noting it conflicts on 21 and 22 as well.
            Hide
            andyjdavis Andrew Davis added a comment -

            Bringing this forward to sprint 17. Looks like it got left behind.

            Show
            andyjdavis Andrew Davis added a comment - Bringing this forward to sprint 17. Looks like it got left behind.
            Hide
            andyjdavis Andrew Davis added a comment -

            After performing some fancy git footwork I believe I have fixed up all the branches.

            Show
            andyjdavis Andrew Davis added a comment - After performing some fancy git footwork I believe I have fixed up all the branches.
            Hide
            stronk7 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
            stronk7 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
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Some notes:

            1) While we should keep current behavior in existing sites, wouldn't be better, for new installations, to set the default of the new setting to no, so admins must opt-in to allow the alternate emails?

            2) As new feature/improvement, do we want this really backported to 21 and 22 ?

            3) This new setting requires docs, it's one new option that needs to be documented, not sure where, but needs that (so I'm adding the docs_required label).

            Otherwise the patch looks ok. Just halting till answer for 1 & 2 arrives, TIA and ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Some notes: 1) While we should keep current behavior in existing sites, wouldn't be better, for new installations, to set the default of the new setting to no, so admins must opt-in to allow the alternate emails? 2) As new feature/improvement, do we want this really backported to 21 and 22 ? 3) This new setting requires docs, it's one new option that needs to be documented, not sure where, but needs that (so I'm adding the docs_required label). Otherwise the patch looks ok. Just halting till answer for 1 & 2 arrives, TIA and ciao
            Hide
            andyjdavis Andrew Davis added a comment -

            1) Youre right. Ive reversed the default so the ability for users to override their email address is off by default. Ive also added some upgrade code that turns that setting on so that upgrading sites will have the setting turned on ensuring there is no change in behaviour for them.

            2) Probably not. I wasnt actually thinking of this as a new feature but I guess it is.

            There is 1 new commit in my master branch. The 2.2 and 2.1 branches are unchanged as theyre probably not going in anyway

            Show
            andyjdavis Andrew Davis added a comment - 1) Youre right. Ive reversed the default so the ability for users to override their email address is off by default. Ive also added some upgrade code that turns that setting on so that upgrading sites will have the setting turned on ensuring there is no change in behaviour for them. 2) Probably not. I wasnt actually thinking of this as a new feature but I guess it is. There is 1 new commit in my master branch. The 2.2 and 2.1 branches are unchanged as theyre probably not going in anyway
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            I'm going to integrate and test this... although I'm going to create one new issue about to clean the subsystems (advanced features) admin page. It should contain exclusively on/off switchers for entire subsystems and not individual settings like the introduced here (and another 2-3 more) that should belong to another (own) settings page (messaging, appearance...).

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - I'm going to integrate and test this... although I'm going to create one new issue about to clean the subsystems (advanced features) admin page. It should contain exclusively on/off switchers for entire subsystems and not individual settings like the introduced here (and another 2-3 more) that should belong to another (own) settings page (messaging, appearance...). Ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            MDL-31471 created about the admin->adv. features cleanup

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - MDL-31471 created about the admin->adv. features cleanup
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Integrated (master only), thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated (master only), thanks!
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Passing, the UI works as expected. Also, new installation leads to disabled setting.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Passing, the UI works as expected. Also, new installation leads to disabled setting.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Many thanks for your collaboration, this code has been integrated upstream and it's available in all the repositories.

            Closing, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Many thanks for your collaboration, this code has been integrated upstream and it's available in all the repositories. Closing, ciao
            Hide
            marycooch Mary Cooch added a comment -

            Just added this clarification to 2.3 docs http://docs.moodle.org/23/en/Messaging_settings#Email so removing label.

            Show
            marycooch Mary Cooch added a comment - Just added this clarification to 2.3 docs http://docs.moodle.org/23/en/Messaging_settings#Email so removing label.

              People

              • Votes:
                1 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  25/Jun/12