From a2b684b1234440f01838b6acb658978c59a432b8 Mon Sep 17 00:00:00 2001 From: Arthur Campbell <51094020+arfacamble@users.noreply.github.com> Date: Tue, 14 Mar 2023 14:55:32 +0000 Subject: [PATCH] CLDC-1826 lettings log codes only download (#1268) * update seeds to add self in review env, change spec to reflect this, update config yml to allow csv exports in review * update interface of relevant methods EmailCsvJob, LettingsLog.to_csv and LettingsLogCsvService consume codes_only flag * update tests including adding a new csv file to test against * update LettingsLogCsvService to output codes only csv * correct minor error and linting * enable codes only download in UI - add link on lettings log index page - pass codes_only flag through params in relevant links and methods - convert flag to boolean in controller methods * ensure link displayed successfully for all renderings of logs_list and params passed through relevant methods in organisations controller * fix existing tests * correct linting thing * correct linting error * update tests for lettings log controller * correct linting errors * update organisations controller tests * make minor changes after code review * remove changes made for testing on review app * make codes only download visible to support users only * change variable names throughout after info on rauby/rails naming conventions, update tests for change in who can view codes only download link * rework csv service for readability, remove delegating methods from lettings log to keep all code to do with mapping between our domain and desired export format in one place * update test name * correct a small typo and remove a duplicated method after clever git merge conflict suggestion * point review app at staging csv bucket for csv download * change variables named codes_only_export to codes_only to avoid inconsistency * write tests to ensure that differetn user roles have the correct permissions around csv download * ensure that non support users may not download codes only exports * correct a small error in a previous commit * correct minor linting error --- .github/workflows/review_pipeline.yml | 2 +- app/controllers/lettings_logs_controller.rb | 16 +- app/controllers/organisations_controller.rb | 6 +- app/helpers/logs_helper.rb | 7 +- app/jobs/email_csv_job.rb | 4 +- app/models/lettings_log.rb | 20 +- app/services/csv/lettings_log_csv_service.rb | 123 ++++++++-- app/views/logs/_log_list.html.erb | 5 +- app/views/logs/download_csv.html.erb | 2 +- app/views/logs/index.html.erb | 12 +- app/views/organisations/logs.html.erb | 12 +- .../form/accessible_autocomplete_spec.rb | 2 +- .../form/progressive_total_field_spec.rb | 2 +- .../lettings_logs_download_codes_only.csv | 2 + spec/models/lettings_log_spec.rb | 75 ++++-- .../requests/lettings_logs_controller_spec.rb | 215 ++++++++++++++---- .../requests/organisations_controller_spec.rb | 61 ++++- .../csv/lettings_log_csv_service_spec.rb | 2 +- 18 files changed, 451 insertions(+), 117 deletions(-) create mode 100644 spec/fixtures/files/lettings_logs_download_codes_only.csv diff --git a/.github/workflows/review_pipeline.yml b/.github/workflows/review_pipeline.yml index 5656898bc..ac883c788 100644 --- a/.github/workflows/review_pipeline.yml +++ b/.github/workflows/review_pipeline.yml @@ -123,7 +123,7 @@ jobs: cf set-env $APP_NAME IMPORT_PAAS_INSTANCE $IMPORT_PAAS_INSTANCE cf set-env $APP_NAME EXPORT_PAAS_INSTANCE "dluhc-core-review-export-bucket" cf set-env $APP_NAME S3_CONFIG $S3_CONFIG - cf set-env $APP_NAME CSV_DOWNLOAD_PAAS_INSTANCE "dluhc-core-review-csv-bucket" + cf set-env $APP_NAME CSV_DOWNLOAD_PAAS_INSTANCE "dluhc-core-staging-csv-bucket" cf set-env $APP_NAME SENTRY_DSN $SENTRY_DSN cf set-env $APP_NAME APP_HOST "https://dluhc-core-review-${{ github.event.pull_request.number }}.london.cloudapps.digital" diff --git a/app/controllers/lettings_logs_controller.rb b/app/controllers/lettings_logs_controller.rb index f86a55a27..e07878b0b 100644 --- a/app/controllers/lettings_logs_controller.rb +++ b/app/controllers/lettings_logs_controller.rb @@ -2,10 +2,16 @@ class LettingsLogsController < LogsController before_action :find_resource, except: %i[create index edit] before_action :session_filters, if: :current_user, only: %i[index email_csv download_csv] before_action :set_session_filters, if: :current_user, only: %i[index email_csv download_csv] + before_action :authenticate_scope!, only: %i[download_csv email_csv] before_action :extract_bulk_upload_from_session_filters, only: [:index] before_action :redirect_if_bulk_upload_resolved, only: [:index] + def authenticate_scope! + codes_only_export = codes_only_export?(params) + head :unauthorized and return unless current_user.support? || !codes_only_export + end + def index respond_to do |format| format.html do @@ -80,13 +86,19 @@ class LettingsLogsController < LogsController def download_csv unpaginated_filtered_logs = filtered_logs(current_user.lettings_logs, search_term, @session_filters) + codes_only = codes_only_export?(params) + + render "download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: email_csv_lettings_logs_path, codes_only: } + end - render "download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: email_csv_lettings_logs_path } + def codes_only_export?(params) + params.require(:codes_only) == "true" end def email_csv all_orgs = params["organisation_select"] == "all" - EmailCsvJob.perform_later(current_user, search_term, @session_filters, all_orgs) + codes_only_export = params.require(:codes_only) == "true" + EmailCsvJob.perform_later(current_user, search_term, @session_filters, all_orgs, nil, codes_only_export) redirect_to csv_confirmation_lettings_logs_path end diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 211283ffe..418526960 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -107,12 +107,14 @@ class OrganisationsController < ApplicationController def download_csv organisation_logs = LettingsLog.all.where(owning_organisation_id: @organisation.id) unpaginated_filtered_logs = filtered_logs(organisation_logs, search_term, @session_filters) + codes_only = params.require(:codes_only) == "true" - render "logs/download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: logs_email_csv_organisation_path } + render "logs/download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: logs_email_csv_organisation_path, codes_only: } end def email_csv - EmailCsvJob.perform_later(current_user, search_term, @session_filters, false, @organisation) + codes_only_export = params.require(:codes_only) == "true" + EmailCsvJob.perform_later(current_user, search_term, @session_filters, false, @organisation, codes_only_export) redirect_to logs_csv_confirmation_organisation_path end diff --git a/app/helpers/logs_helper.rb b/app/helpers/logs_helper.rb index 7653b7e5c..aa132afd6 100644 --- a/app/helpers/logs_helper.rb +++ b/app/helpers/logs_helper.rb @@ -33,10 +33,9 @@ module LogsHelper end end - def csv_download_url_for_controller(controller) - case log_type_for_controller(controller) - when "lettings" - csv_download_lettings_logs_path(search: params["search"]) + def csv_download_url_for_controller(controller_type:, search:, codes_only:) + case log_type_for_controller(controller_type) + when "lettings" then csv_download_lettings_logs_path(search:, codes_only:) end end end diff --git a/app/jobs/email_csv_job.rb b/app/jobs/email_csv_job.rb index c7009645f..6e7888f18 100644 --- a/app/jobs/email_csv_job.rb +++ b/app/jobs/email_csv_job.rb @@ -5,14 +5,14 @@ class EmailCsvJob < ApplicationJob EXPIRATION_TIME = 3.hours.to_i - def perform(user, search_term = nil, filters = {}, all_orgs = false, organisation = nil) # 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 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" 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)) + 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) diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index 108cb1be1..dad1ef705 100644 --- a/app/models/lettings_log.rb +++ b/app/models/lettings_log.rb @@ -425,27 +425,13 @@ class LettingsLog < Log created_by&.is_dpo end - delegate :service_name, :sensitive, :registered_under_care_act, :primary_client_group, :has_other_client_group, :secondary_client_group, :owning_organisation, :managing_organisation, :support_type, :intended_stay, :created_at, prefix: "scheme", to: :scheme, allow_nil: true - delegate :scheme_type, to: :scheme, allow_nil: true - def scheme_code scheme&.id ? "S#{scheme.id}" : nil end - def scheme_owning_organisation_name - scheme_owning_organisation&.name - end - - delegate :postcode, :name, :units, :type_of_unit, :mobility_type, :startdate, prefix: "location", to: :location, allow_nil: true - delegate :location_admin_district, to: :location, allow_nil: true - - # This is not the location_code in the db, location.id is just called code in the UI - def location_code - location&.id - end - - def self.to_csv(user = nil) - Csv::LettingsLogCsvService.new(user).to_csv + def self.to_csv(user = nil, codes_only_export:) + export_type = codes_only_export ? "codes" : "labels" + Csv::LettingsLogCsvService.new(user, export_type:).to_csv end def beds_for_la_rent_range diff --git a/app/services/csv/lettings_log_csv_service.rb b/app/services/csv/lettings_log_csv_service.rb index c8a2cf41e..09d373b01 100644 --- a/app/services/csv/lettings_log_csv_service.rb +++ b/app/services/csv/lettings_log_csv_service.rb @@ -2,8 +2,9 @@ module Csv class LettingsLogCsvService CSV_FIELDS_TO_OMIT = %w[hhmemb net_income_value_check first_time_property_let_as_social_housing renttype needstype postcode_known is_la_inferred totchild totelder totadult net_income_known is_carehome previous_la_known is_previous_la_inferred age1_known age2_known age3_known age4_known age5_known age6_known age7_known age8_known letting_allocation_unknown details_known_2 details_known_3 details_known_4 details_known_5 details_known_6 details_known_7 details_known_8 rent_type_detail wrent wscharge wpschrge wsupchrg wtcharge wtshortfall rent_value_check old_form_id old_id retirement_value_check tshortfall_known pregnancy_value_check hhtype new_old vacdays la prevloc unresolved updated_by_id bulk_upload_id].freeze - def initialize(user) + def initialize(user, export_type:) @user = user + @export_type = export_type set_csv_attributes end @@ -12,30 +13,126 @@ module Csv csv << @attributes LettingsLog.all.find_each do |record| - csv << @attributes.map do |att| - label_from_value(record, att) - end + csv << @attributes.map { |attribute| get_value(attribute, record) } end end end private - def label_from_value(record, att) - if %w[la prevloc].include? att - label_from_boolean_value(record.send(att)) - elsif %w[mrcdate startdate voiddate].include? att - record.send(att)&.to_formatted_s(:govuk_date) + ATTRIBUTES_OF_RELATED_OBJECTS = { + location_code: { + labels: %i[location id], + codes: %i[location id], + }, + location_postcode: { + labels: %i[location postcode], + codes: %i[location postcode], + }, + location_name: { + labels: %i[location name], + codes: %i[location name], + }, + location_units: { + labels: %i[location units], + codes: %i[location units], + }, + location_type_of_unit: { + labels: %i[location type_of_unit], + codes: %i[location type_of_unit_before_type_cast], + }, + location_mobility_type: { + labels: %i[location mobility_type], + codes: %i[location mobility_type_before_type_cast], + }, + location_admin_district: { + labels: %i[location location_admin_district], + codes: %i[location location_admin_district], + }, + location_startdate: { + labels: %i[location startdate], + codes: %i[location startdate], + }, + scheme_service_name: { + labels: %i[scheme service_name], + codes: %i[scheme service_name], + }, + scheme_sensitive: { + labels: %i[scheme sensitive], + codes: %i[scheme sensitive_before_type_cast], + }, + scheme_type: { + labels: %i[scheme scheme_type], + codes: %i[scheme scheme_type_before_type_cast], + }, + scheme_registered_under_care_act: { + labels: %i[scheme registered_under_care_act], + codes: %i[scheme registered_under_care_act_before_type_cast], + }, + scheme_owning_organisation_name: { + labels: %i[scheme owning_organisation name], + codes: %i[scheme owning_organisation name], + }, + scheme_primary_client_group: { + labels: %i[scheme primary_client_group], + codes: %i[scheme primary_client_group_before_type_cast], + }, + scheme_has_other_client_group: { + labels: %i[scheme has_other_client_group], + codes: %i[scheme has_other_client_group_before_type_cast], + }, + scheme_secondary_client_group: { + labels: %i[scheme secondary_client_group], + codes: %i[scheme secondary_client_group_before_type_cast], + }, + scheme_support_type: { + labels: %i[scheme support_type], + codes: %i[scheme support_type_before_type_cast], + }, + scheme_intended_stay: { + labels: %i[scheme intended_stay], + codes: %i[scheme intended_stay_before_type_cast], + }, + scheme_created_at: { + labels: %i[scheme created_at], + codes: %i[scheme created_at], + }, + }.freeze + + def get_value(attribute, record) + attribute = "rent_type" if attribute == "rent_type_detail" # rent_type_detail is the requested column header for rent_type, so as not to confuse with renttype + if ATTRIBUTES_OF_RELATED_OBJECTS.key? attribute.to_sym + call_chain = ATTRIBUTES_OF_RELATED_OBJECTS[attribute.to_sym][@export_type.to_sym] + call_chain.reduce(record) { |object, next_call| object&.send(next_call) } + elsif %w[la prevloc].include? attribute # for all exports we output both the codes and labels for these location attributes + record.send(attribute) + elsif %w[la_label prevloc_label].include? attribute # as above + attribute = attribute.remove("_label") + field_value = record.send(attribute) + get_label(field_value, attribute, record) + elsif %w[mrcdate startdate voiddate].include? attribute + record.send(attribute)&.to_formatted_s(:govuk_date) else - record.form.get_question(att.remove("_label"), record)&.label_from_value(record.send(att.remove("_label"))) || label_from_boolean_value(record.send(att.remove("_label"))) + field_value = record.send(attribute) + case @export_type + when "codes" + field_value + when "labels" + answer_label = get_label(field_value, attribute, record) + answer_label || label_if_boolean_value(field_value) || field_value + end end end - def label_from_boolean_value(value) + def get_label(value, attribute, record) + record.form + .get_question(attribute, record) + &.label_from_value(value) + end + + def label_if_boolean_value(value) return "Yes" if value == true return "No" if value == false - - value end def set_csv_attributes diff --git a/app/views/logs/_log_list.html.erb b/app/views/logs/_log_list.html.erb index 1aeaa03f2..9112cde81 100644 --- a/app/views/logs/_log_list.html.erb +++ b/app/views/logs/_log_list.html.erb @@ -1,7 +1,10 @@

<%= render(SearchResultCaptionComponent.new(searched:, count: pagy.count, item_label:, total_count:, item: "logs", path: request.path)) %> <% if logs&.first&.lettings? %> - <%= govuk_link_to "Download (CSV)", csv_download_url, type: "text/csv" %> + <%= govuk_link_to "Download (CSV)", csv_download_url, type: "text/csv", class: "govuk-!-margin-right-4" %> + <% if @current_user.support? %> + <%= govuk_link_to "Download (CSV, codes only)", csv_codes_only_download_url, type: "text/csv" %> + <% end %> <% end %>

<% logs.map do |log| %> diff --git a/app/views/logs/download_csv.html.erb b/app/views/logs/download_csv.html.erb index 4e0da2704..5aeea8173 100644 --- a/app/views/logs/download_csv.html.erb +++ b/app/views/logs/download_csv.html.erb @@ -11,6 +11,6 @@

We'll send a secure download link to your email address <%= @current_user.email %>.

You've selected <%= count %> logs.

- <%= govuk_button_to "Send email", post_path, method: :post, params: { search: search_term } %> + <%= govuk_button_to "Send email", post_path, method: :post, params: { search: search_term, codes_only: } %> diff --git a/app/views/logs/index.html.erb b/app/views/logs/index.html.erb index 0e4cd76df..2d88c9bd6 100644 --- a/app/views/logs/index.html.erb +++ b/app/views/logs/index.html.erb @@ -66,7 +66,17 @@
<%= render SearchComponent.new(current_user:, search_label: search_label_for_controller(controller), value: @searched) %> <%= govuk_section_break(visible: true, size: "m") %> - <%= render partial: "log_list", locals: { logs: @logs, title: "Logs", pagy: @pagy, searched: @searched, item_label:, total_count: @total_count, csv_download_url: csv_download_url_for_controller(controller) } %> + <%= render partial: "log_list", + locals: { + logs: @logs, + title: "Logs", + pagy: @pagy, + searched: @searched, + item_label:, + total_count: @total_count, + csv_download_url: csv_download_url_for_controller(controller_type: controller, search: @search_term, codes_only: false), + csv_codes_only_download_url: csv_download_url_for_controller(controller_type: controller, search: @search_term, codes_only: true), + } %> <%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "logs" } %>
diff --git a/app/views/organisations/logs.html.erb b/app/views/organisations/logs.html.erb index 94fe8e9bc..bac737e6c 100644 --- a/app/views/organisations/logs.html.erb +++ b/app/views/organisations/logs.html.erb @@ -26,7 +26,17 @@
<%= render SearchComponent.new(current_user:, search_label: "Search by log ID, tenant code, property reference or postcode", value: @searched) %> <%= govuk_section_break(visible: true, size: "m") %> - <%= render partial: "logs/log_list", locals: { logs: @logs, title: "Logs", pagy: @pagy, searched: @searched, item_label:, total_count: @total_count, csv_download_url: logs_csv_download_organisation_path(@organisation, search: @search_term) } %> + <%= render partial: "logs/log_list", + locals: { + logs: @logs, + title: "Logs", + pagy: @pagy, + searched: @searched, + item_label:, + total_count: @total_count, + csv_download_url: logs_csv_download_organisation_path(@organisation, search: @search_term, codes_only: false), + csv_codes_only_download_url: logs_csv_download_organisation_path(@organisation, search: @search_term, codes_only: true), + } %> <%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "logs" } %>
diff --git a/spec/features/form/accessible_autocomplete_spec.rb b/spec/features/form/accessible_autocomplete_spec.rb index fd8102dcc..160d32d38 100644 --- a/spec/features/form/accessible_autocomplete_spec.rb +++ b/spec/features/form/accessible_autocomplete_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" require_relative "helpers" -RSpec.describe "Accessible Automcomplete" do +RSpec.describe "Accessible Autocomplete" do include Helpers let(:user) { FactoryBot.create(:user) } let(:lettings_log) do diff --git a/spec/features/form/progressive_total_field_spec.rb b/spec/features/form/progressive_total_field_spec.rb index e5959d16d..787174c20 100644 --- a/spec/features/form/progressive_total_field_spec.rb +++ b/spec/features/form/progressive_total_field_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" require_relative "helpers" -RSpec.describe "Accessible Automcomplete" do +RSpec.describe "Accessible Autocomplete" do include Helpers let(:user) { FactoryBot.create(:user) } let(:lettings_log) do diff --git a/spec/fixtures/files/lettings_logs_download_codes_only.csv b/spec/fixtures/files/lettings_logs_download_codes_only.csv new file mode 100644 index 000000000..b0bd8be1a --- /dev/null +++ b/spec/fixtures/files/lettings_logs_download_codes_only.csv @@ -0,0 +1,2 @@ +id,status,created_at,updated_at,created_by_name,is_dpo,owning_organisation_name,managing_organisation_name,collection_start_year,needstype,renewal,startdate,rent_type_detail,irproduct_other,tenancycode,propcode,age1,sex1,ecstat1,hhmemb,relat2,age2,sex2,retirement_value_check,ecstat2,armedforces,leftreg,illness,housingneeds_a,housingneeds_b,housingneeds_c,housingneeds_h,is_previous_la_inferred,prevloc_label,prevloc,illness_type_1,illness_type_2,is_la_inferred,la_label,la,postcode_known,postcode_full,previous_la_known,wchair,preg_occ,cbl,earnings,incfreq,net_income_value_check,benefits,hb,period,brent,scharge,pscharge,supcharg,tcharge,offered,layear,ppostcode_full,mrcdate,declaration,ethnic,national,prevten,age3,sex3,ecstat3,age4,sex4,ecstat4,age5,sex5,ecstat5,age6,sex6,ecstat6,age7,sex7,ecstat7,age8,sex8,ecstat8,homeless,underoccupation_benefitcap,reservist,startertenancy,tenancylength,tenancy,rsnvac,unittype_gn,beds,waityear,reasonpref,chr,cap,reasonother,housingneeds_f,housingneeds_g,illness_type_3,illness_type_4,illness_type_8,illness_type_5,illness_type_6,illness_type_7,illness_type_9,illness_type_10,rp_homeless,rp_insan_unsat,rp_medwel,rp_hardship,rp_dontknow,tenancyother,property_owner_organisation,property_manager_organisation,purchaser_code,reason,majorrepairs,hbrentshortfall,property_relet,incref,first_time_property_let_as_social_housing,unitletas,builtype,voiddate,renttype,lettype,totchild,totelder,totadult,net_income_known,nocharge,is_carehome,household_charge,referral,tshortfall,chcharge,ppcodenk,age1_known,age2_known,age3_known,age4_known,age5_known,age6_known,age7_known,age8_known,ethnic_group,letting_allocation_unknown,details_known_2,details_known_3,details_known_4,details_known_5,details_known_6,details_known_7,details_known_8,has_benefits,wrent,wscharge,wpschrge,wsupchrg,wtcharge,wtshortfall,refused,housingneeds,wchchrg,newprop,relat3,relat4,relat5,relat6,relat7,relat8,rent_value_check,old_form_id,lar,irproduct,old_id,joint,tshortfall_known,sheltered,pregnancy_value_check,hhtype,new_old,vacdays,major_repairs_date_value_check,void_date_value_check,housingneeds_type,housingneeds_other,unresolved,updated_by_id,unittype_sh,scheme_code,scheme_service_name,scheme_sensitive,scheme_type,scheme_registered_under_care_act,scheme_owning_organisation_name,scheme_primary_client_group,scheme_has_other_client_group,scheme_secondary_client_group,scheme_support_type,scheme_intended_stay,scheme_created_at,location_code,location_postcode,location_name,location_units,location_type_of_unit,location_mobility_type,location_admin_district,location_startdate +{id},in_progress,2022-02-08 16:52:15 +0000,2022-02-08 16:52:15 +0000,Danny Rojas,false,DLUHC,DLUHC,2021,2,,2 October 2021,2,,,,,,,,,,,,,,,,,,,,false,,,,,false,Westminster,E09000033,,SE1 1TE,,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,2,8,0,0,0,,0,,,,,,,,,,,,,,,,,,,,,,,,0,,,,,,,0,,,,,,,,,,,,,,,,,,,9,1,,,,,,,,6,{scheme_code},{scheme_service_name},{scheme_sensitive},0,1,DLUHC,{scheme_primary_client_group},,{scheme_secondary_client_group},{scheme_support_type},{scheme_intended_stay},2021-04-01 00:00:00 +0100,{location_code},SE1 1TE,Downing Street,20,6,A,Westminster,{location_startdate} diff --git a/spec/models/lettings_log_spec.rb b/spec/models/lettings_log_spec.rb index c1bfafb57..c6f559aec 100644 --- a/spec/models/lettings_log_spec.rb +++ b/spec/models/lettings_log_spec.rb @@ -2685,40 +2685,67 @@ RSpec.describe LettingsLog do let(:user) { FactoryBot.create(:user, organisation: location.scheme.owning_organisation) } let(:expected_content) { csv_export_file.read } - before do - Timecop.freeze(Time.utc(2022, 6, 5)) - lettings_log = FactoryBot.create(:lettings_log, needstype: 2, scheme:, location:, owning_organisation: scheme.owning_organisation, created_by: user, rent_type: 2, startdate: Time.zone.local(2021, 10, 2)) - expected_content.sub!(/\{id\}/, lettings_log["id"].to_s) - expected_content.sub!(/\{scheme_code\}/, "S#{scheme['id']}") - expected_content.sub!(/\{scheme_service_name\}/, scheme["service_name"].to_s) - expected_content.sub!(/\{scheme_sensitive\}/, scheme["sensitive"].to_s) - expected_content.sub!(/\{scheme_primary_client_group\}/, scheme["primary_client_group"].to_s) - expected_content.sub!(/\{scheme_secondary_client_group\}/, scheme["secondary_client_group"].to_s) - expected_content.sub!(/\{scheme_support_type\}/, scheme["support_type"].to_s) - expected_content.sub!(/\{scheme_intended_stay\}/, scheme["intended_stay"].to_s) - expected_content.sub!(/\{location_code\}/, location["id"].to_s) - expected_content.sub!(/\{location_startdate\}/, location["startdate"].to_s) - expected_content.sub!(/\{scheme_id\}/, scheme["service_name"].to_s) - expected_content.sub!(/\{location_id\}/, location["id"].to_s) - end - after do Timecop.unfreeze end - context "with a support user" do - let(:csv_export_file) { File.open("spec/fixtures/files/lettings_logs_download.csv", "r:UTF-8") } + context "with values represented as human readable labels" do + before do + Timecop.freeze(Time.utc(2022, 6, 5)) + lettings_log = FactoryBot.create(:lettings_log, needstype: 2, scheme:, location:, owning_organisation: scheme.owning_organisation, created_by: user, rent_type: 2, startdate: Time.zone.local(2021, 10, 2)) + expected_content.sub!(/\{id\}/, lettings_log["id"].to_s) + expected_content.sub!(/\{scheme_code\}/, "S#{scheme['id']}") + expected_content.sub!(/\{scheme_service_name\}/, scheme["service_name"].to_s) + expected_content.sub!(/\{scheme_sensitive\}/, scheme["sensitive"].to_s) + expected_content.sub!(/\{scheme_primary_client_group\}/, scheme["primary_client_group"].to_s) + expected_content.sub!(/\{scheme_secondary_client_group\}/, scheme["secondary_client_group"].to_s) + expected_content.sub!(/\{scheme_support_type\}/, scheme["support_type"].to_s) + expected_content.sub!(/\{scheme_intended_stay\}/, scheme["intended_stay"].to_s) + expected_content.sub!(/\{location_code\}/, location["id"].to_s) + expected_content.sub!(/\{location_startdate\}/, location["startdate"].to_s) + expected_content.sub!(/\{scheme_id\}/, scheme["service_name"].to_s) + expected_content.sub!(/\{location_id\}/, location["id"].to_s) + end - it "generates a correct csv from a lettings log" do - expect(described_class.to_csv).to eq(expected_content) + context "with a support user" do + 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 + expect(described_class.to_csv(codes_only_export: false)).to eq(expected_content) + end + end + + context "with a non support user" do + 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 + expect(described_class.to_csv(user, codes_only_export: false)).to eq(expected_content) + end end end - context "with a non support user" do - let(:csv_export_file) { File.open("spec/fixtures/files/lettings_logs_download_non_support.csv", "r:UTF-8") } + context "with values represented as codes" do + before do + Timecop.freeze(Time.utc(2022, 6, 5)) + lettings_log = FactoryBot.create(:lettings_log, needstype: 2, scheme:, location:, owning_organisation: scheme.owning_organisation, created_by: user, rent_type: 2, startdate: Time.zone.local(2021, 10, 2)) + expected_content.sub!(/\{id\}/, lettings_log["id"].to_s) + expected_content.sub!(/\{scheme_code\}/, "S#{scheme.id}") + expected_content.sub!(/\{scheme_service_name\}/, scheme.service_name.to_s) + expected_content.sub!(/\{scheme_sensitive\}/, scheme.sensitive_before_type_cast.to_s) + expected_content.sub!(/\{scheme_primary_client_group\}/, scheme.primary_client_group_before_type_cast.to_s) + expected_content.sub!(/\{scheme_secondary_client_group\}/, scheme.secondary_client_group_before_type_cast.to_s) + expected_content.sub!(/\{scheme_support_type\}/, scheme.support_type_before_type_cast.to_s) + expected_content.sub!(/\{scheme_intended_stay\}/, scheme.intended_stay_before_type_cast.to_s) + expected_content.sub!(/\{location_code\}/, location.id.to_s) + expected_content.sub!(/\{location_startdate\}/, location.startdate.to_s) + expected_content.sub!(/\{scheme_id\}/, scheme.service_name.to_s) + expected_content.sub!(/\{location_id\}/, location.id.to_s) + end + + 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 - expect(described_class.to_csv(user)).to eq(expected_content) + expect(described_class.to_csv(codes_only_export: true)).to eq(expected_content) end end end diff --git a/spec/requests/lettings_logs_controller_spec.rb b/spec/requests/lettings_logs_controller_spec.rb index 9db2c3236..4e242cd76 100644 --- a/spec/requests/lettings_logs_controller_spec.rb +++ b/spec/requests/lettings_logs_controller_spec.rb @@ -252,24 +252,36 @@ RSpec.describe LettingsLogsController, type: :request do end it "does have organisation values" do - get "/lettings-logs", headers: headers, params: {} + get "/lettings-logs", headers:, params: {} expect(page).to have_content("Owned by") expect(page).to have_content("Managed by") end it "shows lettings logs for all organisations" do - get "/lettings-logs", headers: headers, params: {} + get "/lettings-logs", headers:, params: {} expect(page).to have_content("LC783") expect(page).to have_content("UA984") end + it "displays CSV download links with the correct paths" do + 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, codes only)", href: "/lettings-logs/csv-download?codes_only=true") + end + context "when there are no logs in the database" do before do LettingsLog.destroy_all end + it "does not display CSV download links" do + get "/lettings-logs", headers:, params: {} + expect(page).not_to have_link("Download (CSV)") + expect(page).not_to have_link("Download (CSV, codes only)") + end + it "page has correct title" do - get "/lettings-logs", headers: headers, params: {} + get "/lettings-logs", headers:, params: {} expect(page).to have_title("Logs - Submit social housing lettings and sales data (CORE) - GOV.UK") end end @@ -292,29 +304,29 @@ RSpec.describe LettingsLogsController, type: :request do end it "shows lettings logs for multiple selected statuses" do - get "/lettings-logs?status[]=in_progress&status[]=completed", headers: headers, params: {} + get "/lettings-logs?status[]=in_progress&status[]=completed", headers:, params: {} expect(page).to have_link(in_progress_lettings_log.id.to_s) expect(page).to have_link(completed_lettings_log.id.to_s) end it "shows lettings logs for one selected status" do - get "/lettings-logs?status[]=in_progress", headers: headers, params: {} + get "/lettings-logs?status[]=in_progress", headers:, params: {} expect(page).to have_link(in_progress_lettings_log.id.to_s) expect(page).not_to have_link(completed_lettings_log.id.to_s) end it "filters on organisation" do - get "/lettings-logs?organisation[]=#{organisation_2.id}", headers: headers, params: {} + get "/lettings-logs?organisation[]=#{organisation_2.id}", headers:, params: {} expect(page).to have_link(completed_lettings_log.id.to_s) expect(page).not_to have_link(in_progress_lettings_log.id.to_s) end it "does not reset the filters" do - get "/lettings-logs?status[]=in_progress", headers: headers, params: {} + get "/lettings-logs?status[]=in_progress", headers:, params: {} expect(page).to have_link(in_progress_lettings_log.id.to_s) expect(page).not_to have_link(completed_lettings_log.id.to_s) - get "/lettings-logs", headers: headers, params: {} + get "/lettings-logs", headers:, params: {} expect(page).to have_link(in_progress_lettings_log.id.to_s) expect(page).not_to have_link(completed_lettings_log.id.to_s) end @@ -338,13 +350,13 @@ RSpec.describe LettingsLogsController, type: :request do end it "shows lettings logs for multiple selected years" do - get "/lettings-logs?years[]=2021&years[]=2022", headers: headers, params: {} + get "/lettings-logs?years[]=2021&years[]=2022", headers:, params: {} expect(page).to have_link(lettings_log_2021.id.to_s) expect(page).to have_link(lettings_log_2022.id.to_s) end it "shows lettings logs for one selected year" do - get "/lettings-logs?years[]=2021", headers: headers, params: {} + get "/lettings-logs?years[]=2021", headers:, params: {} expect(page).to have_link(lettings_log_2021.id.to_s) expect(page).not_to have_link(lettings_log_2022.id.to_s) end @@ -387,14 +399,14 @@ RSpec.describe LettingsLogsController, type: :request do end it "shows lettings logs for multiple selected statuses and years" do - get "/lettings-logs?years[]=2021&years[]=2022&status[]=in_progress&status[]=completed", headers: headers, params: {} + get "/lettings-logs?years[]=2021&years[]=2022&status[]=in_progress&status[]=completed", headers:, params: {} expect(page).to have_link(lettings_log_2021.id.to_s) expect(page).to have_link(lettings_log_2022.id.to_s) expect(page).to have_link(lettings_log_2022_in_progress.id.to_s) end it "shows lettings logs for one selected status" do - get "/lettings-logs?years[]=2022&status[]=in_progress", headers: headers, params: {} + get "/lettings-logs?years[]=2022&status[]=in_progress", headers:, params: {} expect(page).to have_link(lettings_log_2022_in_progress.id.to_s) expect(page).not_to have_link(lettings_log_2021.id.to_s) expect(page).not_to have_link(lettings_log_2022.id.to_s) @@ -512,23 +524,36 @@ RSpec.describe LettingsLogsController, type: :request do end it "does not have organisation columns" do - get "/lettings-logs", headers: headers, params: {} + get "/lettings-logs", headers:, params: {} expect(page).not_to have_content("Owning organisation") expect(page).not_to have_content("Managing organisation") end + it "displays standard CSV download link only, with the correct path" do + get "/lettings-logs", headers:, params: {} + expect(page).to have_link("Download (CSV)", href: "/lettings-logs/csv-download?codes_only=false") + expect(page).not_to have_link("Download (CSV, codes only)") + end + + it "does not display CSV download links if there are no logs" do + LettingsLog.destroy_all + get "/lettings-logs", headers:, params: {} + expect(page).not_to have_link("Download (CSV)") + expect(page).not_to have_link("Download (CSV, codes only)") + end + context "when using a search query" do let(:logs) { FactoryBot.create_list(:lettings_log, 3, :completed, owning_organisation: user.organisation, created_by: user) } let(:log_to_search) { FactoryBot.create(:lettings_log, :completed, owning_organisation: user.organisation, created_by: user) } let(:log_total_count) { LettingsLog.where(owning_organisation: user.organisation).count } it "has search results in the title" do - get "/lettings-logs?search=#{log_to_search.id}", headers: headers, params: {} + get "/lettings-logs?search=#{log_to_search.id}", headers:, params: {} expect(page).to have_title("Logs (1 logs matching ‘#{log_to_search.id}’) - Submit social housing lettings and sales data (CORE) - GOV.UK") end it "shows lettings logs matching the id" do - get "/lettings-logs?search=#{log_to_search.id}", headers: headers, params: {} + get "/lettings-logs?search=#{log_to_search.id}", headers:, params: {} expect(page).to have_link(log_to_search.id.to_s) logs.each do |log| expect(page).not_to have_link(log.id.to_s) @@ -536,7 +561,7 @@ RSpec.describe LettingsLogsController, type: :request do end it "shows lettings logs matching the tenant code" do - get "/lettings-logs?search=#{log_to_search.tenancycode}", headers: headers, params: {} + get "/lettings-logs?search=#{log_to_search.tenancycode}", headers:, params: {} expect(page).to have_link(log_to_search.id.to_s) logs.each do |log| expect(page).not_to have_link(log.id.to_s) @@ -544,7 +569,7 @@ RSpec.describe LettingsLogsController, type: :request do end it "shows lettings logs matching the property reference" do - get "/lettings-logs?search=#{log_to_search.propcode}", headers: headers, params: {} + get "/lettings-logs?search=#{log_to_search.propcode}", headers:, params: {} expect(page).to have_link(log_to_search.id.to_s) logs.each do |log| expect(page).not_to have_link(log.id.to_s) @@ -552,25 +577,27 @@ RSpec.describe LettingsLogsController, type: :request do end it "shows lettings logs matching the property postcode" do - get "/lettings-logs?search=#{log_to_search.postcode_full}", headers: headers, params: {} + get "/lettings-logs?search=#{log_to_search.postcode_full}", headers:, params: {} expect(page).to have_link(log_to_search.id.to_s) logs.each do |log| expect(page).not_to have_link(log.id.to_s) end end - it "includes the search on the CSV link" do + it "includes the search on the CSV links" do search_term = "foo" FactoryBot.create(:lettings_log, created_by: user, owning_organisation: user.organisation, tenancycode: "foo") - get "/lettings-logs?search=#{search_term}", headers: headers, params: {} - expect(page).to have_link("Download (CSV)", href: "/lettings-logs/csv-download?search=#{search_term}") + get "/lettings-logs?search=#{search_term}", headers:, params: {} + download_link = page.find_link("Download (CSV)") + download_link_params = CGI.parse(URI.parse(download_link[:href]).query) + expect(download_link_params).to include("search" => [search_term]) end context "when more than one results with matching postcode" do let!(:matching_postcode_log) { FactoryBot.create(:lettings_log, :completed, owning_organisation: user.organisation, postcode_full: log_to_search.postcode_full) } it "displays all matching logs" do - get "/lettings-logs?search=#{log_to_search.postcode_full}", headers: headers, params: {} + get "/lettings-logs?search=#{log_to_search.postcode_full}", headers:, params: {} expect(page).to have_link(log_to_search.id.to_s) expect(page).to have_link(matching_postcode_log.id.to_s) logs.each do |log| @@ -585,12 +612,12 @@ RSpec.describe LettingsLogsController, type: :request do let(:log_total_count) { LettingsLog.where(owning_organisation: user.organisation).count } it "has title with pagination details for page 1" do - get "/lettings-logs?search=#{logs[0].postcode_full}", headers: headers, params: {} + get "/lettings-logs?search=#{logs[0].postcode_full}", headers:, params: {} expect(page).to have_title("Logs (#{logs.count} logs matching ‘#{postcode}’) (page 1 of 2) - Submit social housing lettings and sales data (CORE) - GOV.UK") end it "has title with pagination details for page 2" do - get "/lettings-logs?search=#{logs[0].postcode_full}&page=2", headers: headers, params: {} + get "/lettings-logs?search=#{logs[0].postcode_full}&page=2", headers:, params: {} expect(page).to have_title("Logs (#{logs.count} logs matching ‘#{postcode}’) (page 2 of 2) - Submit social housing lettings and sales data (CORE) - GOV.UK") end end @@ -621,7 +648,7 @@ RSpec.describe LettingsLogsController, type: :request do let!(:log_matching_filter_and_search) { FactoryBot.create(:lettings_log, :in_progress, owning_organisation: user.organisation, postcode_full: matching_postcode, created_by: user) } it "shows only logs matching both search and filters" do - get "/lettings-logs?search=#{matching_postcode}&status[]=#{matching_status}", headers: headers, params: {} + get "/lettings-logs?search=#{matching_postcode}&status[]=#{matching_status}", headers:, params: {} expect(page).to have_link(log_matching_filter_and_search.id.to_s) expect(page).not_to have_link(log_to_search.id.to_s) logs.each do |log| @@ -631,7 +658,7 @@ RSpec.describe LettingsLogsController, type: :request do end end - context "when there are less than 20 logs" do + context "when there are fewer than 20 logs" do before do get "/lettings-logs", headers:, params: {} end @@ -675,7 +702,7 @@ RSpec.describe LettingsLogsController, type: :request do end it "shows the CSV download link" do - expect(page).to have_link("Download (CSV)", href: "/lettings-logs/csv-download") + expect(page).to have_link("Download (CSV)", href: "/lettings-logs/csv-download?codes_only=false") end it "does not show the organisation filter" do @@ -809,7 +836,7 @@ RSpec.describe LettingsLogsController, type: :request do context "with a user that is not signed in" do it "does not let the user get lettings log tasklist pages they don't have access to" do - get "/lettings-logs/#{lettings_log.id}", headers: headers, params: {} + get "/lettings-logs/#{lettings_log.id}", headers:, params: {} expect(response).to redirect_to("/account/sign-in") end end @@ -975,7 +1002,7 @@ RSpec.describe LettingsLogsController, type: :request do before do sign_in user - get "/lettings-logs/csv-download?search=#{search_term}", headers: + get "/lettings-logs/csv-download?search=#{search_term}&codes_only=false", headers: end it "returns http success" do @@ -1246,8 +1273,91 @@ RSpec.describe LettingsLogsController, type: :request do end end + describe "GET #csv-download" do + let(:page) { Capybara::Node::Simple.new(response.body) } + let(:user) { FactoryBot.create(:user) } + let(:headers) { { "Accept" => "text/html" } } + + before do + allow(user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in user + end + + it "renders a page with the correct header" do + get "/lettings-logs/csv-download?codes_only=false", headers:, params: {} + header = page.find_css("h1") + expect(header.text).to include("Download CSV") + end + + it "renders a form with the correct target containing a button with the correct text" do + get "/lettings-logs/csv-download?codes_only=false", headers:, params: {} + form = page.find("form.button_to") + expect(form[:method]).to eq("post") + expect(form[:action]).to eq("/lettings-logs/email-csv") + expect(form).to have_button("Send email") + end + + it "when query string contains search parameter, form contains hidden field with correct value" do + search_term = "blam" + get "/lettings-logs/csv-download?codes_only=false&search=#{search_term}", headers:, params: {} + hidden_field = page.find("form.button_to").find_field("search", type: "hidden") + expect(hidden_field.value).to eq(search_term) + end + + context "when the user is a data coordinator" do + let(:user) { FactoryBot.create(:user, :data_coordinator) } + + it "when codes_only query parameter is false, form contains hidden field with correct value" do + codes_only = false + get "/lettings-logs/csv-download?codes_only=#{codes_only}", headers:, params: {} + hidden_field = page.find("form.button_to").find_field("codes_only", type: "hidden") + expect(hidden_field.value).to eq(codes_only.to_s) + end + + it "when codes_only query parameter is true, user is not authorized" do + codes_only = true + get "/lettings-logs/csv-download?codes_only=#{codes_only}", headers:, params: {} + expect(response).to have_http_status(:unauthorized) + end + end + + context "when the user is a data provider" do + it "when codes_only query parameter is false, form contains hidden field with correct value" do + codes_only = false + get "/lettings-logs/csv-download?codes_only=#{codes_only}", headers:, params: {} + hidden_field = page.find("form.button_to").find_field("codes_only", type: "hidden") + expect(hidden_field.value).to eq(codes_only.to_s) + end + + it "when codes_only query parameter is true, user is not authorized" do + codes_only = true + get "/lettings-logs/csv-download?codes_only=#{codes_only}", headers:, params: {} + expect(response).to have_http_status(:unauthorized) + end + end + + context "when the user is a support user" do + let(:user) { FactoryBot.create(:user, :support) } + + it "when codes_only query parameter is false, form contains hidden field with correct value" do + codes_only = false + get "/lettings-logs/csv-download?codes_only=#{codes_only}", headers:, params: {} + hidden_field = page.find("form.button_to").find_field("codes_only", type: "hidden") + expect(hidden_field.value).to eq(codes_only.to_s) + end + + it "when codes_only query parameter is true, form contains hidden field with correct value" do + codes_only = true + get "/lettings-logs/csv-download?codes_only=#{codes_only}", headers:, params: {} + hidden_field = page.find("form.button_to").find_field("codes_only", type: "hidden") + expect(hidden_field.value).to eq(codes_only.to_s) + end + end + end + describe "POST #email-csv" do let(:other_organisation) { FactoryBot.create(:organisation) } + let(:user) { FactoryBot.create(:user, :support) } context "when a log exists" do let!(:lettings_log) do @@ -1259,6 +1369,7 @@ RSpec.describe LettingsLogsController, type: :request do end before do + allow(user).to receive(:need_two_factor_authentication?).and_return(false) sign_in user FactoryBot.create(:lettings_log) FactoryBot.create(:lettings_log, @@ -1270,33 +1381,59 @@ RSpec.describe LettingsLogsController, type: :request do it "creates an E-mail job" do expect { - post "/lettings-logs/email-csv", headers:, params: {} - }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false) + post "/lettings-logs/email-csv?codes_only=true", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, nil, true) end it "redirects to the confirmation page" do - post "/lettings-logs/email-csv", headers:, params: {} + post "/lettings-logs/email-csv?codes_only=true", headers:, params: {} expect(response).to redirect_to(csv_confirmation_lettings_logs_path) end it "passes the search term" do expect { - post "/lettings-logs/email-csv?search=#{lettings_log.id}", headers:, params: {} - }.to enqueue_job(EmailCsvJob).with(user, lettings_log.id.to_s, {}, false) + post "/lettings-logs/email-csv?search=#{lettings_log.id}&codes_only=false", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, lettings_log.id.to_s, {}, false, nil, false) end it "passes filter parameters" do expect { - post "/lettings-logs/email-csv?status[]=completed", headers:, params: {} - }.to enqueue_job(EmailCsvJob).with(user, nil, { "status" => %w[completed] }, false) + post "/lettings-logs/email-csv?status[]=completed&codes_only=true", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, nil, { "status" => %w[completed] }, false, nil, true) + end + + it "passes export type flag" do + expect { + post "/lettings-logs/email-csv?codes_only=true", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, nil, true) + expect { + post "/lettings-logs/email-csv?codes_only=false", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, nil, false) end - it "passes a combination of search term and filter parameters" do + it "passes a combination of search term, export type and filter parameters" do postcode = "XX1 1TG" expect { - post "/lettings-logs/email-csv?status[]=completed&search=#{postcode}", headers:, params: {} - }.to enqueue_job(EmailCsvJob).with(user, postcode, { "status" => %w[completed] }, false) + post "/lettings-logs/email-csv?status[]=completed&search=#{postcode}&codes_only=false", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, postcode, { "status" => %w[completed] }, false, nil, false) + end + + context "when the user is not a support user" do + let(:user) { FactoryBot.create(:user, :data_coordinator) } + + it "has permission to download human readable csv" do + codes_only_export = false + expect { + post "/lettings-logs/email-csv?codes_only=#{codes_only_export}", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, nil, false) + end + + it "is not authorized to download codes only csv" do + codes_only_export = true + post "/lettings-logs/email-csv?codes_only=#{codes_only_export}", headers:, params: {} + expect(response).to have_http_status(:unauthorized) + end end end end diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index 004ea538a..e7cb7fea3 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -1142,11 +1142,12 @@ RSpec.describe OrganisationsController, type: :request do context "when they view the logs tab" do before do FactoryBot.create(:lettings_log, owning_organisation: organisation) - get "/organisations/#{organisation.id}/lettings-logs" end - it "has a CSV download button with the correct path if at least 1 log exists" do - expect(page).to have_link("Download (CSV)", href: "/organisations/#{organisation.id}/logs/csv-download") + it "has CSV download buttons with the correct paths if at least 1 log exists" do + get "/organisations/#{organisation.id}/lettings-logs" + expect(page).to have_link("Download (CSV)", href: "/organisations/#{organisation.id}/logs/csv-download?codes_only=false") + expect(page).to have_link("Download (CSV, codes only)", href: "/organisations/#{organisation.id}/logs/csv-download?codes_only=true") end context "when you download the CSV" do @@ -1158,16 +1159,64 @@ RSpec.describe OrganisationsController, type: :request do end it "only includes logs from that organisation" do - get "/organisations/#{organisation.id}/logs/csv-download" + get "/organisations/#{organisation.id}/logs/csv-download?codes_only=false" expect(page).to have_text("You've selected 3 logs.") end it "provides the organisation to the mail job" do expect { - post "/organisations/#{organisation.id}/logs/email-csv?status[]=completed", headers:, params: {} - }.to enqueue_job(EmailCsvJob).with(user, nil, { "status" => %w[completed] }, false, organisation) + post "/organisations/#{organisation.id}/logs/email-csv?status[]=completed&codes_only=false", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, nil, { "status" => %w[completed] }, false, organisation, false) end + + it "provides the export type to the mail job" do + codes_only_export_type = false + expect { + post "/organisations/#{organisation.id}/logs/email-csv?codes_only=#{codes_only_export_type}", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, organisation, codes_only_export_type) + codes_only_export_type = true + expect { + post "/organisations/#{organisation.id}/logs/email-csv?codes_only=#{codes_only_export_type}", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, organisation, codes_only_export_type) + end + end + end + + describe "GET #download_csv" do + it "renders a page with the correct header" do + get "/organisations/#{organisation.id}/logs/csv-download?codes_only=false", headers:, params: {} + header = page.find_css("h1") + expect(header.text).to include("Download CSV") + end + + it "renders a form with the correct target containing a button with the correct text" do + get "/organisations/#{organisation.id}/logs/csv-download?codes_only=false", headers:, params: {} + form = page.find("form.button_to") + expect(form[:method]).to eq("post") + expect(form[:action]).to eq("/organisations/#{organisation.id}/logs/email-csv") + expect(form).to have_button("Send email") + end + + it "when codes_only query parameter is false, form contains hidden field with correct value" do + codes_only = false + get "/organisations/#{organisation.id}/logs/csv-download?codes_only=#{codes_only}", headers:, params: {} + hidden_field = page.find("form.button_to").find_field("codes_only", type: "hidden") + expect(hidden_field.value).to eq(codes_only.to_s) + end + + it "when codes_only query parameter is true, form contains hidden field with correct value" do + codes_only = true + get "/organisations/#{organisation.id}/logs/csv-download?codes_only=#{codes_only}", headers:, params: {} + hidden_field = page.find("form.button_to").find_field("codes_only", type: "hidden") + expect(hidden_field.value).to eq(codes_only.to_s) + end + + it "when query string contains search parameter, form contains hidden field with correct value" do + search_term = "blam" + get "/organisations/#{organisation.id}/logs/csv-download?codes_only=true&search=#{search_term}", headers:, params: {} + hidden_field = page.find("form.button_to").find_field("search", type: "hidden") + expect(hidden_field.value).to eq(search_term) end end diff --git a/spec/services/csv/lettings_log_csv_service_spec.rb b/spec/services/csv/lettings_log_csv_service_spec.rb index 95b3069dc..00955bc37 100644 --- a/spec/services/csv/lettings_log_csv_service_spec.rb +++ b/spec/services/csv/lettings_log_csv_service_spec.rb @@ -222,7 +222,7 @@ RSpec.describe Csv::LettingsLogCsvService do location_mobility_type location_admin_district location_startdate] - csv = CSV.parse(described_class.new(user).to_csv) + csv = CSV.parse(described_class.new(user, export_type: "labels").to_csv) expect(csv.first).to eq(expected_csv_attributes) end end