Add charset() / headerCharset() methods to CakeEmail class #564

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Member

suzuki commented Mar 15, 2012

CakeEmail have many setter / getter methods for mail headers.
But, charset and headerChaset have a only property.

This is a strange. see below :

  • now
$email = new CakeEmail;

$email->charset = 'ISO-2022-JP';       // strange here
$email->headerCharset = 'ISO-2022-JP'; // strange here

$email->from('suzuki@example.com');
$email->to('suzuki@example.jp);
$emaul->subject('TEST MAIL');
$email->send('TEST MAIL BODY');
  • my patch
$email = new CakeEmail;

$email->charset('ISO-2022-JP');       // this is nice accessor.
$email->headerCharset('ISO-2022-JP'); // this is nice accessor.

$email->from('suzuki@example.com');
$email->to('suzuki@example.jp);
$emaul->subject('TEST MAIL');
$email->send('TEST MAIL BODY');

@markstory markstory commented on the diff Mar 15, 2012

lib/Cake/Test/Case/Network/Email/CakeEmailTest.php
+ $this->assertSame($this->CakeEmail->headerCharset(), 'ISO-2022-JP');
+
+ $charset = $this->CakeEmail->headerCharset('Shift_JIS');
+ $this->assertSame($charset, 'Shift_JIS');
+ }
+
+
+/**
+ * Tests for compatible check.
+ * charset property and charset() method.
+ * headerCharset property and headerCharset() method.
+ */
+ public function testCharsetsCompatible() {
+ $this->skipIf(!function_exists('mb_convert_encoding'));
+
+ $checkHeaders = array('from' => true,
@markstory

markstory Mar 15, 2012

Owner

This whitespacing looks a bit inconsistent with CakePHP's style. There should only be one space around the => and array keys should be indented one tab from the surrounding scope.

@markstory markstory commented on the diff Mar 15, 2012

lib/Cake/Test/Case/Network/Email/CakeEmailTest.php
+ // Header Charset : ISO-2022-JP
+ // Boby Charset : UTF-8
+ $oldStyleEmail = $this->getEmailByOldStyleCharset('utf-8', 'iso-2022-jp');
+ $oldStyleHeaders = $oldStyleEmail->getHeaders($checkHeaders);
+
+ $newStyleEmail = $this->getEmailByNewStyleCharset('utf-8', 'iso-20220jp');
+ $newStyleHeaders = $newStyleEmail->getHeaders($checkHeaders);
+
+ $this->assertSame($oldStyleHeaders['From'], $newStyleHeaders['From']);
+ $this->assertSame($oldStyleHeaders['To'], $newStyleHeaders['To']);
+ $this->assertSame($oldStyleHeaders['Cc'], $newStyleHeaders['Cc']);
+ $this->assertSame($oldStyleHeaders['Subject'], $newStyleHeaders['Subject']);
+
+ }
+
+ private function getEmailByOldStyleCharset($charset, $headerCharset) {
@markstory

markstory Mar 15, 2012

Owner

Private functions should have leading _

Owner

markstory commented Mar 15, 2012

I like the idea, and think the methods are good. Thanks for including tests as well. Could you make the pull request against 2.2 as that branch is where new features/API additions are going now, as 2.1 is a stable branch.

suzuki closed this Mar 16, 2012

Member

suzuki commented Mar 16, 2012

Thank you, your comment. I tried to pull request again. #567 .

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