From c22d3c8957b80c1e95884b035a8ebf3d7c54366a Mon Sep 17 00:00:00 2001
From: Jonathan Labreuille <jonathan@mailpoet.com>
Date: Wed, 9 Nov 2016 11:43:08 +0100
Subject: [PATCH] Renamed 'ip' column to 'subscribed_ip' - updated code based
 on PR review

---
 lib/Config/Migrator.php                      |  2 +-
 lib/Models/Subscriber.php                    | 13 ++++++------
 lib/Subscription/Pages.php                   |  3 ---
 tests/unit/API/Endpoints/SubscribersTest.php | 22 +++++++++++++-------
 4 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/lib/Config/Migrator.php b/lib/Config/Migrator.php
index 36c7395105..cc8f4223a8 100644
--- a/lib/Config/Migrator.php
+++ b/lib/Config/Migrator.php
@@ -134,7 +134,7 @@ class Migrator {
       'last_name tinytext NOT NULL DEFAULT "",',
       'email varchar(150) NOT NULL,',
       'status varchar(12) NOT NULL DEFAULT "' . Subscriber::STATUS_UNCONFIRMED . '",',
-      'ip varchar(32) NULL,',
+      'subscribed_ip varchar(32) NULL,',
       'confirmed_ip varchar(32) NULL,',
       'confirmed_at TIMESTAMP NULL,',
       'created_at TIMESTAMP NULL,',
diff --git a/lib/Models/Subscriber.php b/lib/Models/Subscriber.php
index 338468c7d1..a872d90ec3 100644
--- a/lib/Models/Subscriber.php
+++ b/lib/Models/Subscriber.php
@@ -160,18 +160,18 @@ class Subscriber extends Model {
       'signup_confirmation.enabled'
     );
 
-    // set user ip
-    $subscriber_data['ip'] = (isset($_SERVER['REMOTE_ADDR']))
+    $subscriber_data['subscribed_ip'] = (isset($_SERVER['REMOTE_ADDR']))
       ? $_SERVER['REMOTE_ADDR']
       : null;
 
     // make sure we don't allow too many subscriptions with the same ip address
-    $subscription_count = Subscriber::where('ip', $subscriber_data['ip'])
-      ->whereRaw(
+    $subscription_count = Subscriber::where(
+        'subscribed_ip',
+        $subscriber_data['subscribed_ip']
+      )->whereRaw(
         'TIME_TO_SEC(TIMEDIFF(NOW(), created_at)) < ?',
         self::SUBSCRIPTION_LIMIT_COOLDOWN
-      )
-      ->count();
+      )->count();
 
     if($subscription_count > 0) {
       throw new \Exception(__('You need to wait before subscribing again.', 'mailpoet'));
@@ -180,7 +180,6 @@ class Subscriber extends Model {
     $subscriber = self::findOne($subscriber_data['email']);
 
     if($subscriber === false) {
-
       // create new subscriber
       $subscriber = self::createOrUpdate($subscriber_data);
       if($subscriber->getErrors() !== false) {
diff --git a/lib/Subscription/Pages.php b/lib/Subscription/Pages.php
index fc9bbed56a..688452acc2 100644
--- a/lib/Subscription/Pages.php
+++ b/lib/Subscription/Pages.php
@@ -58,12 +58,9 @@ class Pages {
 
   function confirm() {
     if($this->subscriber !== false) {
-      // update status
       $this->subscriber->status = Subscriber::STATUS_SUBSCRIBED;
-      // set confirmed ip & time
       $this->subscriber->confirmed_ip = $_SERVER['REMOTE_ADDR'];
       $this->subscriber->setExpr('confirmed_at', 'NOW()');
-
       $this->subscriber->save();
     }
   }
diff --git a/tests/unit/API/Endpoints/SubscribersTest.php b/tests/unit/API/Endpoints/SubscribersTest.php
index e957df7493..aee9e67967 100644
--- a/tests/unit/API/Endpoints/SubscribersTest.php
+++ b/tests/unit/API/Endpoints/SubscribersTest.php
@@ -361,26 +361,34 @@ class SubscribersTest extends MailPoetTest {
     expect($response->errors[0]['message'])->contains('has no method');
   }
 
-  function testItCanSubscribeAUser() {
-    // set ip address
-    $_SERVER['REMOTE_ADDR'] = '127.0.0.1';
-
+  function testItCannotSubscribeWithoutSegments() {
     $router = new Subscribers();
-
-    // missing segment
     $response = $router->subscribe(array(
       'email' => 'toto@mailpoet.com'
+      // no segments specified
     ));
 
     expect($response->status)->equals(APIResponse::STATUS_BAD_REQUEST);
     expect($response->errors[0]['message'])->equals('Please select a list.');
+  }
 
-    // proper subscription
+  function testItCanSubscribe() {
+    $router = new Subscribers();
     $response = $router->subscribe(array(
       'email' => 'toto@mailpoet.com',
       'segments' => array($this->segment_1->id, $this->segment_2->id)
     ));
     expect($response->status)->equals(APIResponse::STATUS_OK);
+  }
+
+  function testItCannotMassSubscribe() {
+    $_SERVER['REMOTE_ADDR'] = '127.0.0.1';
+
+    $router = new Subscribers();
+    $response = $router->subscribe(array(
+      'email' => 'toto@mailpoet.com',
+      'segments' => array($this->segment_1->id, $this->segment_2->id)
+    ));
 
     try {
       $response = $router->subscribe(array(