-

From 363260815f1a5812d659c6d865ab884b20d46167 Mon Sep 17 00:00:00 2001
From: Adam Phillips <aphillips@scholastic.co.uk>
Date: Thu, 6 Jun 2013 14:15:31 +0100
Subject: [PATCH] Bug 3011: Tell a friend failing to hide the spam trap

There are two parts to this - the spam trap was not being hidden due to missing
CSS and then was erroring on submit as it was missing some code to check how
much time had passed between the initial get request and the post request.

For the first part, the CSS was removed accidentally in commit 62727113a63e0
and so has been added back in. However the id of the containing element has
been removed and a more descriptive class has been added as this seemed more
appropriate. In addition, many references to the #additional_info id were found
in the CSS however as this view is the only place containing an element using
this ID, it is assumed that the remainder are hangovers from when this form was
on more sites and so have been removed.

The second part is essentially a reversion of commit fc11d4d4925 where this
code was thought to be redundant. However it has been reorganised slightly so
that it is abstracted into its own module and included into the appropriate
controller rather than cluttering FatController.

The old tests for the spam trap were re-added when the bug was fixed but have
been converted to the new style.

Also the styles applied to the form have been updated to use the latest styles
and a cancel button has been added.
---
 store/app/controllers/shops/products_controller.rb |  5 ++-
 store/app/stylesheets/lorem/default.less           |  5 ---
 .../stylesheets/patterns/universal_nav/unav.less   |  9 +----
 store/app/stylesheets/pierre/reset.less            |  9 +----
 store/app/stylesheets/shared/forms/base.less       |  8 ++++-
 .../stylesheets/sitespecific/bookfairs/styles.less |  5 ---
 .../stylesheets/sitespecific/education/styles.less |  7 ----
 .../views/shops/products/tell_a_friend.html.erb    | 13 ++++---
 store/lib/session_timing.rb                        | 30 ++++++++++++++++
 .../functional/shops/products_controller_test.rb   | 42 ++++++++++++++++++++++
 10 files changed, 93 insertions(+), 40 deletions(-)
 create mode 100644 store/lib/session_timing.rb

diff --git a/store/app/controllers/shops/products_controller.rb b/store/app/controllers/shops/products_controller.rb
index 30cf8e1..7d4efd7 100644
--- a/store/app/controllers/shops/products_controller.rb
+++ b/store/app/controllers/shops/products_controller.rb
@@ -1,8 +1,11 @@
 class Shops::ProductsController < Shops::ShopsParentController
+  include SessionTiming
+
   before_filter :find_product, :except => [:register]
   before_filter :get_products_promos, :except => [:register]
   before_filter :login_required, :only => [:register]
-  
+  before_filter :store_get_time_in_session, :only => [:tell_a_friend]
+
   def show
     params[:id] = @product.id unless params[:id] # Caching uses the params[:id]
     if !@shop_config.show_currency_switcher? || @shop_session[:currency] == @shop_config.default_currency
diff --git a/store/app/stylesheets/lorem/default.less b/store/app/stylesheets/lorem/default.less
index 565e578..933b411 100644
--- a/store/app/stylesheets/lorem/default.less
+++ b/store/app/stylesheets/lorem/default.less
@@ -27,11 +27,6 @@
  padding-bottom: 0;
 }
 
-form div#additional_info {
-    position:absolute;
-    left:-999em;
-}
-
 #send-to-friend input.text, #send-to-friend textarea { 
  	border: 1px solid #999;
  	font-size: 14px;
diff --git a/store/app/stylesheets/patterns/universal_nav/unav.less b/store/app/stylesheets/patterns/universal_nav/unav.less
index ddd349f..4b8439b 100644
--- a/store/app/stylesheets/patterns/universal_nav/unav.less
+++ b/store/app/stylesheets/patterns/universal_nav/unav.less
@@ -716,13 +716,6 @@ li.hover .subnav {
   display: none;
 }
 
