From 3a1568a31d1fc14647d3eaf6acbea279aa0c611e Mon Sep 17 00:00:00 2001 From: Vlad Date: Wed, 4 Jan 2017 13:04:53 -0500 Subject: [PATCH 1/2] - Fixes mailer error message not being displayed when newsletter preview sending fails --- lib/API/Endpoints/Newsletters.php | 2 +- tests/unit/API/Endpoints/NewslettersTest.php | 189 +++++++++++-------- 2 files changed, 112 insertions(+), 79 deletions(-) diff --git a/lib/API/Endpoints/Newsletters.php b/lib/API/Endpoints/Newsletters.php index bd61725c0f..fae2361c1f 100644 --- a/lib/API/Endpoints/Newsletters.php +++ b/lib/API/Endpoints/Newsletters.php @@ -284,7 +284,7 @@ class Newsletters extends APIEndpoint { if($result['response'] === false) { $error = sprintf( __('The email could not be sent: %s', 'mailpoet'), - $result['error'] + $result['error_message'] ); return $this->errorResponse(array(APIError::BAD_REQUEST => $error)); } else { diff --git a/tests/unit/API/Endpoints/NewslettersTest.php b/tests/unit/API/Endpoints/NewslettersTest.php index 6a3675aa41..4ea834b995 100644 --- a/tests/unit/API/Endpoints/NewslettersTest.php +++ b/tests/unit/API/Endpoints/NewslettersTest.php @@ -1,27 +1,27 @@ newsletter = Newsletter::createOrUpdate(array( - 'subject' => 'My Standard Newsletter', - 'body' => Fixtures::get('newsletter_body_template'), - 'type' => Newsletter::TYPE_STANDARD, - )); + 'subject' => 'My Standard Newsletter', + 'body' => Fixtures::get('newsletter_body_template'), + 'type' => Newsletter::TYPE_STANDARD, + )); $this->post_notification = Newsletter::createOrUpdate(array( - 'subject' => 'My Post Notification', - 'body' => Fixtures::get('newsletter_body_template'), - 'type' => Newsletter::TYPE_NOTIFICATION - )); + 'subject' => 'My Post Notification', + 'body' => Fixtures::get('newsletter_body_template'), + 'type' => Newsletter::TYPE_NOTIFICATION + )); } function testItCanGetANewsletter() { @@ -63,7 +63,8 @@ class NewslettersTest extends MailPoetTest { $router = new Newsletters(); $response = $router->save($valid_data); - $saved_newsletter = Newsletter::filter('filterWithOptions')->findOne($response->data['id']); + $saved_newsletter = Newsletter::filter('filterWithOptions') + ->findOne($response->data['id']); expect($response->status)->equals(APIResponse::STATUS_OK); expect($response->data)->equals($saved_newsletter->asArray()); // newsletter option should be saved @@ -131,7 +132,8 @@ class NewslettersTest extends MailPoetTest { // schedule should be recalculated when options change $newsletter_data['options']['intervalType'] = Scheduler::INTERVAL_IMMEDIATELY; $response = $router->save($newsletter_data); - $saved_newsletter = Newsletter::filter('filterWithOptions')->findOne($response->data['id']); + $saved_newsletter = Newsletter::filter('filterWithOptions') + ->findOne($response->data['id']); expect($response->status)->equals(APIResponse::STATUS_OK); expect($saved_newsletter->schedule)->equals('* * * * *'); } @@ -144,14 +146,18 @@ class NewslettersTest extends MailPoetTest { $newsletter_data = array( 'id' => $this->newsletter->id, 'subject' => 'My Updated Newsletter', - 'segments' => array($segment_1->asArray(), $fake_segment_id) + 'segments' => array( + $segment_1->asArray(), + $fake_segment_id + ) ); $response = $router->save($newsletter_data); expect($response->status)->equals(APIResponse::STATUS_OK); $updated_newsletter = - Newsletter::findOne($this->newsletter->id)->withSegments(); + Newsletter::findOne($this->newsletter->id) + ->withSegments(); expect(count($updated_newsletter->segments))->equals(1); expect($updated_newsletter->segments[0]['name'])->equals('Segment 1'); @@ -161,32 +167,32 @@ class NewslettersTest extends MailPoetTest { $router = new Newsletters(); // set status to sending $response = $router->setStatus(array( - 'id' => $this->newsletter->id, - 'status' => Newsletter::STATUS_SENDING - )); + 'id' => $this->newsletter->id, + 'status' => Newsletter::STATUS_SENDING + )); expect($response->status)->equals(APIResponse::STATUS_OK); expect($response->data['status'])->equals(Newsletter::STATUS_SENDING); // set status to draft $response = $router->setStatus(array( - 'id' => $this->newsletter->id, - 'status' => Newsletter::STATUS_DRAFT - )); + 'id' => $this->newsletter->id, + 'status' => Newsletter::STATUS_DRAFT + )); expect($response->status)->equals(APIResponse::STATUS_OK); expect($response->data['status'])->equals(Newsletter::STATUS_DRAFT); // no status specified throws an error $response = $router->setStatus(array( - 'id' => $this->newsletter->id, - )); + 'id' => $this->newsletter->id, + )); expect($response->status)->equals(APIResponse::STATUS_BAD_REQUEST); expect($response->errors[0]['message']) ->equals('You need to specify a status.'); // invalid newsletter id throws an error $response = $router->setStatus(array( - 'status' => Newsletter::STATUS_DRAFT - )); + 'status' => Newsletter::STATUS_DRAFT + )); expect($response->status)->equals(APIResponse::STATUS_NOT_FOUND); expect($response->errors[0]['message']) ->equals('This newsletter does not exist.'); @@ -202,7 +208,8 @@ class NewslettersTest extends MailPoetTest { $response = $router->restore(array('id' => $this->newsletter->id)); expect($response->status)->equals(APIResponse::STATUS_OK); expect($response->data)->equals( - Newsletter::findOne($this->newsletter->id)->asArray() + Newsletter::findOne($this->newsletter->id) + ->asArray() ); expect($response->data['deleted_at'])->null(); expect($response->meta['count'])->equals(1); @@ -213,7 +220,8 @@ class NewslettersTest extends MailPoetTest { $response = $router->trash(array('id' => $this->newsletter->id)); expect($response->status)->equals(APIResponse::STATUS_OK); expect($response->data)->equals( - Newsletter::findOne($this->newsletter->id)->asArray() + Newsletter::findOne($this->newsletter->id) + ->asArray() ); expect($response->data['deleted_at'])->notNull(); expect($response->meta['count'])->equals(1); @@ -273,23 +281,23 @@ class NewslettersTest extends MailPoetTest { $newsletter_segment = NewsletterSegment::create(); $newsletter_segment->hydrate(array( - 'newsletter_id' => $this->newsletter->id, - 'segment_id' => $segment_1->id - )); + 'newsletter_id' => $this->newsletter->id, + 'segment_id' => $segment_1->id + )); $newsletter_segment->save(); $newsletter_segment = NewsletterSegment::create(); $newsletter_segment->hydrate(array( - 'newsletter_id' => $this->newsletter->id, - 'segment_id' => $segment_2->id - )); + 'newsletter_id' => $this->newsletter->id, + 'segment_id' => $segment_2->id + )); $newsletter_segment->save(); $newsletter_segment = NewsletterSegment::create(); $newsletter_segment->hydrate(array( - 'newsletter_id' => $this->post_notification->id, - 'segment_id' => $segment_2->id - )); + 'newsletter_id' => $this->post_notification->id, + 'segment_id' => $segment_2->id + )); $newsletter_segment->save(); $router = new Newsletters(); @@ -326,34 +334,34 @@ class NewslettersTest extends MailPoetTest { // link standard newsletter to the 2 segments $newsletter_segment = NewsletterSegment::create(); $newsletter_segment->hydrate(array( - 'newsletter_id' => $this->newsletter->id, - 'segment_id' => $segment_1->id - )); + 'newsletter_id' => $this->newsletter->id, + 'segment_id' => $segment_1->id + )); $newsletter_segment->save(); $newsletter_segment = NewsletterSegment::create(); $newsletter_segment->hydrate(array( - 'newsletter_id' => $this->newsletter->id, - 'segment_id' => $segment_2->id - )); + 'newsletter_id' => $this->newsletter->id, + 'segment_id' => $segment_2->id + )); $newsletter_segment->save(); // link post notification to the 2nd segment $newsletter_segment = NewsletterSegment::create(); $newsletter_segment->hydrate(array( - 'newsletter_id' => $this->post_notification->id, - 'segment_id' => $segment_2->id - )); + 'newsletter_id' => $this->post_notification->id, + 'segment_id' => $segment_2->id + )); $newsletter_segment->save(); $router = new Newsletters(); // filter by 1st segment $response = $router->listing(array( - 'filter' => array( - 'segment' => $segment_1->id - ) - )); + 'filter' => array( + 'segment' => $segment_1->id + ) + )); expect($response->status)->equals(APIResponse::STATUS_OK); @@ -363,10 +371,10 @@ class NewslettersTest extends MailPoetTest { // filter by 2nd segment $response = $router->listing(array( - 'filter' => array( - 'segment' => $segment_2->id - ) - )); + 'filter' => array( + 'segment' => $segment_2->id + ) + )); expect($response->status)->equals(APIResponse::STATUS_OK); @@ -378,10 +386,10 @@ class NewslettersTest extends MailPoetTest { $router = new Newsletters(); // get 1st page (limit items per page to 1) $response = $router->listing(array( - 'limit' => 1, - 'sort_by' => 'subject', - 'sort_order' => 'asc' - )); + 'limit' => 1, + 'sort_by' => 'subject', + 'sort_order' => 'asc' + )); expect($response->status)->equals(APIResponse::STATUS_OK); @@ -393,11 +401,11 @@ class NewslettersTest extends MailPoetTest { // get 1st page (limit items per page to 1) $response = $router->listing(array( - 'limit' => 1, - 'offset' => 1, - 'sort_by' => 'subject', - 'sort_order' => 'asc' - )); + 'limit' => 1, + 'offset' => 1, + 'sort_by' => 'subject', + 'sort_order' => 'asc' + )); expect($response->meta['count'])->equals(2); expect($response->data)->count(1); @@ -414,11 +422,11 @@ class NewslettersTest extends MailPoetTest { $router = new Newsletters(); $response = $router->bulkAction(array( - 'listing' => array( - 'selection' => $selection_ids - ), - 'action' => 'delete' - )); + 'listing' => array( + 'selection' => $selection_ids + ), + 'action' => 'delete' + )); expect($response->status)->equals(APIResponse::STATUS_OK); expect($response->meta['count'])->equals(count($selection_ids)); @@ -427,24 +435,24 @@ class NewslettersTest extends MailPoetTest { function testItCanBulkDeleteNewsletters() { $router = new Newsletters(); $response = $router->bulkAction(array( - 'action' => 'trash', - 'listing' => array('group' => 'all') - )); + 'action' => 'trash', + 'listing' => array('group' => 'all') + )); expect($response->status)->equals(APIResponse::STATUS_OK); expect($response->meta['count'])->equals(2); $router = new Newsletters(); $response = $router->bulkAction(array( - 'action' => 'delete', - 'listing' => array('group' => 'trash') - )); + 'action' => 'delete', + 'listing' => array('group' => 'trash') + )); expect($response->status)->equals(APIResponse::STATUS_OK); expect($response->meta['count'])->equals(2); $response = $router->bulkAction(array( - 'action' => 'delete', - 'listing' => array('group' => 'trash') - )); + 'action' => 'delete', + 'listing' => array('group' => 'trash') + )); expect($response->status)->equals(APIResponse::STATUS_OK); expect($response->meta['count'])->equals(0); } @@ -471,6 +479,31 @@ class NewslettersTest extends MailPoetTest { expect($response->status)->equals(APIResponse::STATUS_OK); } + function testItReturnsMaillerErrorWhenSendingFailed() { + $subscriber = 'test@subscriber.com'; + $data = array( + 'subscriber' => $subscriber, + 'id' => $this->newsletter->id, + 'mailer' => Stub::makeEmpty( + '\MailPoet\Mailer\Mailer', + array( + 'send' => function($newsletter, $subscriber) { + expect(is_array($newsletter))->true(); + expect($newsletter['body']['text'])->contains('Hello test'); + expect($subscriber)->equals($subscriber); + return array( + 'response' => false, + 'error_message' => 'failed' + ); + } + ) + ) + ); + $router = new Newsletters(); + $response = $router->sendPreview($data); + expect($response->errors[0]['message'])->equals('The email could not be sent: failed'); + } + function _after() { Newsletter::deleteMany(); NewsletterSegment::deleteMany(); From 449eb28b2ac6ce4b7571654b5bf4d4ecc9ffd984 Mon Sep 17 00:00:00 2001 From: Vlad Date: Mon, 9 Jan 2017 09:09:13 -0500 Subject: [PATCH 2/2] - Updates code formatting --- tests/unit/API/Endpoints/NewslettersTest.php | 212 +++++++++++-------- 1 file changed, 125 insertions(+), 87 deletions(-) diff --git a/tests/unit/API/Endpoints/NewslettersTest.php b/tests/unit/API/Endpoints/NewslettersTest.php index 4ea834b995..690727f8d8 100644 --- a/tests/unit/API/Endpoints/NewslettersTest.php +++ b/tests/unit/API/Endpoints/NewslettersTest.php @@ -11,17 +11,19 @@ use MailPoet\Newsletter\Scheduler\Scheduler; class NewslettersTest extends MailPoetTest { function _before() { - $this->newsletter = Newsletter::createOrUpdate(array( - 'subject' => 'My Standard Newsletter', - 'body' => Fixtures::get('newsletter_body_template'), - 'type' => Newsletter::TYPE_STANDARD, - )); + $this->newsletter = Newsletter::createOrUpdate( + array( + 'subject' => 'My Standard Newsletter', + 'body' => Fixtures::get('newsletter_body_template'), + 'type' => Newsletter::TYPE_STANDARD, + )); - $this->post_notification = Newsletter::createOrUpdate(array( - 'subject' => 'My Post Notification', - 'body' => Fixtures::get('newsletter_body_template'), - 'type' => Newsletter::TYPE_NOTIFICATION - )); + $this->post_notification = Newsletter::createOrUpdate( + array( + 'subject' => 'My Post Notification', + 'body' => Fixtures::get('newsletter_body_template'), + 'type' => Newsletter::TYPE_NOTIFICATION + )); } function testItCanGetANewsletter() { @@ -166,33 +168,41 @@ class NewslettersTest extends MailPoetTest { function testItCanSetANewsletterStatus() { $router = new Newsletters(); // set status to sending - $response = $router->setStatus(array( - 'id' => $this->newsletter->id, - 'status' => Newsletter::STATUS_SENDING - )); + $response = $router->setStatus + (array( + 'id' => $this->newsletter->id, + 'status' => Newsletter::STATUS_SENDING + ) + ); expect($response->status)->equals(APIResponse::STATUS_OK); expect($response->data['status'])->equals(Newsletter::STATUS_SENDING); // set status to draft - $response = $router->setStatus(array( - 'id' => $this->newsletter->id, - 'status' => Newsletter::STATUS_DRAFT - )); + $response = $router->setStatus( + array( + 'id' => $this->newsletter->id, + 'status' => Newsletter::STATUS_DRAFT + ) + ); expect($response->status)->equals(APIResponse::STATUS_OK); expect($response->data['status'])->equals(Newsletter::STATUS_DRAFT); // no status specified throws an error - $response = $router->setStatus(array( - 'id' => $this->newsletter->id, - )); + $response = $router->setStatus( + array( + 'id' => $this->newsletter->id, + ) + ); expect($response->status)->equals(APIResponse::STATUS_BAD_REQUEST); expect($response->errors[0]['message']) ->equals('You need to specify a status.'); // invalid newsletter id throws an error - $response = $router->setStatus(array( - 'status' => Newsletter::STATUS_DRAFT - )); + $response = $router->setStatus( + array( + 'status' => Newsletter::STATUS_DRAFT + ) + ); expect($response->status)->equals(APIResponse::STATUS_NOT_FOUND); expect($response->errors[0]['message']) ->equals('This newsletter does not exist.'); @@ -280,24 +290,30 @@ class NewslettersTest extends MailPoetTest { $segment_2 = Segment::createOrUpdate(array('name' => 'Segment 2')); $newsletter_segment = NewsletterSegment::create(); - $newsletter_segment->hydrate(array( - 'newsletter_id' => $this->newsletter->id, - 'segment_id' => $segment_1->id - )); + $newsletter_segment->hydrate( + array( + 'newsletter_id' => $this->newsletter->id, + 'segment_id' => $segment_1->id + ) + ); $newsletter_segment->save(); $newsletter_segment = NewsletterSegment::create(); - $newsletter_segment->hydrate(array( - 'newsletter_id' => $this->newsletter->id, - 'segment_id' => $segment_2->id - )); + $newsletter_segment->hydrate( + array( + 'newsletter_id' => $this->newsletter->id, + 'segment_id' => $segment_2->id + ) + ); $newsletter_segment->save(); $newsletter_segment = NewsletterSegment::create(); - $newsletter_segment->hydrate(array( - 'newsletter_id' => $this->post_notification->id, - 'segment_id' => $segment_2->id - )); + $newsletter_segment->hydrate( + array( + 'newsletter_id' => $this->post_notification->id, + 'segment_id' => $segment_2->id + ) + ); $newsletter_segment->save(); $router = new Newsletters(); @@ -333,35 +349,43 @@ class NewslettersTest extends MailPoetTest { // link standard newsletter to the 2 segments $newsletter_segment = NewsletterSegment::create(); - $newsletter_segment->hydrate(array( - 'newsletter_id' => $this->newsletter->id, - 'segment_id' => $segment_1->id - )); + $newsletter_segment->hydrate( + array( + 'newsletter_id' => $this->newsletter->id, + 'segment_id' => $segment_1->id + ) + ); $newsletter_segment->save(); $newsletter_segment = NewsletterSegment::create(); - $newsletter_segment->hydrate(array( - 'newsletter_id' => $this->newsletter->id, - 'segment_id' => $segment_2->id - )); + $newsletter_segment->hydrate + (array( + 'newsletter_id' => $this->newsletter->id, + 'segment_id' => $segment_2->id + ) + ); $newsletter_segment->save(); // link post notification to the 2nd segment $newsletter_segment = NewsletterSegment::create(); - $newsletter_segment->hydrate(array( - 'newsletter_id' => $this->post_notification->id, - 'segment_id' => $segment_2->id - )); + $newsletter_segment->hydrate( + array( + 'newsletter_id' => $this->post_notification->id, + 'segment_id' => $segment_2->id + ) + ); $newsletter_segment->save(); $router = new Newsletters(); // filter by 1st segment - $response = $router->listing(array( - 'filter' => array( - 'segment' => $segment_1->id - ) - )); + $response = $router->listing( + array( + 'filter' => array( + 'segment' => $segment_1->id + ) + ) + ); expect($response->status)->equals(APIResponse::STATUS_OK); @@ -370,11 +394,13 @@ class NewslettersTest extends MailPoetTest { expect($response->data[0]['subject'])->equals($this->newsletter->subject); // filter by 2nd segment - $response = $router->listing(array( - 'filter' => array( - 'segment' => $segment_2->id - ) - )); + $response = $router->listing( + array( + 'filter' => array( + 'segment' => $segment_2->id + ) + ) + ); expect($response->status)->equals(APIResponse::STATUS_OK); @@ -385,11 +411,13 @@ class NewslettersTest extends MailPoetTest { function testItCanLimitListing() { $router = new Newsletters(); // get 1st page (limit items per page to 1) - $response = $router->listing(array( - 'limit' => 1, - 'sort_by' => 'subject', - 'sort_order' => 'asc' - )); + $response = $router->listing( + array( + 'limit' => 1, + 'sort_by' => 'subject', + 'sort_order' => 'asc' + ) + ); expect($response->status)->equals(APIResponse::STATUS_OK); @@ -400,12 +428,14 @@ class NewslettersTest extends MailPoetTest { ); // get 1st page (limit items per page to 1) - $response = $router->listing(array( - 'limit' => 1, - 'offset' => 1, - 'sort_by' => 'subject', - 'sort_order' => 'asc' - )); + $response = $router->listing( + array( + 'limit' => 1, + 'offset' => 1, + 'sort_by' => 'subject', + 'sort_order' => 'asc' + ) + ); expect($response->meta['count'])->equals(2); expect($response->data)->count(1); @@ -421,12 +451,14 @@ class NewslettersTest extends MailPoetTest { ); $router = new Newsletters(); - $response = $router->bulkAction(array( - 'listing' => array( - 'selection' => $selection_ids - ), - 'action' => 'delete' - )); + $response = $router->bulkAction( + array( + 'listing' => array( + 'selection' => $selection_ids + ), + 'action' => 'delete' + ) + ); expect($response->status)->equals(APIResponse::STATUS_OK); expect($response->meta['count'])->equals(count($selection_ids)); @@ -434,25 +466,31 @@ class NewslettersTest extends MailPoetTest { function testItCanBulkDeleteNewsletters() { $router = new Newsletters(); - $response = $router->bulkAction(array( - 'action' => 'trash', - 'listing' => array('group' => 'all') - )); + $response = $router->bulkAction( + array( + 'action' => 'trash', + 'listing' => array('group' => 'all') + ) + ); expect($response->status)->equals(APIResponse::STATUS_OK); expect($response->meta['count'])->equals(2); $router = new Newsletters(); - $response = $router->bulkAction(array( - 'action' => 'delete', - 'listing' => array('group' => 'trash') - )); + $response = $router->bulkAction( + array( + 'action' => 'delete', + 'listing' => array('group' => 'trash') + ) + ); expect($response->status)->equals(APIResponse::STATUS_OK); expect($response->meta['count'])->equals(2); - $response = $router->bulkAction(array( - 'action' => 'delete', - 'listing' => array('group' => 'trash') - )); + $response = $router->bulkAction( + array( + 'action' => 'delete', + 'listing' => array('group' => 'trash') + ) + ); expect($response->status)->equals(APIResponse::STATUS_OK); expect($response->meta['count'])->equals(0); }