Call different actions for standard and automated newsletters on Send step

- converted save and setStatus method of Newsletters endpoint
- updated pause/resume mixin for notification & welcome listings
- use 'newsletter_id' instead of 'id' in SendingQueue endpoint (less confusing)
- added NOT_FOUND constant to APIError
- fixed unit test for Newsletters endpoint save and added test for setStatus

assets/js/src/newsletters/listings/mixins.jsx

@@ -9,11 +9,18 @@ const _QueueMixin = {
       endpoint: 'sendingQueue',
       action: 'pause',
       data: {
-        id: newsletter.id
+        newsletter_id: newsletter.id
       }
     }).done(function() {
       jQuery('#resume_'+newsletter.id).show();
       jQuery('#pause_'+newsletter.id).hide();
+    }).fail((response) => {
+      if (response.errors.length > 0) {
+        MailPoet.Notice.error(
+          response.errors.map(function(error) { return error.message; }),
+          { scroll: true }
+        );
+      }
     });
   },
   resumeSending: function(newsletter) {
@@ -21,11 +28,18 @@ const _QueueMixin = {
       endpoint: 'sendingQueue',
       action: 'resume',
       data: {
-        id: newsletter.id
+        newsletter_id: newsletter.id
       }
     }).done(function() {
       jQuery('#pause_'+newsletter.id).show();
       jQuery('#resume_'+newsletter.id).hide();
+    }).fail((response) => {
+      if (response.errors.length > 0) {
+        MailPoet.Notice.error(
+          response.errors.map(function(error) { return error.message; }),
+          { scroll: true }
+        );
+      }
     });
   },
   renderQueueStatus: function(newsletter) {

assets/js/src/newsletters/listings/notification.jsx

@@ -158,20 +158,18 @@ const NewsletterListNotification = React.createClass({
         id: ~~(e.target.getAttribute('data-id')),
         status: e.target.value
       }
-    }).done(function(response) {
+    }).done((response) => {
-      if (response.result === false) {
+      if (response.data.status === 'active') {
-        MailPoet.Notice.error(MailPoet.I18n.t('postNotificationActivationFailed'));
-
-        // reset value to actual newsletter's status
-         e.target.value = response.status;
-      } else {
-        if (response.status === 'active') {
         MailPoet.Notice.success(MailPoet.I18n.t('postNotificationActivated'));
       }
       // force refresh of listing so that groups are updated
       this.forceUpdate();
-      }
+    }).fail((response) => {
-    }.bind(this));
+      MailPoet.Notice.error(MailPoet.I18n.t('postNotificationActivationFailed'));
+
+      // reset value to actual newsletter's status
+      e.target.value = response.status;
+    });
   },
   renderStatus: function(newsletter) {
     return (

assets/js/src/newsletters/listings/welcome.jsx

@@ -156,20 +156,18 @@ const NewsletterListWelcome = React.createClass({
         id: ~~(e.target.getAttribute('data-id')),
         status: e.target.value
       }
-    }).done(function(response) {
+    }).done((response) => {
-      if (response.result === false) {
+      if (response.data.status === 'active') {
-        MailPoet.Notice.error(MailPoet.I18n.t('welcomeEmailActivationFailed'));
-
-        // reset value to actual newsletter's status
-         e.target.value = response.status;
-      } else {
-        if (response.status === 'active') {
         MailPoet.Notice.success(MailPoet.I18n.t('welcomeEmailActivated'));
       }
       // force refresh of listing so that groups are updated
       this.forceUpdate();
-      }
+    }).fail((response) => {
-    }.bind(this));
+      MailPoet.Notice.error(MailPoet.I18n.t('welcomeEmailActivationFailed'));
+
+      // reset value to actual newsletter's status
+      e.target.value = response.status;
+    });
   },
   renderStatus: function(newsletter) {
     let total_sent;

assets/js/src/newsletters/send.jsx

@@ -87,6 +87,15 @@ define(
       handleSend: function(e) {
         e.preventDefault();
 
+        const onFail = (response) => {
+          if (response.errors.length > 0) {
+            MailPoet.Notice.error(
+              response.errors.map(function(error) { return error.message; }),
+              { scroll: true }
+            );
+          }
+        };
+
         if(!this.isValid()) {
           jQuery('#mailpoet_newsletter').parsley().validate();
         } else {
@@ -96,43 +105,57 @@ define(
             endpoint: 'newsletters',
             action: 'save',
             data: this.state.item,
-          }).then((response) => {
+          }).done((response) => {
-            if (response.result === true) {
+            switch (response.data.type) {
+              case 'notification':
+              case 'welcome':
+                return MailPoet.Ajax.post({
+                  endpoint: 'newsletters',
+                  action: 'setStatus',
+                  data: {
+                    id: this.props.params.id,
+                    status: 'active'
+                  }
+                }).done((response) => {
+                  // redirect to listing based on newsletter type
+                  this.context.router.push(`/${ this.state.item.type || '' }`);
+
+                  // display success message depending on newsletter type
+                  if (response.data.type === 'welcome') {
+                    MailPoet.Notice.success(
+                      MailPoet.I18n.t('welcomeEmailActivated')
+                    );
+                  } else if (response.data.type === 'notification') {
+                    MailPoet.Notice.success(
+                      MailPoet.I18n.t('postNotificationActivated')
+                    );
+                  }
+                }).fail(onFail);
+              default:
                 return MailPoet.Ajax.post({
                   endpoint: 'sendingQueue',
                   action: 'add',
                   data: {
-                  id: this.props.params.id
+                    newsletter_id: this.props.params.id
                   }
-              });
-            } else {
-              return response;
-            }
-          }).always(() => {
-            this.setState({ loading: false });
                 }).done((response) => {
                   // redirect to listing based on newsletter type
                   this.context.router.push(`/${ this.state.item.type || '' }`);
-            // display success message depending on newsletter type
+
-            if (this.state.item.type === 'welcome') {
-              MailPoet.Notice.success(MailPoet.I18n.t('welcomeEmailActivated'));
-            } else if (this.state.item.type === 'notification') {
-              MailPoet.Notice.success(MailPoet.I18n.t('postNotificationActivated'));
-            } else {
                   if (response.data.status === 'scheduled') {
-                MailPoet.Notice.success(MailPoet.I18n.t('newsletterHasBeenScheduled'));
+                    MailPoet.Notice.success(
+                      MailPoet.I18n.t('newsletterHasBeenScheduled')
+                    );
                   } else {
-                MailPoet.Notice.success(MailPoet.I18n.t('newsletterBeingSent'));
+                    MailPoet.Notice.success(
-              }
+                      MailPoet.I18n.t('newsletterBeingSent')
-            }
-          }).fail((response) => {
-            if (response.errors.length > 0) {
-              MailPoet.Notice.error(
-                response.errors.map(function(error) { return error.message; }),
-                { scroll: true }
                     );
                   }
-          });
+                }).fail(onFail);
+            }
+          }).always(() => {
+            this.setState({ loading: false });
+          }).fail(onFail);
         }
         return false;
       },

assets/js/src/newsletters/templates.jsx

@@ -134,16 +134,17 @@ define(
             id: this.props.params.id,
             body: body
           }
-        }).done(function(response) {
+        }).done((response) => {
-          if(response.result === true) {
           // TODO: Move this URL elsewhere
-            window.location = 'admin.php?page=mailpoet-newsletter-editor&id=' + this.props.params.id;
+          window.location = 'admin.php?page=mailpoet-newsletter-editor&id=' + response.data.id;
-          } else {
+        }).fail((response) => {
-            response.errors.map(function(error) {
+          if (response.errors.length > 0) {
-              MailPoet.Notice.error(error);
+            MailPoet.Notice.error(
-            });
+              response.errors.map(function(error) { return error.message; }),
+              { scroll: true }
+            );
           }
-        }.bind(this));
+        });
       },
       handleDeleteTemplate: function(template) {
         this.setState({ loading: true });

lib/API/Endpoints/Newsletters.php

@@ -1,5 +1,7 @@
 <?php
 namespace MailPoet\API\Endpoints;
+use MailPoet\API\Endpoint as APIEndpoint;
+use MailPoet\API\Error as APIError;
 
 use MailPoet\Listing;
 use MailPoet\Models\Newsletter;
@@ -18,10 +20,7 @@ if(!defined('ABSPATH')) exit;
 
 require_once(ABSPATH . 'wp-includes/pluggable.php');
 
-class Newsletters {
+class Newsletters extends APIEndpoint {
-  function __construct() {
-  }
-
   function get($id = false) {
     $newsletter = Newsletter::findOne($id);
     if($newsletter === false) {
@@ -55,10 +54,7 @@ class Newsletters {
     $errors = $newsletter->getErrors();
 
     if(!empty($errors)) {
-      return array(
+      return $this->badRequest($errors);
-        'result' => false,
-        'errors' => $errors
-      );
     } else {
       if(!empty($segment_ids)) {
         NewsletterSegment::where('newsletter_id', $newsletter->id)
@@ -92,33 +88,40 @@ class Newsletters {
         }
       }
 
-      return array(
+      return $this->successResponse(
-        'result' => true
+        Newsletter::findOne($newsletter->id)->asArray()
       );
     }
   }
 
   function setStatus($data = array()) {
-    $id = (isset($data['id'])) ? (int)$data['id'] : null;
+    $id = (isset($data['id'])) ? (int)$data['id'] : false;
     $newsletter = Newsletter::findOne($id);
     $status = (isset($data['status']) ? $data['status'] : null);
 
-    $result = false;
+    if(!$status) {
-    $errors = array();
+      return $this->badRequest(array(
-
+        APIError::BAD_REQUEST  => __('You need to specify a status.')
-    if($newsletter !== false && $status !== null) {
+      ));
-      $newsletter->setStatus($status);
-
-      $result = (
-        $newsletter->getErrors() === false && $newsletter->status === $status
-      );
     }
 
-    return array(
+    if($newsletter === false) {
-      'result' => $result,
+      return $this->errorResponse(array(
-      'status' => $newsletter->status
+        APIError::NOT_FOUND => __('This newsletter does not exist.')
+      ));
+    }
+
+    $newsletter->setStatus($status);
+    $errors = $newsletter->getErrors();
+
+    if(!empty($errors)) {
+      return $this->errorResponse($errors);
+    } else {
+      return $this->successResponse(
+        $newsletter->asArray()
       );
     }
+  }
 
   function restore($id) {
     $newsletter = Newsletter::findOne($id);
@@ -189,7 +192,7 @@ class Newsletters {
   }
 
   function sendPreview($data = array()) {
-    $id = (isset($data['id'])) ? (int)$data['id'] : null;
+    $id = (isset($data['id'])) ? (int)$data['id'] : false;
     $newsletter = Newsletter::findOne($id);
 
     if($newsletter === false) {

lib/API/Endpoints/SendingQueue.php

@@ -18,10 +18,14 @@ if(!defined('ABSPATH')) exit;
 
 class SendingQueue extends APIEndpoint {
   function add($data = array()) {
-    $id = (isset($data['id']) ? (int)$data['id'] : null);
+    $newsletter_id = (isset($data['newsletter_id'])
+      ? (int)$data['newsletter_id']
+      : false
+    );
 
     // check that the newsletter exists
-    $newsletter = Newsletter::filter('filterWithOptions')->findOne($id);
+    $newsletter = Newsletter::filter('filterWithOptions')
+      ->findOne($newsletter_id);
 
     if($newsletter === false) {
       return $this->errorResponse(array(
@@ -38,23 +42,7 @@ class SendingQueue extends APIEndpoint {
       ));
     }
 
-    // Automated newsletters (welcome & post notification)
+    // add newsletter to the sending queue
-    if($newsletter->type === Newsletter::TYPE_WELCOME ||
-       $newsletter->type === Newsletter::TYPE_NOTIFICATION
-    ) {
-      // set newsletter status to active
-      $result = $newsletter->setStatus(Newsletter::STATUS_ACTIVE);
-      $errors = $result->getErrors();
-
-      if(!empty($errors)) {
-        return $this->errorResponse($errors);
-      } else {
-        return $this->successResponse(
-          Newsletter::findOne($newsletter->id)->asArray()
-        );
-      }
-    } else {
-      // Standard newsletters
     $queue = SendingQueueModel::whereNull('status')
     ->where('newsletter_id', $newsletter->id)
     ->findOne();
@@ -121,11 +109,13 @@ class SendingQueue extends APIEndpoint {
       );
     }
   }
-  }
 
   function pause($data = array()) {
-    $id = (isset($data['id']) ? (int)$data['id'] : null);
+    $newsletter_id = (isset($data['newsletter_id'])
-    $newsletter = Newsletter::findOne($id);
+      ? (int)$data['newsletter_id']
+      : false
+    );
+    $newsletter = Newsletter::findOne($newsletter_id);
 
     if($newsletter === false) {
       return $this->errorResponse(array(
@@ -148,9 +138,11 @@ class SendingQueue extends APIEndpoint {
   }
 
   function resume($data = array()) {
-    $id = (isset($data['id']) ? (int)$data['id'] : null);
+    $newsletter_id = (isset($data['newsletter_id'])
-    $newsletter = Newsletter::findOne($id);
+      ? (int)$data['newsletter_id']
-
+      : false
+    );
+    $newsletter = Newsletter::findOne($newsletter_id);
     if($newsletter === false) {
       return $this->errorResponse(array(
         APIError::NOT_FOUND => __('This newsletter does not exist.')

lib/API/Error.php

@@ -8,6 +8,7 @@ final class Error {
   const BAD_REQUEST = 'bad_request';
   const UNAUTHORIZED = 'unauthorized';
   const FORBIDDEN = 'forbidden';
+  const NOT_FOUND = 'not_found';
 
   private function __construct() {
 

tests/unit/API/NewslettersTest.php

@@ -1,4 +1,6 @@
 <?php
+use \MailPoet\API\Response as APIResponse;
+use \MailPoet\API\Error as APIError;
 use \MailPoet\API\Endpoints\Newsletters;
 use \MailPoet\Models\Newsletter;
 use \MailPoet\Models\NewsletterSegment;
@@ -39,16 +41,18 @@ class NewslettersTest extends MailPoetTest {
 
     $router = new Newsletters();
     $response = $router->save($valid_data);
-    expect($response['result'])->true();
+    expect($response->status)->equals(APIResponse::STATUS_OK);
-    expect($response)->hasntKey('errors');
+    expect($response->data)->equals(
+      Newsletter::findOne($response->data['id'])->asArray()
+    );
 
     $invalid_data = array(
       'subject' => 'Missing newsletter type'
     );
 
     $response = $router->save($invalid_data);
-    expect($response['result'])->false();
+    expect($response->status)->equals(APIResponse::STATUS_BAD_REQUEST);
-    expect($response['errors'][0])->equals('Please specify a type');
+    expect($response->errors[0]['message'])->equals('Please specify a type');
   }
 
   function testItCanSaveAnExistingNewsletter() {
@@ -59,12 +63,50 @@ class NewslettersTest extends MailPoetTest {
     );
 
     $response = $router->save($newsletter_data);
-    expect($response['result'])->true();
+    expect($response->status)->equals(APIResponse::STATUS_OK);
+    expect($response->data)->equals(
+      Newsletter::findOne($this->newsletter->id)->asArray()
+    );
 
-    $updated_newsletter = Newsletter::findOne($this->newsletter->id());
+    $updated_newsletter = Newsletter::findOne($this->newsletter->id);
     expect($updated_newsletter->subject)->equals('My Updated Newsletter');
   }
 
+  function testItCanSetANewsletterStatus() {
+    $router = new Newsletters();
+    // set status to 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
+    ));
+    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,
+    ));
+    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
+    ));
+    expect($response->status)->equals(APIResponse::STATUS_NOT_FOUND);
+    expect($response->errors[0]['message'])
+      ->equals('This newsletter does not exist.');
+  }
+
   function testItCanRestoreANewsletter() {
     $this->newsletter->trash();