SRCH-1547 fix CTRs in analytics downloads (#655)

- bump cucumber
- add cucumber test for query downloads
- rubocop cleanup

Gemfile

@@ -156,6 +156,9 @@ end
 group :test do
   gem 'capybara-screenshot'
   gem 'simplecov', '~> 0.15.1', require: false
+  # Limiting the cucumber version until v4 is compatible with VCR
+  # https://github.com/vcr/vcr/issues/825
+  gem 'cucumber', '~> 3.0', require: false
   gem 'cucumber-rails', '~> 2.0', require: false
   gem 'resque_spec', '~> 0.17.0'
   gem 'poltergeist', '~> 1.18.1'

Gemfile.lock

@@ -196,7 +196,7 @@ GEM
       descendants_tracker (~> 0.0.4)
       ice_nine (~> 0.11.0)
       thread_safe (~> 0.3, >= 0.3.1)
-    backports (3.15.0)
+    backports (3.20.2)
     bcrypt (3.1.13)
     bcrypt-ruby (3.1.5)
       bcrypt (>= 3.1.3)
@@ -252,7 +252,7 @@ GEM
       compass (~> 1.0.0)
       sass-rails (< 5.1)
       sprockets (< 4.0)
-    concurrent-ruby (1.1.7)
+    concurrent-ruby (1.1.8)
     cookiejar (0.3.3)
     counter_culture (2.3.0)
       activerecord (>= 4.2)
@@ -261,7 +261,7 @@ GEM
     crack (0.4.3)
       safe_yaml (~> 1.0.0)
     crass (1.0.6)
-    cucumber (3.1.2)
+    cucumber (3.2.0)
       builder (>= 2.1.2)
       cucumber-core (~> 3.2.0)
       cucumber-expressions (~> 6.0.1)
@@ -275,12 +275,12 @@ GEM
       cucumber-tag_expressions (~> 1.1.0)
       gherkin (~> 5.0)
     cucumber-expressions (6.0.1)
-    cucumber-rails (2.0.0)
-      capybara (>= 2.12, < 4)
-      cucumber (>= 3.0.2, < 4)
-      mime-types (>= 2.0, < 4)
+    cucumber-rails (2.2.0)
+      capybara (>= 2.18, < 4)
+      cucumber (>= 3.0.2, < 6)
+      mime-types (~> 3.2)
       nokogiri (~> 1.8)
-      railties (>= 4.2, < 7)
+      rails (>= 5.0, < 7)
     cucumber-tag_expressions (1.1.1)
     cucumber-wire (0.0.1)
     curb (0.9.10)
@@ -290,7 +290,7 @@ GEM
     declarative-option (0.1.0)
     descendants_tracker (0.0.4)
       thread_safe (~> 0.3, >= 0.3.1)
-    diff-lcs (1.3)
+    diff-lcs (1.4.4)
     docile (1.1.5)
     dogstatsd-ruby (3.2.0)
     domain_name (0.5.20180417)
@@ -482,20 +482,20 @@ GEM
     memoizable (0.4.2)
       thread_safe (~> 0.3, >= 0.3.1)
     method_source (0.9.2)
-    mime-types (3.3)
+    mime-types (3.3.1)
       mime-types-data (~> 3.2015)
-    mime-types-data (3.2019.1009)
+    mime-types-data (3.2020.1104)
     mimemagic (0.3.5)
     mini_mime (1.0.2)
     mini_portile2 (2.4.0)
-    minitest (5.14.2)
+    minitest (5.14.3)
     mobile-fu (1.4.0)
       rack-mobile-detect
       rails
     mono_logger (1.1.0)
     mry (0.59.0.0)
       rubocop (>= 0.41.0)
-    multi_json (1.14.1)
+    multi_json (1.15.0)
     multi_test (0.1.2)
     multi_xml (0.6.0)
     multipart-post (2.1.1)
@@ -765,7 +765,7 @@ GEM
       railties (>= 3.1)
     typhoeus (1.3.1)
       ethon (>= 0.9.0)
-    tzinfo (1.2.8)
+    tzinfo (1.2.9)
       thread_safe (~> 0.1)
     uber (0.1.0)
     uglifier (4.1.20)
@@ -829,6 +829,7 @@ DEPENDENCIES
   compass-blueprint (~> 1.0.0)
   compass-rails (~> 3.1.0)
   counter_culture (~> 2.3.0)
+  cucumber (~> 3.0)
   cucumber-rails (~> 2.0)
   curb (~> 0.9.4)
   database_cleaner (~> 1.7.0)

app/controllers/sites/query_downloads_controller.rb

@@ -2,11 +2,11 @@
 
 class Sites::QueryDownloadsController < Sites::SetupSiteController
   include CSVResponsive
-  MAX_RESULTS = 1000000
+  MAX_RESULTS = 1_000_000
 
   def show
-    @end_date = request["end_date"].to_date
-    @start_date = request["start_date"].to_date
+    @end_date = request['end_date'].to_date
+    @start_date = request['start_date'].to_date
     filename = [@site.name, @start_date, @end_date].join('_')
     header = [
       'Search Term',
@@ -15,7 +15,7 @@ def show
       'Real CTR',
       'Total (Bots + Humans) Queries',
       'Total Clicks',
-      'Total CTR',
+      'Total CTR'
     ]
     csv_response(filename, header, top_queries)
   end
@@ -38,55 +38,55 @@ def top_queries
         ctr_human,
         query_raw_count,
         click_raw_count,
-        ctr_raw,
+        ctr_raw
       ]
     end
 
     report_array.sort_by { |a| -a[1] }
   end
 
