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.

Sign up for GitHub

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

Jump to bottom

Auto-set template name based on action #6885

Merged
merged 3 commits into from
Aug 17, 2015
Merged

Auto-set template name based on action #6885

merged 3 commits into from
Aug 17, 2015

Conversation

jadb
Copy link
Contributor

@jadb jadb commented Jun 25, 2015

No description provided.

@markstory markstory added this to the 3.1.0 milestone Jun 25, 2015
markstory
markstory reviewed Jun 25, 2015
View reviewed changes
src/Mailer/Mailer.php
@@ -226,6 +226,10 @@ public function send($action, $args = [], $headers = [])

call_user_func_array([$this, $action], $args);

if (empty($this->template)) {
Copy link
Member

Choose a reason for hiding this comment

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

Do we ever thing a joker will use '0.ctp' ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ain't no joke. The struggle is real.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I replace by is_null()?

Copy link
Member

Choose a reason for hiding this comment

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

We use === null.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Keep forgetting, will do that.

@ADmad
Copy link
Member

ADmad commented Jun 28, 2015

@jadb Tests failing

@markstory
Copy link
Member

ping.

@jadb
Copy link
Contributor Author

jadb commented Jul 18, 2015

Looks like CS errors unrelated to this PR.

@markstory
Copy link
Member

Should this have new tests added?

@jadb jadb self-assigned this Jul 19, 2015
@markstory
Copy link
Member

ping

@jadb
Copy link
Contributor Author

jadb commented Aug 11, 2015

I'll do it, sorry missed it. Need to start clearing the stuff I assign to myself for a change, been really busy.

@jadb
Copy link
Contributor Author

jadb commented Aug 17, 2015

Sorry for the delay. I added the tests and amended the other commit.

markstory added a commit that referenced this pull request Aug 17, 2015
@markstory
Merge pull request #6885 from cakephp/jadb-patch-mailer
293d983 
Auto-set template name based on action
@markstory markstory merged commit 293d983 into 3.1 Aug 17, 2015
@markstory markstory deleted the jadb-patch-mailer branch August 17, 2015 23:54
@ADmad
Copy link
Member

ADmad commented Aug 21, 2015

With this change setting custom template using $this->_email->template('custom') in a mailer method no longer works. It still ends up using the method name as template.

You are forced to set $this->template in the mailer which should not be the case.

@markstory
Copy link
Member

@ADmad Seems like something that should be fixed.

@ADmad
Copy link
Member

ADmad commented Aug 21, 2015

@markstory I have opened the new issue #7278 explaining the problems with mailer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jadb jadb

Labels
enhancement
Projects
None yet
Milestone
3.1.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@jadb @ADmad @markstory @bcrowe @dereuromark