From e5e7f89e38106208216d868b8b38d5c6a27d9d86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Meny?= Date: Wed, 6 Jul 2022 13:12:41 +0100 Subject: [PATCH] First set of changes --- app/controllers/schemes_controller.rb | 2 +- app/models/organisation.rb | 4 ++-- app/models/scheme.rb | 8 +++---- app/views/schemes/_scheme_list.html.erb | 2 +- app/views/schemes/details.html.erb | 16 +++++++------- app/views/schemes/new.html.erb | 11 +++++----- ...20705130923_rename_scheme_organisations.rb | 8 +++++++ db/schema.rb | 14 ++++++------ spec/factories/scheme.rb | 2 +- spec/features/schemes_spec.rb | 4 ++-- .../form/setup/questions/scheme_id_spec.rb | 2 +- spec/models/organisation_spec.rb | 2 +- spec/models/scheme_spec.rb | 2 +- spec/requests/schemes_controller_spec.rb | 22 +++++++++---------- 14 files changed, 53 insertions(+), 46 deletions(-) create mode 100644 db/migrate/20220705130923_rename_scheme_organisations.rb diff --git a/app/controllers/schemes_controller.rb b/app/controllers/schemes_controller.rb index 4dd657534..68817507c 100644 --- a/app/controllers/schemes_controller.rb +++ b/app/controllers/schemes_controller.rb @@ -139,7 +139,7 @@ private def authenticate_scope! head :unauthorized and return unless current_user.data_coordinator? || current_user.support? - if %w[show locations primary_client_group confirm_secondary_client_group secondary_client_group support details check_answers].include?(action_name) && !((current_user.organisation == @scheme.organisation) || current_user.support?) + if %w[show locations primary_client_group confirm_secondary_client_group secondary_client_group support details check_answers].include?(action_name) && !((current_user.organisation == @scheme.owning_organisation) || current_user.support?) render_not_found and return end end diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 65fce7e51..3cae243c2 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -4,8 +4,8 @@ class Organisation < ApplicationRecord has_many :managed_case_logs, class_name: "CaseLog", foreign_key: "managing_organisation_id" has_many :data_protection_confirmations has_many :organisation_rent_periods - has_many :owned_schemes, class_name: "Scheme", foreign_key: "stock_owning_organisation_id" - has_many :managed_schemes, class_name: "Scheme" + has_many :owned_schemes, class_name: "Scheme", foreign_key: "owning_organisation_id" + has_many :managed_schemes, class_name: "Scheme", foreign_key: "managing_organisation_id" scope :search_by_name, ->(name) { where("name ILIKE ?", "%#{name}%") } scope :search_by, ->(param) { search_by_name(param) } diff --git a/app/models/scheme.rb b/app/models/scheme.rb index f93c08778..93c0c1e9c 100644 --- a/app/models/scheme.rb +++ b/app/models/scheme.rb @@ -1,6 +1,6 @@ class Scheme < ApplicationRecord - belongs_to :organisation - belongs_to :stock_owning_organisation, optional: true, class_name: "Organisation" + belongs_to :owning_organisation, class_name: "Organisation" + belongs_to :managing_organisation, optional: true, class_name: "Organisation" has_many :locations has_many :case_logs @@ -95,8 +95,8 @@ class Scheme < ApplicationRecord { name: "Service code", value: id_to_display }, { name: "Name", value: service_name }, { name: "Confidential information", value: sensitive }, - { name: "Housing stock owned by", value: stock_owning_organisation&.name }, - { name: "Managed by", value: organisation.name }, + { name: "Housing stock owned by", value: owning_organisation.name }, + { name: "Managed by", value: managing_organisation&.name }, { name: "Type of scheme", value: scheme_type }, { name: "Registered under Care Standards Act 2000", value: registered_under_care_act }, ] diff --git a/app/views/schemes/_scheme_list.html.erb b/app/views/schemes/_scheme_list.html.erb index 8b5c6c85c..547afe9f3 100644 --- a/app/views/schemes/_scheme_list.html.erb +++ b/app/views/schemes/_scheme_list.html.erb @@ -24,7 +24,7 @@ <%= body.row do |row| %> <% row.cell(text: scheme.id_to_display) %> <% row.cell(text: simple_format(scheme_cell(scheme), { class: "govuk-!-font-weight-bold" }, wrapper_tag: "div")) %> - <% row.cell(text: scheme.organisation.name) %> + <% row.cell(text: scheme.managing_organisation.name) %> <% row.cell(text: scheme.created_at.to_formatted_s(:govuk_date)) %> <% end %> <% end %> diff --git a/app/views/schemes/details.html.erb b/app/views/schemes/details.html.erb index 73ef511aa..d45d14fa9 100644 --- a/app/views/schemes/details.html.erb +++ b/app/views/schemes/details.html.erb @@ -29,25 +29,25 @@ <% organisations = Organisation.all.map { |org| OpenStruct.new(id: org.id, name: org.name) } %> <% stock_org_answer_options = null_option + organisations %> - <%= f.govuk_collection_select :stock_owning_organisation_id, - stock_org_answer_options, + <%= f.govuk_collection_select :managing_organisation_id, + organisations, :id, :name, - label: { text: "Which organisation owns the housing stock for this scheme?", size: "m" }, + label: { text: "Which organisation manages this scheme?", size: "m" }, + options: { required: true }, "data-controller": %w[accessible-autocomplete conditional-filter] %> <% if current_user.support? %> - <%= f.govuk_collection_select :organisation_id, - organisations, + <%= f.govuk_collection_select :owning_organisation_id, + stock_org_answer_options, :id, :name, - label: { text: "Which organisation manages this scheme?", size: "m" }, - options: { required: true }, + label: { text: "Which organisation owns the housing stock for this scheme?", size: "m" }, "data-controller": %w[accessible-autocomplete conditional-filter] %> <% end %> <% if current_user.data_coordinator? %> - <%= f.hidden_field :organisation_id, value: current_user.organisation.id %> + <%= f.hidden_field :owning_organisation_id, value: current_user.organisation.id %> <% end %> <% scheme_types_selection = Scheme.scheme_types.keys.excluding("Missing").map { |key, _| OpenStruct.new(id: key, name: key.to_s.humanize) } %> diff --git a/app/views/schemes/new.html.erb b/app/views/schemes/new.html.erb index 194f61c3e..f4cbc2827 100644 --- a/app/views/schemes/new.html.erb +++ b/app/views/schemes/new.html.erb @@ -30,24 +30,23 @@ <% organisations = Organisation.all.map { |org| OpenStruct.new(id: org.id, name: org.name) } %> <% answer_options = null_option + organisations %> - <%= f.govuk_collection_select :stock_owning_organisation_id, + <%= f.govuk_collection_select :managing_organisation_id, answer_options, :id, :name, - label: { text: "Which organisation owns the housing stock for this scheme?", size: "m" }, + label: { text: "Which organisation manages this scheme?", size: "m" }, "data-controller": %w[accessible-autocomplete conditional-filter] %> - <% if current_user.support? %> - <%= f.govuk_collection_select :organisation_id, + <%= f.govuk_collection_select :owning_organisation_id, answer_options, :id, :name, - label: { text: "Which organisation manages this scheme?", size: "m" }, + label: { text: "Which organisation owns the housing stock for this scheme?", size: "m" }, "data-controller": %w[accessible-autocomplete conditional-filter] %> <% end %> <% if current_user.data_coordinator? %> - <%= f.hidden_field :organisation_id, value: current_user.organisation.id %> + <%= f.hidden_field :owning_organisation_id, value: current_user.organisation.id %> <% end %> <% scheme_types_selection = Scheme.scheme_types.keys.excluding("Missing").map { |key, _| OpenStruct.new(id: key, name: key.to_s.humanize) } %> diff --git a/db/migrate/20220705130923_rename_scheme_organisations.rb b/db/migrate/20220705130923_rename_scheme_organisations.rb new file mode 100644 index 000000000..5ea1acebc --- /dev/null +++ b/db/migrate/20220705130923_rename_scheme_organisations.rb @@ -0,0 +1,8 @@ +class RenameSchemeOrganisations < ActiveRecord::Migration[7.0] + def change + change_table :schemes, bulk: true do |t| + t.rename :organisation_id, :owning_organisation_id + t.rename :stock_owning_organisation_id, :managing_organisation_id + end + end +end diff --git a/db/schema.rb b/db/schema.rb index b38a76b1a..31d6d4071 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2022_07_04_135746) do +ActiveRecord::Schema[7.0].define(version: 2022_07_05_130923) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -299,7 +299,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_07_04_135746) do create_table "schemes", force: :cascade do |t| t.string "service_name" - t.bigint "organisation_id", null: false + t.bigint "owning_organisation_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false t.string "primary_client_group" @@ -312,9 +312,9 @@ ActiveRecord::Schema[7.0].define(version: 2022_07_04_135746) do t.string "intended_stay" t.datetime "end_date" t.integer "has_other_client_group" - t.bigint "stock_owning_organisation_id" - t.index ["organisation_id"], name: "index_schemes_on_organisation_id" - t.index ["stock_owning_organisation_id"], name: "index_schemes_on_stock_owning_organisation_id" + t.bigint "managing_organisation_id" + t.index ["managing_organisation_id"], name: "index_schemes_on_managing_organisation_id" + t.index ["owning_organisation_id"], name: "index_schemes_on_owning_organisation_id" end create_table "users", force: :cascade do |t| @@ -373,6 +373,6 @@ ActiveRecord::Schema[7.0].define(version: 2022_07_04_135746) do add_foreign_key "case_logs", "locations" add_foreign_key "case_logs", "schemes" add_foreign_key "locations", "schemes" - add_foreign_key "schemes", "organisations" - add_foreign_key "schemes", "organisations", column: "stock_owning_organisation_id" + add_foreign_key "schemes", "organisations", column: "managing_organisation_id" + add_foreign_key "schemes", "organisations", column: "owning_organisation_id" end diff --git a/spec/factories/scheme.rb b/spec/factories/scheme.rb index b16cc75cf..df314894c 100644 --- a/spec/factories/scheme.rb +++ b/spec/factories/scheme.rb @@ -9,7 +9,7 @@ FactoryBot.define do intended_stay { %w[M P S V X].sample } primary_client_group { %w[O H M L A G F B D E I S N R Q P X].sample } secondary_client_group { %w[O H M L A G F B D E I S N R Q P X].sample } - organisation + owning_organisation { FactoryBot.create(:organisation) } created_at { Time.zone.now } end end diff --git a/spec/features/schemes_spec.rb b/spec/features/schemes_spec.rb index 62107aaac..2c29413bf 100644 --- a/spec/features/schemes_spec.rb +++ b/spec/features/schemes_spec.rb @@ -4,8 +4,8 @@ RSpec.describe "Schemes scheme Features" do context "when viewing list of schemes" do context "when I am signed as a coordinator user and there are schemes in the database" do let!(:user) { FactoryBot.create(:user, :data_coordinator, last_sign_in_at: Time.zone.now) } - let!(:schemes) { FactoryBot.create_list(:scheme, 5, organisation: user.organisation) } - let!(:scheme_to_search) { FactoryBot.create(:scheme, organisation: user.organisation) } + let!(:schemes) { FactoryBot.create_list(:scheme, 5, owning_organisation: user.organisation) } + let!(:scheme_to_search) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } before do visit("/logs") diff --git a/spec/models/form/setup/questions/scheme_id_spec.rb b/spec/models/form/setup/questions/scheme_id_spec.rb index 2a71fb2a5..e319dd5ae 100644 --- a/spec/models/form/setup/questions/scheme_id_spec.rb +++ b/spec/models/form/setup/questions/scheme_id_spec.rb @@ -47,7 +47,7 @@ RSpec.describe Form::Setup::Questions::SchemeId, type: :model do let(:case_log) { FactoryBot.create(:case_log, created_by: user) } before do - FactoryBot.create(:scheme, organisation_id: organisation_2.id) + FactoryBot.create(:scheme, owning_organisation: organisation_2) end it "has the correct answer_options based on the schemes the user's organisation owns or manages" do diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb index 3addfd76b..fd2088b6d 100644 --- a/spec/models/organisation_spec.rb +++ b/spec/models/organisation_spec.rb @@ -4,7 +4,7 @@ RSpec.describe Organisation, type: :model do describe "#new" do let(:user) { FactoryBot.create(:user) } let!(:organisation) { user.organisation } - let!(:scheme) { FactoryBot.create(:scheme, organisation:, stock_owning_organisation: organisation) } + let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: organisation, managing_organisation: organisation) } it "has expected fields" do expect(organisation.attribute_names).to include("name", "phone", "provider_type") diff --git a/spec/models/scheme_spec.rb b/spec/models/scheme_spec.rb index 1581bb1e0..96c9b86a7 100644 --- a/spec/models/scheme_spec.rb +++ b/spec/models/scheme_spec.rb @@ -5,7 +5,7 @@ RSpec.describe Scheme, type: :model do let(:scheme) { FactoryBot.create(:scheme) } it "belongs to an organisation" do - expect(scheme.organisation).to be_a(Organisation) + expect(scheme.owning_organisation).to be_a(Organisation) end describe "scopes" do diff --git a/spec/requests/schemes_controller_spec.rb b/spec/requests/schemes_controller_spec.rb index 4dc88fdcb..0160db535 100644 --- a/spec/requests/schemes_controller_spec.rb +++ b/spec/requests/schemes_controller_spec.rb @@ -203,7 +203,7 @@ RSpec.describe SchemesController, type: :request do context "when signed in as a data coordinator user" do let(:user) { FactoryBot.create(:user, :data_coordinator) } - let!(:specific_scheme) { FactoryBot.create(:scheme, organisation: user.organisation) } + let!(:specific_scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } before do sign_in user @@ -287,7 +287,7 @@ RSpec.describe SchemesController, type: :request do context "when signed in as a data coordinator user" do let(:user) { FactoryBot.create(:user, :data_coordinator) } - let!(:scheme) { FactoryBot.create(:scheme, organisation: user.organisation) } + let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } let!(:locations) { FactoryBot.create_list(:location, 3, scheme:) } before do @@ -600,7 +600,7 @@ RSpec.describe SchemesController, type: :request do end context "when required organisation id param is missing" do - let(:params) { { "scheme" => { "service_name" => "qweqwer", "sensitive" => "Yes", "organisation_id" => "", "scheme_type" => "Foyer", "registered_under_care_act" => "Yes – part registered as a care home", "total_units" => "1" } } } + let(:params) { { "scheme" => { "service_name" => "qweqwer", "sensitive" => "Yes", "owning_organisation_id" => "", "scheme_type" => "Foyer", "registered_under_care_act" => "Yes – part registered as a care home", "total_units" => "1" } } } it "displays the new page with an error message" do post "/schemes", params: params @@ -635,7 +635,7 @@ RSpec.describe SchemesController, type: :request do context "when signed in as a data coordinator" do let(:user) { FactoryBot.create(:user, :data_coordinator) } - let(:scheme_to_update) { FactoryBot.create(:scheme, organisation: user.organisation) } + let(:scheme_to_update) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } before do sign_in user @@ -820,7 +820,7 @@ RSpec.describe SchemesController, type: :request do context "when signed in as a support" do let(:user) { FactoryBot.create(:user, :support) } - let(:scheme_to_update) { FactoryBot.create(:scheme, organisation: user.organisation) } + let(:scheme_to_update) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } before do allow(user).to receive(:need_two_factor_authentication?).and_return(false) @@ -1041,7 +1041,7 @@ RSpec.describe SchemesController, type: :request do context "when signed in as a data coordinator" do let(:user) { FactoryBot.create(:user, :data_coordinator) } - let!(:scheme) { FactoryBot.create(:scheme, organisation: user.organisation) } + let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } let!(:another_scheme) { FactoryBot.create(:scheme) } before do @@ -1107,7 +1107,7 @@ RSpec.describe SchemesController, type: :request do context "when signed in as a data coordinator" do let(:user) { FactoryBot.create(:user, :data_coordinator) } - let!(:scheme) { FactoryBot.create(:scheme, organisation: user.organisation) } + let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } let!(:another_scheme) { FactoryBot.create(:scheme) } before do @@ -1173,7 +1173,7 @@ RSpec.describe SchemesController, type: :request do context "when signed in as a data coordinator" do let(:user) { FactoryBot.create(:user, :data_coordinator) } - let!(:scheme) { FactoryBot.create(:scheme, organisation: user.organisation) } + let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } let!(:another_scheme) { FactoryBot.create(:scheme) } before do @@ -1239,7 +1239,7 @@ RSpec.describe SchemesController, type: :request do context "when signed in as a data coordinator" do let(:user) { FactoryBot.create(:user, :data_coordinator) } - let!(:scheme) { FactoryBot.create(:scheme, organisation: user.organisation) } + let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } let!(:another_scheme) { FactoryBot.create(:scheme) } before do @@ -1305,7 +1305,7 @@ RSpec.describe SchemesController, type: :request do context "when signed in as a data coordinator" do let(:user) { FactoryBot.create(:user, :data_coordinator) } - let!(:scheme) { FactoryBot.create(:scheme, organisation: user.organisation) } + let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } let!(:another_scheme) { FactoryBot.create(:scheme) } before do @@ -1371,7 +1371,7 @@ RSpec.describe SchemesController, type: :request do context "when signed in as a data coordinator" do let(:user) { FactoryBot.create(:user, :data_coordinator) } - let!(:scheme) { FactoryBot.create(:scheme, organisation: user.organisation) } + let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } let!(:another_scheme) { FactoryBot.create(:scheme) } before do