Browse Source

change variable names throughout after info on rauby/rails naming conventions, update tests for change in who can view codes only download link

pull/1268/head
Arthur Campbell 3 years ago
parent
commit
0abcc7ac16
  1. 8
      app/controllers/lettings_logs_controller.rb
  2. 8
      app/controllers/organisations_controller.rb
  3. 4
      app/jobs/email_csv_job.rb
  4. 4
      app/models/lettings_log.rb
  5. 10
      app/services/csv/lettings_log_csv_service.rb
  6. 2
      app/views/logs/download_csv.html.erb
  7. 6
      spec/models/lettings_log_spec.rb
  8. 28
      spec/requests/lettings_logs_controller_spec.rb
  9. 10
      spec/requests/organisations_controller_spec.rb
  10. 2
      spec/services/csv/lettings_log_csv_service_spec.rb

8
app/controllers/lettings_logs_controller.rb

@ -80,15 +80,15 @@ class LettingsLogsController < LogsController
def download_csv def download_csv
unpaginated_filtered_logs = filtered_logs(current_user.lettings_logs, search_term, @session_filters) unpaginated_filtered_logs = filtered_logs(current_user.lettings_logs, search_term, @session_filters)
is_codes_only_export = params.require(:codes_only) == "true" codes_only_export = params.require(:codes_only) == "true"
render "download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: email_csv_lettings_logs_path, is_codes_only_export: } render "download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: email_csv_lettings_logs_path, codes_only_export: }
end end
def email_csv def email_csv
all_orgs = params["organisation_select"] == "all" all_orgs = params["organisation_select"] == "all"
is_codes_only_export = params.require(:is_codes_only_export) == "true" codes_only_export = params.require(:codes_only_export) == "true"
EmailCsvJob.perform_later(current_user, search_term, @session_filters, all_orgs, nil, is_codes_only_export) EmailCsvJob.perform_later(current_user, search_term, @session_filters, all_orgs, nil, codes_only_export)
redirect_to csv_confirmation_lettings_logs_path redirect_to csv_confirmation_lettings_logs_path
end end

8
app/controllers/organisations_controller.rb

@ -107,14 +107,14 @@ class OrganisationsController < ApplicationController
def download_csv def download_csv
organisation_logs = LettingsLog.all.where(owning_organisation_id: @organisation.id) organisation_logs = LettingsLog.all.where(owning_organisation_id: @organisation.id)
unpaginated_filtered_logs = filtered_logs(organisation_logs, search_term, @session_filters) unpaginated_filtered_logs = filtered_logs(organisation_logs, search_term, @session_filters)
is_codes_only_export = params.require(:codes_only) == "true" codes_only_export = params.require(:codes_only) == "true"
render "logs/download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: logs_email_csv_organisation_path, is_codes_only_export: } render "logs/download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: logs_email_csv_organisation_path, codes_only_export: }
end end
def email_csv def email_csv
is_codes_only_export = params.require(:is_codes_only_export) == "true" codes_only_export = params.require(:codes_only_export) == "true"
EmailCsvJob.perform_later(current_user, search_term, @session_filters, false, @organisation, is_codes_only_export) EmailCsvJob.perform_later(current_user, search_term, @session_filters, false, @organisation, codes_only_export)
redirect_to logs_csv_confirmation_organisation_path redirect_to logs_csv_confirmation_organisation_path
end end

4
app/jobs/email_csv_job.rb