-#additional_info {
-  position: absolute;
-  left: -999em;
-  top: 0;
-  overflow: hidden;
-}
-
 /* shop-search */
 #site-search { /* The search form at the top of the left hand colum */
   bottom: 14px;
@@ -839,4 +832,4 @@ li.hover .subnav {
 
 .js #site-search .search-options {
   margin-top: -66px;
-}
\ No newline at end of file
+}
diff --git a/store/app/stylesheets/pierre/reset.less b/store/app/stylesheets/pierre/reset.less
index e4d0ea2..20f95eb 100644
--- a/store/app/stylesheets/pierre/reset.less
+++ b/store/app/stylesheets/pierre/reset.less
@@ -1,13 +1,6 @@
 /* == PLEASE *DO NOT* EDIT THIS FILE, NOT EVEN A LITTLE BIT. CONTACT JOHN OR STEFF FOR MORE INFO. THANKS == */      
 
 
-/* jo added quick fix for invisible capture on send to a friend  19th January 2011  added to shop too, think this needs to go into Lorem et al */
-form div#additional_info{  
-    position:absolute;
-    left:-999em;
-    
-}
-
 /* http://meyerweb.com/eric/thoughts/2007/05/01/reset-reloaded/ */
 
 html, body, div, span, applet, object, iframe,
@@ -55,4 +48,4 @@ q:before, q:after {
 }
 blockquote, q {
 	quotes: "" "";
-}
\ No newline at end of file
+}
diff --git a/store/app/stylesheets/shared/forms/base.less b/store/app/stylesheets/shared/forms/base.less
index 8fc8de2..8989fb5 100644
--- a/store/app/stylesheets/shared/forms/base.less
+++ b/store/app/stylesheets/shared/forms/base.less
@@ -187,6 +187,11 @@
       padding:5px;
     }
   }
+
+  .spam-trap {
+      position:absolute;
+      left:-999em;
+  }
 }
 
 .lt-ie8 .form {
@@ -215,4 +220,5 @@
   button {
     overflow:visible;
   }
-}
\ No newline at end of file
+}
+
diff --git a/store/app/stylesheets/sitespecific/bookfairs/styles.less b/store/app/stylesheets/sitespecific/bookfairs/styles.less
index 544101e..2f10d56 100644
--- a/store/app/stylesheets/sitespecific/bookfairs/styles.less
+++ b/store/app/stylesheets/sitespecific/bookfairs/styles.less
@@ -893,11 +893,6 @@ body.region-ie .UKonly, body.region-uk .IEonly {
 }
 
 
-form div#additional_info {
-  position:absolute;
-  left:-999em;
-}
-
 
 
 /*  Sneak preview CSS
diff --git a/store/app/stylesheets/sitespecific/education/styles.less b/store/app/stylesheets/sitespecific/education/styles.less
index 693634a..fab31bf 100644
--- a/store/app/stylesheets/sitespecific/education/styles.less
+++ b/store/app/stylesheets/sitespecific/education/styles.less
@@ -38,13 +38,6 @@
  right: 0;
 } 
 
-/* jo added quick fix for invisible capture on send to a friend  19th January 2011  added to shop too, think this needs to go into Lorem et al */
-form div#additional_info{  
-    position:absolute;
-    left:-999em;
-    
-}
-
 
 /* 1. ==@import less files
 -------------------------------------------------------------- */
diff --git a/store/app/views/shops/products/tell_a_friend.html.erb b/store/app/views/shops/products/tell_a_friend.html.erb
index 0d9f755..7b9a8d4 100644
--- a/store/app/views/shops/products/tell_a_friend.html.erb
+++ b/store/app/views/shops/products/tell_a_friend.html.erb
@@ -6,7 +6,7 @@
 
 <!--googleoff: all-->
 
-<%= form_tag({:action => "tell_a_friend", :id => @product.id}, :class => "six-columns block-labels") do -%>
+<%= form_tag({:action => "tell_a_friend", :id => @product.id}, :class => "six-columns form") do -%>
   <p><em><%= required_field %> indicates a required field.</em></p>
 
   <fieldset>
