-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feature/restore reply to and validation #8
Feature/restore reply to and validation #8
Conversation
- re-add the ability to set Reply-To Mail Header from reply_to property json data - add email validation for mail attributes in template model rules and before sending email
…urn-Path and reply-to from schema
…d of throwing a forbidden exception
public function safeUp() | ||
{ | ||
|
||
$this->addColumn('app_dmstr_contact_template','return_path', $this->string()->null()->defaultValue(null)->after('to_email')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use {{%dmstr_contact_template}} instead of specifying the prefix directly. This could lead to potential side-effects
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, i've just copied the "wrong" existing migration: https://github.com/dmstr/yii2-contact-module/blob/master/src/migrations/m201012_141325_alter_app_dmstr_contact_template_table.php
;-)
public function safeUp() | ||
{ | ||
|
||
$this->addColumn('app_dmstr_contact_template','reply_to_schema_property', $this->string()->null()->defaultValue(null)->after('return_path')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use {{%dmstr_contact_template}} instead of specifying the prefix directly. This could lead to potential side-effects
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above.
src/views/default/done-expired.php
Outdated
|
||
<div class="container text-center"> | ||
<div class="row"> | ||
<div class=" col-xs-12 alert alert-success"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use an extra div for the alert. Horizontal padding could lead to shifting layout.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, will change
Background:
reply_to_email
was added to the template, which - if set - is used as a "fixed" reply-to for all sended mails. This can be useful, but is not a generic solution to set the reply-to header.This PR:
reply_to_email
address - if set - has priority.As we need these "features" also in "older" projects, this PR should be merged in the
release/2.0.x
branch first, and must then be ported to the3.*
andmaster
branches.