Browse Source

Merge pull request #528 from communitiesuk/believe-import-fixes

Believe import fixes
pull/537/head
baarkerlounger 3 years ago committed by GitHub
parent
commit
cf3603ad17
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 1
      app/controllers/case_logs_controller.rb
  2. 1
      app/models/bulk_upload.rb
  3. 3
      app/models/case_log.rb
  4. 7
      app/services/imports/case_logs_import_service.rb
  5. 8
      app/services/imports/user_import_service.rb
  6. 7
      db/migrate/20220427160536_add_created_by_to_case_logs.rb
  7. 4
      db/schema.rb
  8. 2
      spec/controllers/admin/case_logs_controller_spec.rb
  9. 1
      spec/factories/case_log.rb
  10. 1
      spec/factories/user.rb
  11. 1
      spec/features/form/form_navigation_spec.rb
  12. 1
      spec/fixtures/exports/case_logs.xml
  13. 39
      spec/models/case_log_spec.rb
  14. 3
      spec/requests/case_logs_controller_spec.rb
  15. 1
      spec/services/exports/case_log_export_service_spec.rb
  16. 3
      spec/services/imports/case_logs_import_service_spec.rb
  17. 1
      spec/services/imports/user_import_service_spec.rb

1
app/controllers/case_logs_controller.rb

@ -104,6 +104,7 @@ private
{ {
"owning_organisation_id" => current_user.organisation.id, "owning_organisation_id" => current_user.organisation.id,
"managing_organisation_id" => current_user.organisation.id, "managing_organisation_id" => current_user.organisation.id,
"created_by_id" => current_user.id,
} }
end end

1
app/models/bulk_upload.rb

@ -32,6 +32,7 @@ class BulkUpload
case_log = CaseLog.create!( case_log = CaseLog.create!(
owning_organisation: current_user.organisation, owning_organisation: current_user.organisation,
managing_organisation: current_user.organisation, managing_organisation: current_user.organisation,
created_by: current_user,
) )
map_row(row).each do |attr_key, attr_val| map_row(row).each do |attr_key, attr_val|
update = case_log.update(attr_key => attr_val) update = case_log.update(attr_key => attr_val)

3
app/models/case_log.rb

@ -32,6 +32,7 @@ class CaseLog < ApplicationRecord
belongs_to :owning_organisation, class_name: "Organisation" belongs_to :owning_organisation, class_name: "Organisation"
belongs_to :managing_organisation, class_name: "Organisation" belongs_to :managing_organisation, class_name: "Organisation"
belongs_to :created_by, class_name: "User"
scope :for_organisation, ->(org) { where(owning_organisation: org).or(where(managing_organisation: org)) } scope :for_organisation, ->(org) { where(owning_organisation: org).or(where(managing_organisation: org)) }
scope :filter_by_status, ->(status, _user = nil) { where status: } scope :filter_by_status, ->(status, _user = nil) { where status: }
@ -45,7 +46,7 @@ class CaseLog < ApplicationRecord
scope :filter_by_user, lambda { |selected_user, user| scope :filter_by_user, lambda { |selected_user, user|
if !selected_user.include?("all") && user.present? if !selected_user.include?("all") && user.present?
where(id: PaperTrail::Version.where(item_type: "CaseLog", event: "create", whodunnit: user.to_global_id.uri.to_s).map(&:item_id)) where(created_by: user)
end end
} }

7
app/services/imports/case_logs_import_service.rb

