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

clean up EMAIL variables, use EMAIL_FROM where possible #257

Merged
merged 8 commits into from Feb 16, 2018
Merged

clean up EMAIL variables, use EMAIL_FROM where possible #257

merged 8 commits into from Feb 16, 2018

Conversation

wilsonc86
Copy link
Contributor

@wilsonc86 wilsonc86 commented Feb 16, 2018

This change is Reviewable

@coveralls
Copy link

coveralls commented Feb 16, 2018

Pull Request Test Coverage Report for Build 363

  • 8 of 9 (88.89%) changed or added relevant lines in 6 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+3.1%) to 51.659%

Changes Missing Coverage Covered Lines Changed/Added Lines %
wildlifelicensing/apps/returns/emails.py 0 1 0.0%
Files with Coverage Reduction New Missed Lines %
wildlifelicensing/apps/applications/emails.py 1 77.67%
Totals Coverage Status
Change from base Build 356: 3.1%
Covered Lines: 9764
Relevant Lines: 18901

💛 - Coveralls

@scottp-dpaw
Copy link

ledger/settings_base.py, line 121 at r1 (raw file):

EMAIL_PORT = env('EMAIL_PORT', 25)
EMAIL_FROM = env('EMAIL_FROM', ADMINS[0])
DEFAULT_FROM_EMAIL = EMAIL_FROM

this is actually a Django variable; https://docs.djangoproject.com/en/2.0/ref/settings/#std:setting-DEFAULT_FROM_EMAIL . this line should be copied into wildlifelicensing/settings.py and parkstay/settings.py


Comments from Reviewable

@scottp-dpaw
Copy link

wildlifelicensing/apps/main/pdf.py, line 5 at r1 (raw file):

from datetime import date

from ledger.settings_base import EMAIL_FROM

from django.settings import EMAIL_FROM


Comments from Reviewable

@scottp-dpaw
Copy link

ledger/emails/emails.py, line 37 at r1 (raw file):

        Send an email using EmailMultiAlternatives with text and html.
        :param to_addresses: a string or a list of addresses
        :param from_address: if None the settings.EMAIL_FROM is used

django default is to use settings.DEFAULT_FROM_EMAIL


Comments from Reviewable

@scottp-dpaw
Copy link

wildlifelicensing/apps/emails/emails.py, line 43 at r1 (raw file):

        Send an email using EmailMultiAlternatives with text and html.
        :param to_addresses: a string or a list of addresses
        :param from_address: if None the settings.EMAIL_FROM is used

django default is to use settings.DEFAULT_FROM_EMAIL


Comments from Reviewable

@scottp-dpaw
Copy link

Reviewed 9 of 15 files at r1.
Review status: 9 of 15 files reviewed at latest revision, 4 unresolved discussions.


Comments from Reviewable

@wilsonc86
Copy link
Contributor Author

Review status: 9 of 13 files reviewed at latest revision, 4 unresolved discussions.


ledger/settings_base.py, line 121 at r1 (raw file):

Previously, scottp-dpaw wrote…

this is actually a Django variable; https://docs.djangoproject.com/en/2.0/ref/settings/#std:setting-DEFAULT_FROM_EMAIL . this line should be copied into wildlifelicensing/settings.py and parkstay/settings.py

Done.


wildlifelicensing/apps/main/pdf.py, line 5 at r1 (raw file):

Previously, scottp-dpaw wrote…

from django.settings import EMAIL_FROM

Done.


ledger/emails/emails.py, line 37 at r1 (raw file):

Previously, scottp-dpaw wrote…

django default is to use settings.DEFAULT_FROM_EMAIL

Done.


wildlifelicensing/apps/emails/emails.py, line 43 at r1 (raw file):

Previously, scottp-dpaw wrote…

django default is to use settings.DEFAULT_FROM_EMAIL

Done.


Comments from Reviewable

@scottp-dpaw
Copy link

:lgtm:


Reviewed 6 of 6 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@dbca-asi dbca-asi merged commit e7d49df into dbca-wa:master Feb 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants