Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Fixes #5962 #6157

Merged
merged 4 commits into from
Apr 30, 2014
Merged

Fixes #5962 #6157

merged 4 commits into from
Apr 30, 2014

Conversation

samsonasik
Copy link
Contributor

Fixes #5962 . Based on Zend\Mime\Message::generateMessage() comment description, If no part had been added, an empty string is returned.

@@ -107,8 +107,12 @@ public function getMime()
public function generateMessage($EOL = Mime::LINEEND)
{
if (!$this->isMultiPart()) {
$part = current($this->parts);
$body = $part->getContent($EOL);
if (!empty($this->parts)) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you swap this to remove the else?

@Ocramius Ocramius added this to the 2.3.2 milestone Apr 29, 2014
@Ocramius Ocramius self-assigned this Apr 29, 2014
@samsonasik
Copy link
Contributor Author

@Ocramius done ;) please let me know if I missed something , Thank you ;)

*/
public function testPassEmptyArrayIntoSetPartsShouldReturnEmptyString()
{
$mailMessage = new Mail\Message();
Copy link
Member

Choose a reason for hiding this comment

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

Do you really need the mail dependency to testZend\Mime?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is to proof that this test fix the issue based on #5962 explanation, should I remove it ?

Copy link
Member

Choose a reason for hiding this comment

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

@samsonasik an integration test in ZendTest\Mail would be better (plus the unit test here)

@samsonasik
Copy link
Contributor Author

@Ocramius done ;) I added test in ZendTest\Mail and remove dependency of Zend\Mail in ZendTest\Mime\MessageTest. please let me know if I missed something , Thank you ;)

@Ocramius
Copy link
Member

@samsonasik thanks, this is much cleaner!

@Ocramius Ocramius merged commit 8c161a3 into zendframework:master Apr 30, 2014
Ocramius added a commit that referenced this pull request Apr 30, 2014
Ocramius added a commit that referenced this pull request Apr 30, 2014
@samsonasik samsonasik deleted the patch-mime-emptyparts branch April 30, 2014 12:38
weierophinney pushed a commit to zendframework/zend-mail that referenced this pull request May 14, 2015
gianarb pushed a commit to zendframework/zend-mime that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-mime that referenced this pull request May 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fatal Error on /Mime/Message.php on line 111
2 participants