diff --git a/app/controllers/lettings_logs_controller.rb b/app/controllers/lettings_logs_controller.rb index e07878b0b..c97f5f45d 100644 --- a/app/controllers/lettings_logs_controller.rb +++ b/app/controllers/lettings_logs_controller.rb @@ -15,7 +15,7 @@ class LettingsLogsController < LogsController def index respond_to do |format| format.html do - all_logs = current_user.lettings_logs + all_logs = current_user.lettings_logs.visible unpaginated_filtered_logs = filtered_logs(all_logs, search_term, @session_filters) @search_term = search_term @@ -140,7 +140,7 @@ private end def find_resource - @log = LettingsLog.find_by(id: params[:id]) + @log = LettingsLog.visible.find_by(id: params[:id]) end def post_create_redirect_url(log) diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 418526960..1bd4694ec 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -90,7 +90,7 @@ class OrganisationsController < ApplicationController end def lettings_logs - organisation_logs = LettingsLog.where(owning_organisation_id: @organisation.id) + organisation_logs = LettingsLog.visible.where(owning_organisation_id: @organisation.id) unpaginated_filtered_logs = filtered_logs(organisation_logs, search_term, @session_filters) respond_to do |format| @@ -105,7 +105,7 @@ class OrganisationsController < ApplicationController end def download_csv - organisation_logs = LettingsLog.all.where(owning_organisation_id: @organisation.id) + organisation_logs = LettingsLog.visible.where(owning_organisation_id: @organisation.id) unpaginated_filtered_logs = filtered_logs(organisation_logs, search_term, @session_filters) codes_only = params.require(:codes_only) == "true" diff --git a/app/jobs/email_csv_job.rb b/app/jobs/email_csv_job.rb index 6e7888f18..f313c87b8 100644 --- a/app/jobs/email_csv_job.rb +++ b/app/jobs/email_csv_job.rb @@ -6,7 +6,7 @@ class EmailCsvJob < ApplicationJob EXPIRATION_TIME = 3.hours.to_i def perform(user, search_term = nil, filters = {}, all_orgs = false, organisation = nil, codes_only_export = false) # rubocop:disable Style/OptionalBooleanParameter - sidekiq can't serialise named params - unfiltered_logs = organisation.present? && user.support? ? LettingsLog.where(owning_organisation_id: organisation.id) : user.lettings_logs + unfiltered_logs = organisation.present? && user.support? ? LettingsLog.visible.where(owning_organisation_id: organisation.id) : user.lettings_logs.visible 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" diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index 4a0651a29..9e55bd0e5 100644 --- a/app/models/lettings_log.rb +++ b/app/models/lettings_log.rb @@ -53,6 +53,8 @@ class LettingsLog < Log scope :unresolved, -> { where(unresolved: true) } scope :filter_by_organisation, ->(org, _user = nil) { where(owning_organisation: org).or(where(managing_organisation: org)) } + scope :visible, -> { where(status: %w[not_started in_progress completed]) } + AUTOGENERATED_FIELDS = %w[id status created_at updated_at discarded_at].freeze OPTIONAL_FIELDS = %w[first_time_property_let_as_social_housing tenancycode propcode chcharge].freeze RENT_TYPE_MAPPING_LABELS = { 1 => "Social Rent", 2 => "Affordable Rent", 3 => "Intermediate Rent" }.freeze diff --git a/app/services/exports/lettings_log_export_service.rb b/app/services/exports/lettings_log_export_service.rb index 197e0ed4b..158d52147 100644 --- a/app/services/exports/lettings_log_export_service.rb +++ b/app/services/exports/lettings_log_export_service.rb @@ -115,12 +115,13 @@ module Exports def retrieve_lettings_logs(start_time, full_update) recent_export = LogsExport.order("started_at").last + if !full_update && recent_export params = { from: recent_export.started_at, to: start_time } - LettingsLog.where("updated_at >= :from and updated_at <= :to", params) + LettingsLog.visible.where("updated_at >= :from and updated_at <= :to", params) else params = { to: start_time } - LettingsLog.where("updated_at <= :to", params) + LettingsLog.visible.where("updated_at <= :to", params) end end diff --git a/spec/jobs/email_csv_job_spec.rb b/spec/jobs/email_csv_job_spec.rb index 04776aac7..29dceff59 100644 --- a/spec/jobs/email_csv_job_spec.rb +++ b/spec/jobs/email_csv_job_spec.rb @@ -61,6 +61,7 @@ describe EmailCsvJob do context "when writing to S3" do before do FactoryBot.create_list(:lettings_log, 4, owning_organisation: other_organisation) + FactoryBot.create(:lettings_log, owning_organisation: other_organisation, status: "pending", skip_update_status: true) end def expect_csv diff --git a/spec/requests/lettings_logs_controller_spec.rb b/spec/requests/lettings_logs_controller_spec.rb index 23df8e1b8..d95e0a3fd 100644 --- a/spec/requests/lettings_logs_controller_spec.rb +++ b/spec/requests/lettings_logs_controller_spec.rb @@ -224,6 +224,15 @@ RSpec.describe LettingsLogsController, type: :request do tenancycode: "UA984", ) end + let!(:pending_lettings_log) do + FactoryBot.create( + :lettings_log, + created_by: user, + tenancycode: "LC999", + status: "pending", + skip_update_status: true, + ) + end context "when displaying a collection of logs" do let(:headers) { { "Accept" => "text/html" } } @@ -261,6 +270,7 @@ RSpec.describe LettingsLogsController, type: :request do get "/lettings-logs", headers:, params: {} expect(page).to have_content("LC783") expect(page).to have_content("UA984") + expect(page).not_to have_content(pending_lettings_log.tenancycode) end it "displays CSV download links with the correct paths" do @@ -841,6 +851,24 @@ RSpec.describe LettingsLogsController, type: :request do end end + context "when viewing a pending log" do + let(:completed_lettings_log) do + FactoryBot.create( + :lettings_log, + :completed, + owning_organisation: user.organisation, + managing_organisation: user.organisation, + created_by: user, + status: "pending", + skip_update_status: true, + ) + end + + it "returns 404" do + expect(response).to have_http_status(:not_found) + end + end + context "when editing a lettings log" do let(:headers) { { "Accept" => "text/html" } } @@ -1319,9 +1347,12 @@ RSpec.describe LettingsLogsController, type: :request do end context "when a lettings log deletion fails" do + let(:mock_scope) { instance_double("LettingsLog::ActiveRecord_Relation", find_by: lettings_log) } + before do - allow(LettingsLog).to receive(:find_by).and_return(lettings_log) + allow(LettingsLog).to receive(:visible).and_return(mock_scope) allow(lettings_log).to receive(:delete).and_return(false) + delete "/lettings-logs/#{id}", headers: end diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index e7cb7fea3..15cfba390 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -591,6 +591,7 @@ RSpec.describe OrganisationsController, type: :request do before do FactoryBot.create_list(:lettings_log, number_of_org1_lettings_logs, created_by: user) + FactoryBot.create(:lettings_log, created_by: user, status: "pending", skip_update_status: true) FactoryBot.create_list(:lettings_log, number_of_org2_lettings_logs, created_by: nil, owning_organisation_id: unauthorised_organisation.id, managing_organisation_id: unauthorised_organisation.id) get "/organisations/#{organisation.id}/lettings-logs", headers:, params: {} @@ -598,7 +599,8 @@ RSpec.describe OrganisationsController, type: :request do it "only shows logs for that organisation" do expect(page).to have_content("#{number_of_org1_lettings_logs} total logs") - organisation.lettings_logs.map(&:id).each do |lettings_log_id| + + organisation.lettings_logs.visible.map(&:id).each do |lettings_log_id| expect(page).to have_link lettings_log_id.to_s, href: "/lettings-logs/#{lettings_log_id}" end @@ -1155,6 +1157,7 @@ RSpec.describe OrganisationsController, type: :request do before do FactoryBot.create_list(:lettings_log, 2, owning_organisation: organisation) + FactoryBot.create(:lettings_log, owning_organisation: organisation, status: "pending", skip_update_status: true) FactoryBot.create_list(:lettings_log, 2, owning_organisation: other_organisation) end diff --git a/spec/services/exports/lettings_log_export_service_spec.rb b/spec/services/exports/lettings_log_export_service_spec.rb index dbc4b246b..6580a6acd 100644 --- a/spec/services/exports/lettings_log_export_service_spec.rb +++ b/spec/services/exports/lettings_log_export_service_spec.rb @@ -59,6 +59,35 @@ RSpec.describe Exports::LettingsLogExportService do end end + context "when one pending lettings log exists" do + before do + FactoryBot.create( + :lettings_log, + :completed, + status: "pending", + skip_update_status: true, + propcode: "123", + ppostcode_full: "SE2 6RT", + postcode_full: "NW1 5TY", + tenancycode: "BZ737", + startdate: Time.zone.local(2022, 2, 2, 10, 36, 49), + voiddate: Time.zone.local(2019, 11, 3), + mrcdate: Time.zone.local(2020, 5, 5, 10, 36, 49), + tenancylength: 5, + underoccupation_benefitcap: 4, + ) + end + + it "generates a master manifest with CSV headers but no data" do + actual_content = nil + expected_content = "zip-name,date-time zipped folder generated,zip-file-uri\n" + allow(storage_service).to receive(:write_file).with(expected_master_manifest_filename, any_args) { |_, arg2| actual_content = arg2&.string } + + export_service.export_xml_lettings_logs + expect(actual_content).to eq(expected_content) + end + end + context "and one lettings log is available for export" do let!(:lettings_log) { FactoryBot.create(:lettings_log, :completed, propcode: "123", ppostcode_full: "SE2 6RT", postcode_full: "NW1 5TY", tenancycode: "BZ737", startdate: Time.zone.local(2022, 2, 2, 10, 36, 49), voiddate: Time.zone.local(2019, 11, 3), mrcdate: Time.zone.local(2020, 5, 5, 10, 36, 49), tenancylength: 5, underoccupation_benefitcap: 4) }