Merge 50dbd0a into 11e1386

app/controllers/request_controller.rb

@@ -14,6 +14,7 @@ class RequestController < ApplicationController
   before_action :set_info_request, only: [:show]
   before_action :redirect_new_form_to_pro_version, only: [:select_authority, :new]
   before_action :set_in_pro_area, only: [:select_authority, :show]
+  before_action :setup_results_pagination, only: [:list, :similar]
 
   helper_method :state_transitions_empty?
 
@@ -110,13 +111,6 @@ def details
   # Requests similar to this one
   def similar
     short_cache
-    @per_page = 25
-    @page = (params[:page] || "1").to_i
-
-    # Later pages are very expensive to load
-    if @page > MAX_RESULTS / PER_PAGE
-      raise ActiveRecord::RecordNotFound, "Sorry. No pages after #{MAX_RESULTS / PER_PAGE}."
-    end
 
     @info_request = InfoRequest.find_by_url_title!(params[:url_title])
 
@@ -133,34 +127,36 @@ def similar
 
   def list
     medium_cache
-    @view = params[:view]
-    unless @page # used in cache case, as perform_search sets @page as side effect
-      @page = get_search_page_from_params
-    end
-    @per_page = PER_PAGE
-    @max_results = MAX_RESULTS
-    if @view == "recent"
-      return redirect_to request_list_all_url(action: "list", view: "all", page: @page), status: :moved_permanently
-    end
 
-    # Later pages are very expensive to load
-    if @page > MAX_RESULTS / PER_PAGE
-      raise ActiveRecord::RecordNotFound, "Sorry. No pages after #{MAX_RESULTS / PER_PAGE}."
-    end
+    @filters = params.slice(:query, :request_date_after, :request_date_before)
+    @filters[:latest_status] = params[:view]
+    @tag = @filters[:tag] = params[:tag]
 
-    @filters = params.merge(latest_status: @view)
+    @results = InfoRequest.request_list(
+      @filters, @page, @per_page, @max_results
+    )
 
-    if @page > 1
+    @category = InfoRequest.category_list.find_by(category_tag: @tag) if @tag
+    if @category
+      @title = n_('Found {{count}} request in the category ‘{{category}}’',
+                  'Found {{count}} requests in the category ‘{{category}}’',
+                  @results[:matches_estimated],
+                  count: @results[:matches_estimated],
+                  category: @category.title)
+    elsif @tag
+      @title = n_('Found {{count}} request tagged ‘{{tag_name}}’',
+                  'Found {{count}} requests tagged ‘{{tag_name}}’',
+                  @results[:matches_estimated],
+                  count: @results[:matches_estimated],
+                  tag_name: @tag)
+    elsif @page > 1
       @title = _("Browse and search requests (page {{count}})", count: @page)
     else
       @title = _('Browse and search requests')
     end
 
     @track_thing = TrackThing.create_track_for_search_query(InfoRequestEvent.make_query_from_params(@filters))
     @feed_autodetect = [ { url: do_track_url(@track_thing, 'feed'), title: @track_thing.params[:title_in_rss], has_json: true } ]
-
-    # Don't let robots go more than 20 pages in
-    @no_crawl = true if @page > 20
   end
 
   # Page new form posts to
@@ -474,6 +470,21 @@ def outgoing_message_params
     params.require(:outgoing_message).permit(:body, :what_doing)
   end
 
+  def setup_results_pagination
+    @page ||= get_search_page_from_params
+    @per_page = PER_PAGE
+    @max_results = MAX_RESULTS
+
+    # Don't let robots go more than 20 pages in
+    @no_crawl = true if @page > 20
+
+    # Later pages are very expensive to load
+    return if @page <= MAX_RESULTS / PER_PAGE
+
+    raise ActiveRecord::RecordNotFound,
+      "Sorry. No pages after #{MAX_RESULTS / PER_PAGE}."
+  end
+
   def can_update_status(info_request)
     # Don't allow status update on external requests, otherwise accept param
     info_request.is_external? ? false : params[:update_status] == "1"

app/views/general/_frontpage_requests_list.html.erb

@@ -41,5 +41,5 @@
     <% end %>
   </ul>
 
-  <%= link_to _("Browse all requests &rarr;"), request_list_all_path, :class => 'button-secondary' %>
+  <%= link_to _("Browse all requests &rarr;"), request_list_path, :class => 'button-secondary' %>
 </div>

app/views/request/_category.html.erb

@@ -0,0 +1,2 @@
+<%= render_notes(@category.all_notes) %>
+<%= @category.body %>

app/views/request/_list_results.html.erb

@@ -1,4 +1,3 @@
-<% @results = InfoRequest.request_list(@filters, @page, @per_page, @max_results) %>
 <% if @results[:results].empty? %>
   <p> <%= _('No requests of this sort yet.')%></p>
 <% else %>

app/views/request/_request_filter_form.html.erb

@@ -11,7 +11,7 @@
             <% if controller?("public_body") %>
               <%= link_to label, url_for(:controller => "public_body", :action => "show", :view => status, :url_name => @public_body.url_name) + "?" + request.query_string + '#results' %>
             <% else %>
-              <%= link_to label, url_for(:controller => "request", :action => "list", :view => status) + "?" + request.query_string + '#results' %>
+              <%= link_to label, url_for(:controller => "request", :action => "list", :view => status, :tag => @tag) + "?" + request.query_string + '#results' %>
             <% end %>
           <% else %>
             <span><%= label %></span>

app/views/request/list.html.erb

@@ -2,6 +2,8 @@
   <h1><%= @title %></h1>
   <%= render :partial => 'request/request_search_form',
              :locals => { :after_form_fields => render(:partial => 'request/request_filter_form') } %>