@ -172,6 +172,11 @@ module Imports
previous_status = field_value(xml_doc, "meta", "status") previous_status = field_value(xml_doc, "meta", "status")
owner_id = field_value(xml_doc, "meta", "owner-user-id").strip
if owner_id.present?
attributes["created_by"] = User.find_by(old_user_id: owner_id)
end
case_log = CaseLog.new(attributes) case_log = CaseLog.new(attributes)
save_case_log(case_log, attributes) save_case_log(case_log, attributes)
compute_differences(case_log, attributes) compute_differences(case_log, attributes)
@ -361,7 +366,7 @@ module Imports
def previous_postcode_known(xml_doc, previous_postcode) def previous_postcode_known(xml_doc, previous_postcode)
previous_postcode_known = string_or_nil(xml_doc, "Q12bnot") previous_postcode_known = string_or_nil(xml_doc, "Q12bnot")
if previous_postcode_known == "Temporary or Unknown" if previous_postcode_known == "Temporary_or_Unknown"
0 0
elsif previous_postcode.nil? elsif previous_postcode.nil?
nil nil

8
app/services/imports/user_import_service.rb

@ -14,13 +14,19 @@ module Imports
organisation = Organisation.find_by(old_org_id: user_field_value(xml_document, "institution")) organisation = Organisation.find_by(old_org_id: user_field_value(xml_document, "institution"))
old_user_id = user_field_value(xml_document, "id") old_user_id = user_field_value(xml_document, "id")
name = user_field_value(xml_document, "full-name") name = user_field_value(xml_document, "full-name")
email = user_field_value(xml_document, "email") email = user_field_value(xml_document, "email").downcase.strip
deleted = user_field_value(xml_document, "deleted")
date_deactivated = user_field_value(xml_document, "date-deactivated")
if User.find_by(old_user_id:, organisation:) if User.find_by(old_user_id:, organisation:)
@logger.warn("User #{name} with old user id #{old_user_id} is already present, skipping.") @logger.warn("User #{name} with old user id #{old_user_id} is already present, skipping.")
elsif deleted == "true" || date_deactivated.present?
@logger.warn("User #{name} with old user id #{old_user_id} is deleted or deactivated, skipping.")
elsif (user = User.find_by(email:, organisation:)) elsif (user = User.find_by(email:, organisation:))
is_dpo = user.is_data_protection_officer? || is_dpo?(user_field_value(xml_document, "user-type")) is_dpo = user.is_data_protection_officer? || is_dpo?(user_field_value(xml_document, "user-type"))
role = highest_role(user.role, role(user_field_value(xml_document, "user-type"))) role = highest_role(user.role, role(user_field_value(xml_document, "user-type")))
user.update!(role:, is_dpo:) user.update!(role:, is_dpo:)
@logger.info("Found duplicated email, updating user #{user.id} with role #{role} and is_dpo #{is_dpo}")
else else
User.create!( User.create!(
email:, email:,

7
db/migrate/20220427160536_add_created_by_to_case_logs.rb

@ -0,0 +1,7 @@
class AddCreatedByToCaseLogs < ActiveRecord::Migration[7.0]
def change
change_table :case_logs, bulk: true do |t|
t.belongs_to :created_by, class_name: "User"
end
end
end

4
db/schema.rb

@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema[7.0].define(version: 2022_04_26_122618) do ActiveRecord::Schema[7.0].define(version: 2022_04_27_160536) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
@ -220,6 +220,8 @@ ActiveRecord::Schema[7.0].define(version: 2022_04_26_122618) do
t.integer "irproduct" t.integer "irproduct"
t.string "old_id" t.string "old_id"
t.integer "joint" t.integer "joint"
t.bigint "created_by_id"
t.index ["created_by_id"], name: "index_case_logs_on_created_by_id"
t.index ["managing_organisation_id"], name: "index_case_logs_on_managing_organisation_id" t.index ["managing_organisation_id"], name: "index_case_logs_on_managing_organisation_id"
t.index ["old_id"], name: "index_case_logs_on_old_id", unique: true t.index ["old_id"], name: "index_case_logs_on_old_id", unique: true
t.index ["owning_organisation_id"], name: "index_case_logs_on_owning_organisation_id" t.index ["owning_organisation_id"], name: "index_case_logs_on_owning_organisation_id"

2
spec/controllers/admin/case_logs_controller_spec.rb

@ -11,6 +11,7 @@ describe Admin::CaseLogsController, type: :controller do
let(:resource_title) { "Logs" } let(:resource_title) { "Logs" }
let(:valid_session) { {} } let(:valid_session) { {} }
let(:admin_user) { FactoryBot.create(:admin_user) } let(:admin_user) { FactoryBot.create(:admin_user) }
let(:user) { FactoryBot.create(:user) }
describe "Get case logs" do describe "Get case logs" do
let!(:case_log) { FactoryBot.create(:case_log, :in_progress) } let!(:case_log) { FactoryBot.create(:case_log, :in_progress) }
@ -35,6 +36,7 @@ describe Admin::CaseLogsController, type: :controller do
"case_log": { "case_log": {
"owning_organisation_id": owning_organisation.id, "owning_organisation_id": owning_organisation.id,
"managing_organisation_id": managing_organisation.id, "managing_organisation_id": managing_organisation.id,
"created_by_id": user.id,
}, },
} }
end end