-  private
-
   def ctr(click_count, query_count)
-    return '--' if click_count == 0 || query_count == 0
+    return '--' if click_count.zero? || query_count.zero?
 
     sprintf('%.1f%%', click_count.to_f * 100 / query_count)
   end
 
-  def date_range_top_n_query
-    @date_range_top_n_query ||= DateRangeTopNQuery.new(@site.name,
-                                                       'search',
-                                                       @start_date,
-                                                       @end_date,
-                                                       field: 'params.query.raw',
-                                                       size: MAX_RESULTS)
+  def date_range_top_n_query(type)
+    DateRangeTopNQuery.new(
+      @site.name,
+      type,
+      @start_date,
+      @end_date,
+      field: 'params.query.raw',
+      size: MAX_RESULTS
+    )
   end
 
   def query_raw_count_array
     @query_raw_count_array ||= begin
-      rtu_top_queries = RtuTopQueries.new(date_range_top_n_query.body, false)
+      rtu_top_queries = RtuTopQueries.new(date_range_top_n_query('search').body, false)
       rtu_top_queries.top_n
     end
   end
 
   def query_human_count_hash
     @query_human_count_hash ||= begin
-      rtu_top_human_queries = RtuTopQueries.new(date_range_top_n_query.body, true)
-      Hash[rtu_top_human_queries.top_n]
+      top_human_queries = RtuTopQueries.new(date_range_top_n_query('search').body, true)
+      Hash[top_human_queries.top_n]
     end
   end
 
   def click_raw_count_hash
     @click_raw_count_hash ||= begin
-      rtu_top_clicks = RtuTopClicks.new(date_range_top_n_query.body, false)
-      Hash[rtu_top_clicks.top_n]
+      top_clicks = RtuTopClicks.new(date_range_top_n_query('click').body, false)
+      Hash[top_clicks.top_n]
     end
   end
 
   def click_human_count_hash
     @click_human_count_hash ||= begin
-      rtu_top_human_clicks = RtuTopClicks.new(date_range_top_n_query.body, true)
-      Hash[rtu_top_human_clicks.top_n]
+      top_human_clicks = RtuTopClicks.new(date_range_top_n_query('click').body, true)
+      Hash[top_human_clicks.top_n]
     end
   end
 end