@ -5,14 +5,14 @@ class EmailCsvJob < ApplicationJob
EXPIRATION_TIME = 3.hours.to_i EXPIRATION_TIME = 3.hours.to_i
def perform(user, search_term = nil, filters = {}, all_orgs = false, organisation = nil, is_codes_only_export = false) # rubocop:disable Style/OptionalBooleanParameter - sidekiq can't serialise named params def perform(user, search_term = nil, filters = {}, all_orgs = false, organisation = nil, codes_only_export = false) # rubocop:disable Style/OptionalBooleanParameter - sidekiq can't serialise named params
unfiltered_logs = organisation.present? && user.support? ? LettingsLog.where(owning_organisation_id: organisation.id) : user.lettings_logs unfiltered_logs = organisation.present? && user.support? ? LettingsLog.where(owning_organisation_id: organisation.id) : user.lettings_logs
filtered_logs = FilterService.filter_logs(unfiltered_logs, search_term, filters, all_orgs, user) filtered_logs = FilterService.filter_logs(unfiltered_logs, search_term, filters, all_orgs, user)
filename = organisation.present? ? "logs-#{organisation.name}-#{Time.zone.now}.csv" : "logs-#{Time.zone.now}.csv" filename = organisation.present? ? "logs-#{organisation.name}-#{Time.zone.now}.csv" : "logs-#{Time.zone.now}.csv"
storage_service = Storage::S3Service.new(Configuration::EnvConfigurationService.new, ENV["CSV_DOWNLOAD_PAAS_INSTANCE"]) storage_service = Storage::S3Service.new(Configuration::EnvConfigurationService.new, ENV["CSV_DOWNLOAD_PAAS_INSTANCE"])
storage_service.write_file(filename, BYTE_ORDER_MARK + filtered_logs.to_csv(user, is_codes_only_export:)) storage_service.write_file(filename, BYTE_ORDER_MARK + filtered_logs.to_csv(user, codes_only_export:))
url = storage_service.get_presigned_url(filename, EXPIRATION_TIME) url = storage_service.get_presigned_url(filename, EXPIRATION_TIME)

4
app/models/lettings_log.rb

@ -448,8 +448,8 @@ class LettingsLog < Log
location&.id location&.id
end end
def self.to_csv(user = nil, is_codes_only_export:) def self.to_csv(user = nil, codes_only_export:)
Csv::LettingsLogCsvService.new(user).to_csv(is_codes_only_export:) Csv::LettingsLogCsvService.new(user).to_csv(codes_only_export:)
end end
def beds_for_la_rent_range def beds_for_la_rent_range

10
app/services/csv/lettings_log_csv_service.rb

@ -7,13 +7,13 @@ module Csv
set_csv_attributes set_csv_attributes
end end
def to_csv(is_codes_only_export:) def to_csv(codes_only_export:)
CSV.generate(headers: true) do |csv| CSV.generate(headers: true) do |csv|
csv << @attributes csv << @attributes
LettingsLog.all.find_each do |record| LettingsLog.all.find_each do |record|
csv << @attributes.map do |att| csv << @attributes.map do |att|
label_from_value(record, att, is_codes_only_export:) label_from_value(record, att, codes_only_export:)
end end
end end
end end
@ -21,18 +21,18 @@ module Csv
private private
def label_from_value(record, att, is_codes_only_export:) def label_from_value(record, att, codes_only_export:)
if %w[la prevloc].include? att if %w[la prevloc].include? att
record.send(att) record.send(att)
elsif %w[mrcdate startdate voiddate].include? att elsif %w[mrcdate startdate voiddate].include? att
record.send(att)&.to_formatted_s(:govuk_date) record.send(att)&.to_formatted_s(:govuk_date)
elsif is_codes_only_export && att.start_with?("location_", "scheme_") elsif codes_only_export && att.start_with?("location_", "scheme_")
att += "_before_type_cast" unless %w[location_code scheme_code scheme_owning_organisation_name scheme_created_at location_startdate].include? att att += "_before_type_cast" unless %w[location_code scheme_code scheme_owning_organisation_name scheme_created_at location_startdate].include? att
record.send(att) record.send(att)
else else
att = att.remove("_label", "_detail") # a couple of csv column headers have suffixes for the user that are not reflected in the app domain att = att.remove("_label", "_detail") # a couple of csv column headers have suffixes for the user that are not reflected in the app domain
field_value = record.send(att) field_value = record.send(att)
if is_codes_only_export if codes_only_export
field_value field_value
else else
answer_label = record.form answer_label = record.form

2
app/views/logs/download_csv.html.erb

@ -11,6 +11,6 @@
<p class="govuk-body">We'll send a secure download link to your email address <strong><%= @current_user.email %></strong>.</p> <p class="govuk-body">We'll send a secure download link to your email address <strong><%= @current_user.email %></strong>.</p>
<p class="govuk-body">You've selected <%= count %> logs.</p> <p class="govuk-body">You've selected <%= count %> logs.</p>
<%= govuk_button_to "Send email", post_path, method: :post, params: { search: search_term, is_codes_only_export: } %> <%= govuk_button_to "Send email", post_path, method: :post, params: { search: search_term, codes_only_export: } %>
</div> </div>
</div> </div>