+
+  <%= render partial: 'category' if @category %>
 </div>
 
 <div id="header_right" class="sidebar header_right">

config/routes.rb

@@ -75,28 +75,23 @@ def matches?(request)
   get '/body_statistics' => redirect('/statistics#public_bodies'), :as => :public_bodies_statistics
 
   ##### Request controller
-  match '/list/recent' => 'request#list',
-        :as => :request_list_recent,
-        :view => 'recent',
-        :via => :get
-  match '/list/all' => 'request#list',
-        :as => :request_list_all,
-        :view => 'all',
-        :via => :get
-  match '/list/successful' => 'request#list',
+  get '/list/all' => redirect('/list')
+  get '/list/recent' => redirect('/list')
+  match '/list(/:tag)/successful' => 'request#list',
         :as => :request_list_successful,
         :view => 'successful',
         :via => :get
-  match '/list/unsuccessful' => 'request#list',
+  match '/list(/:tag)/unsuccessful' => 'request#list',
         :as => :request_list_unsuccessful,
         :view => 'unsuccessful',
         :via => :get
-  match '/list/awaiting' => 'request#list',
+  match '/list(/:tag)/awaiting' => 'request#list',
         :as => :request_list_awaiting,
         :view => 'awaiting',
         :via => :get
-  match '/list' => 'request#list',
+  match '/list(/:tag)' => 'request#list',
         :as => :request_list,
+        :view => 'all',
         :via => :get
 
   match '/select_authority' => 'request#select_authority',

doc/CHANGES.md

@@ -2,6 +2,7 @@
 
 ## Highlighted Features
 
+* Allow requests to be listed and filtered by tag (Graeme Porteous)
 * Allow categories to have notes associated with them (Graeme Porteous)
 * Add styling option and rich text editor to the notes admin (Graeme Porteous)
 * Strengthen 2FA warning. Users *must* remember to keep this code safe (Gareth

lib/xapian_queries.rb

@@ -70,11 +70,18 @@ def get_date_range_from_params(params)
     query
   end
 
+  def get_tag_params(params)
+    return '' unless params[:tag]
+
+    " tag:#{params[:tag]}"
+  end
+
   def make_query_from_params(params)
     query = params[:query] || ''
     query += get_date_range_from_params(params)
     query += get_request_variety_from_params(params)
     query += get_status_from_params(params)
+    query += get_tag_params(params)
     query
   end
 end

spec/controllers/request_controller_spec.rb

@@ -1,11 +1,6 @@
 require 'spec_helper'
 
 RSpec.describe RequestController, "when listing recent requests" do
-  before(:each) do
-    load_raw_emails_data
-    update_xapian_index
-  end
-
   it "should be successful" do
     get :list, params: { view: 'all' }
     expect(response).to be_successful
@@ -17,11 +12,6 @@
   end
 
   it "should return 404 for pages we don't want to serve up" do
-    xap_results = double(
-      ActsAsXapian::Search,
-      results: (1..25).to_a.map { |m| { model: m } },
-      matches_estimated: 1_000_000
-    )
     expect {
       get :list, params: { view: 'all', page: 100 }
     }.to raise_error(ActiveRecord::RecordNotFound)
@@ -37,6 +27,41 @@
     expect { get :list, params: { view: 'all', page: "-1" } }.not_to raise_error
     expect(assigns[:page]).to eq(1)
   end
+
+  it 'sets title based on page' do
+    get :list, params: { view: 'all' }
+    expect(assigns[:title]).to eq('Browse and search requests')
+
+    get :list, params: { view: 'all', page: 2 }
+    expect(assigns[:title]).to eq('Browse and search requests (page 2)')
+  end
+
+  it 'sets title based on if tag matches an request category' do
+    FactoryBot.create(:category, :info_request,
+                      title: 'Climate requests', category_tag: 'climate')
+
+    update_xapian_index
+    get :list, params: { view: 'all', tag: 'climate' }
+    expect(assigns[:title]).
+      to eq("Found 0 requests in the category ‘Climate requests’")
+
+    FactoryBot.create(:info_request, tag_string: 'climate')
+    update_xapian_index
+    get :list, params: { view: 'all', tag: 'climate' }
+    expect(assigns[:title]).
+      to eq("Found 1 request in the category ‘Climate requests’")
+  end
+
+  it 'sets title based on if tag does not match an request category' do
+    update_xapian_index
+    get :list, params: { view: 'all', tag: 'other' }
+    expect(assigns[:title]).to eq("Found 0 requests tagged ‘other’")
+
+    FactoryBot.create(:info_request, tag_string: 'other')
+    update_xapian_index
+    get :list, params: { view: 'all', tag: 'other' }
+    expect(assigns[:title]).to eq("Found 1 request tagged ‘other’")
+  end
 end
 
 RSpec.describe RequestController, "when showing one request" do

spec/controllers/track_controller_spec.rb

@@ -367,7 +367,7 @@
         and_return(track_thing)
       expect(track_thing).to receive(:save).and_call_original
       get :track_list, params: { view: 'recent', feed: 'track' }
-      expect(response).to redirect_to("/list?view=recent")
+      expect(response).to redirect_to("/list")
     end
 
     it "should redirect with an error message if the query is too long" do
@@ -378,7 +378,7 @@
         and_return(long_track)
       get :track_list, params: { view: 'recent', feed: 'track' }
       expect(flash[:error]).to match('too long')
-      expect(response).to redirect_to("/list?view=recent")
+      expect(response).to redirect_to("/list")
     end
   end
 

spec/factories/categories.rb

@@ -20,5 +20,9 @@
     trait :public_body do
       parents { [PublicBody.category_root] }
     end
+
+    trait :info_request do
+      parents { [InfoRequest.category_root] }
+    end
   end
 end