diff --git a/app/controllers/logs_controller.rb b/app/controllers/logs_controller.rb index de4a35bdb..599f3a7b9 100644 --- a/app/controllers/logs_controller.rb +++ b/app/controllers/logs_controller.rb @@ -63,7 +63,7 @@ private end def org_params - owning_organisation_id = current_user.organisation.holds_own_stock? ? current_user.organisation.id : nil + owning_organisation_id = instance_of?(SalesLogsController) || current_user.organisation.holds_own_stock? ? current_user.organisation.id : nil { "owning_organisation_id" => owning_organisation_id, "created_by_id" => current_user.id, diff --git a/app/models/form_handler.rb b/app/models/form_handler.rb index 1ef74b265..645efb1b4 100644 --- a/app/models/form_handler.rb +++ b/app/models/form_handler.rb @@ -158,6 +158,7 @@ class FormHandler def use_real_forms! @directories = ["config/forms"] + @lettings_forms = get_lettings_forms @forms = get_all_forms end diff --git a/app/views/devise/confirmations/invalid.html.erb b/app/views/devise/confirmations/invalid.html.erb index 8d3ad96df..32219f0f4 100644 --- a/app/views/devise/confirmations/invalid.html.erb +++ b/app/views/devise/confirmations/invalid.html.erb @@ -7,6 +7,5 @@
It looks like you have requested a newer join link than this one. Check your emails and follow the most recent link instead.
- diff --git a/app/views/devise/confirmations/new.html.erb b/app/views/devise/confirmations/new.html.erb index 5f36dc388..3fc34aee6 100644 --- a/app/views/devise/confirmations/new.html.erb +++ b/app/views/devise/confirmations/new.html.erb @@ -6,11 +6,11 @@ <%= content_for(:title) %> <%= form_for(resource, as: resource_name, url: confirmation_path(resource_name), html: { method: :post }) do |f| %> - -For security reasons, your join link expired - get another one using the button below (valid for 3 hours).
+For security reasons, your link expired - get another one using the button below (valid for 24 hours).
<%= f.hidden_field :email, value: (resource.pending_reconfirmation? ? resource.unconfirmed_email : resource.email) %> - <%= f.govuk_submit "Get a new join link" %> + + <%= f.govuk_submit "Get a new link" %> <% end %> diff --git a/config/initializers/multi_logger.rb b/config/initializers/multi_logger.rb index d39d786c1..6c0b4eb27 100644 --- a/config/initializers/multi_logger.rb +++ b/config/initializers/multi_logger.rb @@ -1,17 +1,19 @@ class MultiLogger - def initialize(*targets) - @targets = targets + def initialize(file_logger) + @rails_logger = Rails.logger + @file_logger = file_logger end def info(data) - @targets.each { |t| t.info(data) } + @rails_logger.info(data) end def warn(data) - @targets.each { |t| t.warn(data) } + @rails_logger.warn(data) end def error(data) - @targets.each { |t| t.error(data) } + @rails_logger.error(data) + @file_logger.error(data) end end diff --git a/lib/tasks/full_import.rake b/lib/tasks/full_import.rake index c9d95d2a8..f614d4314 100644 --- a/lib/tasks/full_import.rake +++ b/lib/tasks/full_import.rake @@ -9,7 +9,7 @@ namespace :import do s3_service = Storage::S3Service.new(PlatformHelper.is_paas? ? Configuration::PaasConfigurationService.new : Configuration::EnvConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"]) csv = CSV.parse(s3_service.get_file_io(institutions_csv_name), headers: true) logs_string = StringIO.new - logger = MultiLogger.new(Rails.logger, Logger.new(logs_string)) + logger = MultiLogger.new(Logger.new(logs_string)) org_count = csv.length initial_import_list = [ @@ -37,7 +37,7 @@ namespace :import do end log_file = "#{File.basename(institutions_csv_name, File.extname(institutions_csv_name))}_#{File.basename(archive_path, File.extname(archive_path))}_initial.log" - s3_service.write_file(log_file, logs_string.string) + s3_service.write_file(log_file, logs_string.string) if logs_string.string.present? logs_string.rewind logs_string.truncate(0) end @@ -54,7 +54,7 @@ namespace :import do csv = CSV.parse(s3_service.get_file_io(institutions_csv_name), headers: true) org_count = csv.length logs_string = StringIO.new - logger = MultiLogger.new(Rails.logger, Logger.new(logs_string)) + logger = MultiLogger.new(Logger.new(logs_string)) logs_import_list = [ Import.new(Imports::LettingsLogsImportService, :create_logs, "logs", logger), @@ -78,7 +78,7 @@ namespace :import do end log_file = "#{File.basename(institutions_csv_name, File.extname(institutions_csv_name))}_#{File.basename(archive_path, File.extname(archive_path))}_logs.log" - s3_service.write_file(log_file, logs_string.string) + s3_service.write_file(log_file, logs_string.string) if logs_string.string.present? logs_string.rewind logs_string.truncate(0) end diff --git a/spec/lib/tasks/full_import_spec.rb b/spec/lib/tasks/full_import_spec.rb index 0b8d3bd9a..b95b370cd 100644 --- a/spec/lib/tasks/full_import_spec.rb +++ b/spec/lib/tasks/full_import_spec.rb @@ -77,10 +77,10 @@ describe "full import", type: :task do allow(Imports::OrganisationRentPeriodImportService).to receive(:new).and_return(instance_double(Imports::OrganisationRentPeriodImportService)) end - it "creates a report using given organisation csv" do + it "does not write a report if there were no errors" do expect(Storage::S3Service).to receive(:new).with(env_config_service, instance_name) - expect(storage_service).to receive(:write_file).with("some_name_1_initial.log", / INFO -- : Performing initial imports for organisation org1/) - expect(storage_service).to receive(:write_file).with("some_name_2_initial.log", / INFO -- : Performing initial imports for organisation org2/) + expect(storage_service).not_to receive(:write_file).with("some_name_1_initial.log", "") + expect(storage_service).not_to receive(:write_file).with("some_name_2_initial.log", "") task.invoke("some_name.csv") end @@ -106,10 +106,10 @@ describe "full import", type: :task do allow(Imports::SalesLogsImportService).to receive(:new).and_return(instance_double(Imports::SalesLogsImportService)) end - it "creates a report using given organisation csv" do + it "does not write a report if there were no errors" do expect(Storage::S3Service).to receive(:new).with(env_config_service, instance_name) - expect(storage_service).to receive(:write_file).with("some_name_1_logs.log", / INFO -- : Importing logs for organisation org1, expecting 0 logs/) - expect(storage_service).to receive(:write_file).with("some_name_2_logs.log", / INFO -- : Importing logs for organisation org2, expecting 0 logs/) + expect(storage_service).not_to receive(:write_file).with("some_name_1_logs.log", "") + expect(storage_service).not_to receive(:write_file).with("some_name_2_logs.log", "") task.invoke("some_name.csv") end diff --git a/spec/models/form_spec.rb b/spec/models/form_spec.rb index c812f8172..5547d64ea 100644 --- a/spec/models/form_spec.rb +++ b/spec/models/form_spec.rb @@ -188,7 +188,6 @@ RSpec.describe Form, type: :model do around do |example| FormHandler.instance.use_real_forms! - example.run end diff --git a/spec/models/lettings_log_spec.rb b/spec/models/lettings_log_spec.rb index 489a31b9b..36c793c79 100644 --- a/spec/models/lettings_log_spec.rb +++ b/spec/models/lettings_log_spec.rb @@ -1575,6 +1575,7 @@ RSpec.describe LettingsLog do it "clears underoccupation_benefitcap if log is no longer in 2021/22" do expect { lettings_log.update!(renewal: 1) }.to change(lettings_log, :underoccupation_benefitcap).to 2 Timecop.return + Singleton.__init__(FormHandler) expect { lettings_log.update!(startdate: Time.zone.local(2023, 1, 1)) }.to change(lettings_log, :underoccupation_benefitcap).from(2).to nil end @@ -2040,6 +2041,7 @@ RSpec.describe LettingsLog do before do Timecop.freeze(Time.zone.local(2022, 4, 2)) + Singleton.__init__(FormHandler) lettings_log.update!(startdate: Time.zone.local(2022, 4, 2), scheme:) Timecop.unfreeze end diff --git a/spec/requests/auth/confirmations_controller_spec.rb b/spec/requests/auth/confirmations_controller_spec.rb index aa7f93d65..e7e2e1815 100644 --- a/spec/requests/auth/confirmations_controller_spec.rb +++ b/spec/requests/auth/confirmations_controller_spec.rb @@ -40,7 +40,7 @@ RSpec.describe Auth::ConfirmationsController, type: :request do end it "shows the expired page" do - expect(page).to have_content("For security reasons, your join link expired - get another one using the button below (valid for 3 hours).") + expect(page).to have_content("For security reasons, your link expired - get another one using the button below (valid for 24 hours).") end end diff --git a/spec/requests/sales_logs_controller_spec.rb b/spec/requests/sales_logs_controller_spec.rb index 0305690f8..8a21f3fa4 100644 --- a/spec/requests/sales_logs_controller_spec.rb +++ b/spec/requests/sales_logs_controller_spec.rb @@ -106,6 +106,61 @@ RSpec.describe SalesLogsController, type: :request do expect(whodunnit_actor).to be_a(User) expect(whodunnit_actor.id).to eq(user.id) end + + context "when creating a new log" do + context "when the user is support" do + let(:organisation) { FactoryBot.create(:organisation) } + let(:support_user) { FactoryBot.create(:user, :support, organisation:) } + + before do + allow(support_user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in support_user + post "/sales-logs", headers: + end + + it "sets the stock-owning org as nil" do + created_id = response.location.match(/[0-9]+/)[0] + sales_log = SalesLog.find_by(id: created_id) + expect(sales_log.owning_organisation).to eq(nil) + end + end + + context "when the user is not support" do + context "when the user's org holds stock" do + let(:organisation) { FactoryBot.create(:organisation, name: "User org", holds_own_stock: true) } + let(:user) { FactoryBot.create(:user, :data_coordinator, organisation:) } + + before do + RequestHelper.stub_http_requests + sign_in user + post "/sales-logs", headers: + end + + it "sets the stock-owning org as the user's org" do + created_id = response.location.match(/[0-9]+/)[0] + sales_log = SalesLog.find_by(id: created_id) + expect(sales_log.owning_organisation.name).to eq("User org") + end + end + + context "when the user's org doesn't hold stock" do + let(:organisation) { FactoryBot.create(:organisation, name: "User org", holds_own_stock: false) } + let(:user) { FactoryBot.create(:user, :data_coordinator, organisation:) } + + before do + RequestHelper.stub_http_requests + sign_in user + post "/sales-logs", headers: + end + + it "sets the stock-owning org as user's org" do + created_id = response.location.match(/[0-9]+/)[0] + sales_log = SalesLog.find_by(id: created_id) + expect(sales_log.owning_organisation.name).to eq("User org") + end + end + end + end end end diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb index 751444a8e..6e5fb0d8b 100644 --- a/spec/services/imports/lettings_logs_import_service_spec.rb +++ b/spec/services/imports/lettings_logs_import_service_spec.rb @@ -14,7 +14,9 @@ RSpec.describe Imports::LettingsLogsImportService do end let(:storage_service) { instance_double(Storage::S3Service) } - let(:logger) { instance_double(ActiveSupport::Logger) } + let(:logs_string) { StringIO.new } + let(:file_logger) { Logger.new(logs_string) } + let(:logger) { MultiLogger.new(file_logger) } let(:real_2021_2022_form) { Form.new("config/forms/2021_2022.json") } let(:real_2022_2023_form) { Form.new("config/forms/2022_2023.json") } @@ -328,6 +330,24 @@ RSpec.describe Imports::LettingsLogsImportService do end end + context "and submitted log has errors" do + let(:lettings_log_id) { "0ead17cb-1668-442d-898c-0d52879ff592" } + + before do + lettings_log_xml.at_xpath("//meta:status").content = "submitted" + lettings_log_xml.at_xpath("//xmlns:HHMEMB").content = "32" + end + + it "logs the error" do + expect { lettings_log_service.send(:create_log, lettings_log_xml) } + .to raise_error(ActiveRecord::RecordInvalid) + + expect(logs_string.string).to include("Failed to import") + expect(logs_string.string).to include("ERROR -- : Validation error: Field hhmemb") + expect(logs_string.string).to include("Error message: outside_the_range") + end + end + context "and this is an internal transfer from a non social housing" do before do lettings_log_xml.at_xpath("//xmlns:Q11").content = "9 Residential care home" @@ -1048,7 +1068,9 @@ RSpec.describe Imports::LettingsLogsImportService do end let(:storage_service) { instance_double(Storage::S3Service) } - let(:logger) { instance_double(ActiveSupport::Logger) } + let(:logs_string) { StringIO.new } + let(:file_logger) { Logger.new(logs_string) } + let(:logger) { MultiLogger.new(file_logger) } let(:real_2021_2022_form) { Form.new("config/forms/2021_2022.json") } let(:real_2022_2023_form) { Form.new("config/forms/2022_2023.json") } diff --git a/spec/services/imports/organisation_import_service_spec.rb b/spec/services/imports/organisation_import_service_spec.rb index f9a47abe8..d4d3be1e5 100644 --- a/spec/services/imports/organisation_import_service_spec.rb +++ b/spec/services/imports/organisation_import_service_spec.rb @@ -3,7 +3,7 @@ require "rails_helper" RSpec.describe Imports::OrganisationImportService do let(:storage_service) { instance_double(Storage::S3Service) } let(:logs_string) { StringIO.new } - let(:logger) { MultiLogger.new(Rails.logger, Logger.new(logs_string)) } + let(:logger) { MultiLogger.new(logs_string) } let(:folder_name) { "organisations" } let(:filenames) { %w[my_folder/my_file1.xml my_folder/my_file2.xml] } let(:fixture_directory) { "spec/fixtures/imports/institution" } @@ -87,7 +87,7 @@ RSpec.describe Imports::OrganisationImportService do expect { import_service.create_organisations(folder_name) }.to change(Organisation, :count).by(1) expect { import_service.create_organisations(folder_name) }.to change(Organisation, :count).by(0) - expect(logs_string.string).to include("Organisation my_new_organisation is already present with old visible ID 1, skipping.") + expect(logs_string.string).not_to include("Organisation my_new_organisation is already present with old visible ID 1, skipping.") expect(Organisation).to exist(old_visible_id: "1", name: "HA Ltd") end