6
spec/models/lettings_log_spec.rb

@ -2711,7 +2711,7 @@ RSpec.describe LettingsLog do
let(:csv_export_file) { File.open("spec/fixtures/files/lettings_logs_download.csv", "r:UTF-8") } let(:csv_export_file) { File.open("spec/fixtures/files/lettings_logs_download.csv", "r:UTF-8") }
it "generates a correct csv from a lettings log" do it "generates a correct csv from a lettings log" do
expect(described_class.to_csv(is_codes_only_export: false)).to eq(expected_content) expect(described_class.to_csv(codes_only_export: false)).to eq(expected_content)
end end
end end
@ -2719,7 +2719,7 @@ RSpec.describe LettingsLog do
let(:csv_export_file) { File.open("spec/fixtures/files/lettings_logs_download_non_support.csv", "r:UTF-8") } let(:csv_export_file) { File.open("spec/fixtures/files/lettings_logs_download_non_support.csv", "r:UTF-8") }
it "generates a correct csv from a lettings log" do it "generates a correct csv from a lettings log" do
expect(described_class.to_csv(user, is_codes_only_export: false)).to eq(expected_content) expect(described_class.to_csv(user, codes_only_export: false)).to eq(expected_content)
end end
end end
end end
@ -2745,7 +2745,7 @@ RSpec.describe LettingsLog do
let(:csv_export_file) { File.open("spec/fixtures/files/lettings_logs_download_codes_only.csv", "r:UTF-8") } let(:csv_export_file) { File.open("spec/fixtures/files/lettings_logs_download_codes_only.csv", "r:UTF-8") }
it "generates a correct csv from a lettings log" do it "generates a correct csv from a lettings log" do
expect(described_class.to_csv(is_codes_only_export: true)).to eq(expected_content) expect(described_class.to_csv(codes_only_export: true)).to eq(expected_content)
end end
end end
end end

28
spec/requests/lettings_logs_controller_spec.rb

