Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bulk course emails are rejected by email server because of an invalid "from" address #102

Closed
regisb opened this issue Sep 23, 2021 · 9 comments
Assignees
Milestone

Comments

@regisb
Copy link
Contributor

regisb commented Sep 23, 2021

See this conversation: https://discuss.overhang.io/t/sending-bulk-email-triggers-smtprecipientsrefused-error/1923/10

When sending bulk course emails, by default the "from" address is built as follows: f"{courseid}-{bulk_email_from}". (source) In many cases, sender verification will fail for this address because it will not exist on the email server.

This is all the more surprising for the user, as they have properly defined the BULK_EMAIL_DEFAULT_FROM_EMAIL setting. A way to bypass this strange behaviour is to enable the EMAIL_USE_DEFAULT_FROM_FOR_BULK toggle -- but it's counter intuitive that we need to manually enable a toggle to use the "default" behaviour.

Thus, I propose the following changes:

  1. Switch the EMAIL_USE_DEFAULT_FROM_FOR_BULK toggle default to True (i.e: rename EMAIL_USE_DEFAULT_FROM_FOR_BULK to EMAIL_USE_COURSE_ID_FROM_FOR_BULK).
  2. When this toggle is True (EDIT: clarity) EMAIL_USE_COURSE_ID_FROM_FOR_BULK is False, the “from” address should default to settings.DEFAULT_FROM_EMAIL only if settings.BULK_EMAIL_DEFAULT_FROM_EMAIL is undefined. In other words:
if is_email_use_default_from_bulk_enabled():
    from_addr = settings.BULK_EMAIL_DEFAULT_FROM_EMAIL or settings.DEFAULT_FROM_EMAIL
else:
    ...
@Drakko25
Copy link

Hey @regisb I am looking at this issue. I've create the PR by now is a draft to check if the CI passes https://github.com/edx/edx-platform/pull/29001. Please let me know if any issues. Regards.

@regisb
Copy link
Contributor Author

regisb commented Oct 14, 2021

I realized by reading @Drakko25's PR that my proposed solution was unclear. I amended my initial post above. Let's talk further on edx/edx-platform#29001.

@BbrSofiane
Copy link
Member

Needs to be merged in maple.master

@nizarmah
Copy link

@BbrSofiane I'd like to merge this into maple.master. cc @regisb

nizarmah pushed a commit to open-craft/edx-platform that referenced this issue Dec 14, 2021
* pull request against `master`: Update from_addr for default from bulk emails (openedx#29001)
* related openedx issue: openedx/wg-build-test-release#102

Co-authored-by: Kevin Valencia <kevin@bitmaker.la>
(cherry picked from commit fa258de)
@nizarmah
Copy link

@regisb @BbrSofiane I created the backport PR to the maple master branch. Would either of you have time to review the pull request? Otherwise, do you have an idea who would be best to reach out to?

edx-community-bot added a commit to openedx/edx-platform that referenced this issue Dec 14, 2021
## Description

Backports course bulk email from address fix to the Maple master branch.

## Supporting information

* Original pull request: #29001 
* Reported issue: openedx/wg-build-test-release#102

## Deadline

* Best merged before December 20, 2021
@regisb
Copy link
Contributor Author

regisb commented Dec 14, 2021

I realize that this issue still affects Lilac, but since it is resolved in Maple I'm closing this.

@regisb regisb closed this as completed Dec 14, 2021
@nizarmah
Copy link

@regisb would you like me to create a backport pull request to lilac master, or is that not going to be necessary?

@regisb
Copy link
Contributor Author

regisb commented Dec 16, 2021

If you have time, that would be great but not strictly necessary, I guess, since we are 4 days away from the release.

nizarmah pushed a commit to open-craft/edx-platform that referenced this issue Dec 17, 2021
* pull request against `master`: Update from_addr for default from bulk emails (openedx#29001)
* related openedx issue: openedx/wg-build-test-release#102

Co-authored-by: Kevin Valencia <kevin@bitmaker.la>
(cherry picked from commit fa258de)
@nizarmah
Copy link

@regisb the backport PR to lilac is ready for your review 😃

mariajgrimaldi pushed a commit to eduNEXT/edunext-platform that referenced this issue Jan 5, 2022
* pull request against `master`: Update from_addr for default from bulk emails (#29001)
* related openedx issue: openedx/wg-build-test-release#102

Co-authored-by: Kevin Valencia <kevin@bitmaker.la>
(cherry picked from commit fa258de)
MaferMazu pushed a commit to eduNEXT/edunext-platform that referenced this issue Jan 5, 2022
* pull request against `master`: Update from_addr for default from bulk emails (#29001)
* related openedx issue: openedx/wg-build-test-release#102

Co-authored-by: Kevin Valencia <kevin@bitmaker.la>
(cherry picked from commit fa258de)
MaferMazu pushed a commit to eduNEXT/edunext-platform that referenced this issue Jan 5, 2022
* pull request against `master`: Update from_addr for default from bulk emails (#29001)
* related openedx issue: openedx/wg-build-test-release#102

Co-authored-by: Kevin Valencia <kevin@bitmaker.la>
(cherry picked from commit fa258de)
MaferMazu pushed a commit to eduNEXT/edunext-platform that referenced this issue Jan 6, 2022
* pull request against `master`: Update from_addr for default from bulk emails (#29001)
* related openedx issue: openedx/wg-build-test-release#102

Co-authored-by: Kevin Valencia <kevin@bitmaker.la>
(cherry picked from commit fa258de)
MaferMazu pushed a commit to eduNEXT/edunext-platform that referenced this issue Jan 7, 2022
* pull request against `master`: Update from_addr for default from bulk emails (#29001)
* related openedx issue: openedx/wg-build-test-release#102

Co-authored-by: Kevin Valencia <kevin@bitmaker.la>
(cherry picked from commit fa258de)
MaferMazu pushed a commit to eduNEXT/edunext-platform that referenced this issue Jan 10, 2022
* pull request against `master`: Update from_addr for default from bulk emails (#29001)
* related openedx issue: openedx/wg-build-test-release#102

Co-authored-by: Kevin Valencia <kevin@bitmaker.la>
(cherry picked from commit fa258de)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

4 participants