@@ -24,14 +24,17 @@
       <%= text_area_tag :message, @message, :size => "40x20" %>
     </li>
 
-    <div id="additional_info">
+    <li class="spam-trap">
       <%= label_tag :more_info, "We're trying to fool the bots, do not fill this in" %>
       <%= text_field_tag :more_info %>
-    </div>
+    </li>
 
-    <li><%= submit_tag "Send" %></li>
+    <li class="form-actions">
+      <%= submit_tag 'Send', :class => 'submit form-action' %>
+      <%= link_to 'Cancel', url_for_product(@product) %>
+    </li>
 	</ul>
 	</fieldset>
 <% end %>
 
-<!--googleon: all-->
\ No newline at end of file
+<!--googleon: all-->
diff --git a/store/lib/session_timing.rb b/store/lib/session_timing.rb
new file mode 100644
index 0000000..c541fb8
--- /dev/null
+++ b/store/lib/session_timing.rb
@@ -0,0 +1,30 @@
+# Module for working with Session Timings. These are used by the Spam trap as
+# one way of filtering out spam submissions by bots.
+#
+# To use these, the module first needs to be included into the controller. Then the
+# action for the get request that displays the form should contain a call to
+# store_get_time_in_session whilst the action that handles the submission post
+# request should test against acceptable_time_between_get_and_now? to see if it
+# should actually process the submitted data.
+module SessionTiming
+
+  # It is assumed that a person filling out and submitting a form will take
+  # longer than an automated spam-bot. Therefore this tests that the minimum
+  # time required to complete the form has elapsed since the previous get
+  # request, the assumption being that if the request is too quick, it is being
+  # performed by a bot not a human.
+  def acceptable_time_between_get_and_now?
+    page_get_timestamp = session[:get_request_time]
+    if !page_get_timestamp || ((page_get_timestamp + MIN_TIME_TO_POST_FORM) > Time.now.to_i)
+      return false
+    end
+    true
+  end
+
+  # If the request is a get request, the time of the request is stored in the
+  # user's session.
+  def store_get_time_in_session
+    session[:get_request_time] = Time.now.to_i if request.get?
+  end
+
+end
diff --git a/store/test/functional/shops/products_controller_test.rb b/store/test/functional/shops/products_controller_test.rb
index f351d85..14f7434 100644
--- a/store/test/functional/shops/products_controller_test.rb
+++ b/store/test/functional/shops/products_controller_test.rb
@@ -294,6 +294,48 @@ class Shops::ProductsControllerTest < ActionController::TestCase
     end
   end
 
+  on UK_SHOP_HOST do
+    action :tell_a_friend, method: :post do
+      setup do
+        @product = products(:learning_maths)
+        @params = {
+          id: @product.id,
+          friend_address: 'mail@mail.com',
+          message: 'I LOVE THIS BOOK'
+        }
+      end
+
+      context 'when the time since the last get request is more than MIN_TIME_TO_POST_FORM' do
+        setup do
+          @session = {get_request_time: (Time.now - MIN_TIME_TO_POST_FORM - 5).to_i}
+        end
+
+        should_change "LoggedActivity.count", 1
+        should_redirect to: -> {product_path(@product)}
+
+        context 'but the more info field has been filled in' do
+          setup do
+            @params[:more_info] = 'something'
+          end
+
+          should_not_change "LoggedActivity.count"
+          should_work
+          should_flash type: :error, text: /There was a problem sending the email/
+        end
+      end
+
+      context 'when the time since the last get request is less than MIN_TIME_TO_POST_FORM' do
+        setup do
+          @session = {get_request_time: (Time.now - MIN_TIME_TO_POST_FORM + 5).to_i}
+        end
+
+        should_not_change "LoggedActivity.count"
+        should_work
+        should_flash type: :error, text: /Sorry we could not send the email/
+      end
+    end
+  end
+
   def test_show_invalid
     get :show, :id => 2435
     assert_response :not_found
-- 
1.8.2.3