Moodle
  1. Moodle
  2. MDL-29615

Controlling Different eMail addresses in Moodle

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor 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
    • Rank:
      19129

      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.

        Issue Links

          Activity

          Hide
          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
          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
          Michael de Raadt added a comment -

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

          Show
          Michael de Raadt added a comment - Perhaps adding a permission would be better than adding a setting.
          Hide
          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
          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 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 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 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 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
          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
          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
          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
          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 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 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
          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
          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 Agarwal added a comment -

          +1 to use default in such cases

          Show
          Ankit Agarwal added a comment - +1 to use default in such cases
          Hide
          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
          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
          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
          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
          Rajesh Taneja added a comment -

          Looks Good Andrew

          Show
          Rajesh Taneja added a comment - Looks Good Andrew
          Hide
          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
          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
          Eloy Lafuente (stronk7) added a comment -

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

          Show
          Eloy Lafuente (stronk7) added a comment - differences in version between 2.2 and 2.3dev are not allowed at this stage, sorry!
          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          Sam Hemelryk added a comment -

          Also noting it conflicts on 21 and 22 as well.

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

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

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

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

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

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

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

          Integrated (master only), thanks!

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

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

          Show
          Eloy Lafuente (stronk7) added a comment - Passing, the UI works as expected. Also, new installation leads to disabled setting.
          Hide
          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
          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
          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
          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: