Reorganized new API + added legacy API support + new API

- Updated Settings Router to new standards
- Updated settings.html to reflect API change with better error handling
- Updated Settings API unit tests

assets/js/src/ajax.js

@@ -8,9 +8,7 @@ define('ajax', ['mailpoet', 'jquery', 'underscore'], function(MailPoet, jQuery,
         endpoint: null,
         action: null,
         token: null,
-        data: {},
+        data: {}
-        onSuccess: function(data, textStatus, xhr) {},
-        onError: function(xhr, textStatus, errorThrown) {}
       },
       get: function(options) {
         return this.request('get', options);
@@ -72,9 +70,7 @@ define('ajax', ['mailpoet', 'jquery', 'underscore'], function(MailPoet, jQuery,
             url: this.options.url,
             type : 'post',
             data: params,
-            dataType: 'json',
+            dataType: 'json'
-            success : this.options.onSuccess,
-            error : this.options.onError
           });
         }
 

lib/API/API.php

@@ -35,14 +35,34 @@ class API {
   }
 
   function setupAdmin() {
-    $this->verifyToken();
+    if($this->checkToken() === false) {
-    $this->checkPermissions();
+      $this->errorResponse(
-    return $this->processRoute();
+        array('unauthorized' => __('This request is not authorized.')),
+        array(),
+        APIResponse::STATUS_UNAUTHORIZED
+      )->send();
+    }
+
+    if($this->checkPermissions() === false) {
+      $this->errorResponse(
+        array('forbidden' => __('You do not have the required permissions.')),
+        array(),
+        APIResponse::STATUS_FORBIDDEN
+      )->send();
+    }
+
+    $this->processRoute();
   }
 
   function setupPublic() {
-    $this->verifyToken();
+    if($this->checkToken() === false) {
-    return $this->processRoute();
+      $response = new ErrorResponse(array(
+        'unauthorized' => __('This request is not authorized.')
+      ), APIResponse::STATUS_UNAUTHORIZED);
+      $response->send();
+    }
+
+    $this->processRoute();
   }
 
   function processRoute() {
@@ -72,9 +92,18 @@ class API {
     try {
       $endpoint = new $endpoint();
       $response = $endpoint->$method($data);
-      wp_send_json($response);
+
+      // TODO: remove this condition once the API unification is complete
+      if(is_object($response)) {
+        $response->send();
+      } else {
+        // LEGACY API
+        wp_send_json($response);
+      }
     } catch(\Exception $e) {
-      exit;
+      $this->errorResponse(array(
+        $e->getMessage()
+      ))->send();
     }
   }
 
@@ -86,18 +115,32 @@ class API {
   }
 
   function checkPermissions() {
-    if(!current_user_can('manage_options')) {
+    return current_user_can('manage_options');
-      die();
-    }
   }
 
-  function verifyToken() {
+  function checkToken() {
-    if(
+    return (
-      empty($_POST['token'])
+      isset($_POST['token'])
-      ||
+      &&
-      !wp_verify_nonce($_POST['token'], 'mailpoet_token')
+      wp_verify_nonce($_POST['token'], 'mailpoet_token')
-    ) {
+    );
-      die();
-    }
   }
-}
+
+  function successResponse(
+    $data = array(), $meta = array(), $status = APIResponse::STATUS_OK
+  ) {
+
+    return new SuccessResponse($data, $meta, $status);
+  }
+
+  function errorResponse(
+    $errors = array(), $meta = array(), $status = APIResponse::STATUS_NOT_FOUND
+  ) {
+
+    return new ErrorResponse($errors, $meta, $status);
+  }
+
+  function badRequest($errors = array(), $meta = array()) {
+    return new ErrorResponse($errors, $meta, APIResponse::STATUS_BAD_REQUEST);
+  }
+}

lib/API/APIResponse.php

@@ -0,0 +1,88 @@
+<?php
+namespace MailPoet\API;
+abstract class APIResponse {
+  const STATUS_OK = 200;
+  const STATUS_BAD_REQUEST = 400;
+  const STATUS_UNAUTHORIZED = 401;
+  const STATUS_FORBIDDEN = 403;
+  const STATUS_NOT_FOUND = 404;
+
+  public $status;
+  public $meta;
+
+  function __construct($status, $meta = array()) {
+    $this->status = $status;
+    $this->meta = $meta;
+  }
+
+  function send() {
+    status_header($this->status);
+
+    $data = $this->getData();
+    $response = array();
+
+    if(!empty($this->meta)) {
+      $response['meta'] = $this->meta;
+    }
+    if(!empty($data)) {
+      $response = array_merge($response, $data);
+    }
+
+    if(empty($response)) {
+      die();
+    } else {
+      wp_send_json($response);
+    }
+  }
+
+  abstract function getData();
+}
+
+class SuccessResponse extends APIResponse {
+  public $data;
+
+  function __construct($data = array(), $meta = array(), $status = self::STATUS_OK) {
+    parent::__construct($status, $meta);
+    $this->data = $data;
+  }
+
+  function getData() {
+    if(empty($this->data)) {
+      return false;
+    } else {
+      return array(
+        'data' => $this->data
+      );
+    }
+  }
+}
+
+class ErrorResponse extends APIResponse {
+  public $errors;
+
+  function __construct($errors = array(), $meta = array(), $status = self::STATUS_NOT_FOUND) {
+    parent::__construct($status, $meta);
+    $this->errors = $this->formatErrors($errors);
+  }
+
+  function getData() {
+    if(empty($this->errors)) {
+      return false;
+    } else {
+      return array(
+        'errors' => $this->errors
+      );
+    }
+  }
+
+  function formatErrors($errors = array()) {
+    $formatted_errors = array();
+    foreach($errors as $error => $message) {
+      $formatted_errors[] = array(
+        'error' => $error,
+        'message' => $message
+      );
+    }
+    return $formatted_errors;
+  }
+}

lib/API/AutomatedLatestContent.php

@@ -1,6 +1,6 @@
 <?php
-namespace MailPoet\API;
+namespace MailPoet\API
-
+    ;
 
 if(!defined('ABSPATH')) exit;
 

lib/API/Cron.php

@@ -1,6 +1,6 @@
 <?php
-namespace MailPoet\API;
+namespace MailPoet\API
-
+    ;
 
 use MailPoet\Cron\CronHelper;
 use MailPoet\Cron\Supervisor;

lib/API/CustomFields.php

@@ -1,5 +1,6 @@
 <?php
-namespace MailPoet\API;
+namespace MailPoet\API
+    ;
 
 use \MailPoet\Models\CustomField;
 

lib/API/Forms.php

@@ -1,5 +1,6 @@
 <?php
-namespace MailPoet\API;
+namespace MailPoet\API
+    ;
 
 use \MailPoet\Models\Form;
 use \MailPoet\Models\StatisticsForms;

lib/API/ImportExport.php

@@ -1,6 +1,6 @@
 <?php
-namespace MailPoet\API;
+namespace MailPoet\API
-
+    ;
 
 use MailPoet\Subscribers\ImportExport\Import\MailChimp;
 use MailPoet\Models\CustomField;

lib/API/Mailer.php

@@ -1,6 +1,6 @@
 <?php
-namespace MailPoet\API;
+namespace MailPoet\API
-
+    ;
 
 if(!defined('ABSPATH')) exit;
 

lib/API/NewsletterTemplates.php

@@ -1,6 +1,6 @@
 <?php
-namespace MailPoet\API;
+namespace MailPoet\API
-
+    ;
 
 use MailPoet\Models\NewsletterTemplate;
 

lib/API/Newsletters.php

@@ -1,6 +1,6 @@
 <?php
-namespace MailPoet\API;
+namespace MailPoet\API
-
+    ;
 
 use MailPoet\Listing;
 use MailPoet\Models\Newsletter;

lib/API/Segments.php

@@ -1,5 +1,6 @@
 <?php
-namespace MailPoet\API;
+namespace MailPoet\API
+    ;
 
 use \MailPoet\Models\Segment;
 use \MailPoet\Models\SubscriberSegment;

lib/API/SendingQueue.php

@@ -1,6 +1,6 @@
 <?php
-namespace MailPoet\API;
+namespace MailPoet\API
-
+    ;
 
 use MailPoet\Mailer\Mailer;
 use MailPoet\Models\Newsletter;

lib/API/Settings.php

@@ -1,27 +1,26 @@
 <?php
 namespace MailPoet\API;
-
 use \MailPoet\Models\Setting;
 
 if(!defined('ABSPATH')) exit;
 
-class Settings {
+class Settings extends API {
   function __construct() {
   }
 
   function get() {
     $settings = Setting::getAll();
-    return $settings;
+    return $this->successResponse($settings);
   }
 
   function set($settings = array()) {
     if(empty($settings)) {
-      return false;
+      return $this->badRequest();
     } else {
       foreach($settings as $name => $value) {
         Setting::setValue($name, $value);
       }
-      return true;
+      return $this->successResponse();
     }
   }
 }

lib/API/Setup.php

@@ -1,5 +1,6 @@
 <?php
-namespace MailPoet\API;
+namespace MailPoet\API
+    ;
 
 use \MailPoet\Config\Activator;
 

lib/API/Subscribers.php

@@ -1,6 +1,6 @@
 <?php
-namespace MailPoet\API;
+namespace MailPoet\API
-
+    ;
 
 use MailPoet\Listing;
 use MailPoet\Models\Subscriber;

tests/unit/API/CustomFieldsTest.php

@@ -1,7 +1,6 @@
 <?php
 
-use \MailPoet\API
+use \MailPoet\API\CustomFields;
-    \CustomFields;
 use \MailPoet\Models\CustomField;
 
 class CustomFieldsTest extends MailPoetTest {

tests/unit/API/FormsTest.php

@@ -1,6 +1,5 @@
 <?php
-use \MailPoet\API
+use \MailPoet\API\Forms;
-    \Forms;
 use \MailPoet\Models\Form;
 use \MailPoet\Models\Segment;
 

tests/unit/API/NewsletterTemplatesTest.php

@@ -1,6 +1,5 @@
 <?php
-use \MailPoet\API
+use \MailPoet\API\NewsletterTemplates;
-    \NewsletterTemplates;
 use \MailPoet\Models\NewsletterTemplate;
 
 class NewsletterTemplatesTest extends MailPoetTest {

tests/unit/API/NewslettersTest.php

@@ -1,6 +1,5 @@
 <?php
-use \MailPoet\API
+use \MailPoet\API\Newsletters;
-    \Newsletters;
 use \MailPoet\Models\Newsletter;
 use \MailPoet\Models\NewsletterSegment;
 use \MailPoet\Models\NewsletterTemplate;

tests/unit/API/SegmentsTest.php

@@ -1,6 +1,5 @@
 <?php
-use \MailPoet\API
+use \MailPoet\API\Segments;
-    \Segments;
 use \MailPoet\Models\Segment;
 
 class SegmentsTest extends MailPoetTest {

tests/unit/API/SettingsTest.php

@@ -1,6 +1,6 @@
 <?php
-use \MailPoet\API
+use \MailPoet\API\APIResponse;
-    \Settings;
+use \MailPoet\API\Settings;
 use \MailPoet\Models\Setting;
 
 class SettingsTest extends MailPoetTest {
@@ -11,13 +11,16 @@ class SettingsTest extends MailPoetTest {
   function testItCanGetSettings() {
     $router = new Settings();
 
-    $settings = $router->get();
+    $response = $router->get();
-    expect($settings)->notEmpty();
+    expect($response->status)->equals(APIResponse::STATUS_OK);
-    expect($settings['some']['setting']['key'])->true();
+
+    expect($response->data)->notEmpty();
+    expect($response->data['some']['setting']['key'])->true();
 
     Setting::deleteMany();
-    $settings = $router->get();
+    $response = $router->get();
-    expect($settings)->equals(Setting::getDefaults());
+    expect($response->status)->equals(APIResponse::STATUS_OK);
+    expect($response->data)->equals(Setting::getDefaults());
   }
 
   function testItCanSetSettings() {
@@ -33,16 +36,16 @@ class SettingsTest extends MailPoetTest {
     $router = new Settings();
 
     $response = $router->set(/* missing data */);
-    expect($response)->false();
+    expect($response->status)->equals(APIResponse::STATUS_BAD_REQUEST);
 
     $response = $router->set($new_settings);
-    expect($response)->true();
+    expect($response->status)->equals(APIResponse::STATUS_OK);
 
-    $settings = $router->get();
+    $response = $router->get();
-
+    expect($response->status)->equals(APIResponse::STATUS_OK);
-    expect($settings['some']['setting'])->hasntKey('key');
+    expect($response->data['some']['setting'])->hasntKey('key');
-    expect($settings['some']['setting']['new_key'])->true();
+    expect($response->data['some']['setting']['new_key'])->true();
-    expect($settings['some']['new_setting'])->true();
+    expect($response->data['some']['new_setting'])->true();
   }
 
   function _after() {

tests/unit/API/SetupTest.php

@@ -1,6 +1,5 @@
 <?php
-use \MailPoet\API
+use \MailPoet\API\Setup;
-    \Setup;
 use \MailPoet\Models\Setting;
 
 class SetupTest extends MailPoetTest {

tests/unit/API/SubscribersTest.php

@@ -1,7 +1,6 @@
 <?php
 
-use \MailPoet\API
+use \MailPoet\API\Subscribers;
-    \Subscribers;
 use \MailPoet\Models\Subscriber;
 use \MailPoet\Models\Segment;
 

views/settings.html

@@ -77,25 +77,21 @@
             endpoint: 'settings',
             action: 'set',
             data: settings_data
-          }).done(function(response) {
+          }).done(function() {
-            if(response === true) {
+            MailPoet.Notice.success(
-              MailPoet.Notice.success(
+              "<%= __('Settings saved') %>",
-                "<%= __('Settings saved') %>",
+              { scroll: true }
-                { scroll: true }
+            );
-              );
+            MailPoet.Modal.loading(false);
-            } else {
+          }).error(function(xhr) {
+            var response = xhr.responseJSON;
+            if(response.errors !== undefined) {
               MailPoet.Notice.error(
-                "<%= __('Settings could not be saved') %>",
+                response.errors.map(function(error) { return error.message; }),
                 { scroll: true }
               );
             }
             MailPoet.Modal.loading(false);
-          }).error(function(errors) {
-            MailPoet.Notice.error(
-              "<%= __('An error occurred. Please check the settings.') %>",
-              { scroll: true }
-            );
-            MailPoet.Modal.loading(false);
           });
         }
 

views/settings/advanced.html

@@ -41,7 +41,8 @@
               type="radio"
               name="task_scheduler[method]"
               value="WordPress"
-              <% if (settings.task_scheduler.method == 'WordPress') %>
+              <% if not(settings.task_scheduler.method)
+                or (settings.task_scheduler.method == 'WordPress') %>
               checked="checked"
               <% endif %>
             /><%= __('Visitors to your website (recommended)') %>