Merge pull request #798 from mailpoet/form_segments_fix

Filter lists that can be subscribed to using any particular form [MAILPOET-777]

lib/API/Endpoints/Subscribers.php

@@ -71,6 +71,7 @@ class Subscribers extends APIEndpoint {
       ? (array)$data['segments']
       : array()
     );
+    $segment_ids = $form->filterSegments($segment_ids);
     unset($data['segments']);
 
     if(empty($segment_ids)) {
@@ -97,6 +98,7 @@ class Subscribers extends APIEndpoint {
 
         $form = $form->asArray();
 
+        if(!empty($form['settings']['on_success'])) {
           if($form['settings']['on_success'] === 'page') {
             // redirect to a page on a success, pass the page url in the meta
             $meta['redirect_url'] = get_permalink($form['settings']['success_page']);
@@ -104,6 +106,7 @@ class Subscribers extends APIEndpoint {
             $meta['redirect_url'] = $form['settings']['success_url'];
           }
         }
+      }
 
       return $this->successResponse(
         Subscriber::findOne($subscriber->id)->asArray(),

lib/Models/Form.php

@@ -65,6 +65,23 @@ class Form extends Model {
     return $fields ?: false;
   }
 
+  function filterSegments(array $segment_ids = array()) {
+    $form = $this->asArray();
+    if(empty($form['settings']['segments'])) {
+      return array();
+    }
+
+    if(!empty($form['settings']['segments_selected_by'])
+      && $form['settings']['segments_selected_by'] == 'user'
+    ) {
+      $segment_ids = array_intersect($segment_ids, $form['settings']['segments']);
+    } else {
+      $segment_ids = $form['settings']['segments'];
+    }
+
+    return $segment_ids;
+  }
+
   static function search($orm, $search = '') {
     return $orm->whereLike('name', '%'.$search.'%');
   }

tests/unit/API/Endpoints/SubscribersTest.php

@@ -32,7 +32,14 @@ class SubscribersTest extends MailPoetTest {
 
     $this->form = Form::createOrUpdate(array(
       'name' => 'My Form',
-      'body' => Fixtures::get('form_body_template')
+      'body' => Fixtures::get('form_body_template'),
+      'settings' => array(
+        'segments_selected_by' => 'user',
+        'segments' => array(
+          $this->segment_1->id,
+          $this->segment_2->id
+        )
+      )
     ));
 
     // setup mailer
@@ -393,7 +400,7 @@ class SubscribersTest extends MailPoetTest {
     expect($response->errors[0]['message'])->equals('Please specify a valid form ID.');
   }
 
-  function testItCannotSubscribeWithoutSegments() {
+  function testItCannotSubscribeWithoutSegmentsIfTheyAreSelectedByUser() {
     $router = new Subscribers();
     $response = $router->subscribe(array(
       'email' => 'toto@mailpoet.com',
@@ -415,6 +422,45 @@ class SubscribersTest extends MailPoetTest {
     expect($response->status)->equals(APIResponse::STATUS_OK);
   }
 
+  function testItCanSubscribeWithoutSegmentsIfTheyAreSelectedByAdmin() {
+    $form = $this->form->asArray();
+    $form['settings']['segments_selected_by'] = 'admin';
+    $this->form->settings = $form['settings'];
+    $this->form->save();
+
+    $router = new Subscribers();
+    $response = $router->subscribe(array(
+      'email' => 'toto@mailpoet.com',
+      'form_id' => $this->form->id
+      // no segments specified
+    ));
+
+    expect($response->status)->equals(APIResponse::STATUS_OK);
+    $subscriber = Subscriber::findOne($response->data['id']);
+    $subscriber_segments = $subscriber->segments()->findArray();
+    expect($subscriber_segments)->count(2);
+    expect($subscriber_segments[0]['id'])->equals($form['settings']['segments'][0]);
+    expect($subscriber_segments[1]['id'])->equals($form['settings']['segments'][1]);
+  }
+
+  function testItCannotSubscribeIfFormHasNoSegmentsDefined() {
+    $form = $this->form->asArray();
+    $form['settings']['segments_selected_by'] = 'admin';
+    unset($form['settings']['segments']);
+    $this->form->settings = $form['settings'];
+    $this->form->save();
+
+    $router = new Subscribers();
+    $response = $router->subscribe(array(
+      'email' => 'toto@mailpoet.com',
+      'form_id' => $this->form->id,
+      'segments' => array($this->segment_1->id, $this->segment_2->id)
+    ));
+
+    expect($response->status)->equals(APIResponse::STATUS_BAD_REQUEST);
+    expect($response->errors[0]['message'])->equals('Please select a list.');
+  }
+
   function testItCanFilterOutNonFormFieldsWhenSubscribing() {
     $router = new Subscribers();
     $response = $router->subscribe(array(
@@ -429,10 +475,9 @@ class SubscribersTest extends MailPoetTest {
       'bogus' => 'hahaha'
     ));
     expect($response->status)->equals(APIResponse::STATUS_OK);
-    $subscriber = Subscriber::findOne($response->data['id']);
+    expect($response->data['first_name'])->equals('aaa');
-    expect($subscriber->first_name)->equals('aaa');
+    expect($response->data['last_name'])->isEmpty();
-    expect($subscriber->last_name)->isEmpty();
+    expect(isset($response->data['bogus']))->false();
-    expect(isset($subscriber->bogus))->false();
   }
 
   function testItCannotMassSubscribe() {

views/form/widget.html

@@ -18,11 +18,6 @@
       <input type="hidden" name="endpoint" value="subscribers" />
       <input type="hidden" name="method" value="subscribe" />
 
-      <% if not(form.settings.segments_selected_by == 'user') %>
-        <% for segment in form.settings.segments %>
-        <input type="hidden" name="segments[]" value="<%= segment %>" />
-        <% endfor %>
-      <% endif %>
       <%= html | raw %>
       <div class="mailpoet_message">
         <p