From 7adf58feaef60d4ef8740c0aef080c6836b5ba70 Mon Sep 17 00:00:00 2001 From: Jack S Date: Tue, 18 Oct 2022 15:36:19 +0100 Subject: [PATCH 1/4] wip --- .../organisation_relationships_controller.rb | 20 +++++++++++++++++++ app/helpers/navigation_items_helper.rb | 16 +++++++++++++++ app/models/organisation.rb | 6 ++++++ app/models/organisation_relationship.rb | 6 ++++-- .../managing_agents.html.erb | 3 +++ config/initializers/feature_toggle.rb | 6 ++++++ config/routes.rb | 1 + ...3607_rename_organisations_agents_column.rb | 4 ++++ db/seeds.rb | 15 +++++++++++++- spec/models/organisation_spec.rb | 4 ++++ 10 files changed, 78 insertions(+), 3 deletions(-) create mode 100644 app/controllers/organisation_relationships_controller.rb create mode 100644 app/views/organisation_relationships/managing_agents.html.erb create mode 100644 db/migrate/20221018143607_rename_organisations_agents_column.rb diff --git a/app/controllers/organisation_relationships_controller.rb b/app/controllers/organisation_relationships_controller.rb new file mode 100644 index 000000000..e784c23e1 --- /dev/null +++ b/app/controllers/organisation_relationships_controller.rb @@ -0,0 +1,20 @@ +class OrganisationRelationshipsController < ApplicationController + include Pagy::Backend + + before_action :authenticate_user! + + def managing_agents + # kick out if org isn't the current org + + @managing_agents = OrganisationRelationships.where( + owning_organisation_id: organisation.id, + relationship_type: :managing, + ) + end + +private + + def organisation + @organisation ||= Organisation.find(params[:id]) + end +end diff --git a/app/helpers/navigation_items_helper.rb b/app/helpers/navigation_items_helper.rb index 3ac422c2d..a5f701743 100644 --- a/app/helpers/navigation_items_helper.rb +++ b/app/helpers/navigation_items_helper.rb @@ -24,6 +24,7 @@ module NavigationItemsHelper FeatureToggle.sales_log_enabled? ? NavigationItem.new("Sales logs", sales_logs_path, sales_logs_current?(path)) : nil, NavigationItem.new("Users", users_organisation_path(current_user.organisation), subnav_users_path?(path)), NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}", subnav_details_path?(path)), + managing_agents_item, ].compact end end @@ -91,4 +92,19 @@ private def subnav_details_path?(path) path.include?("/organisations") && path.include?("/details") end + + def managing_agents_path?(path) + path.include?("/managing-agents") + end + + def managing_agents_item + return unless FeatureToggle.managing_agents_enabled? + return unless current_user.organisation.holds_own_stock? + + NavigationItem.new( + "Managing agents", + "/organisations/#{current_user.organisation.id}/managing-agents", + managing_agents_path?(path), + ) + end end diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 9eb039ab1..d21717206 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -13,6 +13,12 @@ class Organisation < ApplicationRecord has_many :child_organisation_relationships, foreign_key: :parent_organisation_id, class_name: "OrganisationRelationship" has_many :child_organisations, through: :child_organisation_relationships + has_many :managing_agents, + -> { joins(:parent_organisation_relationships).where({ parent_organisation_relationships: { relationship_type: OrganisationRelationship::MANAGING } }) }, + class_name: "Organisation", + through: :parent_organisation_relationships, + source: :child_organisation + scope :search_by_name, ->(name) { where("name ILIKE ?", "%#{name}%") } scope :search_by, ->(param) { search_by_name(param) } diff --git a/app/models/organisation_relationship.rb b/app/models/organisation_relationship.rb index 5c50c3e0f..acda8d2c4 100644 --- a/app/models/organisation_relationship.rb +++ b/app/models/organisation_relationship.rb @@ -2,9 +2,11 @@ class OrganisationRelationship < ApplicationRecord belongs_to :child_organisation, class_name: "Organisation" belongs_to :parent_organisation, class_name: "Organisation" + OWNING = "owning".freeze + MANAGING = "managing".freeze RELATIONSHIP_TYPE = { - "owning": 0, - "managing": 1, + OWNING => 0, + MANAGING => 1, }.freeze enum relationship_type: RELATIONSHIP_TYPE diff --git a/app/views/organisation_relationships/managing_agents.html.erb b/app/views/organisation_relationships/managing_agents.html.erb new file mode 100644 index 000000000..a9b698239 --- /dev/null +++ b/app/views/organisation_relationships/managing_agents.html.erb @@ -0,0 +1,3 @@ +<% @managing_agents.each do |managing_agent|%> + <%= managing_agent.inspect %> +<% end %> diff --git a/config/initializers/feature_toggle.rb b/config/initializers/feature_toggle.rb index 0e01531ca..64b75e6b7 100644 --- a/config/initializers/feature_toggle.rb +++ b/config/initializers/feature_toggle.rb @@ -8,4 +8,10 @@ class FeatureToggle false end + + def self.managing_agents_enabled? + return true unless Rails.env.production? + + false + end end diff --git a/config/routes.rb b/config/routes.rb index 0be5f5b6a..2dffe036e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -79,6 +79,7 @@ Rails.application.routes.draw do post "logs/email-csv", to: "organisations#email_csv" get "logs/csv-confirmation", to: "lettings_logs#csv_confirmation" get "schemes", to: "organisations#schemes" + get "managing-agents", to: "organisation_relationships#managing_agents" end end diff --git a/db/migrate/20221018143607_rename_organisations_agents_column.rb b/db/migrate/20221018143607_rename_organisations_agents_column.rb new file mode 100644 index 000000000..0397f4afd --- /dev/null +++ b/db/migrate/20221018143607_rename_organisations_agents_column.rb @@ -0,0 +1,4 @@ +class RenameOrganisationsAgentsColumn < ActiveRecord::Migration[7.0] + def change + end +end diff --git a/db/seeds.rb b/db/seeds.rb index 8491db6b6..f8d421999 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -13,7 +13,7 @@ unless Rails.env.test? address_line1: "2 Marsham Street", address_line2: "London", postcode: "SW1P 4DF", - holds_own_stock: false, + holds_own_stock: true, other_stock_owners: "None", managing_agents: "None", provider_type: "LA", @@ -26,6 +26,19 @@ unless Rails.env.test? end end + Organisation.find_or_create_by!( + name: "DLUHC", + address_line1: "2 Marsham Street", + address_line2: "London", + postcode: "SW1P 4DF", + holds_own_stock: true, + other_stock_owners: "None", + managing_agents: "None", + provider_type: "LA", + child_organisations: [org] + ) + + if Rails.env.development? && User.count.zero? User.create!( name: "Provider", diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb index b6011cf20..e2336bd0c 100644 --- a/spec/models/organisation_spec.rb +++ b/spec/models/organisation_spec.rb @@ -41,6 +41,10 @@ RSpec.describe Organisation, type: :model do it "has correct parent" do expect(child_organisation.parent_organisations.first).to eq(organisation) end + + it "has correct managing agents" do + expect(child_organisation.managing_agents).to eq([organisation]) + end end context "with owning association" do From 1943357db1857154e4726622ace101f259cc97fb Mon Sep 17 00:00:00 2001 From: Jack S Date: Wed, 19 Oct 2022 09:28:26 +0100 Subject: [PATCH 2/4] Rename managing_agents column --- app/models/organisation.rb | 2 +- .../20221019082625_rename_managing_agents_column.rb | 5 +++++ db/schema.rb | 4 ++-- db/seeds.rb | 8 ++++---- 4 files changed, 12 insertions(+), 7 deletions(-) create mode 100644 db/migrate/20221019082625_rename_managing_agents_column.rb diff --git a/app/models/organisation.rb b/app/models/organisation.rb index d21717206..bd57838b8 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -89,7 +89,7 @@ class Organisation < ApplicationRecord { name: "Rent_periods", value: rent_period_labels, editable: false, format: :bullet }, { name: "Owns housing stock", value: holds_own_stock ? "Yes" : "No", editable: false }, { name: "Other stock owners", value: other_stock_owners, editable: false }, - { name: "Managing agents", value: managing_agents, editable: false }, + { name: "Managing agents", value: managing_agents_label, editable: false }, { name: "Data protection agreement", value: data_protection_agreement_string, editable: false }, ] end diff --git a/db/migrate/20221019082625_rename_managing_agents_column.rb b/db/migrate/20221019082625_rename_managing_agents_column.rb new file mode 100644 index 000000000..dd03a7435 --- /dev/null +++ b/db/migrate/20221019082625_rename_managing_agents_column.rb @@ -0,0 +1,5 @@ +class RenameManagingAgentsColumn < ActiveRecord::Migration[7.0] + def change + rename_column :organisations, :managing_agents, :managing_agents_label + end +end diff --git a/db/schema.rb b/db/schema.rb index 59a0dbd24..80f37a6b0 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_10_17_095918) do +ActiveRecord::Schema[7.0].define(version: 2022_10_19_082625) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -297,7 +297,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_10_17_095918) do t.string "postcode" t.boolean "holds_own_stock" t.string "other_stock_owners" - t.string "managing_agents" + t.string "managing_agents_label" t.datetime "created_at", null: false t.datetime "updated_at", null: false t.boolean "active" diff --git a/db/seeds.rb b/db/seeds.rb index f8d421999..dfc479cb8 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -15,7 +15,7 @@ unless Rails.env.test? postcode: "SW1P 4DF", holds_own_stock: true, other_stock_owners: "None", - managing_agents: "None", + managing_agents_label: "None", provider_type: "LA", ) do info = "Seeded DLUHC Organisation" @@ -33,9 +33,9 @@ unless Rails.env.test? postcode: "SW1P 4DF", holds_own_stock: true, other_stock_owners: "None", - managing_agents: "None", + managing_agents_label: "None", provider_type: "LA", - child_organisations: [org] + child_organisations: [org], ) @@ -78,7 +78,7 @@ unless Rails.env.test? postcode: "BA21 4AT", holds_own_stock: false, other_stock_owners: "None", - managing_agents: "None", + managing_agents_label: "None", provider_type: "LA", ) From baa052f802286f485dd48a6c6fb8a2cea454292e Mon Sep 17 00:00:00 2001 From: Jack S Date: Wed, 19 Oct 2022 11:23:53 +0100 Subject: [PATCH 3/4] add managing relationship --- app/models/organisation.rb | 8 +-- spec/factories/organisation.rb | 5 -- spec/factories/organisation_relationship.rb | 14 +++++ spec/models/organisation_spec.rb | 64 +++++++++++++++------ 4 files changed, 62 insertions(+), 29 deletions(-) create mode 100644 spec/factories/organisation_relationship.rb diff --git a/app/models/organisation.rb b/app/models/organisation.rb index bd57838b8..e86be1e9f 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -13,15 +13,11 @@ class Organisation < ApplicationRecord has_many :child_organisation_relationships, foreign_key: :parent_organisation_id, class_name: "OrganisationRelationship" has_many :child_organisations, through: :child_organisation_relationships - has_many :managing_agents, - -> { joins(:parent_organisation_relationships).where({ parent_organisation_relationships: { relationship_type: OrganisationRelationship::MANAGING } }) }, - class_name: "Organisation", - through: :parent_organisation_relationships, - source: :child_organisation + has_many :managing_agent_relationships, -> { where(relationship_type: OrganisationRelationship::MANAGING) }, foreign_key: :child_organisation_id, class_name: "OrganisationRelationship" + has_many :managing_agents, through: :managing_agent_relationships, source: :parent_organisation scope :search_by_name, ->(name) { where("name ILIKE ?", "%#{name}%") } scope :search_by, ->(param) { search_by_name(param) } - has_paper_trail auto_strip_attributes :name diff --git a/spec/factories/organisation.rb b/spec/factories/organisation.rb index 8b02f030a..fa40663ac 100644 --- a/spec/factories/organisation.rb +++ b/spec/factories/organisation.rb @@ -17,9 +17,4 @@ FactoryBot.define do created_at { Time.zone.now } updated_at { Time.zone.now } end - - factory :organisation_relationship do - child_organisation { FactoryBot.create(:organisation) } - parent_organisation { FactoryBot.create(:organisation) } - end end diff --git a/spec/factories/organisation_relationship.rb b/spec/factories/organisation_relationship.rb new file mode 100644 index 000000000..418599902 --- /dev/null +++ b/spec/factories/organisation_relationship.rb @@ -0,0 +1,14 @@ +FactoryBot.define do + factory :organisation_relationship do + child_organisation { FactoryBot.create(:organisation) } + parent_organisation { FactoryBot.create(:organisation) } + + trait :owning do + relationship_type { OrganisationRelationship::OWNING } + end + + trait :managing do + relationship_type { OrganisationRelationship::MANAGING } + end + end +end diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb index e2336bd0c..1d9e557ec 100644 --- a/spec/models/organisation_spec.rb +++ b/spec/models/organisation_spec.rb @@ -27,39 +27,67 @@ RSpec.describe Organisation, type: :model do .to raise_error(ActiveRecord::RecordInvalid, "Validation failed: Provider type #{I18n.t('validations.organisation.provider_type_missing')}") end - context "with managing association" do - let(:child_organisation) { FactoryBot.create(:organisation, name: "DLUHC Child") } + context "with parent/child associations", :aggregate_failures do + let!(:child_organisation) { FactoryBot.create(:organisation, name: "DLUHC Child") } + let!(:grandchild_organisation) { FactoryBot.create(:organisation, name: "DLUHC Grandchild") } before do - FactoryBot.create(:organisation_relationship, child_organisation:, parent_organisation: organisation, relationship_type: 0) - end + FactoryBot.create( + :organisation_relationship, + :managing, + child_organisation:, + parent_organisation: organisation, + ) - it "has correct child" do - expect(organisation.child_organisations.first).to eq(child_organisation) + FactoryBot.create( + :organisation_relationship, + :managing, + child_organisation: grandchild_organisation, + parent_organisation: child_organisation, + ) end - it "has correct parent" do - expect(child_organisation.parent_organisations.first).to eq(organisation) + it "has correct child_organisations" do + expect(organisation.child_organisations).to eq([child_organisation]) + expect(child_organisation.child_organisations).to eq([grandchild_organisation]) end - it "has correct managing agents" do - expect(child_organisation.managing_agents).to eq([organisation]) + it "has correct parent_organisations" do + expect(child_organisation.parent_organisations).to eq([organisation]) + expect(grandchild_organisation.parent_organisations).to eq([child_organisation]) end end - context "with owning association" do - let(:child_organisation) { FactoryBot.create(:organisation, name: "DLUHC Child") } + context "with managing association", :aggregate_failures do + let!(:child_organisation) { FactoryBot.create(:organisation, name: "DLUHC Child") } + let!(:grandchild_organisation) { FactoryBot.create(:organisation, name: "DLUHC Grandchild") } before do - FactoryBot.create(:organisation_relationship, child_organisation:, parent_organisation: organisation, relationship_type: 1) - end + FactoryBot.create( + :organisation_relationship, + :managing, + child_organisation:, + parent_organisation: organisation, + ) + + FactoryBot.create( + :organisation_relationship, + :owning, + child_organisation:, + parent_organisation: organisation, + ) - it "has correct child" do - expect(organisation.child_organisations.first).to eq(child_organisation) + FactoryBot.create( + :organisation_relationship, + :managing, + child_organisation: grandchild_organisation, + parent_organisation: child_organisation, + ) end - it "has correct parent" do - expect(child_organisation.parent_organisations.first).to eq(organisation) + it "has correct managing_agents" do + expect(child_organisation.managing_agents).to eq([organisation]) + expect(grandchild_organisation.managing_agents).to eq([child_organisation]) end end From 4743fb6fba02be331441a2c31854f0224b8528b0 Mon Sep 17 00:00:00 2001 From: Jack S Date: Wed, 19 Oct 2022 12:20:53 +0100 Subject: [PATCH 4/4] f --- .../organisation_relationships_controller.rb | 7 ++--- app/helpers/navigation_items_helper.rb | 8 +++--- .../managing_agents.html.erb | 7 ++++- db/seeds.rb | 26 +++++++++++-------- 4 files changed, 28 insertions(+), 20 deletions(-) diff --git a/app/controllers/organisation_relationships_controller.rb b/app/controllers/organisation_relationships_controller.rb index e784c23e1..1677239d0 100644 --- a/app/controllers/organisation_relationships_controller.rb +++ b/app/controllers/organisation_relationships_controller.rb @@ -4,12 +4,9 @@ class OrganisationRelationshipsController < ApplicationController before_action :authenticate_user! def managing_agents - # kick out if org isn't the current org + # kick out if cannot access org - @managing_agents = OrganisationRelationships.where( - owning_organisation_id: organisation.id, - relationship_type: :managing, - ) + @managing_agents = organisation.managing_agents end private diff --git a/app/helpers/navigation_items_helper.rb b/app/helpers/navigation_items_helper.rb index a5f701743..270a563e4 100644 --- a/app/helpers/navigation_items_helper.rb +++ b/app/helpers/navigation_items_helper.rb @@ -2,7 +2,7 @@ module NavigationItemsHelper NavigationItem = Struct.new(:text, :href, :current, :classes) def primary_items(path, current_user) - if current_user.support? + items = if current_user.support? [ NavigationItem.new("Organisations", organisations_path, organisations_current?(path)), NavigationItem.new("Users", "/users", users_current?(path)), @@ -24,9 +24,11 @@ module NavigationItemsHelper FeatureToggle.sales_log_enabled? ? NavigationItem.new("Sales logs", sales_logs_path, sales_logs_current?(path)) : nil, NavigationItem.new("Users", users_organisation_path(current_user.organisation), subnav_users_path?(path)), NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}", subnav_details_path?(path)), - managing_agents_item, ].compact end + + # figure out correct rules + items << managing_agents_item(path) end def secondary_items(path, current_organisation_id) @@ -97,7 +99,7 @@ private path.include?("/managing-agents") end - def managing_agents_item + def managing_agents_item(path) return unless FeatureToggle.managing_agents_enabled? return unless current_user.organisation.holds_own_stock? diff --git a/app/views/organisation_relationships/managing_agents.html.erb b/app/views/organisation_relationships/managing_agents.html.erb index a9b698239..a4a8aa6ad 100644 --- a/app/views/organisation_relationships/managing_agents.html.erb +++ b/app/views/organisation_relationships/managing_agents.html.erb @@ -1,3 +1,8 @@ +<% title = format_title(nil, current_user.support? ? "About this organisation" : "Your managing agents", current_user, nil, nil, @organisation.name) %> +<% content_for :title, title %> + +<%= render partial: "organisations/headings", locals: { main: "Your managing agents", sub: nil } %> + <% @managing_agents.each do |managing_agent|%> - <%= managing_agent.inspect %> + <%= managing_agent.name %> <% end %> diff --git a/db/seeds.rb b/db/seeds.rb index dfc479cb8..e5e4eacaa 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -8,6 +8,17 @@ # rubocop:disable Rails/Output unless Rails.env.test? + managing_agent = Organisation.find_or_create_by!( + name: "Managing Agent", + address_line1: "2 Marsham Street", + address_line2: "London", + postcode: "SW1P 4DF", + holds_own_stock: true, + other_stock_owners: "None", + managing_agents_label: "None", + provider_type: "LA", + ) + org = Organisation.find_or_create_by!( name: "DLUHC", address_line1: "2 Marsham Street", @@ -26,19 +37,12 @@ unless Rails.env.test? end end - Organisation.find_or_create_by!( - name: "DLUHC", - address_line1: "2 Marsham Street", - address_line2: "London", - postcode: "SW1P 4DF", - holds_own_stock: true, - other_stock_owners: "None", - managing_agents_label: "None", - provider_type: "LA", - child_organisations: [org], + OrganisationRelationship.create!( + child_organisation: org, + parent_organisation: managing_agent, + relationship_type: OrganisationRelationship::MANAGING, ) - if Rails.env.development? && User.count.zero? User.create!( name: "Provider",