From 6818a511803d1d2eb781c899858273deecef39f7 Mon Sep 17 00:00:00 2001 From: Dushan Despotovic Date: Thu, 28 Apr 2022 14:30:41 +0100 Subject: [PATCH] Fix all failing specs This commit fixes the tests that failed as a result of the introduction of the created_by column in the case log table. The tests were failing as it is now necessary to have a created_by for a case log --- app/controllers/case_logs_controller.rb | 2 + app/models/bulk_upload.rb | 1 + .../imports/case_logs_import_service.rb | 4 +- .../admin/case_logs_controller_spec.rb | 2 +- spec/factories/user.rb | 3 +- spec/features/form/form_navigation_spec.rb | 1 + spec/fixtures/exports/case_logs.xml | 1 + spec/models/case_log_spec.rb | 38 ++++++++++++------- spec/requests/case_logs_controller_spec.rb | 3 ++ .../exports/case_log_export_service_spec.rb | 1 + .../imports/case_logs_import_service_spec.rb | 3 ++ 11 files changed, 42 insertions(+), 17 deletions(-) diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index 34e257fb3..cde7ad501 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -104,10 +104,12 @@ private { "owning_organisation_id" => current_user.organisation.id, "managing_organisation_id" => current_user.organisation.id, + "created_by_id" => current_user.id } end def api_case_log_params + return {} unless params[:case_log] permitted = params.require(:case_log).permit(CaseLog.editable_fields) diff --git a/app/models/bulk_upload.rb b/app/models/bulk_upload.rb index 9b8c8a9ba..1c971c4d5 100644 --- a/app/models/bulk_upload.rb +++ b/app/models/bulk_upload.rb @@ -32,6 +32,7 @@ class BulkUpload case_log = CaseLog.create!( owning_organisation: current_user.organisation, managing_organisation: current_user.organisation, + created_by: current_user, ) map_row(row).each do |attr_key, attr_val| update = case_log.update(attr_key => attr_val) diff --git a/app/services/imports/case_logs_import_service.rb b/app/services/imports/case_logs_import_service.rb index ee6328a16..ab0f41d71 100644 --- a/app/services/imports/case_logs_import_service.rb +++ b/app/services/imports/case_logs_import_service.rb @@ -172,9 +172,9 @@ module Imports previous_status = field_value(xml_doc, "meta", "status") - owner_id = field_value(xml_doc, "meta", "owner-user-id") + owner_id = field_value(xml_doc, "meta", "owner-user-id").strip if owner_id.present? - attributes["created_by"] = User.find_by(old_id: owner_id) + attributes["created_by"] = User.find_by(old_user_id: owner_id) end case_log = CaseLog.new(attributes) diff --git a/spec/controllers/admin/case_logs_controller_spec.rb b/spec/controllers/admin/case_logs_controller_spec.rb index 6c9a0cbe4..a94e06585 100644 --- a/spec/controllers/admin/case_logs_controller_spec.rb +++ b/spec/controllers/admin/case_logs_controller_spec.rb @@ -36,7 +36,7 @@ describe Admin::CaseLogsController, type: :controller do "case_log": { "owning_organisation_id": owning_organisation.id, "managing_organisation_id": managing_organisation.id, - "created_by": user, + "created_by_id": user.id, }, } end diff --git a/spec/factories/user.rb b/spec/factories/user.rb index 9b8edb23f..b62006a97 100644 --- a/spec/factories/user.rb +++ b/spec/factories/user.rb @@ -4,7 +4,8 @@ FactoryBot.define do name { "Danny Rojas" } password { "pAssword1" } organisation - role { "data_provider" } + role { "data_provider" } + old_user_id { 2 } trait :data_coordinator do role { "data_coordinator" } end diff --git a/spec/features/form/form_navigation_spec.rb b/spec/features/form/form_navigation_spec.rb index ef8bc7448..adc7d6bce 100644 --- a/spec/features/form/form_navigation_spec.rb +++ b/spec/features/form/form_navigation_spec.rb @@ -10,6 +10,7 @@ RSpec.describe "Form Navigation" do :in_progress, owning_organisation: user.organisation, managing_organisation: user.organisation, + created_by: user, ) end let(:id) { case_log.id } diff --git a/spec/fixtures/exports/case_logs.xml b/spec/fixtures/exports/case_logs.xml index 1d712eed8..33a8f6941 100644 --- a/spec/fixtures/exports/case_logs.xml +++ b/spec/fixtures/exports/case_logs.xml @@ -162,6 +162,7 @@ + {created_by_id} 1 diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index 30931890a..e5f9af085 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -3,11 +3,12 @@ require "rails_helper" RSpec.describe CaseLog do let(:owning_organisation) { FactoryBot.create(:organisation) } let(:different_managing_organisation) { FactoryBot.create(:organisation) } + let(:created_by_user) { FactoryBot.create(:user) } describe "#form" do - let(:case_log) { FactoryBot.build(:case_log) } - let(:case_log_2) { FactoryBot.build(:case_log, startdate: Time.zone.local(2022, 1, 1)) } - let(:case_log_year_2) { FactoryBot.build(:case_log, startdate: Time.zone.local(2023, 5, 1)) } + let(:case_log) { FactoryBot.build(:case_log, created_by: created_by_user) } + let(:case_log_2) { FactoryBot.build(:case_log, startdate: Time.zone.local(2022, 1, 1), created_by: created_by_user) } + let(:case_log_year_2) { FactoryBot.build(:case_log, startdate: Time.zone.local(2023, 5, 1), created_by: created_by_user) } it "has returns the correct form based on the start date" do expect(case_log.form_name).to be_nil @@ -19,7 +20,7 @@ RSpec.describe CaseLog do end context "when a date outside the collection window is passed" do - let(:case_log) { FactoryBot.build(:case_log, startdate: Time.zone.local(2015, 1, 1)) } + let(:case_log) { FactoryBot.build(:case_log, startdate: Time.zone.local(2015, 1, 1), created_by: created_by_user) } it "returns the first form" do expect(case_log.form).to be_a(Form) @@ -34,6 +35,7 @@ RSpec.describe CaseLog do described_class.create( owning_organisation:, managing_organisation: owning_organisation, + created_by: created_by_user, ) end @@ -45,7 +47,7 @@ RSpec.describe CaseLog do end describe "#update" do - let(:case_log) { FactoryBot.create(:case_log) } + let(:case_log) { FactoryBot.create(:case_log, created_by: created_by_user) } let(:validator) { case_log._validators[nil].first } after do @@ -199,6 +201,7 @@ RSpec.describe CaseLog do described_class.create({ managing_organisation: owning_organisation, owning_organisation:, + created_by: created_by_user, postcode_full: "M1 1AE", ppostcode_full: "M2 2AE", startdate: Time.gm(2021, 10, 10), @@ -1134,6 +1137,7 @@ RSpec.describe CaseLog do described_class.create({ managing_organisation: owning_organisation, owning_organisation:, + created_by: created_by_user, postcode_known: 1, postcode_full: "M1 1AE", }) @@ -1221,6 +1225,7 @@ RSpec.describe CaseLog do described_class.create({ managing_organisation: owning_organisation, owning_organisation:, + created_by: created_by_user, previous_postcode_known: 1, ppostcode_full: "M1 1AE", }) @@ -1305,6 +1310,7 @@ RSpec.describe CaseLog do described_class.create({ managing_organisation: owning_organisation, owning_organisation:, + created_by: created_by_user, brent: 5.77, scharge: 10.01, pscharge: 3, @@ -1323,6 +1329,7 @@ RSpec.describe CaseLog do described_class.create!({ managing_organisation: owning_organisation, owning_organisation:, + created_by: created_by_user, hhmemb: 3, relat2: "X", relat3: "C", @@ -1376,6 +1383,7 @@ RSpec.describe CaseLog do described_class.create({ managing_organisation: owning_organisation, owning_organisation:, + created_by: created_by_user, renewal: 1, startdate: Time.zone.local(2021, 4, 10), }) @@ -1421,6 +1429,7 @@ RSpec.describe CaseLog do described_class.create({ managing_organisation: owning_organisation, owning_organisation:, + created_by: created_by_user, age1_known: 1, sex1: "R", relat2: "R", @@ -1440,6 +1449,7 @@ RSpec.describe CaseLog do described_class.create({ managing_organisation: owning_organisation, owning_organisation:, + created_by: created_by_user, }) end @@ -1467,6 +1477,7 @@ RSpec.describe CaseLog do described_class.create({ managing_organisation: owning_organisation, owning_organisation:, + created_by: created_by_user, needstype: 2, }) end @@ -1621,6 +1632,7 @@ RSpec.describe CaseLog do described_class.create({ managing_organisation: owning_organisation, owning_organisation:, + created_by: created_by_user, first_time_property_let_as_social_housing: 1, }) end @@ -1629,6 +1641,7 @@ RSpec.describe CaseLog do described_class.create({ managing_organisation: owning_organisation, owning_organisation:, + created_by: created_by_user, first_time_property_let_as_social_housing: 0, }) end @@ -1799,8 +1812,8 @@ RSpec.describe CaseLog do end describe "scopes" do - let!(:case_log_1) { FactoryBot.create(:case_log, :in_progress, startdate: Time.utc(2021, 5, 3)) } - let!(:case_log_2) { FactoryBot.create(:case_log, :completed, startdate: Time.utc(2021, 5, 3)) } + let!(:case_log_1) { FactoryBot.create(:case_log, :in_progress, startdate: Time.utc(2021, 5, 3), created_by: created_by_user) } + let!(:case_log_2) { FactoryBot.create(:case_log, :completed, startdate: Time.utc(2021, 5, 3), created_by: created_by_user) } before do FactoryBot.create(:case_log, startdate: Time.utc(2022, 6, 3)) @@ -1839,23 +1852,22 @@ RSpec.describe CaseLog do end context "when filtering by user" do - let!(:user) { FactoryBot.create(:user) } before do - PaperTrail::Version.find_by(item_id: case_log_1.id, event: "create").update!(whodunnit: user.to_global_id.uri.to_s) - PaperTrail::Version.find_by(item_id: case_log_2.id, event: "create").update!(whodunnit: user.to_global_id.uri.to_s) + PaperTrail::Version.find_by(item_id: case_log_1.id, event: "create").update!(whodunnit: created_by_user.to_global_id.uri.to_s) + PaperTrail::Version.find_by(item_id: case_log_2.id, event: "create").update!(whodunnit: created_by_user.to_global_id.uri.to_s) end it "allows filtering on current user" do - expect(described_class.filter_by_user(%w[yours], user).count).to eq(2) + expect(described_class.filter_by_user(%w[yours], created_by_user).count).to eq(2) end it "returns all logs when all logs selected" do - expect(described_class.filter_by_user(%w[all], user).count).to eq(3) + expect(described_class.filter_by_user(%w[all], created_by_user).count).to eq(3) end it "returns all logs when all and your users selected" do - expect(described_class.filter_by_user(%w[all yours], user).count).to eq(3) + expect(described_class.filter_by_user(%w[all yours], created_by_user).count).to eq(3) end end end diff --git a/spec/requests/case_logs_controller_spec.rb b/spec/requests/case_logs_controller_spec.rb index 04fd7e404..25265c8cf 100644 --- a/spec/requests/case_logs_controller_spec.rb +++ b/spec/requests/case_logs_controller_spec.rb @@ -3,6 +3,7 @@ require "rails_helper" RSpec.describe CaseLogsController, type: :request do let(:owning_organisation) { FactoryBot.create(:organisation) } let(:managing_organisation) { owning_organisation } + let(:user) { FactoryBot.create(:user) } let(:api_username) { "test_user" } let(:api_password) { "test_password" } let(:basic_credentials) do @@ -38,6 +39,7 @@ RSpec.describe CaseLogsController, type: :request do { "owning_organisation_id": owning_organisation.id, "managing_organisation_id": managing_organisation.id, + "created_by_id": user.id, "tenant_code": tenant_code, "age1": age1, "postcode_full": postcode_full, @@ -90,6 +92,7 @@ RSpec.describe CaseLogsController, type: :request do "case_log" => { "owning_organisation_id" => owning_organisation.id, "managing_organisation_id" => managing_organisation.id, + "created_by_id" => user.id, }, } end diff --git a/spec/services/exports/case_log_export_service_spec.rb b/spec/services/exports/case_log_export_service_spec.rb index 2b8846d05..2eb6171b0 100644 --- a/spec/services/exports/case_log_export_service_spec.rb +++ b/spec/services/exports/case_log_export_service_spec.rb @@ -16,6 +16,7 @@ RSpec.describe Exports::CaseLogExportService do export_template.sub!(/\{id\}/, case_log["id"].to_s) export_template.sub!(/\{owning_org_id\}/, case_log["owning_organisation_id"].to_s) export_template.sub!(/\{managing_org_id\}/, case_log["managing_organisation_id"].to_s) + export_template.sub!(/\{created_by_id\}/, case_log["created_by_id"].to_s) end context "when exporting daily case logs" do diff --git a/spec/services/imports/case_logs_import_service_spec.rb b/spec/services/imports/case_logs_import_service_spec.rb index 9fc4e8734..92ee96d3f 100644 --- a/spec/services/imports/case_logs_import_service_spec.rb +++ b/spec/services/imports/case_logs_import_service_spec.rb @@ -31,6 +31,9 @@ RSpec.describe Imports::CaseLogsImportService do WebMock.stub_request(:get, /api.postcodes.io\/postcodes\/LS166FT/) .to_return(status: 200, body: '{"status":200,"result":{"codes":{"admin_district":"E08000035"}}}', headers: {}) + + FactoryBot.create(:user, old_user_id: "c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa" ) + FactoryBot.create(:user, old_user_id: "e29c492473446dca4d50224f2bb7cf965a261d6f" ) end it "successfully create all case logs" do