1
spec/factories/case_log.rb

@ -2,6 +2,7 @@ FactoryBot.define do
factory :case_log do factory :case_log do
owning_organisation { FactoryBot.create(:organisation) } owning_organisation { FactoryBot.create(:organisation) }
managing_organisation { FactoryBot.create(:organisation) } managing_organisation { FactoryBot.create(:organisation) }
created_by { FactoryBot.create(:user) }
trait :about_completed do trait :about_completed do
renewal { 0 } renewal { 0 }
needstype { 1 } needstype { 1 }

1
spec/factories/user.rb

@ -5,6 +5,7 @@ FactoryBot.define do
password { "pAssword1" } password { "pAssword1" }
organisation organisation
role { "data_provider" } role { "data_provider" }
old_user_id { 2 }
trait :data_coordinator do trait :data_coordinator do
role { "data_coordinator" } role { "data_coordinator" }
end end

1
spec/features/form/form_navigation_spec.rb

@ -10,6 +10,7 @@ RSpec.describe "Form Navigation" do
:in_progress, :in_progress,
owning_organisation: user.organisation, owning_organisation: user.organisation,
managing_organisation: user.organisation, managing_organisation: user.organisation,
created_by: user,
) )
end end
let(:id) { case_log.id } let(:id) { case_log.id }

1
spec/fixtures/exports/case_logs.xml vendored

@ -162,6 +162,7 @@
<irproduct/> <irproduct/>
<old_id/> <old_id/>
<joint/> <joint/>
<created_by_id>{created_by_id}</created_by_id>
<providertype>1</providertype> <providertype>1</providertype>
</form> </form>
</forms> </forms>

39
spec/models/case_log_spec.rb