features/admin_center_clicks_and_queries.feature

@@ -26,3 +26,8 @@ Feature: Clicks and Queries stats
     Then I should see "Referrers"
     And I should see "Your site has not received any queries with referrers yet."
 
+    When I follow "Monthly Reports"
+    Then I should see "Change Month"
+    When I follow "csv"
+    Then I should get a CSV download
+    And the downloaded file should include "Search Term"

features/step_definitions/web_steps.rb

@@ -240,3 +240,11 @@ def with_scope(locator)
     end
   end
 end
+
+Then /^I should get a ([^\"]*) download$/ do |type|
+  page.driver.response.headers['Content-Type'].should match type.downcase
+end
+
+Then /^the downloaded file should include "([^\"]*)"$/ do |content|
+  page.driver.response.body.should match content
+end

spec/controllers/sites/query_downloads_controller_spec.rb

@@ -1,11 +1,21 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe Sites::QueryDownloadsController do
   render_views
-  fixtures :users, :affiliates, :memberships
   before { activate_authlogic }
 
   describe '#show' do
+    subject(:show) do
+      get :show, params: {
+        site_id: site.id,
+        start_date: '2014-06-08',
+        end_date: '2014-06-14',
+        format: 'csv'
+      }
+    end
+
     it_should_behave_like 'restricted to approved user', :get, :show, site_id: 100
 
     context 'when affiliate is downloading CSV data' do
@@ -24,22 +34,38 @@
           size: 1_000_000
         ]
       end
-      let(:query) { instance_double(DateRangeTopNQuery, body: '') }
+      let(:date_range_top_n_click_args) do
+        [
+          site.name,
+          'click',
+          Date.new(2014, 6, 8),
+          Date.new(2014, 6, 14),
+          field: 'params.query.raw',
+          size: 1_000_000
+        ]
+      end
+      let(:search_query) { instance_double(DateRangeTopNQuery, body: 'search query') }
+      let(:click_query) { instance_double(DateRangeTopNQuery, body: 'click query') }
 
       before do
-        expect(DateRangeTopNQuery).to receive(:new).
-          with(*date_range_top_n_query_args).and_return(query)
-        allow(ES::ELK.client_reader).to receive(:search).and_return(top_queries_response, top_human_queries_response, top_clicks_response, top_human_clicks_response)
+        allow(DateRangeTopNQuery).to receive(:new).
+          with(*date_range_top_n_query_args).and_return(search_query)
+        allow(DateRangeTopNQuery).to receive(:new).
+          with(*date_range_top_n_click_args).and_return(click_query)
+        allow(ES::ELK.client_reader).to receive(:search).
+          with(hash_including(body: 'search query')).and_return(
+            top_queries_response,
+            top_human_queries_response
+          )
+        allow(ES::ELK.client_reader).to receive(:search).
+          with(hash_including(body: 'click query')).and_return(
+            top_clicks_response,
+            top_human_clicks_response
+          )
       end
 
-      it 'should generate a CSV of human and bot traffic for some date range, sorted by human count' do
-        get :show,
-            params: {
-              site_id: site.id,
-              start_date: '2014-06-08',
-              end_date: '2014-06-14',
-              format: 'csv'
-            }
+      it 'generates a CSV of human/bot traffic for a date range, sorted by human count' do
+        show
         expect(response.content_type).to eq("text/csv")
         expect(response.headers["Content-Disposition"]).to eq("attachment;filename=nps.gov_2014-06-08_2014-06-14.csv")
         expect(response.body).to start_with("Search Term,Real (Humans only) Queries,Real Clicks,Real CTR,Total (Bots + Humans) Queries,Total Clicks,Total CTR\njobs,9,15,166.7%,10,15,150.0%\nchartres,1,20,2000.0%,1,1,100.0%\n")