@ -532,7 +532,7 @@ RSpec.describe LettingsLogsController, type: :request do
it "displays CSV download links with the correct paths" do it "displays CSV download links with the correct paths" do
get "/lettings-logs", headers:, params: {} get "/lettings-logs", headers:, params: {}
expect(page).to have_link("Download (CSV)", href: "/lettings-logs/csv-download?codes_only=false") expect(page).to have_link("Download (CSV)", href: "/lettings-logs/csv-download?codes_only=false")
expect(page).to have_link("Download (CSV, codes only)", href: "/lettings-logs/csv-download?codes_only=true") expect(page).not_to have_link("Download (CSV, codes only)")
end end
it "does not display CSV download links if there are no logs" do it "does not display CSV download links if there are no logs" do
@ -590,10 +590,7 @@ RSpec.describe LettingsLogsController, type: :request do
get "/lettings-logs?search=#{search_term}", headers:, params: {} get "/lettings-logs?search=#{search_term}", headers:, params: {}
download_link = page.find_link("Download (CSV)") download_link = page.find_link("Download (CSV)")
download_link_params = CGI.parse(URI.parse(download_link[:href]).query) download_link_params = CGI.parse(URI.parse(download_link[:href]).query)
codes_only_download_link = page.find_link("Download (CSV, codes only)")
codes_only_download_link_params = CGI.parse(URI.parse(codes_only_download_link[:href]).query)
expect(download_link_params).to include("search" => [search_term]) expect(download_link_params).to include("search" => [search_term])
expect(codes_only_download_link_params).to include("search" => [search_term])
end end
context "when more than one results with matching postcode" do context "when more than one results with matching postcode" do
@ -661,7 +658,7 @@ RSpec.describe LettingsLogsController, type: :request do
end end
end end
context "when there are less than 20 logs" do context "when there are fewer than 20 logs" do
before do before do
get "/lettings-logs", headers:, params: {} get "/lettings-logs", headers:, params: {}
end end
@ -704,9 +701,8 @@ RSpec.describe LettingsLogsController, type: :request do
expect(page).to have_title("Logs - Submit social housing lettings and sales data (CORE) - GOV.UK") expect(page).to have_title("Logs - Submit social housing lettings and sales data (CORE) - GOV.UK")
end end
it "shows the CSV download links" do it "shows the CSV download link" do
expect(page).to have_link("Download (CSV)", href: "/lettings-logs/csv-download?codes_only=false") expect(page).to have_link("Download (CSV)", href: "/lettings-logs/csv-download?codes_only=false")
expect(page).to have_link("Download (CSV, codes only)", href: "/lettings-logs/csv-download?codes_only=true")
end end
it "does not show the organisation filter" do it "does not show the organisation filter" do
@ -1304,14 +1300,14 @@ RSpec.describe LettingsLogsController, type: :request do
it "when codes_only query parameter is false, form contains hidden field with correct value" do it "when codes_only query parameter is false, form contains hidden field with correct value" do
codes_only = false codes_only = false
get "/lettings-logs/csv-download?codes_only=#{codes_only}", headers:, params: {} get "/lettings-logs/csv-download?codes_only=#{codes_only}", headers:, params: {}
hidden_field = page.find("form.button_to").find_field("is_codes_only_export", type: "hidden") hidden_field = page.find("form.button_to").find_field("codes_only_export", type: "hidden")
expect(hidden_field.value).to eq(codes_only.to_s) expect(hidden_field.value).to eq(codes_only.to_s)
end end
it "when codes_only query parameter is true, form contains hidden field with correct value" do it "when codes_only query parameter is true, form contains hidden field with correct value" do
codes_only = true codes_only = true
get "/lettings-logs/csv-download?codes_only=#{codes_only}", headers:, params: {} get "/lettings-logs/csv-download?codes_only=#{codes_only}", headers:, params: {}
hidden_field = page.find("form.button_to").find_field("is_codes_only_export", type: "hidden") hidden_field = page.find("form.button_to").find_field("codes_only_export", type: "hidden")
expect(hidden_field.value).to eq(codes_only.to_s) expect(hidden_field.value).to eq(codes_only.to_s)
end end
@ -1347,33 +1343,33 @@ RSpec.describe LettingsLogsController, type: :request do
it "creates an E-mail job" do it "creates an E-mail job" do
expect { expect {
post "/lettings-logs/email-csv?is_codes_only_export=true", headers:, params: {} post "/lettings-logs/email-csv?codes_only_export=true", headers:, params: {}
}.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, nil, true) }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, nil, true)
end end
it "redirects to the confirmation page" do it "redirects to the confirmation page" do
post "/lettings-logs/email-csv?is_codes_only_export=true", headers:, params: {} post "/lettings-logs/email-csv?codes_only_export=true", headers:, params: {}
expect(response).to redirect_to(csv_confirmation_lettings_logs_path) expect(response).to redirect_to(csv_confirmation_lettings_logs_path)
end end
it "passes the search term" do it "passes the search term" do
expect { expect {
post "/lettings-logs/email-csv?search=#{lettings_log.id}&is_codes_only_export=false", headers:, params: {} post "/lettings-logs/email-csv?search=#{lettings_log.id}&codes_only_export=false", headers:, params: {}
}.to enqueue_job(EmailCsvJob).with(user, lettings_log.id.to_s, {}, false, nil, false) }.to enqueue_job(EmailCsvJob).with(user, lettings_log.id.to_s, {}, false, nil, false)
end end
it "passes filter parameters" do it "passes filter parameters" do
expect { expect {
post "/lettings-logs/email-csv?status[]=completed&is_codes_only_export=true", headers:, params: {} post "/lettings-logs/email-csv?status[]=completed&codes_only_export=true", headers:, params: {}
}.to enqueue_job(EmailCsvJob).with(user, nil, { "status" => %w[completed] }, false, nil, true) }.to enqueue_job(EmailCsvJob).with(user, nil, { "status" => %w[completed] }, false, nil, true)
end end
it "passes export type flag" do it "passes export type flag" do
expect { expect {
post "/lettings-logs/email-csv?is_codes_only_export=true", headers:, params: {} post "/lettings-logs/email-csv?codes_only_export=true", headers:, params: {}
}.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, nil, true) }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, nil, true)
expect { expect {
post "/lettings-logs/email-csv?is_codes_only_export=false", headers:, params: {} post "/lettings-logs/email-csv?codes_only_export=false", headers:, params: {}
}.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, nil, false) }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, nil, false)
end end
@ -1381,7 +1377,7 @@ RSpec.describe LettingsLogsController, type: :request do
postcode = "XX1 1TG" postcode = "XX1 1TG"
expect { expect {
post "/lettings-logs/email-csv?status[]=completed&search=#{postcode}&is_codes_only_export=false", headers:, params: {} post "/lettings-logs/email-csv?status[]=completed&search=#{postcode}&codes_only_export=false", headers:, params: {}
}.to enqueue_job(EmailCsvJob).with(user, postcode, { "status" => %w[completed] }, false, nil, false) }.to enqueue_job(EmailCsvJob).with(user, postcode, { "status" => %w[completed] }, false, nil, false)
end end
end end

