Open your email inbox and click the link to download your CSV file.
- <%= govuk_link_to "Return to logs", lettings_logs_path %> + <%= govuk_link_to "Return to logs", logs_path_for_controller(controller) %>
diff --git a/app/views/logs/index.html.erb b/app/views/logs/index.html.erb index 04209be73..7a4fcba06 100644 --- a/app/views/logs/index.html.erb +++ b/app/views/logs/index.html.erb @@ -78,8 +78,8 @@ 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), + csv_download_url: csv_download_url_for_controller(controller:, search: @search_term, codes_only: false), + csv_codes_only_download_url: csv_download_url_for_controller(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 bac737e6c..55c130c29 100644 --- a/app/views/organisations/logs.html.erb +++ b/app/views/organisations/logs.html.erb @@ -34,8 +34,8 @@ 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), + csv_download_url: csv_download_url_by_log_type(@log_type, @organisation, search: @search_term, codes_only: false), + csv_codes_only_download_url: csv_download_url_by_log_type(@log_type, @organisation, search: @search_term, codes_only: true), } %> <%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "logs" } %> diff --git a/config/routes.rb b/config/routes.rb index 359e7e121..e815b5137 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -113,9 +113,12 @@ Rails.application.routes.draw do get "users/invite", to: "users/account#new" get "lettings-logs", to: "organisations#lettings_logs" get "sales-logs", to: "organisations#sales_logs" - get "logs/csv-download", to: "organisations#download_csv" - post "logs/email-csv", to: "organisations#email_csv" - get "logs/csv-confirmation", to: "lettings_logs#csv_confirmation" + get "lettings-logs/csv-download", to: "organisations#download_lettings_csv" + post "lettings-logs/email-csv", to: "organisations#email_lettings_csv" + get "lettings-logs/csv-confirmation", to: "lettings_logs#csv_confirmation" + get "sales-logs/csv-download", to: "organisations#download_sales_csv" + post "sales-logs/email-csv", to: "organisations#email_sales_csv" + get "sales-logs/csv-confirmation", to: "sales_logs#csv_confirmation" get "schemes", to: "organisations#schemes" get "stock-owners", to: "organisation_relationships#stock_owners" get "stock-owners/add", to: "organisation_relationships#add_stock_owner" @@ -198,6 +201,10 @@ Rails.application.routes.draw do resources :sales_logs, path: "/sales-logs" do collection do + get "csv-download", to: "sales_logs#download_csv" + post "email-csv", to: "sales_logs#email_csv" + get "csv-confirmation", to: "sales_logs#csv_confirmation" + resources :bulk_upload_sales_logs, path: "bulk-upload-logs" do collection do get :start diff --git a/spec/factories/form.rb b/spec/factories/form.rb new file mode 100644 index 000000000..9694cfaac --- /dev/null +++ b/spec/factories/form.rb @@ -0,0 +1,25 @@ +class FormFixture < Form + attr_accessor :sections, :subsections, :pages, :questions +end + +class FormFactory + def initialize(year:, type:) + @year = year + @type = type + end + + def with_sections(sections) + @sections = sections + self + end + + def build + form = FormFixture.new(nil, @year, [], @type) + @sections.each { |section| section.form = form } + form.sections = @sections + form.subsections = form.sections.flat_map(&:subsections) + form.pages = form.subsections.flat_map(&:pages) + form.questions = form.pages.flat_map(&:questions) + form + end +end diff --git a/spec/factories/page.rb b/spec/factories/page.rb new file mode 100644 index 000000000..2b5b27969 --- /dev/null +++ b/spec/factories/page.rb @@ -0,0 +1,21 @@ +FactoryBot.define do + factory :page, class: "Form::Page" do + id { "page_id" } + initialize_with { new(id, nil, nil) } + trait :with_question do + transient do + question_id { nil } + question { nil } + end + + after :build do |page, evaluator| + page.questions = if (q = evaluator.question) + q.page = page + [q] + else + [build(:question, id: evaluator.question_id, page:)] + end + end + end + end +end diff --git a/spec/factories/question.rb b/spec/factories/question.rb new file mode 100644 index 000000000..e34bc5e75 --- /dev/null +++ b/spec/factories/question.rb @@ -0,0 +1,6 @@ +FactoryBot.define do + factory :question, class: "Form::Question" do + initialize_with { new(id, nil, nil) } + type { "text" } + end +end diff --git a/spec/factories/sales_log.rb b/spec/factories/sales_log.rb index fa5773ef3..b17efec90 100644 --- a/spec/factories/sales_log.rb +++ b/spec/factories/sales_log.rb @@ -120,7 +120,8 @@ FactoryBot.define do has_mscharge { 1 } mscharge { 100 } mortlen { 10 } - pcodenk { 1 } + pcodenk { 0 } + postcode_full { "SW1A 1AA" } is_la_inferred { false } mortgagelender { 5 } extrabor { 1 } diff --git a/spec/factories/section.rb b/spec/factories/section.rb new file mode 100644 index 000000000..71bfd52ae --- /dev/null +++ b/spec/factories/section.rb @@ -0,0 +1,16 @@ +FactoryBot.define do + factory :section, class: "Form::Section" do + id { "section_id" } + initialize_with { new(id, nil, nil) } + trait :with_questions do + transient do + question_ids { nil } + questions { nil } + end + + after :build do |section, evaluator| + section.subsections = [build(:subsection, :with_questions, question_ids: evaluator.question_ids, questions: evaluator.questions, section:)] + end + end + end +end diff --git a/spec/factories/subsection.rb b/spec/factories/subsection.rb new file mode 100644 index 000000000..f4e4b307e --- /dev/null +++ b/spec/factories/subsection.rb @@ -0,0 +1,20 @@ +FactoryBot.define do + factory :subsection, class: "Form::Subsection" do + id { "subsection_id" } + initialize_with { new(id, nil, nil) } + trait :with_questions do + transient do + question_ids { nil } + questions { nil } + end + + after :build do |subsection, evaluator| + subsection.pages = if evaluator.questions + evaluator.questions.map { |question| build(:page, :with_question, question:, subsection:) } + else + evaluator.question_ids.map { |question_id| build(:page, :with_question, question_id:, subsection:) } + end + end + end + end +end diff --git a/spec/fixtures/files/sales_logs_csv_export_codes.csv b/spec/fixtures/files/sales_logs_csv_export_codes.csv new file mode 100644 index 000000000..89a6a27bd --- /dev/null +++ b/spec/fixtures/files/sales_logs_csv_export_codes.csv @@ -0,0 +1,2 @@ +id,status,created_at,updated_at,old_id,collection_start_year,creation_method,is_dpo,owning_organisation_name,created_by_name,day,month,year,purchid,ownershipsch,type,othtype,companybuy,buylivein,jointpur,jointmore,beds,proptype,builtype,pcodenk,uprn,uprn_confirmed,address_line1,address_line2,town_or_city,county,pcode1,pcode2,la_known,la,la_label,wchair,noint,privacynotice,age1,sex1,ethnic_group,ethnic,national,ecstat1,buy1livein,relat2,age2,sex2,ethnic_group2,ethnicbuy2,nationalbuy2,ecstat2,buy2livein,hholdcount,relat3,age3,sex3,ecstat3,relat4,age4,sex4,ecstat4,relat5,age5,sex5,ecstat5,relat6,age6,sex6,ecstat6,prevten,ppcodenk,ppostc1,ppostc2,previous_la_known,prevloc,prevloc_label,pregyrha,pregother,pregla,pregghb,pregblank,buy2living,prevtenbuy2,hhregres,hhregresstill,armedforcesspouse,disabled,wheel,income1nk,income1,inc1mort,income2nk,income2,inc2mort,hb,savingsnk,savings,prevown,prevshared,proplen,staircase,stairbought,stairowned,staircasesale,resale,exday,exmonth,exyear,hoday,homonth,hoyear,lanomagr,soctenant,frombeds,fromprop,socprevten,value,equity,mortgageused,mortgage,mortgagelender,mortgagelenderother,mortlen,extrabor,deposit,cashdis,mrent,has_mscharge,mscharge,discount,grant +,completed,2023-02-08T16:52:15+00:00,2023-02-08T16:52:15+00:00,,2022,single log,false,DLUHC,Danny Rojas,2,2,2023,,2,8,,,,1,1,2,1,1,0,,,,,,,SW1A,1AA,1,E09000003,Barnet,1,2,1,30,X,17,17,18,1,1,P,35,X,,,,1,1,1,C,14,X,9,X,18,X,3,R,40,X,2,R,40,X,1,1,1,,,0,,,1,1,1,1,,,,1,4,5,1,1,0,10000,1,0,10000,1,4,1,,1,2,10,,,,,,,,,,,,,,,,,110000.0,,1,20000.0,5,,10,1,80000.0,1000.0,,1,100.0,,10000.0 diff --git a/spec/fixtures/files/sales_logs_csv_export_labels.csv b/spec/fixtures/files/sales_logs_csv_export_labels.csv new file mode 100644 index 000000000..c17122c18 --- /dev/null +++ b/spec/fixtures/files/sales_logs_csv_export_labels.csv @@ -0,0 +1,2 @@ +id,status,created_at,updated_at,old_id,collection_start_year,creation_method,is_dpo,owning_organisation_name,created_by_name,day,month,year,purchid,ownershipsch,type,othtype,companybuy,buylivein,jointpur,jointmore,beds,proptype,builtype,pcodenk,uprn,uprn_confirmed,address_line1,address_line2,town_or_city,county,pcode1,pcode2,la_known,la,la_label,wchair,noint,privacynotice,age1,sex1,ethnic_group,ethnic,national,ecstat1,buy1livein,relat2,age2,sex2,ethnic_group2,ethnicbuy2,nationalbuy2,ecstat2,buy2livein,hholdcount,relat3,age3,sex3,ecstat3,relat4,age4,sex4,ecstat4,relat5,age5,sex5,ecstat5,relat6,age6,sex6,ecstat6,prevten,ppcodenk,ppostc1,ppostc2,previous_la_known,prevloc,prevloc_label,pregyrha,pregother,pregla,pregghb,pregblank,buy2living,prevtenbuy2,hhregres,hhregresstill,armedforcesspouse,disabled,wheel,income1nk,income1,inc1mort,income2nk,income2,inc2mort,hb,savingsnk,savings,prevown,prevshared,proplen,staircase,stairbought,stairowned,staircasesale,resale,exday,exmonth,exyear,hoday,homonth,hoyear,lanomagr,soctenant,frombeds,fromprop,socprevten,value,equity,mortgageused,mortgage,mortgagelender,mortgagelenderother,mortlen,extrabor,deposit,cashdis,mrent,has_mscharge,mscharge,discount,grant +,completed,2023-02-08T16:52:15+00:00,2023-02-08T16:52:15+00:00,,2022,single log,false,DLUHC,Danny Rojas,2,2,2023,,Yes - a discounted ownership scheme,Right to Acquire (RTA),,,,Yes,Yes,2,Flat or maisonette,Purpose built,Yes,,,,,,,SW1A,1AA,Yes,E09000003,Barnet,Yes,Yes,1,30,Non-binary,Buyer 1 prefers not to say,17,United Kingdom,Full-time - 30 hours or more,Yes,Partner,35,Non-binary,,,,Full-time - 30 hours or more,Yes,1,Child,14,Non-binary,Child under 16,Other,18,Non-binary,"In government training into work, such as New Deal",Person prefers not to say,40,Non-binary,Part-time - Less than 30 hours,Person prefers not to say,40,Non-binary,Full-time - 30 hours or more,Local authority tenant,No,,,No,,,1,1,1,1,,,,Yes,Yes,No,Yes,Yes,Yes,10000,Yes,Yes,10000,Yes,Don’t know ,No,,Yes,2,10,,,,,,,,,,,,,,,,,110000.0,,Yes,20000.0,Cambridge Building Society,,10,Yes,80000.0,1000.0,,Yes,100.0,,10000.0 diff --git a/spec/jobs/email_csv_job_spec.rb b/spec/jobs/email_csv_job_spec.rb index 29dceff59..40f13e022 100644 --- a/spec/jobs/email_csv_job_spec.rb +++ b/spec/jobs/email_csv_job_spec.rb @@ -26,6 +26,7 @@ describe EmailCsvJob do let(:storage_service) { instance_double(Storage::S3Service) } let(:mailer) { instance_double(CsvDownloadMailer) } + let(:sales_log_csv_service) { instance_double(Csv::SalesLogCsvService) } before do FactoryBot.create(:lettings_log, @@ -39,18 +40,44 @@ describe EmailCsvJob do allow(storage_service).to receive(:write_file) allow(storage_service).to receive(:get_presigned_url).and_return(test_url) + allow(Csv::SalesLogCsvService).to receive(:new).and_return(sales_log_csv_service) + allow(sales_log_csv_service).to receive(:prepare_csv).and_return("") + allow(CsvDownloadMailer).to receive(:new).and_return(mailer) allow(mailer).to receive(:send_csv_download_mail) end - it "uses an appropriate filename in S3" do - expect(storage_service).to receive(:write_file).with(/logs-.*\.csv/, anything) - job.perform(user) + context "when exporting lettings logs" do + it "uses an appropriate filename in S3" do + expect(storage_service).to receive(:write_file).with(/lettings-logs-.*\.csv/, anything) + job.perform(user) + end + + it "includes the organisation name in the filename when one is provided" do + expect(storage_service).to receive(:write_file).with(/lettings-logs-#{organisation.name}-.*\.csv/, anything) + job.perform(user, nil, {}, nil, organisation) + end end - it "includes the organisation name in the filename when one is provided" do - expect(storage_service).to receive(:write_file).with(/logs-#{organisation.name}-.*\.csv/, anything) - job.perform(user, nil, {}, nil, organisation) + context "when exporting sales logs" do + it "uses an appropriate filename in S3" do + expect(storage_service).to receive(:write_file).with(/sales-logs-.*\.csv/, anything) + job.perform(user, nil, {}, nil, nil, nil, "sales") + end + + it "includes the organisation name in the filename when one is provided" do + expect(storage_service).to receive(:write_file).with(/sales-logs-#{organisation.name}-.*\.csv/, anything) + job.perform(user, nil, {}, nil, organisation, nil, "sales") + end + + it "creates a SalesLogCsvService with the correct export type" do + expect(Csv::SalesLogCsvService).to receive(:new).with(export_type: "labels") + codes_only = false + job.perform(user, nil, {}, nil, nil, codes_only, "sales") + expect(Csv::SalesLogCsvService).to receive(:new).with(export_type: "codes") + codes_only = true + job.perform(user, nil, {}, nil, nil, codes_only, "sales") + end end it "sends an E-mail with the presigned URL and duration" do diff --git a/spec/models/form_handler_spec.rb b/spec/models/form_handler_spec.rb index bbeb56e55..547ed2f21 100644 --- a/spec/models/form_handler_spec.rb +++ b/spec/models/form_handler_spec.rb @@ -219,5 +219,56 @@ RSpec.describe FormHandler do end end end + + describe "#ordered_sales_questions_for_all_years" do + let(:result) { described_class.instance.ordered_sales_questions_for_all_years } + let(:now) { Time.zone.now } + + it "returns an array of questions" do + section = build(:section, :with_questions, question_ids: %w[1 2 3]) + sales_form = FormFactory.new(year: 1936, type: "sales") + .with_sections([section]) + .build + described_class.instance.use_fake_forms!({ only_sales: sales_form }) + expect(result).to(satisfy { |result| result.all? { |element| element.is_a?(Form::Question) } }) + end + + it "does not return multiple questions with the same id" do + first_section = build(:section, :with_questions, question_ids: %w[1 2 3]) + second_section = build(:section, :with_questions, question_ids: %w[2 3 4 5]) + sales_form = FormFactory.new(year: 1936, type: "sales") + .with_sections([first_section, second_section]) + .build + described_class.instance.use_fake_forms!({ only_sales: sales_form }) + expect(result.map(&:id)).to eq %w[1 2 3 4 5] + end + + it "returns the questions in the same order as the form" do + first_section = build(:section, :with_questions, question_ids: %w[1 2 3]) + second_section = build(:section, :with_questions, question_ids: %w[4 5 6]) + sales_form = FormFactory.new(year: 1945, type: "sales") + .with_sections([first_section, second_section]) + .build + described_class.instance.use_fake_forms!({ only_sales: sales_form }) + expect(result.map(&:id)).to eq %w[1 2 3 4 5 6] + end + + it "inserts questions from all years in their correct positions" do + original_section = build(:section, :with_questions, question_ids: %w[1 1a_deprecated 2 3]) + new_section = build(:section, :with_questions, question_ids: %w[1 2 2a_new 3]) + original_form = FormFactory.new(year: 1066, type: "sales") + .with_sections([original_section]) + .build + new_form = FormFactory.new(year: 1485, type: "sales") + .with_sections([new_section]) + .build + fake_forms = { + earlier_sales: original_form, + newer_sales: new_form, + } + described_class.instance.use_fake_forms!(fake_forms) + expect(result.map(&:id)).to eq %w[1 1a_deprecated 2 2a_new 3] + end + end # rubocop:enable RSpec/PredicateMatcher end diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index d526f218f..6013a5813 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -367,7 +367,7 @@ RSpec.describe OrganisationsController, type: :request do end end - context "when viewing logs for other organisation" do + context "when viewing lettings logs for other organisation" do it "does not display the lettings logs" do get "/organisations/#{unauthorised_organisation.id}/lettings-logs", headers:, params: {} expect(response).to have_http_status(:unauthorized) @@ -375,13 +375,13 @@ RSpec.describe OrganisationsController, type: :request do it "prevents CSV download" do expect { - post "/organisations/#{unauthorised_organisation.id}/logs/email-csv", headers:, params: {} + post "/organisations/#{unauthorised_organisation.id}/lettings-logs/email-csv", headers:, params: {} }.not_to enqueue_job(EmailCsvJob) expect(response).to have_http_status(:unauthorized) end end - context "when viewing logs for your organisation" do + context "when viewing lettings logs for your organisation" do it "does not display the logs" do get "/organisations/#{organisation.id}/lettings-logs", headers:, params: {} expect(response).to have_http_status(:unauthorized) @@ -389,7 +389,35 @@ RSpec.describe OrganisationsController, type: :request do it "prevents CSV download" do expect { - post "/organisations/#{organisation.id}/logs/email-csv", headers:, params: {} + post "/organisations/#{organisation.id}/lettings-logs/email-csv", headers:, params: {} + }.not_to enqueue_job(EmailCsvJob) + expect(response).to have_http_status(:unauthorized) + end + end + + context "when viewing sales logs for other organisation" do + it "does not display the sales logs" do + get "/organisations/#{unauthorised_organisation.id}/sales-logs", headers:, params: {} + expect(response).to have_http_status(:unauthorized) + end + + it "prevents CSV download" do + expect { + post "/organisations/#{unauthorised_organisation.id}/sales-logs/email-csv", headers:, params: {} + }.not_to enqueue_job(EmailCsvJob) + expect(response).to have_http_status(:unauthorized) + end + end + + context "when viewing sales logs for your organisation" do + it "does not display the logs" do + get "/organisations/#{organisation.id}/sales-logs", headers:, params: {} + expect(response).to have_http_status(:unauthorized) + end + + it "prevents CSV download" do + expect { + post "/organisations/#{organisation.id}/sales-logs/email-csv", headers:, params: {} }.not_to enqueue_job(EmailCsvJob) expect(response).to have_http_status(:unauthorized) end @@ -544,7 +572,7 @@ RSpec.describe OrganisationsController, type: :request do end end - context "when viewing logs for other organisation" do + context "when viewing lettings logs for other organisation" do it "does not display the logs" do get "/organisations/#{unauthorised_organisation.id}/lettings-logs", headers:, params: {} expect(response).to have_http_status(:unauthorized) @@ -552,13 +580,13 @@ RSpec.describe OrganisationsController, type: :request do it "prevents CSV download" do expect { - post "/organisations/#{unauthorised_organisation.id}/logs/email-csv", headers:, params: {} + post "/organisations/#{unauthorised_organisation.id}/lettings-logs/email-csv", headers:, params: {} }.not_to enqueue_job(EmailCsvJob) expect(response).to have_http_status(:unauthorized) end end - context "when viewing logs for your organisation" do + context "when viewing lettings logs for your organisation" do it "does not display the logs" do get "/organisations/#{organisation.id}/lettings-logs", headers:, params: {} expect(response).to have_http_status(:unauthorized) @@ -566,7 +594,35 @@ RSpec.describe OrganisationsController, type: :request do it "prevents CSV download" do expect { - post "/organisations/#{organisation.id}/logs/email-csv", headers:, params: {} + post "/organisations/#{organisation.id}/lettings-logs/email-csv", headers:, params: {} + }.not_to enqueue_job(EmailCsvJob) + expect(response).to have_http_status(:unauthorized) + end + end + + context "when viewing sales logs for other organisation" do + it "does not display the logs" do + get "/organisations/#{unauthorised_organisation.id}/sales-logs", headers:, params: {} + expect(response).to have_http_status(:unauthorized) + end + + it "prevents CSV download" do + expect { + post "/organisations/#{unauthorised_organisation.id}/sales-logs/email-csv", headers:, params: {} + }.not_to enqueue_job(EmailCsvJob) + expect(response).to have_http_status(:unauthorized) + end + end + + context "when viewing sales logs for your organisation" do + it "does not display the logs" do + get "/organisations/#{organisation.id}/sales-logs", headers:, params: {} + expect(response).to have_http_status(:unauthorized) + end + + it "prevents CSV download" do + expect { + post "/organisations/#{organisation.id}/sales-logs/email-csv", headers:, params: {} }.not_to enqueue_job(EmailCsvJob) expect(response).to have_http_status(:unauthorized) end @@ -1176,15 +1232,15 @@ RSpec.describe OrganisationsController, type: :request do sign_in user end - context "when they view the logs tab" do + context "when they view the lettings logs tab" do before do FactoryBot.create(:lettings_log, owning_organisation: organisation) end 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") + expect(page).to have_link("Download (CSV)", href: "/organisations/#{organisation.id}/lettings-logs/csv-download?codes_only=false") + expect(page).to have_link("Download (CSV, codes only)", href: "/organisations/#{organisation.id}/lettings-logs/csv-download?codes_only=true") end context "when you download the CSV" do @@ -1197,62 +1253,151 @@ RSpec.describe OrganisationsController, type: :request do end it "only includes logs from that organisation" do - get "/organisations/#{organisation.id}/logs/csv-download?codes_only=false" + get "/organisations/#{organisation.id}/lettings-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&codes_only=false", headers:, params: {} + post "/organisations/#{organisation.id}/lettings-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: {} + post "/organisations/#{organisation.id}/lettings-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: {} + post "/organisations/#{organisation.id}/lettings-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 + context "when they view the sales logs tab" do + before do + FactoryBot.create(:sales_log, owning_organisation: organisation) + end + + it "has CSV download buttons with the correct paths if at least 1 log exists" do + get "/organisations/#{organisation.id}/sales-logs" + expect(page).to have_link("Download (CSV)", href: "/organisations/#{organisation.id}/sales-logs/csv-download?codes_only=false") + expect(page).to have_link("Download (CSV, codes only)", href: "/organisations/#{organisation.id}/sales-logs/csv-download?codes_only=true") + end + + context "when you download the CSV" do + let(:other_organisation) { FactoryBot.create(:organisation) } + + before do + FactoryBot.create_list(:sales_log, 2, owning_organisation: organisation) + FactoryBot.create(:sales_log, owning_organisation: organisation, status: "pending", skip_update_status: true) + FactoryBot.create_list(:sales_log, 2, owning_organisation: other_organisation) + end + + it "only includes logs from that organisation" do + get "/organisations/#{organisation.id}/sales-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}/sales-logs/email-csv?status[]=completed&codes_only=false", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, nil, { "status" => %w[completed] }, false, organisation, false, "sales") + end + + it "provides the log type to the mail job" do + log_type = "sales" + expect { + post "/organisations/#{organisation.id}/sales-logs/email-csv?status[]=completed&codes_only=false", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, nil, { "status" => %w[completed] }, false, organisation, false, log_type) + end + + it "provides the export type to the mail job" do + codes_only_export_type = false + expect { + post "/organisations/#{organisation.id}/sales-logs/email-csv?codes_only=#{codes_only_export_type}", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, organisation, codes_only_export_type, "sales") + codes_only_export_type = true + expect { + post "/organisations/#{organisation.id}/sales-logs/email-csv?codes_only=#{codes_only_export_type}", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, organisation, codes_only_export_type, "sales") + end + end + end + + describe "GET #download_lettings_csv" do + it "renders a page with the correct header" do + get "/organisations/#{organisation.id}/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 "/organisations/#{organisation.id}/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("/organisations/#{organisation.id}/lettings-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}/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 "/organisations/#{organisation.id}/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 query string contains search parameter, form contains hidden field with correct value" do + search_term = "blam" + get "/organisations/#{organisation.id}/lettings-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 + + describe "GET #download_sales_csv" do it "renders a page with the correct header" do - get "/organisations/#{organisation.id}/logs/csv-download?codes_only=false", headers:, params: {} + get "/organisations/#{organisation.id}/sales-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: {} + get "/organisations/#{organisation.id}/sales-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[:action]).to eq("/organisations/#{organisation.id}/sales-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: {} + get "/organisations/#{organisation.id}/sales-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: {} + get "/organisations/#{organisation.id}/sales-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: {} + get "/organisations/#{organisation.id}/sales-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 diff --git a/spec/requests/sales_logs_controller_spec.rb b/spec/requests/sales_logs_controller_spec.rb index e400366df..6927d294a 100644 --- a/spec/requests/sales_logs_controller_spec.rb +++ b/spec/requests/sales_logs_controller_spec.rb @@ -148,6 +148,16 @@ RSpec.describe SalesLogsController, type: :request do expect(page).to have_content(other_organisation.name) end + it "shows a link for labelled CSV download of logs" do + get "/sales-logs", headers: headers, params: {} + expect(page).to have_link("Download (CSV)", href: "/sales-logs/csv-download?codes_only=false") + end + + it "shows a link for codes only CSV download of logs" do + get "/sales-logs", headers: headers, params: {} + expect(page).to have_link("Download (CSV, codes only)", href: "/sales-logs/csv-download?codes_only=true") + end + context "when there are no logs in the database" do before do SalesLog.destroy_all @@ -157,6 +167,12 @@ RSpec.describe SalesLogsController, type: :request do get "/sales-logs", headers: headers, params: {} expect(page).to have_title("Logs - Submit social housing lettings and sales data (CORE) - GOV.UK") end + + it "does not show CSV download links" do + get "/sales-logs", headers: headers, params: {} + expect(page).not_to have_link("Download (CSV)") + expect(page).not_to have_link("Download (CSV, codes only)") + end end context "when there is a pending log" do @@ -298,6 +314,19 @@ RSpec.describe SalesLogsController, type: :request do expect(page).not_to have_content("Managing organisation") end + it "displays standard CSV download link only, with the correct path" do + get "/sales-logs", headers:, params: {} + expect(page).to have_link("Download (CSV)", href: "/sales-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 + SalesLog.destroy_all + get "/sales-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(:sales_log, 3, :completed, owning_organisation: user.organisation, created_by: user) } let(:log_to_search) { FactoryBot.create(:sales_log, :completed, owning_organisation: user.organisation, created_by: user) } @@ -316,6 +345,13 @@ RSpec.describe SalesLogsController, type: :request do end end + it "displays the labelled CSV download link, with the search included in the query params" do + get "/sales-logs?search=#{log_to_search.id}", 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" => [log_to_search.id.to_s]) + end + context "when search query doesn't match any logs" do it "doesn't display any logs" do get "/sales-logs?search=foobar", headers:, params: {} @@ -352,7 +388,7 @@ RSpec.describe SalesLogsController, 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 "/sales-logs", headers:, params: {} end @@ -469,5 +505,151 @@ RSpec.describe SalesLogsController, type: :request do page.assert_selector(".govuk-button", text: "Create a new lettings log", count: 0) end end + + context "when requesting CSV download" do + let(:headers) { { "Accept" => "text/html" } } + let(:search_term) { "foot" } + let(:codes_only) { false } + + before do + allow(user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in user + get "/sales-logs/csv-download?search=#{search_term}&codes_only=#{codes_only}", headers: + end + + it "returns http success" do + expect(response).to have_http_status(:success) + end + + it "shows a confirmation button" do + expect(page).to have_button("Send email") + end + + it "has a hidden field with the search term" do + expect(page).to have_field("search", type: "hidden", with: search_term) + end + + context "when user is not support" do + context "and export type is not codes only" do + it "has a hidden field with the export type" do + expect(page).to have_field("codes_only", type: "hidden", with: codes_only) + end + end + + context "and export type is codes only" do + let(:codes_only) { true } + + it "the user is not authorised" do + expect(response).to have_http_status(:unauthorized) + end + end + end + + context "when user is support" do + let(:user) { FactoryBot.create(:user, :support) } + + context "and export type is not codes only" do + it "has a hidden field with the export type" do + expect(page).to have_field("codes_only", type: "hidden", with: codes_only) + end + end + + context "and export type is codes only" do + let(:codes_only) { true } + + it "has a hidden field with the export type" do + expect(page).to have_field("codes_only", type: "hidden", with: codes_only) + end + end + end + end + + context "when confirming the CSV email" do + let(:headers) { { "Accept" => "text/html" } } + + it "confirms that the user will receive an email with the requested CSV" do + sign_in user + get "/sales-logs/csv-confirmation" + expect(CGI.unescape_html(response.body)).to include("We’re sending you an email") + end + end + end + + describe "POST #email-csv" do + let(:other_organisation) { FactoryBot.create(:organisation) } + let(:user) { FactoryBot.create(:user, :support) } + let!(:sales_log) do + FactoryBot.create( + :sales_log, + created_by: user, + ) + end + + before do + allow(user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in user + FactoryBot.create(:sales_log) + FactoryBot.create(:sales_log, + :completed, + owning_organisation:, + created_by: user) + end + + it "creates an E-mail job with the correct log type" do + expect { + post "/sales-logs/email-csv?codes_only=true", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, nil, true, "sales") + end + + it "redirects to the confirmation page" do + post "/sales-logs/email-csv?codes_only=true", headers:, params: {} + expect(response).to redirect_to(csv_confirmation_sales_logs_path) + end + + it "passes the search term" do + expect { + post "/sales-logs/email-csv?search=#{sales_log.id}&codes_only=false", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, sales_log.id.to_s, {}, false, nil, false, "sales") + end + + it "passes filter parameters" do + expect { + post "/sales-logs/email-csv?status[]=completed&codes_only=true", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, nil, { "status" => %w[completed] }, false, nil, true, "sales") + end + + it "passes export type flag" do + expect { + post "/sales-logs/email-csv?codes_only=true", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, nil, true, "sales") + expect { + post "/sales-logs/email-csv?codes_only=false", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, nil, false, "sales") + end + + it "passes a combination of search term, export type and filter parameters" do + postcode = "XX1 1TG" + + expect { + post "/sales-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, "sales") + 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 "/sales-logs/email-csv?codes_only=#{codes_only_export}", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, nil, false, "sales") + end + + it "is not authorized to download codes only csv" do + codes_only_export = true + post "/sales-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/services/csv/sales_log_csv_service_spec.rb b/spec/services/csv/sales_log_csv_service_spec.rb new file mode 100644 index 000000000..96099aa70 --- /dev/null +++ b/spec/services/csv/sales_log_csv_service_spec.rb @@ -0,0 +1,189 @@ +require "rails_helper" + +RSpec.describe Csv::SalesLogCsvService do + let(:form_handler_mock) { instance_double(FormHandler) } + let(:organisation) { create(:organisation) } + let!(:log) { create(:sales_log, :completed, owning_organisation: organisation) } + let(:service) { described_class.new(export_type: "labels") } + let(:csv) { CSV.parse(service.prepare_csv(SalesLog.all)) } + + it "calls the form handler to get all questions in order when initialized" do + allow(FormHandler).to receive(:instance).and_return(form_handler_mock) + allow(form_handler_mock).to receive(:ordered_sales_questions_for_all_years).and_return([]) + described_class.new(export_type: "codes") + expect(form_handler_mock).to have_received(:ordered_sales_questions_for_all_years) + end + + it "returns a string" do + result = service.prepare_csv(SalesLog.all) + expect(result).to be_a String + end + + it "returns a csv with headers" do + expect(csv.first.first).to eq "id" + expect(csv.second.first).not_to be log.id.to_s + end + + context "when stubbing :ordered_sales_questions_for_all_years" do + let(:sales_form) do + FormFactory.new(year: 1936, type: "sales") + .with_sections([build(:section, :with_questions, question_ids:, questions:)]) + .build + end + let(:question_ids) { nil } + let(:questions) { nil } + + before do + allow(FormHandler).to receive(:instance).and_return(form_handler_mock) + allow(form_handler_mock).to receive(:form_name_from_start_year) + allow(form_handler_mock).to receive(:get_form).and_return(sales_form) + allow(form_handler_mock).to receive(:ordered_sales_questions_for_all_years).and_return(sales_form.questions) + end + + context "when it returns questions with particular ids" do + let(:question_ids) { %w[type age1 buy1livein exdate] } + + it "includes log attributes related to questions to the headers" do + headers = csv.first + expect(headers).to include(*question_ids.first(3)) + end + + it "removes some log attributes related to questions from the headers and replaces them with their derived values in the correct order" do + headers = csv.first + expect(headers).not_to include "exdate" + expect(headers.last(4)).to eq %w[buy1livein exday exmonth exyear] + end + end + + context "when it returns questions with particular features" do + let(:questions) do + [ + build(:question, id: "attribute_value_check", type: "interruption_screen"), + build(:question, id: "something_or_other_known", type: "radio"), + build(:question, id: "whatchamacallit_asked", type: "radio"), + build(:question, id: "ownershipsch"), + build(:question, id: "checkbox_question", type: "checkbox", answer_options: { "pregyrha" => {}, "pregother" => {} }), + build(:question, id: "type"), + ] + end + + it "does not add questions for checks, whether some other attribute is known or whether something else was asked" do + headers = csv.first + expect(headers).not_to include "attribute_value_check" + expect(headers).not_to include "something_or_other_known" + expect(headers).not_to include "whatchamacallit_asked" + end + + it "does not add the id of checkbox questions, but adds the related attributes of the log in the correct order" do + headers = csv.first + expect(headers.last(4)).to eq %w[ownershipsch pregyrha pregother type] + end + end + end + + it "includes attributes not related to questions to the headers" do + headers = csv.first + expect(headers).to include(*%w[id status created_at updated_at old_id]) + end + + it "returns a csv with the correct number of logs" do + create_list(:sales_log, 15) + log_count = SalesLog.count + expected_row_count_with_headers = log_count + 1 + expect(csv.size).to be expected_row_count_with_headers + end + + context "when exporting with human readable labels" do + it "gives answers to radio questions as their labels" do + national_column_index = csv.first.index("national") + national_value = csv.second[national_column_index] + expect(national_value).to eq "United Kingdom" + relat2_column_index = csv.first.index("relat2") + relat2_value = csv.second[relat2_column_index] + expect(relat2_value).to eq "Partner" + end + + it "gives answers to free input questions as the user input" do + age1_column_index = csv.first.index("age1") + age1_value = csv.second[age1_column_index] + expect(age1_value).to eq 30.to_s + postcode_part1, postcode_part2 = log.postcode_full.split + postcode_part1_column_index = csv.first.index("pcode1") + postcode_part1_value = csv.second[postcode_part1_column_index] + expect(postcode_part1_value).to eq postcode_part1 + postcode_part2_column_index = csv.first.index("pcode2") + postcode_part2_value = csv.second[postcode_part2_column_index] + expect(postcode_part2_value).to eq postcode_part2 + end + + it "exports the code for the local authority under the heading 'la'" do + la_column_index = csv.first.index("la") + la_value = csv.second[la_column_index] + expect(la_value).to eq "E09000003" + end + + it "exports the label for the local authority under the heading 'la_label'" do + la_label_column_index = csv.first.index("la_label") + la_label_value = csv.second[la_label_column_index] + expect(la_label_value).to eq "Barnet" + end + + it "exports the CSV with all values correct" do + expected_content = CSV.read("spec/fixtures/files/sales_logs_csv_export_labels.csv") + values_to_delete = %w[id] + values_to_delete.each do |attribute| + index = csv.first.index(attribute) + csv.second[index] = nil + end + expect(csv).to eq expected_content + end + end + + context "when exporting values as codes" do + let(:service) { described_class.new(export_type: "codes") } + + it "gives answers to radio questions as their codes" do + national_column_index = csv.first.index("national") + national_value = csv.second[national_column_index] + expect(national_value).to eq 18.to_s + relat2_column_index = csv.first.index("relat2") + relat2_value = csv.second[relat2_column_index] + expect(relat2_value).to eq "P" + end + + it "gives answers to free input questions as the user input" do + age1_column_index = csv.first.index("age1") + age1_value = csv.second[age1_column_index] + expect(age1_value).to eq 30.to_s + postcode_part1, postcode_part2 = log.postcode_full.split + postcode_part1_column_index = csv.first.index("pcode1") + postcode_part1_value = csv.second[postcode_part1_column_index] + expect(postcode_part1_value).to eq postcode_part1 + postcode_part2_column_index = csv.first.index("pcode2") + postcode_part2_value = csv.second[postcode_part2_column_index] + expect(postcode_part2_value).to eq postcode_part2 + end + + it "exports the code for the local authority under the heading 'la'" do + la_column_index = csv.first.index("la") + la_value = csv.second[la_column_index] + expect(la_value).to eq "E09000003" + end + + it "exports the label for the local authority under the heading 'la_label'" do + la_label_column_index = csv.first.index("la_label") + la_label_value = csv.second[la_label_column_index] + expect(la_label_value).to eq "Barnet" + end + + it "exports the CSV with all values correct" do + expected_content = CSV.read("spec/fixtures/files/sales_logs_csv_export_codes.csv") + values_to_delete = %w[id] + values_to_delete.each do |attribute| + index = csv.first.index(attribute) + csv.second[index] = nil + end + expect(csv).to eq expected_content + end + end +end