Skip to content

SMTP notify enhancements: full HTML emails and custom product_name in email headers#7533

Merged
balloob merged 2 commits into
home-assistant:devfrom
azogue:html-mails
May 15, 2017
Merged

SMTP notify enhancements: full HTML emails and custom product_name in email headers#7533
balloob merged 2 commits into
home-assistant:devfrom
azogue:html-mails

Conversation

@azogue
Copy link
Copy Markdown
Member

@azogue azogue commented May 10, 2017 •
edited
Loading

Description:

  • Ability to send full HTML emails, with or without inline attached images.
  • Customize product_name identifier in the FROM and X-MAILER email headers.

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#2607

Example entry for configuration.yaml (if applicable):

notify:

- name: NOTIFIER_NAME

  platform: smtp
  server: smtp.gmail.com

  port: 587
  timeout: 15

  sender: john@gmail.com

  starttls: true

  username: john@gmail.com

  password: thePassword

  recipient:

      - james@gmail.com

      - bob@gmail.com
  product_name: My Home

Checklist:

@azogue
SMTP notify enhancements: HTML emails and customization options
eadbf6f 
- Send full HTML emails, with or without inline attached images.
- Custom `timeout`.
- Custom `product_name` identifier for the `FROM` and `X-MAILER` email headers.
- New HTML email test
@mention-bot
Copy link
Copy Markdown

mention-bot commented May 10, 2017

@azogue, thanks for your PR! By analyzing the history of the files in this pull request, we identified @stratosmacker, @pvizeli and @partofthething to be potential reviewers.

@azogue azogue mentioned this pull request May 10, 2017
Merged
@pvizeli
Copy link
Copy Markdown
Member

pvizeli commented May 10, 2017

What is the benefit for custom X-MAILER? I see only a grow up config

@azogue
Copy link
Copy Markdown
Member Author

azogue commented May 11, 2017

The X-MAILER header is changed for consistency with the change in the FROM header, it does not have its specific option in the configuration.

The change in the FROM header does seem to me relevant as it sets the display name on the e-mail readers, instead of the ugly raw e-mail. More so when handling multiple instances of Home Assistant.

balloob
balloob reviewed May 13, 2017
View reviewed changes
Comment thread homeassistant/components/notify/smtp.py Outdated
msg['From'] = self._sender
msg['X-Mailer'] = 'HomeAssistant'
msg['From'] = '{} <{}>'.format(self._product_name, self._sender)
msg['X-Mailer'] = self._product_name
Copy link
Copy Markdown
Member

@balloob balloob May 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The X-Mailer should stick to HomeAssistant, as that's the product that emails it.

balloob
balloob reviewed May 13, 2017
View reviewed changes
Comment thread homeassistant/components/notify/smtp.py Outdated
vol.Optional(CONF_STARTTLS, default=DEFAULT_STARTTLS): cv.boolean,
vol.Optional(CONF_USERNAME): cv.string,
vol.Optional(CONF_PASSWORD): cv.string,
vol.Optional(CONF_PRODUCT_NAME, default=DEFAULT_PRODUCT_NAME): cv.string,
Copy link
Copy Markdown
Member

@balloob balloob May 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the word product name, can you use CONF_SENDER_NAME instead?

@azogue
sender_name instead of product_name
4e57108 
 - Change `sender_name` instead of `product_name`.
 - No changes in `X-Mailer` header.
 - `From` header as always unless you define the new `sender_name` parameter.
@azogue
Copy link
Copy Markdown
Member Author

azogue commented May 13, 2017

@balloob, changes are done here and in the doc, so I think it's ready.

Do you want me to edit the PR subject and description?

balloob
balloob approved these changes May 15, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks 🐬

@balloob balloob merged commit d030419 into home-assistant:dev May 15, 2017
@azogue azogue deleted the html-mails branch June 22, 2017 09:43
@home-assistant home-assistant locked and limited conversation to collaborators Oct 20, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@balloob balloob balloob approved these changes

Assignees

No one assigned

Labels

cla-signed integration: smtp platform

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@azogue @mention-bot @pvizeli @balloob @balloobbot @homeassistant