Browse Source

Cldc 1278 log search follow up (#623)

* test to download only search results

* testing I can download only logs matching both search and filter

* rubocop

* matching style for title

* ILIKE for search

* rubocop

* using ILIKE

* upated rack due to vulnerability

* refactoring

* refactoring

* refactored test

* refactored matching search results on multiple page tests for title

* no pagination title

* no pagination title for when no logs

* fixed all title related tests for filter and search

* rubocop

* removing extra space in the view - unneccesary

* better test when searching for postcode

* failing lint?

* refactored condition

* refactored content to links

* refactored content to links in case logs
pull/626/head
J G 3 years ago committed by GitHub
parent
commit
40b751b9fc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 2
      Gemfile.lock
  2. 11
      app/controllers/case_logs_controller.rb
  3. 9
      app/controllers/organisations_controller.rb
  4. 6
      app/models/case_log.rb
  5. 4
      app/views/case_logs/index.html.erb
  6. 5
      app/views/organisations/logs.html.erb
  7. 48
      spec/requests/case_logs_controller_spec.rb
  8. 41
      spec/requests/organisations_controller_spec.rb

2
Gemfile.lock

@ -270,7 +270,7 @@ GEM
puma (5.6.4) puma (5.6.4)
nio4r (~> 2.0) nio4r (~> 2.0)
racc (1.6.0) racc (1.6.0)
rack (2.2.3) rack (2.2.3.1)
rack-attack (6.6.1) rack-attack (6.6.1)
rack (>= 1.0, < 3) rack (>= 1.0, < 3)
rack-mini-profiler (2.3.4) rack-mini-profiler (2.3.4)

11
app/controllers/case_logs_controller.rb

@ -12,21 +12,16 @@ class CaseLogsController < ApplicationController
set_session_filters set_session_filters
all_logs = current_user.case_logs all_logs = current_user.case_logs
unpaginated_filtered_logs = filtered_case_logs(filtered_collection(all_logs, search_term))
@pagy, @case_logs = pagy( @pagy, @case_logs = pagy(unpaginated_filtered_logs)
filtered_case_logs(
filtered_collection(
all_logs, search_term
),
),
)
@searched = search_term.presence @searched = search_term.presence
@total_count = all_logs.size @total_count = all_logs.size
respond_to do |format| respond_to do |format|
format.html format.html
format.csv do format.csv do
send_data filtered_case_logs(current_user.case_logs).to_csv, filename: "logs-#{Time.zone.now}.csv" send_data unpaginated_filtered_logs.to_csv, filename: "logs-#{Time.zone.now}.csv"
end end
end end
end end

9
app/controllers/organisations_controller.rb

@ -55,14 +55,9 @@ class OrganisationsController < ApplicationController
set_session_filters(specific_org: true) set_session_filters(specific_org: true)
organisation_logs = CaseLog.all.where(owning_organisation_id: @organisation.id) organisation_logs = CaseLog.all.where(owning_organisation_id: @organisation.id)
unpaginated_filtered_logs = filtered_case_logs(filtered_collection(organisation_logs, search_term))
@pagy, @case_logs = pagy( @pagy, @case_logs = pagy(unpaginated_filtered_logs)
filtered_case_logs(
filtered_collection(
organisation_logs, search_term
),
),
)
@searched = search_term.presence @searched = search_term.presence
@total_count = organisation_logs.size @total_count = organisation_logs.size

6
app/models/case_log.rb

@ -52,9 +52,9 @@ class CaseLog < ApplicationRecord
} }
scope :filter_by_id, ->(id) { where(id:) } scope :filter_by_id, ->(id) { where(id:) }
scope :filter_by_tenant_code, ->(code) { where("lower(tenant_code) = ?", code.downcase) } scope :filter_by_tenant_code, ->(tenant_code) { where("tenant_code ILIKE ?", "%#{tenant_code}%") }
scope :filter_by_propcode, ->(code) { where("lower(propcode) = ?", code.downcase) } scope :filter_by_propcode, ->(propcode) { where("propcode ILIKE ?", "%#{propcode}%") }
scope :filter_by_postcode, ->(code) { where(postcode_full: code.upcase.gsub(/\s+/, "")) } scope :filter_by_postcode, ->(postcode_full) { where("postcode_full ILIKE ?", "%#{postcode_full.gsub(/\s+/, '')}%") }
scope :search_by, lambda { |param| scope :search_by, lambda { |param|
filter_by_id(param) filter_by_id(param)
.or(filter_by_tenant_code(param)) .or(filter_by_tenant_code(param))

4
app/views/case_logs/index.html.erb

@ -1,9 +1,9 @@
<% item_label = @pagy.count > 1 ? "logs" : "log" %> <% item_label = @pagy.count > 1 ? "logs" : "log" %>
<% if @searched.present? %> <% if @searched.present? %>
<% title = "Logs (search results for ‘#{@searched}’#{@pagy.last > 1 ? ", page #{@pagy.page} of #{@pagy.last}" : ''}) - Submit social housing and sales data (CORE) - GOV.UK" %> <% title = "Your organisation (#{@pagy.count} #{item_label} matching ‘#{@searched}’ of #{@total_count} total logs)" %>
<% else %> <% else %>
<% title = "Logs #{@pagy.last > 1 ? "(page #{@pagy.page} of #{@pagy.last}) " : ''}- Submit social housing and sales data (CORE) - GOV.UK" %> <% title = "Your organisation (Logs)" %>
<% end %> <% end %>
<% content_for :title, title %> <% content_for :title, title %>

5
app/views/organisations/logs.html.erb

@ -1,9 +1,8 @@
<% item_label = @pagy.count > 1 ? "logs" : "log" %> <% item_label = @pagy.count > 1 ? "logs" : "log" %>
<% if @searched.present? %> <% if @searched.present? %>
<% title = "Logs (search results for ‘#{@searched}’#{@pagy.last > 1 ? ", page #{@pagy.page} of #{@pagy.last}" : ''}) - Submit social housing and sales data (CORE) - GOV.UK" %> <% title = "Your organisation (#{@pagy.count} #{item_label} matching ‘#{@searched}’ of #{@total_count} total logs)" %>
<% else %> <% else %>
<% title = "Logs #{@pagy.last > 1 ? "(page #{@pagy.page} of #{@pagy.last}) " : ''}- Submit social housing and sales data (CORE) - GOV.UK" %> <% title = "Your organisation (Logs)" %>
<% end %> <% end %>
<% content_for :title, title %> <% content_for :title, title %>

48
spec/requests/case_logs_controller_spec.rb

@ -191,7 +191,7 @@ RSpec.describe CaseLogsController, type: :request do
it "page has correct title" do it "page has correct title" do
get "/logs", headers: headers, params: {} get "/logs", headers: headers, params: {}
expect(page).to have_title("Logs - Submit social housing and sales data (CORE) - GOV.UK") expect(page).to have_title("Your organisation (Logs) - Submit social housing lettings and sales data (CORE) - GOV.UK")
end end
end end
@ -322,17 +322,18 @@ RSpec.describe CaseLogsController, type: :request do
context "when using a search query" do context "when using a search query" do
let(:logs) { FactoryBot.create_list(:case_log, 3, :completed, owning_organisation: user.organisation) } let(:logs) { FactoryBot.create_list(:case_log, 3, :completed, owning_organisation: user.organisation) }
let(:log_to_search) { FactoryBot.create(:case_log, :completed, owning_organisation: user.organisation) } let(:log_to_search) { FactoryBot.create(:case_log, :completed, owning_organisation: user.organisation) }
let(:log_total_count) { CaseLog.where(owning_organisation: user.organisation).count }
it "has search results in the title" do it "has search results in the title" do
get "/logs?search=#{log_to_search.id}", headers: headers, params: {} get "/logs?search=#{log_to_search.id}", headers: headers, params: {}
expect(page).to have_content("Logs (search results for#{log_to_search.id}’) - Submit social housing and sales data (CORE) - GOV.UK") expect(page).to have_title("Your organisation (1 log matching#{log_to_search.id} of #{log_total_count} total logs) - Submit social housing lettings and sales data (CORE) - GOV.UK")
end end
it "shows case logs matching the id" do it "shows case logs matching the id" do
get "/logs?search=#{log_to_search.id}", headers: headers, params: {} get "/logs?search=#{log_to_search.id}", headers: headers, params: {}
expect(page).to have_content(log_to_search.id.to_s) expect(page).to have_link(log_to_search.id.to_s)
logs.each do |log| logs.each do |log|
expect(page).not_to have_content(log.id.to_s) expect(page).not_to have_link(log.id.to_s)
end end
end end
@ -346,7 +347,7 @@ RSpec.describe CaseLogsController, type: :request do
it "shows case logs matching the property reference" do it "shows case logs matching the property reference" do
get "/logs?search=#{log_to_search.propcode}", headers: headers, params: {} get "/logs?search=#{log_to_search.propcode}", headers: headers, params: {}
expect(page).to have_content(log_to_search.id.to_s) expect(page).to have_link(log_to_search.id.to_s)
logs.each do |log| logs.each do |log|
expect(page).not_to have_link(log.id.to_s) expect(page).not_to have_link(log.id.to_s)
end end
@ -366,7 +367,7 @@ RSpec.describe CaseLogsController, type: :request do
it "displays all matching logs" do it "displays all matching logs" do
get "/logs?search=#{log_to_search.postcode_full}", headers: headers, params: {} get "/logs?search=#{log_to_search.postcode_full}", headers: headers, params: {}
expect(page).to have_link(log_to_search.id.to_s) expect(page).to have_link(log_to_search.id.to_s)
expect(page).to have_content(matching_postcode_log.id) expect(page).to have_link(matching_postcode_log.id.to_s)
logs.each do |log| logs.each do |log|
expect(page).not_to have_link(log.id.to_s) expect(page).not_to have_link(log.id.to_s)
end end
@ -374,16 +375,18 @@ RSpec.describe CaseLogsController, type: :request do
end end
context "when there are more than 1 page of search results" do context "when there are more than 1 page of search results" do
let(:logs) { FactoryBot.create_list(:case_log, 30, :completed, owning_organisation: user.organisation, postcode_full: "XX1 1YY") } let(:postcode) { "XX11YY" }
let(:logs) { FactoryBot.create_list(:case_log, 30, :completed, owning_organisation: user.organisation, postcode_full: postcode) }
let(:log_total_count) { CaseLog.where(owning_organisation: user.organisation).count }
it "has title with pagination details for page 1" do it "has title with pagination details for page 1" do
get "/logs?search=#{logs[0].postcode_full}", headers: headers, params: {} get "/logs?search=#{logs[0].postcode_full}", headers: headers, params: {}
expect(page).to have_content("Logs (search results for ‘#{logs[0].postcode_full}’, page 1 of 2) - Submit social housing and sales data (CORE) - GOV.UK") expect(page).to have_title("Your organisation (#{logs.count} logs matching ‘#{postcode}’ of #{log_total_count} total logs) (page 1 of 2) - Submit social housing lettings and sales data (CORE) - GOV.UK")
end end
it "has title with pagination details for page 2" do it "has title with pagination details for page 2" do
get "/logs?search=#{logs[0].postcode_full}&page=2", headers: headers, params: {} get "/logs?search=#{logs[0].postcode_full}&page=2", headers: headers, params: {}
expect(page).to have_content("Logs (search results for ‘#{logs[0].postcode_full}’, page 2 of 2) - Submit social housing and sales data (CORE) - GOV.UK") expect(page).to have_title("Your organisation (#{logs.count} logs matching ‘#{postcode}’ of #{log_total_count} total logs) (page 2 of 2) - Submit social housing lettings and sales data (CORE) - GOV.UK")
end end
end end
@ -463,7 +466,7 @@ RSpec.describe CaseLogsController, type: :request do
end end
it "does not have pagination in the title" do it "does not have pagination in the title" do
expect(page).to have_title("Logs - Submit social housing and sales data (CORE) - GOV.UK") expect(page).to have_title("Your organisation (Logs) - Submit social housing lettings and sales data (CORE) - GOV.UK")
end end
it "shows the download csv link" do it "shows the download csv link" do
@ -539,7 +542,7 @@ RSpec.describe CaseLogsController, type: :request do
end end
it "has pagination in the title" do it "has pagination in the title" do
expect(page).to have_title("Logs (page 1 of 2) - Submit social housing and sales data (CORE) - GOV.UK") expect(page).to have_title("Your organisation (Logs) (page 1 of 2) - Submit social housing lettings and sales data (CORE) - GOV.UK")
end end
end end
@ -564,7 +567,7 @@ RSpec.describe CaseLogsController, type: :request do
end end
it "has pagination in the title" do it "has pagination in the title" do
expect(page).to have_title("Logs (page 2 of 2) - Submit social housing and sales data (CORE) - GOV.UK") expect(page).to have_title("Your organisation (Logs) (page 2 of 2) - Submit social housing lettings and sales data (CORE) - GOV.UK")
end end
end end
end end
@ -753,6 +756,27 @@ RSpec.describe CaseLogsController, type: :request do
csv = CSV.parse(response.body) csv = CSV.parse(response.body)
expect(csv.count).to eq(2) expect(csv.count).to eq(2)
end end
it "dowloads searched logs" do
get "/logs?search=#{case_log.id}", headers:, params: {}
csv = CSV.parse(response.body)
expect(csv.count).to eq(2)
end
context "when both filter and search applied" do
let(:postcode) { "XX1 1TG" }
before do
FactoryBot.create(:case_log, :in_progress, postcode_full: postcode, owning_organisation: organisation)
FactoryBot.create(:case_log, :completed, postcode_full: postcode, owning_organisation: organisation)
end
it "dowloads logs matching both csv and filter logs" do
get "/logs?status[]=completed&search=#{postcode}", headers:, params: {}
csv = CSV.parse(response.body)
expect(csv.count).to eq(2)
end
end
end end
context "when there are more than 20 logs" do context "when there are more than 20 logs" do

41
spec/requests/organisations_controller_spec.rb

@ -407,41 +407,42 @@ RSpec.describe OrganisationsController, type: :request do
context "when using a search query" do context "when using a search query" do
let(:logs) { FactoryBot.create_list(:case_log, 3, :completed, owning_organisation: user.organisation) } let(:logs) { FactoryBot.create_list(:case_log, 3, :completed, owning_organisation: user.organisation) }
let(:log_to_search) { FactoryBot.create(:case_log, :completed, owning_organisation: user.organisation) } let(:log_to_search) { FactoryBot.create(:case_log, :completed, owning_organisation: user.organisation) }
let(:log_total_count) { CaseLog.where(owning_organisation: user.organisation).count }
it "has search results in the title" do it "has search results in the title" do
get "/organisations/#{organisation.id}/logs?search=#{log_to_search.id}", headers: headers, params: {} get "/organisations/#{organisation.id}/logs?search=#{log_to_search.id}", headers: headers, params: {}
expect(page).to have_content("Logs (search results for#{log_to_search.id}’) - Submit social housing and sales data (CORE) - GOV.UK") expect(page).to have_title("Your organisation (1 log matching#{log_to_search.id} of #{log_total_count} total logs) - Submit social housing lettings and sales data (CORE) - GOV.UK")
end end
it "shows case logs matching the id" do it "shows case logs matching the id" do
get "/organisations/#{organisation.id}/logs?search=#{log_to_search.id}", headers: headers, params: {} get "/organisations/#{organisation.id}/logs?search=#{log_to_search.id}", headers: headers, params: {}
expect(page).to have_content(log_to_search.id) expect(page).to have_link(log_to_search.id.to_s)
logs.each do |log| logs.each do |log|
expect(page).not_to have_content(log.id) expect(page).not_to have_link(log.id.to_s)
end end
end end
it "shows case logs matching the tenant code" do it "shows case logs matching the tenant code" do
get "/organisations/#{organisation.id}/logs?search=#{log_to_search.tenant_code}", headers: headers, params: {} get "/organisations/#{organisation.id}/logs?search=#{log_to_search.tenant_code}", headers: headers, params: {}
expect(page).to have_content(log_to_search.id) expect(page).to have_link(log_to_search.id.to_s)
logs.each do |log| logs.each do |log|
expect(page).not_to have_content(log.id) expect(page).not_to have_link(log.id.to_s)
end end
end end
it "shows case logs matching the property reference" do it "shows case logs matching the property reference" do
get "/organisations/#{organisation.id}/logs?search=#{log_to_search.propcode}", headers: headers, params: {} get "/organisations/#{organisation.id}/logs?search=#{log_to_search.propcode}", headers: headers, params: {}
expect(page).to have_content(log_to_search.id) expect(page).to have_link(log_to_search.id.to_s)
logs.each do |log| logs.each do |log|
expect(page).not_to have_content(log.id) expect(page).not_to have_link(log.id.to_s)
end end
end end
it "shows case logs matching the property postcode" do it "shows case logs matching the property postcode" do
get "/organisations/#{organisation.id}/logs?search=#{log_to_search.postcode_full}", headers: headers, params: {} get "/organisations/#{organisation.id}/logs?search=#{log_to_search.postcode_full}", headers: headers, params: {}
expect(page).to have_content(log_to_search.id) expect(page).to have_link(log_to_search.id.to_s)
logs.each do |log| logs.each do |log|
expect(page).not_to have_content(log.id) expect(page).not_to have_link(log.id.to_s)
end end
end end
@ -450,25 +451,27 @@ RSpec.describe OrganisationsController, type: :request do
it "displays all matching logs" do it "displays all matching logs" do
get "/organisations/#{organisation.id}/logs?search=#{log_to_search.postcode_full}", headers: headers, params: {} get "/organisations/#{organisation.id}/logs?search=#{log_to_search.postcode_full}", headers: headers, params: {}
expect(page).to have_content(log_to_search.id) expect(page).to have_link(log_to_search.id.to_s)
expect(page).to have_content(matching_postcode_log.id) expect(page).to have_link(matching_postcode_log.id.to_s)
logs.each do |log| logs.each do |log|
expect(page).not_to have_content(log.id) expect(page).not_to have_link(log.id.to_s)
end end
end end
end end
context "when there are more than 1 page of search results" do context "when there are more than 1 page of search results" do
let(:logs) { FactoryBot.create_list(:case_log, 30, :completed, owning_organisation: user.organisation, postcode_full: "XX1 1YY") } let(:postcode) { "XX11YY" }
let(:logs) { FactoryBot.create_list(:case_log, 30, :completed, owning_organisation: user.organisation, postcode_full: postcode) }
let(:log_total_count) { CaseLog.where(owning_organisation: user.organisation).count }
it "has title with pagination details for page 1" do it "has title with pagination details for page 1" do
get "/organisations/#{organisation.id}/logs?search=#{logs[0].postcode_full}", headers: headers, params: {} get "/organisations/#{organisation.id}/logs?search=#{logs[0].postcode_full}", headers: headers, params: {}
expect(page).to have_content("Logs (search results for ‘#{logs[0].postcode_full}’, page 1 of 2) - Submit social housing and sales data (CORE) - GOV.UK") expect(page).to have_title("Your organisation (#{logs.count} logs matching ‘#{postcode}’ of #{log_total_count} total logs) (page 1 of 2) - Submit social housing lettings and sales data (CORE) - GOV.UK")
end end
it "has title with pagination details for page 2" do it "has title with pagination details for page 2" do
get "/organisations/#{organisation.id}/logs?search=#{logs[0].postcode_full}&page=2", headers: headers, params: {} get "/organisations/#{organisation.id}/logs?search=#{logs[0].postcode_full}&page=2", headers: headers, params: {}
expect(page).to have_content("Logs (search results for ‘#{logs[0].postcode_full}’, page 2 of 2) - Submit social housing and sales data (CORE) - GOV.UK") expect(page).to have_title("Your organisation (#{logs.count} logs matching ‘#{postcode}’ of #{log_total_count} total logs) (page 2 of 2) - Submit social housing lettings and sales data (CORE) - GOV.UK")
end end
end end
@ -476,9 +479,9 @@ RSpec.describe OrganisationsController, type: :request do
it "doesn't display any logs" do it "doesn't display any logs" do
get "/organisations/#{organisation.id}/logs?search=foobar", headers:, params: {} get "/organisations/#{organisation.id}/logs?search=foobar", headers:, params: {}
logs.each do |log| logs.each do |log|
expect(page).not_to have_content(log.id) expect(page).not_to have_link(log.id.to_s)
end end
expect(page).not_to have_content(log_to_search.id) expect(page).not_to have_link(log_to_search.id.to_s)
end end
end end
@ -486,9 +489,9 @@ RSpec.describe OrganisationsController, type: :request do
it "doesn't display any logs" do it "doesn't display any logs" do
get "/organisations/#{organisation.id}/logs?search=", headers:, params: {} get "/organisations/#{organisation.id}/logs?search=", headers:, params: {}
logs.each do |log| logs.each do |log|
expect(page).not_to have_content(log.id) expect(page).not_to have_link(log.id.to_s)
end end
expect(page).not_to have_content(log_to_search.id) expect(page).not_to have_link(log_to_search.id.to_s)
end end
end end

Loading…
Cancel
Save