10
spec/requests/organisations_controller_spec.rb

@ -1166,18 +1166,18 @@ RSpec.describe OrganisationsController, type: :request do
it "provides the organisation to the mail job" do it "provides the organisation to the mail job" do
expect { expect {
post "/organisations/#{organisation.id}/logs/email-csv?status[]=completed&is_codes_only_export=false", headers:, params: {} post "/organisations/#{organisation.id}/logs/email-csv?status[]=completed&codes_only_export=false", headers:, params: {}
}.to enqueue_job(EmailCsvJob).with(user, nil, { "status" => %w[completed] }, false, organisation, false) }.to enqueue_job(EmailCsvJob).with(user, nil, { "status" => %w[completed] }, false, organisation, false)
end end
it "provides the export type to the mail job" do it "provides the export type to the mail job" do
codes_only_export_type = false codes_only_export_type = false
expect { expect {
post "/organisations/#{organisation.id}/logs/email-csv?is_codes_only_export=#{codes_only_export_type}", headers:, params: {} post "/organisations/#{organisation.id}/logs/email-csv?codes_only_export=#{codes_only_export_type}", headers:, params: {}
}.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, organisation, codes_only_export_type) }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, organisation, codes_only_export_type)
codes_only_export_type = true codes_only_export_type = true
expect { expect {
post "/organisations/#{organisation.id}/logs/email-csv?is_codes_only_export=#{codes_only_export_type}", headers:, params: {} post "/organisations/#{organisation.id}/logs/email-csv?codes_only_export=#{codes_only_export_type}", headers:, params: {}
}.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, organisation, codes_only_export_type) }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, organisation, codes_only_export_type)
end end
end end
@ -1201,14 +1201,14 @@ RSpec.describe OrganisationsController, type: :request do
it "when codes_only query parameter is false, form contains hidden field with correct value" do it "when codes_only query parameter is false, form contains hidden field with correct value" do
codes_only = false codes_only = false
get "/organisations/#{organisation.id}/logs/csv-download?codes_only=#{codes_only}", headers:, params: {} get "/organisations/#{organisation.id}/logs/csv-download?codes_only=#{codes_only}", headers:, params: {}
hidden_field = page.find("form.button_to").find_field("is_codes_only_export", type: "hidden") hidden_field = page.find("form.button_to").find_field("codes_only_export", type: "hidden")
expect(hidden_field.value).to eq(codes_only.to_s) expect(hidden_field.value).to eq(codes_only.to_s)
end end
it "when codes_only query parameter is true, form contains hidden field with correct value" do it "when codes_only query parameter is true, form contains hidden field with correct value" do
codes_only = true codes_only = true
get "/organisations/#{organisation.id}/logs/csv-download?codes_only=#{codes_only}", headers:, params: {} get "/organisations/#{organisation.id}/logs/csv-download?codes_only=#{codes_only}", headers:, params: {}
hidden_field = page.find("form.button_to").find_field("is_codes_only_export", type: "hidden") hidden_field = page.find("form.button_to").find_field("codes_only_export", type: "hidden")
expect(hidden_field.value).to eq(codes_only.to_s) expect(hidden_field.value).to eq(codes_only.to_s)
end end

2
spec/services/csv/lettings_log_csv_service_spec.rb

@ -222,7 +222,7 @@ RSpec.describe Csv::LettingsLogCsvService do
location_mobility_type location_mobility_type
location_admin_district location_admin_district
location_startdate] location_startdate]
csv = CSV.parse(described_class.new(user).to_csv(is_codes_only_export: false)) csv = CSV.parse(described_class.new(user).to_csv(codes_only_export: false))
expect(csv.first).to eq(expected_csv_attributes) expect(csv.first).to eq(expected_csv_attributes)
end end
end end

Loading…
Cancel
Save