@ -3,11 +3,12 @@ require "rails_helper"
RSpec.describe CaseLog do RSpec.describe CaseLog do
let(:owning_organisation) { FactoryBot.create(:organisation) } let(:owning_organisation) { FactoryBot.create(:organisation) }
let(:different_managing_organisation) { FactoryBot.create(:organisation) } let(:different_managing_organisation) { FactoryBot.create(:organisation) }
let(:created_by_user) { FactoryBot.create(:user) }
describe "#form" do describe "#form" do
let(:case_log) { FactoryBot.build(:case_log) } 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)) } 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)) } 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 it "has returns the correct form based on the start date" do
expect(case_log.form_name).to be_nil expect(case_log.form_name).to be_nil
@ -19,7 +20,7 @@ RSpec.describe CaseLog do
end end
context "when a date outside the collection window is passed" do 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 it "returns the first form" do
expect(case_log.form).to be_a(Form) expect(case_log.form).to be_a(Form)
@ -34,6 +35,7 @@ RSpec.describe CaseLog do
described_class.create( described_class.create(
owning_organisation:, owning_organisation:,
managing_organisation: owning_organisation, managing_organisation: owning_organisation,
created_by: created_by_user,
) )
end end
@ -45,7 +47,7 @@ RSpec.describe CaseLog do
end end
describe "#update" do 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 } let(:validator) { case_log._validators[nil].first }
after do after do
@ -199,6 +201,7 @@ RSpec.describe CaseLog do
described_class.create({ described_class.create({
managing_organisation: owning_organisation, managing_organisation: owning_organisation,
owning_organisation:, owning_organisation:,
created_by: created_by_user,
postcode_full: "M1 1AE", postcode_full: "M1 1AE",
ppostcode_full: "M2 2AE", ppostcode_full: "M2 2AE",
startdate: Time.gm(2021, 10, 10), startdate: Time.gm(2021, 10, 10),
@ -1134,6 +1137,7 @@ RSpec.describe CaseLog do
described_class.create({ described_class.create({
managing_organisation: owning_organisation, managing_organisation: owning_organisation,
owning_organisation:, owning_organisation:,
created_by: created_by_user,
postcode_known: 1, postcode_known: 1,
postcode_full: "M1 1AE", postcode_full: "M1 1AE",
}) })
@ -1221,6 +1225,7 @@ RSpec.describe CaseLog do
described_class.create({ described_class.create({
managing_organisation: owning_organisation, managing_organisation: owning_organisation,
owning_organisation:, owning_organisation:,
created_by: created_by_user,
previous_postcode_known: 1, previous_postcode_known: 1,
ppostcode_full: "M1 1AE", ppostcode_full: "M1 1AE",
}) })
@ -1305,6 +1310,7 @@ RSpec.describe CaseLog do
described_class.create({ described_class.create({
managing_organisation: owning_organisation, managing_organisation: owning_organisation,
owning_organisation:, owning_organisation:,
created_by: created_by_user,
brent: 5.77, brent: 5.77,
scharge: 10.01, scharge: 10.01,
pscharge: 3, pscharge: 3,
@ -1323,6 +1329,7 @@ RSpec.describe CaseLog do
described_class.create!({ described_class.create!({
managing_organisation: owning_organisation, managing_organisation: owning_organisation,
owning_organisation:, owning_organisation:,
created_by: created_by_user,
hhmemb: 3, hhmemb: 3,
relat2: "X", relat2: "X",
relat3: "C", relat3: "C",
@ -1376,6 +1383,7 @@ RSpec.describe CaseLog do
described_class.create({ described_class.create({
managing_organisation: owning_organisation, managing_organisation: owning_organisation,
owning_organisation:, owning_organisation:,
created_by: created_by_user,
renewal: 1, renewal: 1,
startdate: Time.zone.local(2021, 4, 10), startdate: Time.zone.local(2021, 4, 10),
}) })
@ -1421,6 +1429,7 @@ RSpec.describe CaseLog do
described_class.create({ described_class.create({
managing_organisation: owning_organisation, managing_organisation: owning_organisation,
owning_organisation:, owning_organisation:,
created_by: created_by_user,
age1_known: 1, age1_known: 1,
sex1: "R", sex1: "R",
relat2: "R", relat2: "R",
@ -1440,6 +1449,7 @@ RSpec.describe CaseLog do
described_class.create({ described_class.create({
managing_organisation: owning_organisation, managing_organisation: owning_organisation,
owning_organisation:, owning_organisation:,
created_by: created_by_user,
}) })
end end
@ -1467,6 +1477,7 @@ RSpec.describe CaseLog do
described_class.create({ described_class.create({
managing_organisation: owning_organisation, managing_organisation: owning_organisation,
owning_organisation:, owning_organisation:,
created_by: created_by_user,
needstype: 2, needstype: 2,
}) })
end end
@ -1621,6 +1632,7 @@ RSpec.describe CaseLog do
described_class.create({ described_class.create({
managing_organisation: owning_organisation, managing_organisation: owning_organisation,
owning_organisation:, owning_organisation:,
created_by: created_by_user,
first_time_property_let_as_social_housing: 1, first_time_property_let_as_social_housing: 1,
}) })
end end
@ -1629,6 +1641,7 @@ RSpec.describe CaseLog do
described_class.create({ described_class.create({
managing_organisation: owning_organisation, managing_organisation: owning_organisation,
owning_organisation:, owning_organisation:,
created_by: created_by_user,
first_time_property_let_as_social_housing: 0, first_time_property_let_as_social_housing: 0,
}) })
end end
@ -1799,8 +1812,8 @@ RSpec.describe CaseLog do
end end
describe "scopes" do describe "scopes" do
let!(:case_log_1) { FactoryBot.create(:case_log, :in_progress, 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)) } let!(:case_log_2) { FactoryBot.create(:case_log, :completed, startdate: Time.utc(2021, 5, 3), created_by: created_by_user) }
before do before do
FactoryBot.create(:case_log, startdate: Time.utc(2022, 6, 3)) FactoryBot.create(:case_log, startdate: Time.utc(2022, 6, 3))
@ -1839,23 +1852,21 @@ RSpec.describe CaseLog do
end end
context "when filtering by user" do context "when filtering by user" do
let!(:user) { FactoryBot.create(:user) }
before do 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_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: 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 end
it "allows filtering on current user" do 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 end
it "returns all logs when all logs selected" do 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 end
it "returns all logs when all and your users selected" do 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 end
end end

3
spec/requests/case_logs_controller_spec.rb

@ -3,6 +3,7 @@ require "rails_helper"
RSpec.describe CaseLogsController, type: :request do RSpec.describe CaseLogsController, type: :request do
let(:owning_organisation) { FactoryBot.create(:organisation) } let(:owning_organisation) { FactoryBot.create(:organisation) }
let(:managing_organisation) { owning_organisation } let(:managing_organisation) { owning_organisation }
let(:user) { FactoryBot.create(:user) }
let(:api_username) { "test_user" } let(:api_username) { "test_user" }
let(:api_password) { "test_password" } let(:api_password) { "test_password" }
let(:basic_credentials) do let(:basic_credentials) do
@ -38,6 +39,7 @@ RSpec.describe CaseLogsController, type: :request do
{ {
"owning_organisation_id": owning_organisation.id, "owning_organisation_id": owning_organisation.id,
"managing_organisation_id": managing_organisation.id, "managing_organisation_id": managing_organisation.id,
"created_by_id": user.id,
"tenant_code": tenant_code, "tenant_code": tenant_code,
"age1": age1, "age1": age1,
"postcode_full": postcode_full, "postcode_full": postcode_full,
@ -90,6 +92,7 @@ RSpec.describe CaseLogsController, type: :request do
"case_log" => { "case_log" => {
"owning_organisation_id" => owning_organisation.id, "owning_organisation_id" => owning_organisation.id,
"managing_organisation_id" => managing_organisation.id, "managing_organisation_id" => managing_organisation.id,
"created_by_id" => user.id,
}, },
} }
end end

1
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!(/\{id\}/, case_log["id"].to_s)
export_template.sub!(/\{owning_org_id\}/, case_log["owning_organisation_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!(/\{managing_org_id\}/, case_log["managing_organisation_id"].to_s)
export_template.sub!(/\{created_by_id\}/, case_log["created_by_id"].to_s)
end end
context "when exporting daily case logs" do context "when exporting daily case logs" do

3
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/) WebMock.stub_request(:get, /api.postcodes.io\/postcodes\/LS166FT/)
.to_return(status: 200, body: '{"status":200,"result":{"codes":{"admin_district":"E08000035"}}}', headers: {}) .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 end
it "successfully create all case logs" do it "successfully create all case logs" do

1
spec/services/imports/user_import_service_spec.rb

@ -17,6 +17,7 @@ RSpec.describe Imports::UserImportService do
allow(storage_service).to receive(:get_file_io) allow(storage_service).to receive(:get_file_io)
.with("user_directory/#{old_user_id}.xml") .with("user_directory/#{old_user_id}.xml")
.and_return(user_file) .and_return(user_file)
allow(logger).to receive(:info)
end end
it "successfully create a user with the expected data" do it "successfully create a user with the expected data" do

Loading…
Cancel
Save