Change the way params are parsed from JS (especially arrays). Fixing …

…super select functionality and adding a regression spec. Fixes #79. For super select, stop requesting new results from API as soon as the number of documents returned is below the chosen number of documents per page.

app/assets/javascripts/script.js

@@ -389,18 +389,7 @@ jQuery(function(d, $){
       // *all* of the articles from the search, not just those on the current page.
       // (Subject to the article limit.)
       selectAllSearchResults : function(e) {
-        var url_params = window.location.search.substr(1);  // Remove leading "?"
-
-        // Convert to dict for ajax call.
-        var data = {};
-        var pairs = url_params.split("&");
-        for (i in pairs) {
-          var split = pairs[i].split("=");
-
-          // We need to convert "+" to " ", which none of the stock javascript functions
-          // seem to handle correctly.  See http://unixpapa.com/js/querystring.html
-          data[split[0]] = decodeURIComponent(split[1].replace(/\+/g, " "));
-        }
+        var data = queryString.parse(window.location.search);
 
         $("#gray-out-screen").css({
           opacity: 0.7,

app/controllers/home_controller.rb

@@ -37,8 +37,6 @@ def get_article_count
 
   # Queries solr for the results used by select_all_search_results.
   def get_all_results
-    page = params.delete(:current_page)
-
     # For efficiency, we want to query solr for the smallest number of results.
     # However, this is difficult because the user may have already selected
     # some articles from various pages of the search results, and there is no
@@ -47,16 +45,16 @@ def get_all_results
     # various pathological cases such as the user having checked every other
     # search result.
     limit = APP_CONFIG["article_limit"] * 2
-
+    params[:rows] = rows = 200
     # solr usually returns 500s if you try to retreive all 1000 articles at once,
     # so we do paging here (with a larger page size than in the UI).
-    params[:rows] = 200
+
     results = []
-    for page in 1 .. (limit / params[:rows])
+    for page in 1 .. (limit / rows)
       params[:current_page] = page
       docs, _ = Search.find(params)
-      break if docs.empty?
       results += docs
+      break if docs.size < rows
     end
     results
   end

app/controllers/search_controller.rb

@@ -43,7 +43,7 @@ def journals
     else
       # Add a "All Journals" entry
       { SolrRequest::ALL_JOURNALS => SolrRequest::ALL_JOURNALS }.
-        merge(SolrRequest.get_journals.invert)
+        merge(SolrRequest.get_journals)
     end
   end
 end

app/views/search/advanced.html.slim

@@ -3,7 +3,7 @@
 
     h2.text-light-normal
       ' Construct Your Search
-      = link_to "Simple Search", search_path
+      = link_to "Simple Search", root_path
 
     .input-holder
       = select_tag "queryFieldId", options_for_select({ \

app/views/search/simple.html.slim

@@ -18,7 +18,7 @@
         = label_tag(:institution)
         = text_field_tag(:institution)
       .input-holder
-        = label_tag(:publication_days_ago, "Publication date")
+        = label_tag(:publication_days_ago, "Publication Date")
         = select_tag(:publication_days_ago, \
             options_for_select( \
                 [["All Time", -1], \
@@ -34,13 +34,11 @@
         = text_field_tag(:subject, nil, :class => "subject-autocomplete")
       .input-holder
         = label_tag(:cross_published_journal_name, "Journal")
-        = select_tag( \
-            "filterJournals[]", \
-            options_for_select(@journals), \
-            class: "styled" \
+        = collection_select( \
+            "filterJournals", "", @journals, :first, :last, class: "styled" \
           )
       .input-holder
-        = label_tag(:financial_disclosure, "Funder name or funding number")
+        = label_tag(:financial_disclosure, "Funder Name or Funding Number")
         = text_field_tag(:financial_disclosure)
     .input-holder
       = submit_tag "Search", :class => "button-hover-dark-black submit-btn"

spec/features/filter_by_journal_spec.rb

@@ -0,0 +1,23 @@
+require "spec_helper"
+
+if Search.plos?
+  describe "filter by journal", type: :feature, vcr: true do
+    it "considers journal filtering for select all", js: true do
+      ## Before VCR, requests to foreign APIs can take more than the default 2s
+      Capybara.default_wait_time = 20
+
+      visit "/"
+      fill_in "author", with: "Eisen"
+      select "PLOS ONE", from: "filterJournals_"
+      click_button "Search"
+      page.should have_content "journals: PLOS ONE"
+      page.should have_content "1 - 25 of 38 results"
+      page.should have_content "for author: Eisen"
+      expect(page).to have_button("Preview List (0)", disabled: true)
+      find(".select-all-articles-link", text: "select all").click
+      click_link "Select the remaining 13 articles"
+      wait_for_ajax
+      expect(page).to have_button("Preview List (38)")
+    end
+  end
+end

spec/support/wait_for_ajax.rb

@@ -0,0 +1,15 @@
+module WaitForAjax
+  def wait_for_ajax
+    Timeout.timeout(Capybara.default_wait_time) do
+      loop until finished_all_ajax_requests?
+    end
+  end
+
+  def finished_all_ajax_requests?
+    page.evaluate_script('jQuery.active').zero?
+  end
+end
+
+RSpec.configure do |config|
+  config.include WaitForAjax, type: :feature
+end