From 4dd2c7e1340ed718abea2b006728254e094c4f9e Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Wed, 5 Mar 2025 14:07:54 +0000 Subject: [PATCH 01/36] Add questions to page, wip --- app/frontend/controllers/index.js | 3 ++ .../controllers/organisations_controller.js | 14 +++++++++ app/models/organisation.rb | 9 ++++++ app/views/organisations/new.html.erb | 31 +++++++++++++++++++ ...add_new_question_fields_to_organisation.rb | 7 +++++ db/schema.rb | 24 ++++++++------ 6 files changed, 79 insertions(+), 9 deletions(-) create mode 100644 app/frontend/controllers/organisations_controller.js create mode 100644 db/migrate/20250227085622_add_new_question_fields_to_organisation.rb diff --git a/app/frontend/controllers/index.js b/app/frontend/controllers/index.js index 944e32e2d..35385e46e 100644 --- a/app/frontend/controllers/index.js +++ b/app/frontend/controllers/index.js @@ -19,6 +19,8 @@ import FilterLayoutController from './filter_layout_controller.js' import TabsController from './tabs_controller.js' +import OrganisationsController from './organisations_controller.js' + application.register('accessible-autocomplete', AccessibleAutocompleteController) application.register('conditional-filter', ConditionalFilterController) application.register('conditional-question', ConditionalQuestionController) @@ -27,3 +29,4 @@ application.register('numeric-question', NumericQuestionController) application.register('filter-layout', FilterLayoutController) application.register('search', SearchController) application.register('tabs', TabsController) +application.register('organisations', OrganisationsController) diff --git a/app/frontend/controllers/organisations_controller.js b/app/frontend/controllers/organisations_controller.js new file mode 100644 index 000000000..855081dab --- /dev/null +++ b/app/frontend/controllers/organisations_controller.js @@ -0,0 +1,14 @@ +import { Controller } from '@hotwired/stimulus' + +export default class extends Controller { + static targets = ["groupSelect"] + + connect() { + this.toggleGroupSelect() + } + + toggleGroupSelect() { + const groupMemberYes = this.element.querySelector('input[name="organisation[group_member]"]:checked')?.value === 'true' + this.groupSelectTarget.style.display = groupMemberYes ? 'block' : 'none' + } +} diff --git a/app/models/organisation.rb b/app/models/organisation.rb index c3d0a8ca0..a1ccc95f8 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -53,7 +53,16 @@ class Organisation < ApplicationRecord PRP: 2, }.freeze + PROFIT_STATUS = { + non_profit: 1, + profit: 2, + local_authority: 3, + }.freeze + enum :provider_type, PROVIDER_TYPE + enum :profit_status, PROFIT_STATUS + + attribute :group_member, :boolean alias_method :la?, :LA? diff --git a/app/views/organisations/new.html.erb b/app/views/organisations/new.html.erb index 20d4d2cc6..0b3c85cbd 100644 --- a/app/views/organisations/new.html.erb +++ b/app/views/organisations/new.html.erb @@ -56,6 +56,37 @@ :name, legend: { text: "Does the organisation hold its own stock?", size: "m" } %> + <%= f.govuk_collection_radio_buttons :group_member, + [OpenStruct.new(id: true, name: "Yes"), OpenStruct.new(id: false, name: "No")], + :id, + :name, + legend: { text: "Is this organisation part of a housing provider group structure?", size: "m" }, + "data-action": "change->group-member#toggleGroupSelect" %> + + <% group_organisation_options = Organisation.visible.map { |org| OpenStruct.new(id: org.id, name: org.name) } %> + +
+ <%= f.govuk_collection_select :group, + group_organisation_options, + :id, + :name, + label: { text: "Search for an organisation that is part of the same group as this organisation", size: "m" }, + "data-controller": %w[accessible-autocomplete conditional-filter organisations] %> +
+ + <% profit_options = [ + OpenStruct.new(id: 1, name: "Non-profit"), + OpenStruct.new(id: 2, name: "Profit"), + OpenStruct.new(id: 3, name: "Local authority") + ] %> + + <%= f.govuk_collection_select :profit_status, + profit_options, + :id, + :name, + label: { text: "Is the organisation for-profit?", size: "m" }, + options: { disabled: [""], selected: @organisation.profit_status || "" } %> + <%= f.govuk_check_boxes_fieldset :rent_periods, legend: { text: "What are the rent periods for the organisation?" } do %> <% @rent_periods.map do |key, period| %> diff --git a/db/migrate/20250227085622_add_new_question_fields_to_organisation.rb b/db/migrate/20250227085622_add_new_question_fields_to_organisation.rb new file mode 100644 index 000000000..08b3ff2bf --- /dev/null +++ b/db/migrate/20250227085622_add_new_question_fields_to_organisation.rb @@ -0,0 +1,7 @@ +class AddNewQuestionFieldsToOrganisation < ActiveRecord::Migration[7.2] + def change + add_column :organisations, :profit_status, :integer + add_column :organisations, :group_member, :boolean + add_column :organisations, :group, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 894bb1638..f00ffb5d4 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.2].define(version: 2025_01_10_150609) do +ActiveRecord::Schema[7.2].define(version: 2025_02_27_085622) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -84,7 +84,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_10_150609) do t.datetime "last_accessed" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.check_constraint "log_type::text = ANY (ARRAY['lettings'::character varying, 'sales'::character varying]::text[])", name: "log_type_check" + t.check_constraint "log_type::text = ANY (ARRAY['lettings'::character varying::text, 'sales'::character varying::text])", name: "log_type_check" t.check_constraint "year >= 2000 AND year <= 2099", name: "year_check" end @@ -245,14 +245,14 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_10_150609) do t.integer "hb" t.integer "hbrentshortfall" t.integer "property_relet" - t.datetime "mrcdate", precision: nil + t.datetime "mrcdate" t.integer "incref" - t.datetime "startdate", precision: nil + t.datetime "startdate" t.integer "armedforces" t.integer "first_time_property_let_as_social_housing" t.integer "unitletas" t.integer "builtype" - t.datetime "voiddate", precision: nil + t.datetime "voiddate" t.bigint "owning_organisation_id" t.bigint "managing_organisation_id" t.integer "renttype" @@ -373,6 +373,8 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_10_150609) do t.integer "partner_under_16_value_check" t.integer "multiple_partners_value_check" t.bigint "created_by_id" + t.boolean "manual_address_entry_selected", default: false + t.integer "referral_type" t.index ["assigned_to_id"], name: "index_lettings_logs_on_assigned_to_id" t.index ["bulk_upload_id"], name: "index_lettings_logs_on_bulk_upload_id" t.index ["created_by_id"], name: "index_lettings_logs_on_created_by_id" @@ -546,6 +548,9 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_10_150609) do t.datetime "available_from" t.datetime "discarded_at" t.datetime "schemes_deduplicated_at" + t.integer "profit_status" + t.boolean "group_member" + t.integer "group" t.index ["absorbing_organisation_id"], name: "index_organisations_on_absorbing_organisation_id" t.index ["name"], name: "index_organisations_on_name", unique: true t.index ["old_visible_id"], name: "index_organisations_on_old_visible_id", unique: true @@ -762,13 +767,14 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_10_150609) do t.integer "partner_under_16_value_check" t.integer "multiple_partners_value_check" t.bigint "created_by_id" - t.integer "has_management_fee" - t.decimal "management_fee", precision: 10, scale: 2 t.integer "firststair" t.integer "numstair" t.decimal "mrentprestaircasing", precision: 10, scale: 2 t.datetime "lasttransaction" t.datetime "initialpurchase" + t.integer "has_management_fee" + t.decimal "management_fee", precision: 10, scale: 2 + t.boolean "manual_address_entry_selected", default: false t.index ["assigned_to_id"], name: "index_sales_logs_on_assigned_to_id" t.index ["bulk_upload_id"], name: "index_sales_logs_on_bulk_upload_id" t.index ["created_by_id"], name: "index_sales_logs_on_created_by_id" @@ -822,8 +828,8 @@ ActiveRecord::Schema[7.2].define(version: 2025_01_10_150609) do t.string "name" t.bigint "organisation_id" t.integer "sign_in_count", default: 0, null: false - t.datetime "current_sign_in_at", precision: nil - t.datetime "last_sign_in_at", precision: nil + t.datetime "current_sign_in_at" + t.datetime "last_sign_in_at" t.string "current_sign_in_ip" t.string "last_sign_in_ip" t.integer "role" From 02243bc6eb6a127c5464f1f6085c068b2bf141ae Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Wed, 5 Mar 2025 14:34:14 +0000 Subject: [PATCH 02/36] Add select as conditional question in radio like in logs --- app/frontend/controllers/index.js | 3 -- .../controllers/organisations_controller.js | 14 --------- app/views/organisations/new.html.erb | 31 ++++++++++--------- 3 files changed, 16 insertions(+), 32 deletions(-) delete mode 100644 app/frontend/controllers/organisations_controller.js diff --git a/app/frontend/controllers/index.js b/app/frontend/controllers/index.js index 35385e46e..944e32e2d 100644 --- a/app/frontend/controllers/index.js +++ b/app/frontend/controllers/index.js @@ -19,8 +19,6 @@ import FilterLayoutController from './filter_layout_controller.js' import TabsController from './tabs_controller.js' -import OrganisationsController from './organisations_controller.js' - application.register('accessible-autocomplete', AccessibleAutocompleteController) application.register('conditional-filter', ConditionalFilterController) application.register('conditional-question', ConditionalQuestionController) @@ -29,4 +27,3 @@ application.register('numeric-question', NumericQuestionController) application.register('filter-layout', FilterLayoutController) application.register('search', SearchController) application.register('tabs', TabsController) -application.register('organisations', OrganisationsController) diff --git a/app/frontend/controllers/organisations_controller.js b/app/frontend/controllers/organisations_controller.js deleted file mode 100644 index 855081dab..000000000 --- a/app/frontend/controllers/organisations_controller.js +++ /dev/null @@ -1,14 +0,0 @@ -import { Controller } from '@hotwired/stimulus' - -export default class extends Controller { - static targets = ["groupSelect"] - - connect() { - this.toggleGroupSelect() - } - - toggleGroupSelect() { - const groupMemberYes = this.element.querySelector('input[name="organisation[group_member]"]:checked')?.value === 'true' - this.groupSelectTarget.style.display = groupMemberYes ? 'block' : 'none' - } -} diff --git a/app/views/organisations/new.html.erb b/app/views/organisations/new.html.erb index 0b3c85cbd..68040ece6 100644 --- a/app/views/organisations/new.html.erb +++ b/app/views/organisations/new.html.erb @@ -56,23 +56,24 @@ :name, legend: { text: "Does the organisation hold its own stock?", size: "m" } %> - <%= f.govuk_collection_radio_buttons :group_member, - [OpenStruct.new(id: true, name: "Yes"), OpenStruct.new(id: false, name: "No")], - :id, - :name, - legend: { text: "Is this organisation part of a housing provider group structure?", size: "m" }, - "data-action": "change->group-member#toggleGroupSelect" %> - <% group_organisation_options = Organisation.visible.map { |org| OpenStruct.new(id: org.id, name: org.name) } %> -
- <%= f.govuk_collection_select :group, - group_organisation_options, - :id, - :name, - label: { text: "Search for an organisation that is part of the same group as this organisation", size: "m" }, - "data-controller": %w[accessible-autocomplete conditional-filter organisations] %> -
+ <%= f.govuk_radio_buttons_fieldset :group_member, + legend: { text: "Is this organisation part of a housing provider group structure?", size: "m" } do %> + <%= f.govuk_radio_button :group_member, true, + label: { text: "Yes" }, + "data-controller": "conditional-question", + "data-action": "click->conditional-question#displayConditional", + "data-info": { conditional_questions: { group: [true] }, type: "organisation" }.to_json do %> + <%= f.govuk_collection_select :group, + group_organisation_options, + :id, + :name, + label: { text: "Search for an organisation that is part of the same group as this organisation", size: "m" }, + "data-controller": %w[accessible-autocomplete conditional-filter] %> + <% end %> + <%= f.govuk_radio_button :group_member, false, label: { text: "No" } %> + <% end %> <% profit_options = [ OpenStruct.new(id: 1, name: "Non-profit"), From 75ec9e413973d5a65369d2bc2cef357b0d15aa7a Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Wed, 5 Mar 2025 14:37:26 +0000 Subject: [PATCH 03/36] Add existing value --- app/views/organisations/new.html.erb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/views/organisations/new.html.erb b/app/views/organisations/new.html.erb index 68040ece6..be557c239 100644 --- a/app/views/organisations/new.html.erb +++ b/app/views/organisations/new.html.erb @@ -70,6 +70,7 @@ :id, :name, label: { text: "Search for an organisation that is part of the same group as this organisation", size: "m" }, + options: { disabled: [""], selected: @organisation.group || "" }, "data-controller": %w[accessible-autocomplete conditional-filter] %> <% end %> <%= f.govuk_radio_button :group_member, false, label: { text: "No" } %> From 2c14c0dd8c291dc3ee0e3fc04be7d0f6d1cc5441 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Wed, 5 Mar 2025 14:43:23 +0000 Subject: [PATCH 04/36] Add blank default --- app/views/organisations/new.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/organisations/new.html.erb b/app/views/organisations/new.html.erb index be557c239..84aee1d80 100644 --- a/app/views/organisations/new.html.erb +++ b/app/views/organisations/new.html.erb @@ -56,7 +56,7 @@ :name, legend: { text: "Does the organisation hold its own stock?", size: "m" } %> - <% group_organisation_options = Organisation.visible.map { |org| OpenStruct.new(id: org.id, name: org.name) } %> + <% group_organisation_options = [OpenStruct.new(id: "", name: "")] + Organisation.visible.map { |org| OpenStruct.new(id: org.id, name: org.name) } %> <%= f.govuk_radio_buttons_fieldset :group_member, legend: { text: "Is this organisation part of a housing provider group structure?", size: "m" } do %> From df7f31f41f32674250ec1ff869efef3417a5c26c Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Wed, 5 Mar 2025 15:02:39 +0000 Subject: [PATCH 05/36] Add values to about this organisation page --- app/helpers/organisations_helper.rb | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/app/helpers/organisations_helper.rb b/app/helpers/organisations_helper.rb index 19c77b357..8cf7f4ea6 100644 --- a/app/helpers/organisations_helper.rb +++ b/app/helpers/organisations_helper.rb @@ -10,17 +10,26 @@ module OrganisationsHelper end def display_organisation_attributes(organisation) - [ + attributes = [ { name: "Organisation ID", value: "ORG#{organisation.id}", editable: false }, { name: "Address", value: organisation.address_string, editable: true }, { name: "Telephone number", value: organisation.phone, editable: true }, { name: "Registration number", value: organisation.housing_registration_no || "", editable: false }, { name: "Type of provider", value: organisation.display_provider_type, editable: false }, { name: "Owns housing stock", value: organisation.holds_own_stock ? "Yes" : "No", editable: false }, - { name: "Rent periods", value: organisation.rent_period_labels, editable: true, format: :bullet }, - { name: "Data Sharing Agreement" }, - { name: "Status", value: status_tag(organisation.status) + delete_organisation_text(organisation), editable: false }, + { name: "Part of group", value: organisation.group_member ? "Yes" : "No", editable: current_user.support? }, ] + + if organisation.group_member + attributes << { name: "Group number", value: organisation.group, editable: current_user.support? } + end + + attributes << { name: "For profit", value: organisation.profit_status, editable: current_user.support? } + attributes << { name: "Rent periods", value: organisation.rent_period_labels, editable: true, format: :bullet } + attributes << { name: "Data Sharing Agreement" } + attributes << { name: "Status", value: status_tag(organisation.status) + delete_organisation_text(organisation), editable: false } + + attributes end def organisation_name_row(user:, organisation:, summary_list:) @@ -64,6 +73,7 @@ module OrganisationsHelper def organisation_details_link_message(attribute) text = lowercase_first_letter(attribute[:name]) return "Add #{text}" if attribute[:name] == "Rent periods" + return "Select profit status" if attribute[:name] == "For profit" "Enter #{text}" end From 88c768b2ec36cccdff77b779d9f9ab76a6a04a6b Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Wed, 5 Mar 2025 15:07:26 +0000 Subject: [PATCH 06/36] Move grabbing values out of view --- app/helpers/organisations_helper.rb | 4 +++ app/views/organisations/new.html.erb | 38 +++++++++++----------------- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/app/helpers/organisations_helper.rb b/app/helpers/organisations_helper.rb index 8cf7f4ea6..a3a6232dc 100644 --- a/app/helpers/organisations_helper.rb +++ b/app/helpers/organisations_helper.rb @@ -77,4 +77,8 @@ module OrganisationsHelper "Enter #{text}" end + + def group_organisation_options + [OpenStruct.new(id: "", name: "")] + Organisation.visible.map { |org| OpenStruct.new(id: org.id, name: org.name) } + end end diff --git a/app/views/organisations/new.html.erb b/app/views/organisations/new.html.erb index 84aee1d80..b0385f330 100644 --- a/app/views/organisations/new.html.erb +++ b/app/views/organisations/new.html.erb @@ -56,38 +56,30 @@ :name, legend: { text: "Does the organisation hold its own stock?", size: "m" } %> - <% group_organisation_options = [OpenStruct.new(id: "", name: "")] + Organisation.visible.map { |org| OpenStruct.new(id: org.id, name: org.name) } %> - <%= f.govuk_radio_buttons_fieldset :group_member, legend: { text: "Is this organisation part of a housing provider group structure?", size: "m" } do %> <%= f.govuk_radio_button :group_member, true, - label: { text: "Yes" }, - "data-controller": "conditional-question", - "data-action": "click->conditional-question#displayConditional", - "data-info": { conditional_questions: { group: [true] }, type: "organisation" }.to_json do %> + label: { text: "Yes" }, + "data-controller": "conditional-question", + "data-action": "click->conditional-question#displayConditional", + "data-info": { conditional_questions: { group: [true] }, type: "organisation" }.to_json do %> <%= f.govuk_collection_select :group, - group_organisation_options, - :id, - :name, - label: { text: "Search for an organisation that is part of the same group as this organisation", size: "m" }, - options: { disabled: [""], selected: @organisation.group || "" }, - "data-controller": %w[accessible-autocomplete conditional-filter] %> + group_organisation_options, + :id, + :name, + label: { text: "Search for an organisation that is part of the same group as this organisation", size: "m" }, + options: { disabled: [""], selected: @organisation.group || "" }, + "data-controller": %w[accessible-autocomplete conditional-filter] %> <% end %> <%= f.govuk_radio_button :group_member, false, label: { text: "No" } %> <% end %> - <% profit_options = [ - OpenStruct.new(id: 1, name: "Non-profit"), - OpenStruct.new(id: 2, name: "Profit"), - OpenStruct.new(id: 3, name: "Local authority") - ] %> - <%= f.govuk_collection_select :profit_status, - profit_options, - :id, - :name, - label: { text: "Is the organisation for-profit?", size: "m" }, - options: { disabled: [""], selected: @organisation.profit_status || "" } %> + Organisation::PROFIT_STATUS.map { |key, value| OpenStruct.new(id: value, name: key.to_s.humanize) }, + :id, + :name, + label: { text: "Is the organisation for-profit?", size: "m" }, + options: { disabled: [""], selected: @organisation.profit_status || "" } %> <%= f.govuk_check_boxes_fieldset :rent_periods, legend: { text: "What are the rent periods for the organisation?" } do %> From ee05d27b57cc56d754230869cae81eb26406d9d1 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Wed, 5 Mar 2025 15:44:20 +0000 Subject: [PATCH 07/36] Update new org page with js --- app/frontend/controllers/index.js | 3 +++ .../controllers/organisations_controller.js | 17 +++++++++++++++++ app/helpers/organisations_helper.rb | 8 +++++++- app/views/organisations/new.html.erb | 4 +++- 4 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 app/frontend/controllers/organisations_controller.js diff --git a/app/frontend/controllers/index.js b/app/frontend/controllers/index.js index 944e32e2d..771532c3a 100644 --- a/app/frontend/controllers/index.js +++ b/app/frontend/controllers/index.js @@ -19,6 +19,8 @@ import FilterLayoutController from './filter_layout_controller.js' import TabsController from './tabs_controller.js' +import OrganisationsController from './organisations_controller.js' + application.register('accessible-autocomplete', AccessibleAutocompleteController) application.register('conditional-filter', ConditionalFilterController) application.register('conditional-question', ConditionalQuestionController) @@ -27,3 +29,4 @@ application.register('numeric-question', NumericQuestionController) application.register('filter-layout', FilterLayoutController) application.register('search', SearchController) application.register('tabs', TabsController) +application.register('organisations', OrganisationsController ) diff --git a/app/frontend/controllers/organisations_controller.js b/app/frontend/controllers/organisations_controller.js new file mode 100644 index 000000000..0a8934418 --- /dev/null +++ b/app/frontend/controllers/organisations_controller.js @@ -0,0 +1,17 @@ +import { Controller } from '@hotwired/stimulus' + +export default class extends Controller { + updateProfitStatusOptions(event) { + const providerType = event.target.value; + const profitStatusSelect = document.getElementById('organisation-profit-status-field'); + + if (profitStatusSelect) { + profitStatusSelect.disabled = false; + + if (providerType === "LA") { + profitStatusSelect.value = "3"; + profitStatusSelect.disabled = true; + } + } + } +} diff --git a/app/helpers/organisations_helper.rb b/app/helpers/organisations_helper.rb index a3a6232dc..90103cee7 100644 --- a/app/helpers/organisations_helper.rb +++ b/app/helpers/organisations_helper.rb @@ -79,6 +79,12 @@ module OrganisationsHelper end def group_organisation_options - [OpenStruct.new(id: "", name: "")] + Organisation.visible.map { |org| OpenStruct.new(id: org.id, name: org.name) } + null_option = [OpenStruct.new(id: "", name: "Select an option")] + organisations = Organisation.visible.map { |org| OpenStruct.new(id: org.id, name: org.name) } + null_option + organisations + end + + def profit_status_options + Organisation::PROFIT_STATUS.map { |key, value| OpenStruct.new(id: value, name: key.to_s.humanize) } end end diff --git a/app/views/organisations/new.html.erb b/app/views/organisations/new.html.erb index b0385f330..b338d23bd 100644 --- a/app/views/organisations/new.html.erb +++ b/app/views/organisations/new.html.erb @@ -48,6 +48,8 @@ :id, :name, label: { text: "Organisation type", size: "m" }, + "data-controller": "organisations", + "data-action": "change->organisations#updateProfitStatusOptions", options: { disabled: [""], selected: @organisation.provider_type || "" } %> <%= f.govuk_collection_radio_buttons :holds_own_stock, @@ -75,7 +77,7 @@ <% end %> <%= f.govuk_collection_select :profit_status, - Organisation::PROFIT_STATUS.map { |key, value| OpenStruct.new(id: value, name: key.to_s.humanize) }, + profit_status_options, :id, :name, label: { text: "Is the organisation for-profit?", size: "m" }, From 8c5a28a514e34c990e6302a6ad1e24848aba3cd9 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Wed, 5 Mar 2025 17:51:15 +0000 Subject: [PATCH 08/36] Selecting the profit status dependent on the provider type --- app/controllers/organisations_controller.rb | 2 +- .../controllers/organisations_controller.js | 16 +++++++++++--- app/helpers/organisations_helper.rb | 22 ++++++++++++++----- app/models/organisation.rb | 6 +++++ 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index ea0b154e5..8c073d1af 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -341,7 +341,7 @@ private end def org_params - params.require(:organisation).permit(:name, :address_line1, :address_line2, :postcode, :phone, :holds_own_stock, :provider_type, :housing_registration_no, :active) + params.require(:organisation).permit(:name, :address_line1, :address_line2, :postcode, :phone, :holds_own_stock, :provider_type, :housing_registration_no, :active, :profit_status, :group_member, :group) end def rent_period_params diff --git a/app/frontend/controllers/organisations_controller.js b/app/frontend/controllers/organisations_controller.js index 0a8934418..56042e42b 100644 --- a/app/frontend/controllers/organisations_controller.js +++ b/app/frontend/controllers/organisations_controller.js @@ -1,16 +1,26 @@ import { Controller } from '@hotwired/stimulus' +const profitStatusSelect = document.getElementById('organisation-profit-status-field'); +const localAuthorityOption = profitStatusSelect.querySelector('option[value="local_authority"]'); +const nonProfitOption = profitStatusSelect.querySelector('option[value="non_profit"]'); +const profitOption = profitStatusSelect.querySelector('option[value="profit"]'); export default class extends Controller { updateProfitStatusOptions(event) { const providerType = event.target.value; - const profitStatusSelect = document.getElementById('organisation-profit-status-field'); if (profitStatusSelect) { profitStatusSelect.disabled = false; + localAuthorityOption.hidden = false; + nonProfitOption.hidden = false; + profitOption.hidden = false; if (providerType === "LA") { - profitStatusSelect.value = "3"; - profitStatusSelect.disabled = true; + profitStatusSelect.value = "local_authority"; + nonProfitOption.hidden = true; + profitOption.hidden = true; + } else if (providerType === "PRP") { + profitStatusSelect.value = ""; + localAuthorityOption.hidden = true; } } } diff --git a/app/helpers/organisations_helper.rb b/app/helpers/organisations_helper.rb index 90103cee7..2a011af64 100644 --- a/app/helpers/organisations_helper.rb +++ b/app/helpers/organisations_helper.rb @@ -24,7 +24,7 @@ module OrganisationsHelper attributes << { name: "Group number", value: organisation.group, editable: current_user.support? } end - attributes << { name: "For profit", value: organisation.profit_status, editable: current_user.support? } + attributes << { name: "For profit", value: organisation.display_profit_status, editable: current_user.support? } attributes << { name: "Rent periods", value: organisation.rent_period_labels, editable: true, format: :bullet } attributes << { name: "Data Sharing Agreement" } attributes << { name: "Status", value: status_tag(organisation.status) + delete_organisation_text(organisation), editable: false } @@ -79,12 +79,24 @@ module OrganisationsHelper end def group_organisation_options - null_option = [OpenStruct.new(id: "", name: "Select an option")] - organisations = Organisation.visible.map { |org| OpenStruct.new(id: org.id, name: org.name) } + null_option = [OpenStruct.new(id: "", name: "Select an option", group: nil)] + organisations = Organisation.visible.map { |org| OpenStruct.new(id: org.id, name: org.name, group: org.group) } null_option + organisations end - def profit_status_options - Organisation::PROFIT_STATUS.map { |key, value| OpenStruct.new(id: value, name: key.to_s.humanize) } + def profit_status_options(provider_type = nil) + null_option = [OpenStruct.new(id: "", name: "Select an option")] + profit_statuses = Organisation::PROFIT_STATUS.map do |key, _value| + OpenStruct.new(id: key, name: Organisation::DISPLAY_PROFIT_STATUS[key]) + end + + case provider_type + when "LA" + profit_statuses.select! { |option| option.id == :local_authority } + when "PRP" + profit_statuses.reject! { |option| option.id == :local_authority } + end + + null_option + profit_statuses end end diff --git a/app/models/organisation.rb b/app/models/organisation.rb index a1ccc95f8..5d292cb13 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -144,6 +144,12 @@ class Organisation < ApplicationRecord DISPLAY_PROVIDER_TYPE[provider_type.to_sym] end + DISPLAY_PROFIT_STATUS = { "non_profit": "Non-profit", "profit": "Profit", "local_authority": "Local Authority" }.freeze + + def display_profit_status + DISPLAY_PROFIT_STATUS.fetch(profit_status&.to_sym, "") + end + def has_managing_agents? managing_agents.count.positive? end From fbc913088678cde53a3c045f5b3e60159f3e88f0 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Wed, 5 Mar 2025 17:51:29 +0000 Subject: [PATCH 09/36] Add to org edit page for support users --- app/views/organisations/edit.html.erb | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/app/views/organisations/edit.html.erb b/app/views/organisations/edit.html.erb index b0bd05bf6..34bec62e9 100644 --- a/app/views/organisations/edit.html.erb +++ b/app/views/organisations/edit.html.erb @@ -33,6 +33,33 @@ autocomplete: "tel", width: 20 %> + <% if current_user.support? %> + <%= f.govuk_radio_buttons_fieldset :group_member, + legend: { text: "Is this organisation part of a housing provider group structure?", size: "m" } do %> + <%= f.govuk_radio_button :group_member, true, + label: { text: "Yes" }, + "data-controller": "conditional-question", + "data-action": "click->conditional-question#displayConditional", + "data-info": { conditional_questions: { group: [true] }, type: "organisation" }.to_json do %> + <%= f.govuk_collection_select :group, + group_organisation_options, + :id, + :name, + label: { text: "Search for an organisation that is part of the same group as this organisation", size: "m" }, + options: { disabled: [""], selected: @organisation.group || "" }, + "data-controller": %w[accessible-autocomplete conditional-filter] %> + <% end %> + <%= f.govuk_radio_button :group_member, false, label: { text: "No" } %> + <% end %> + + <%= f.govuk_collection_select :profit_status, + profit_status_options(@organisation.provider_type), + :id, + :name, + label: { text: "Is the organisation for-profit?", size: "m" }, + options: { disabled: [""], selected: @organisation.profit_status || "" } %> + <% end %> + <%= f.govuk_check_boxes_fieldset :rent_periods, legend: { text: "What are the rent periods for the organisation?" }, hint: { text: "It is not possible to deselect rent periods that are used in logs" } do %> From b88801c9aed9b7d2d2b57fcb5b969cc5ed2b7206 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Thu, 6 Mar 2025 09:32:20 +0000 Subject: [PATCH 10/36] Add group logic --- app/controllers/organisations_controller.rb | 8 ++++- app/helpers/organisations_helper.rb | 33 +++++++++++++++++-- app/models/organisation.rb | 23 ++++++++++++- app/views/organisations/edit.html.erb | 4 +-- app/views/organisations/new.html.erb | 4 +-- config/locales/en.yml | 4 +++ ...add_new_question_fields_to_organisation.rb | 1 + db/schema.rb | 1 + 8 files changed, 70 insertions(+), 8 deletions(-) diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 8c073d1af..d2154a824 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -90,6 +90,7 @@ class OrganisationsController < ApplicationController def create selected_rent_periods = rent_period_params[:rent_periods].compact_blank @organisation = Organisation.new(org_params) + @organisation.group = org_params[:group_member] ? helpers.assign_group_number(@organisation.id, org_params[:group_member_id]) : nil if @organisation.save OrganisationRentPeriod.transaction do selected_rent_periods.each { |period| OrganisationRentPeriod.create!(organisation: @organisation, rent_period: period) } @@ -127,6 +128,7 @@ class OrganisationsController < ApplicationController def update if (current_user.data_coordinator? && org_params[:active].nil?) || current_user.support? + @organisation.group = org_params[:group_member] ? helpers.assign_group_number(@organisation.id, org_params[:group_member_id]) : nil if @organisation.update(org_params) case org_params[:active] when "false" @@ -341,7 +343,7 @@ private end def org_params - params.require(:organisation).permit(:name, :address_line1, :address_line2, :postcode, :phone, :holds_own_stock, :provider_type, :housing_registration_no, :active, :profit_status, :group_member, :group) + params.require(:organisation).permit(:name, :address_line1, :address_line2, :postcode, :phone, :holds_own_stock, :provider_type, :housing_registration_no, :active, :profit_status, :group_member, :group_member_id) end def rent_period_params @@ -385,4 +387,8 @@ private end end end + + def next_available_group_number + Organisation.maximum(:group).to_i + 1 + end end diff --git a/app/helpers/organisations_helper.rb b/app/helpers/organisations_helper.rb index 2a011af64..c1cc08420 100644 --- a/app/helpers/organisations_helper.rb +++ b/app/helpers/organisations_helper.rb @@ -21,7 +21,7 @@ module OrganisationsHelper ] if organisation.group_member - attributes << { name: "Group number", value: organisation.group, editable: current_user.support? } + attributes << { name: "Group number", value: "GROUP#{organisation.group}", editable: current_user.support? } end attributes << { name: "For profit", value: organisation.display_profit_status, editable: current_user.support? } @@ -80,10 +80,24 @@ module OrganisationsHelper def group_organisation_options null_option = [OpenStruct.new(id: "", name: "Select an option", group: nil)] - organisations = Organisation.visible.map { |org| OpenStruct.new(id: org.id, name: org.name, group: org.group) } + organisations = Organisation.visible.map { |org| OpenStruct.new(id: org.id, name: group_organisation_options_name(org)) } null_option + organisations end + def group_organisation_options_name(org) + "#{org.name}#{group_organisation_options_group_text(org)}" + end + + def group_organisation_options_group_text(org) + return "" unless org.oldest_group_member + + " (GROUP#{org.oldest_group_member&.group})" + end + + def group_organisation_options_group(org) + org.oldest_group_member&.group || next_available_group_number + end + def profit_status_options(provider_type = nil) null_option = [OpenStruct.new(id: "", name: "Select an option")] profit_statuses = Organisation::PROFIT_STATUS.map do |key, _value| @@ -99,4 +113,19 @@ module OrganisationsHelper null_option + profit_statuses end + + def assign_group_number(current_org_id, selected_org_id) + selected_org = Organisation.find_by(id: selected_org_id) + if selected_org&.group.present? + selected_org.group + else + next_group_number = next_available_group_number + selected_org.update!(group_member: true, group_member_id: current_org_id, group: next_group_number) if selected_org + next_group_number + end + end + + def next_available_group_number + Organisation.maximum(:group).to_i + 1 + end end diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 5d292cb13..9acea5138 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -69,6 +69,9 @@ class Organisation < ApplicationRecord validates :name, presence: { message: I18n.t("validations.organisation.name_missing") } validates :name, uniqueness: { case_sensitive: false, message: I18n.t("validations.organisation.name_not_unique") } validates :provider_type, presence: { message: I18n.t("validations.organisation.provider_type_missing") } + validates :group_member_id, presence: { message: I18n.t("validations.organisation.group_missing") }, if: :group_member? + + validate :validate_profit_status def self.find_by_id_on_multiple_fields(id) return if id.nil? @@ -144,7 +147,7 @@ class Organisation < ApplicationRecord DISPLAY_PROVIDER_TYPE[provider_type.to_sym] end - DISPLAY_PROFIT_STATUS = { "non_profit": "Non-profit", "profit": "Profit", "local_authority": "Local Authority" }.freeze + DISPLAY_PROFIT_STATUS = { "non_profit": "Non-profit", "profit": "Profit", "local_authority": "Local authority" }.freeze def display_profit_status DISPLAY_PROFIT_STATUS.fetch(profit_status&.to_sym, "") @@ -217,4 +220,22 @@ class Organisation < ApplicationRecord def has_visible_schemes? owned_schemes.visible.count.positive? end + + def oldest_group_member + return nil if group.blank? + + Organisation.visible.where(group:).order(:created_at).first + end + +private + + def validate_profit_status + if provider_type == "LA" && profit_status != "local_authority" + errors.add(:profit_status, I18n.t("validations.organisation.profit_status.must_be_LA")) + end + + if provider_type == "PRP" && profit_status == "local_authority" + errors.add(:profit_status, I18n.t("validations.organisation.profit_status.must_not_be_LA")) + end + end end diff --git a/app/views/organisations/edit.html.erb b/app/views/organisations/edit.html.erb index 34bec62e9..c6649a30a 100644 --- a/app/views/organisations/edit.html.erb +++ b/app/views/organisations/edit.html.erb @@ -41,12 +41,12 @@ "data-controller": "conditional-question", "data-action": "click->conditional-question#displayConditional", "data-info": { conditional_questions: { group: [true] }, type: "organisation" }.to_json do %> - <%= f.govuk_collection_select :group, + <%= f.govuk_collection_select :group_member_id, group_organisation_options, :id, :name, label: { text: "Search for an organisation that is part of the same group as this organisation", size: "m" }, - options: { disabled: [""], selected: @organisation.group || "" }, + options: { disabled: [""], selected: @organisation.oldest_group_member&.id || "" }, "data-controller": %w[accessible-autocomplete conditional-filter] %> <% end %> <%= f.govuk_radio_button :group_member, false, label: { text: "No" } %> diff --git a/app/views/organisations/new.html.erb b/app/views/organisations/new.html.erb index b338d23bd..d5aa33164 100644 --- a/app/views/organisations/new.html.erb +++ b/app/views/organisations/new.html.erb @@ -65,12 +65,12 @@ "data-controller": "conditional-question", "data-action": "click->conditional-question#displayConditional", "data-info": { conditional_questions: { group: [true] }, type: "organisation" }.to_json do %> - <%= f.govuk_collection_select :group, + <%= f.govuk_collection_select :group_member_id, group_organisation_options, :id, :name, label: { text: "Search for an organisation that is part of the same group as this organisation", size: "m" }, - options: { disabled: [""], selected: @organisation.group || "" }, + options: { disabled: [""], selected: @organisation.oldest_group_member&.id || "" }, "data-controller": %w[accessible-autocomplete conditional-filter] %> <% end %> <%= f.govuk_radio_button :group_member, false, label: { text: "No" } %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 6ca3ea322..bbee236be 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -218,6 +218,10 @@ en: name_missing: "Enter the name of the organisation." name_not_unique: "An organisation with this name already exists. Use the organisation that already exists or add a location or other identifier to the name. For example: Organisation name (City)." provider_type_missing: "Select the organisation type." + group_missing: "Select a group member." + profit_status: + must_be_LA: "This organisation is a local authority, it's profit status must also be local authority." + must_not_be_LA: "This organisation is a private registered provider, it's profit status cannot be local authority." stock_owner: blank: "You must choose a stock owner." already_added: "You have already added this stock owner." diff --git a/db/migrate/20250227085622_add_new_question_fields_to_organisation.rb b/db/migrate/20250227085622_add_new_question_fields_to_organisation.rb index 08b3ff2bf..dad16dc76 100644 --- a/db/migrate/20250227085622_add_new_question_fields_to_organisation.rb +++ b/db/migrate/20250227085622_add_new_question_fields_to_organisation.rb @@ -2,6 +2,7 @@ class AddNewQuestionFieldsToOrganisation < ActiveRecord::Migration[7.2] def change add_column :organisations, :profit_status, :integer add_column :organisations, :group_member, :boolean + add_column :organisations, :group_member_id, :integer add_column :organisations, :group, :integer end end diff --git a/db/schema.rb b/db/schema.rb index f00ffb5d4..1840a3263 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -550,6 +550,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_02_27_085622) do t.datetime "schemes_deduplicated_at" t.integer "profit_status" t.boolean "group_member" + t.integer "group_member_id" t.integer "group" t.index ["absorbing_organisation_id"], name: "index_organisations_on_absorbing_organisation_id" t.index ["name"], name: "index_organisations_on_name", unique: true From 6f9483bbaf63c2e1980f89190a24bb4055385547 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Thu, 6 Mar 2025 09:55:08 +0000 Subject: [PATCH 11/36] Fix bug on existing orgs when group remains unchanged --- app/controllers/organisations_controller.rb | 4 +++- app/models/organisation.rb | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index d2154a824..05130dc5a 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -128,7 +128,9 @@ class OrganisationsController < ApplicationController def update if (current_user.data_coordinator? && org_params[:active].nil?) || current_user.support? - @organisation.group = org_params[:group_member] ? helpers.assign_group_number(@organisation.id, org_params[:group_member_id]) : nil + if org_params[:group_member] && org_params[:group_member_id] + @organisation.group = helpers.assign_group_number(@organisation.id, org_params[:group_member_id]) + end if @organisation.update(org_params) case org_params[:active] when "false" diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 9acea5138..090557025 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -230,6 +230,8 @@ class Organisation < ApplicationRecord private def validate_profit_status + return if profit_status.nil? + if provider_type == "LA" && profit_status != "local_authority" errors.add(:profit_status, I18n.t("validations.organisation.profit_status.must_be_LA")) end From 625a9978f19d39b7e4a8197255c917ce1b877f54 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Thu, 6 Mar 2025 10:00:52 +0000 Subject: [PATCH 12/36] Clear group details if no longer part of a group --- app/models/organisation.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 090557025..0dac3d530 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -63,6 +63,7 @@ class Organisation < ApplicationRecord enum :profit_status, PROFIT_STATUS attribute :group_member, :boolean + before_save :clear_group_member_fields_if_not_group_member alias_method :la?, :LA? @@ -240,4 +241,11 @@ private errors.add(:profit_status, I18n.t("validations.organisation.profit_status.must_not_be_LA")) end end + + def clear_group_member_fields_if_not_group_member + unless group_member + self.group_member_id = nil + self.group = nil + end + end end From add83cf83e69e3c56601ef9bbcb91e33a5562da1 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Thu, 6 Mar 2025 11:38:04 +0000 Subject: [PATCH 13/36] Lint --- ...27085622_add_new_question_fields_to_organisation.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/db/migrate/20250227085622_add_new_question_fields_to_organisation.rb b/db/migrate/20250227085622_add_new_question_fields_to_organisation.rb index dad16dc76..538a23ef7 100644 --- a/db/migrate/20250227085622_add_new_question_fields_to_organisation.rb +++ b/db/migrate/20250227085622_add_new_question_fields_to_organisation.rb @@ -1,8 +1,10 @@ class AddNewQuestionFieldsToOrganisation < ActiveRecord::Migration[7.2] def change - add_column :organisations, :profit_status, :integer - add_column :organisations, :group_member, :boolean - add_column :organisations, :group_member_id, :integer - add_column :organisations, :group, :integer + change_table :organisations, bulk: true do |t| + t.integer :profit_status + t.boolean :group_member + t.integer :group_member_id + t.integer :group + end end end From fa6239c7854e1ea4814d7bfba929d2a017e0bf6c Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Thu, 6 Mar 2025 11:57:07 +0000 Subject: [PATCH 14/36] JS Lint --- app/frontend/controllers/index.js | 2 +- .../controllers/organisations_controller.js | 34 +++++++++---------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/app/frontend/controllers/index.js b/app/frontend/controllers/index.js index 771532c3a..35385e46e 100644 --- a/app/frontend/controllers/index.js +++ b/app/frontend/controllers/index.js @@ -29,4 +29,4 @@ application.register('numeric-question', NumericQuestionController) application.register('filter-layout', FilterLayoutController) application.register('search', SearchController) application.register('tabs', TabsController) -application.register('organisations', OrganisationsController ) +application.register('organisations', OrganisationsController) diff --git a/app/frontend/controllers/organisations_controller.js b/app/frontend/controllers/organisations_controller.js index 56042e42b..e3468d7ce 100644 --- a/app/frontend/controllers/organisations_controller.js +++ b/app/frontend/controllers/organisations_controller.js @@ -1,26 +1,26 @@ import { Controller } from '@hotwired/stimulus' -const profitStatusSelect = document.getElementById('organisation-profit-status-field'); -const localAuthorityOption = profitStatusSelect.querySelector('option[value="local_authority"]'); -const nonProfitOption = profitStatusSelect.querySelector('option[value="non_profit"]'); -const profitOption = profitStatusSelect.querySelector('option[value="profit"]'); +const profitStatusSelect = document.getElementById('organisation-profit-status-field') +const localAuthorityOption = profitStatusSelect.querySelector('option[value="local_authority"]') +const nonProfitOption = profitStatusSelect.querySelector('option[value="non_profit"]') +const profitOption = profitStatusSelect.querySelector('option[value="profit"]') export default class extends Controller { - updateProfitStatusOptions(event) { - const providerType = event.target.value; + updateProfitStatusOptions (event) { + const providerType = event.target.value if (profitStatusSelect) { - profitStatusSelect.disabled = false; - localAuthorityOption.hidden = false; - nonProfitOption.hidden = false; - profitOption.hidden = false; + profitStatusSelect.disabled = false + localAuthorityOption.hidden = false + nonProfitOption.hidden = false + profitOption.hidden = false - if (providerType === "LA") { - profitStatusSelect.value = "local_authority"; - nonProfitOption.hidden = true; - profitOption.hidden = true; - } else if (providerType === "PRP") { - profitStatusSelect.value = ""; - localAuthorityOption.hidden = true; + if (providerType === 'LA') { + profitStatusSelect.value = 'local_authority' + nonProfitOption.hidden = true + profitOption.hidden = true + } else if (providerType === 'PRP') { + profitStatusSelect.value = '' + localAuthorityOption.hidden = true } } } From 59971ae0a8e487d93538ea07e3f1ec41c07266c1 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Mon, 10 Mar 2025 08:45:13 +0000 Subject: [PATCH 15/36] Restore schema --- db/schema.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 7120b6ea0..f81fcd7f7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -84,7 +84,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_02_27_085622) do t.datetime "last_accessed" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.check_constraint "log_type::text = ANY (ARRAY['lettings'::character varying::text, 'sales'::character varying::text])", name: "log_type_check" + t.check_constraint "log_type::text = ANY (ARRAY['lettings'::character varying, 'sales'::character varying]::text[])", name: "log_type_check" t.check_constraint "year >= 2000 AND year <= 2099", name: "year_check" end @@ -245,14 +245,14 @@ ActiveRecord::Schema[7.2].define(version: 2025_02_27_085622) do t.integer "hb" t.integer "hbrentshortfall" t.integer "property_relet" - t.datetime "mrcdate" + t.datetime "mrcdate", precision: nil t.integer "incref" - t.datetime "startdate" + t.datetime "startdate", precision: nil t.integer "armedforces" t.integer "first_time_property_let_as_social_housing" t.integer "unitletas" t.integer "builtype" - t.datetime "voiddate" + t.datetime "voiddate", precision: nil t.bigint "owning_organisation_id" t.bigint "managing_organisation_id" t.integer "renttype" @@ -768,13 +768,13 @@ ActiveRecord::Schema[7.2].define(version: 2025_02_27_085622) do t.integer "partner_under_16_value_check" t.integer "multiple_partners_value_check" t.bigint "created_by_id" + t.integer "has_management_fee" + t.decimal "management_fee", precision: 10, scale: 2 t.integer "firststair" t.integer "numstair" t.decimal "mrentprestaircasing", precision: 10, scale: 2 t.datetime "lasttransaction" t.datetime "initialpurchase" - t.integer "has_management_fee" - t.decimal "management_fee", precision: 10, scale: 2 t.boolean "manual_address_entry_selected", default: false t.index ["assigned_to_id"], name: "index_sales_logs_on_assigned_to_id" t.index ["bulk_upload_id"], name: "index_sales_logs_on_bulk_upload_id" @@ -829,8 +829,8 @@ ActiveRecord::Schema[7.2].define(version: 2025_02_27_085622) do t.string "name" t.bigint "organisation_id" t.integer "sign_in_count", default: 0, null: false - t.datetime "current_sign_in_at" - t.datetime "last_sign_in_at" + t.datetime "current_sign_in_at", precision: nil + t.datetime "last_sign_in_at", precision: nil t.string "current_sign_in_ip" t.string "last_sign_in_ip" t.integer "role" From 812787fd09e45433508dfeda276ff57b1410f74d Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Mon, 10 Mar 2025 14:30:39 +0000 Subject: [PATCH 16/36] Remove manual_address_selected --- ...219122817_add_manual_address_entry_selected_to_logs.rb | 6 ------ ...2913_remove_manual_address_entry_selected_from_logs.rb | 8 ++++++++ 2 files changed, 8 insertions(+), 6 deletions(-) delete mode 100644 db/migrate/20250219122817_add_manual_address_entry_selected_to_logs.rb create mode 100644 db/migrate/20250310142913_remove_manual_address_entry_selected_from_logs.rb diff --git a/db/migrate/20250219122817_add_manual_address_entry_selected_to_logs.rb b/db/migrate/20250219122817_add_manual_address_entry_selected_to_logs.rb deleted file mode 100644 index a0d0f7529..000000000 --- a/db/migrate/20250219122817_add_manual_address_entry_selected_to_logs.rb +++ /dev/null @@ -1,6 +0,0 @@ -class AddManualAddressEntrySelectedToLogs < ActiveRecord::Migration[7.2] - def change - add_column :sales_logs, :manual_address_entry_selected, :boolean, default: false - add_column :lettings_logs, :manual_address_entry_selected, :boolean, default: false - end -end diff --git a/db/migrate/20250310142913_remove_manual_address_entry_selected_from_logs.rb b/db/migrate/20250310142913_remove_manual_address_entry_selected_from_logs.rb new file mode 100644 index 000000000..c5bbd32ea --- /dev/null +++ b/db/migrate/20250310142913_remove_manual_address_entry_selected_from_logs.rb @@ -0,0 +1,8 @@ +class RemoveManualAddressEntrySelectedFromLogs < ActiveRecord::Migration[7.2] + class RemoveManualAddressEntrySelectedFromLogs < ActiveRecord::Migration[7.2] + def change + remove_column :sales_logs, :manual_address_entry_selected, :boolean if column_exists?(:sales_logs, :manual_address_entry_selected) + remove_column :lettings_logs, :manual_address_entry_selected, :boolean if column_exists?(:lettings_logs, :manual_address_entry_selected) + end + end +end From 8ebd56036fd87a89844e8657e39d4d23742fd6d8 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Mon, 10 Mar 2025 14:34:57 +0000 Subject: [PATCH 17/36] Revert "Remove manual_address_selected" This reverts commit 812787fd09e45433508dfeda276ff57b1410f74d. --- ...219122817_add_manual_address_entry_selected_to_logs.rb | 6 ++++++ ...2913_remove_manual_address_entry_selected_from_logs.rb | 8 -------- 2 files changed, 6 insertions(+), 8 deletions(-) create mode 100644 db/migrate/20250219122817_add_manual_address_entry_selected_to_logs.rb delete mode 100644 db/migrate/20250310142913_remove_manual_address_entry_selected_from_logs.rb diff --git a/db/migrate/20250219122817_add_manual_address_entry_selected_to_logs.rb b/db/migrate/20250219122817_add_manual_address_entry_selected_to_logs.rb new file mode 100644 index 000000000..a0d0f7529 --- /dev/null +++ b/db/migrate/20250219122817_add_manual_address_entry_selected_to_logs.rb @@ -0,0 +1,6 @@ +class AddManualAddressEntrySelectedToLogs < ActiveRecord::Migration[7.2] + def change + add_column :sales_logs, :manual_address_entry_selected, :boolean, default: false + add_column :lettings_logs, :manual_address_entry_selected, :boolean, default: false + end +end diff --git a/db/migrate/20250310142913_remove_manual_address_entry_selected_from_logs.rb b/db/migrate/20250310142913_remove_manual_address_entry_selected_from_logs.rb deleted file mode 100644 index c5bbd32ea..000000000 --- a/db/migrate/20250310142913_remove_manual_address_entry_selected_from_logs.rb +++ /dev/null @@ -1,8 +0,0 @@ -class RemoveManualAddressEntrySelectedFromLogs < ActiveRecord::Migration[7.2] - class RemoveManualAddressEntrySelectedFromLogs < ActiveRecord::Migration[7.2] - def change - remove_column :sales_logs, :manual_address_entry_selected, :boolean if column_exists?(:sales_logs, :manual_address_entry_selected) - remove_column :lettings_logs, :manual_address_entry_selected, :boolean if column_exists?(:lettings_logs, :manual_address_entry_selected) - end - end -end From a802c3dc65e62e1314f20184a0fa3a2f981fe81c Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Tue, 11 Mar 2025 08:54:39 +0000 Subject: [PATCH 18/36] Revert "Revert "Remove manual_address_selected"" This reverts commit 8ebd56036fd87a89844e8657e39d4d23742fd6d8. --- ...219122817_add_manual_address_entry_selected_to_logs.rb | 6 ------ ...2913_remove_manual_address_entry_selected_from_logs.rb | 8 ++++++++ 2 files changed, 8 insertions(+), 6 deletions(-) delete mode 100644 db/migrate/20250219122817_add_manual_address_entry_selected_to_logs.rb create mode 100644 db/migrate/20250310142913_remove_manual_address_entry_selected_from_logs.rb diff --git a/db/migrate/20250219122817_add_manual_address_entry_selected_to_logs.rb b/db/migrate/20250219122817_add_manual_address_entry_selected_to_logs.rb deleted file mode 100644 index a0d0f7529..000000000 --- a/db/migrate/20250219122817_add_manual_address_entry_selected_to_logs.rb +++ /dev/null @@ -1,6 +0,0 @@ -class AddManualAddressEntrySelectedToLogs < ActiveRecord::Migration[7.2] - def change - add_column :sales_logs, :manual_address_entry_selected, :boolean, default: false - add_column :lettings_logs, :manual_address_entry_selected, :boolean, default: false - end -end diff --git a/db/migrate/20250310142913_remove_manual_address_entry_selected_from_logs.rb b/db/migrate/20250310142913_remove_manual_address_entry_selected_from_logs.rb new file mode 100644 index 000000000..c5bbd32ea --- /dev/null +++ b/db/migrate/20250310142913_remove_manual_address_entry_selected_from_logs.rb @@ -0,0 +1,8 @@ +class RemoveManualAddressEntrySelectedFromLogs < ActiveRecord::Migration[7.2] + class RemoveManualAddressEntrySelectedFromLogs < ActiveRecord::Migration[7.2] + def change + remove_column :sales_logs, :manual_address_entry_selected, :boolean if column_exists?(:sales_logs, :manual_address_entry_selected) + remove_column :lettings_logs, :manual_address_entry_selected, :boolean if column_exists?(:lettings_logs, :manual_address_entry_selected) + end + end +end From faa222835f2ca4201be6c2a948520d7b31a2a80e Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Tue, 11 Mar 2025 09:41:39 +0000 Subject: [PATCH 19/36] Update migration --- ...remove_manual_address_entry_selected_from_logs.rb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/db/migrate/20250310142913_remove_manual_address_entry_selected_from_logs.rb b/db/migrate/20250310142913_remove_manual_address_entry_selected_from_logs.rb index c5bbd32ea..bdf2d63e4 100644 --- a/db/migrate/20250310142913_remove_manual_address_entry_selected_from_logs.rb +++ b/db/migrate/20250310142913_remove_manual_address_entry_selected_from_logs.rb @@ -1,8 +1,10 @@ class RemoveManualAddressEntrySelectedFromLogs < ActiveRecord::Migration[7.2] - class RemoveManualAddressEntrySelectedFromLogs < ActiveRecord::Migration[7.2] - def change - remove_column :sales_logs, :manual_address_entry_selected, :boolean if column_exists?(:sales_logs, :manual_address_entry_selected) - remove_column :lettings_logs, :manual_address_entry_selected, :boolean if column_exists?(:lettings_logs, :manual_address_entry_selected) - end + def up + remove_column :sales_logs, :manual_address_entry_selected, :boolean if column_exists?(:sales_logs, :manual_address_entry_selected) + remove_column :lettings_logs, :manual_address_entry_selected, :boolean if column_exists?(:lettings_logs, :manual_address_entry_selected) + end + + def down + # Do nothing end end From 85df3f01a6306c2c7b1f30e2b91baceeb399c1d2 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Tue, 11 Mar 2025 09:51:40 +0000 Subject: [PATCH 20/36] Remove file --- ...3_remove_manual_address_entry_selected_from_logs.rb | 10 ---------- 1 file changed, 10 deletions(-) delete mode 100644 db/migrate/20250310142913_remove_manual_address_entry_selected_from_logs.rb diff --git a/db/migrate/20250310142913_remove_manual_address_entry_selected_from_logs.rb b/db/migrate/20250310142913_remove_manual_address_entry_selected_from_logs.rb deleted file mode 100644 index bdf2d63e4..000000000 --- a/db/migrate/20250310142913_remove_manual_address_entry_selected_from_logs.rb +++ /dev/null @@ -1,10 +0,0 @@ -class RemoveManualAddressEntrySelectedFromLogs < ActiveRecord::Migration[7.2] - def up - remove_column :sales_logs, :manual_address_entry_selected, :boolean if column_exists?(:sales_logs, :manual_address_entry_selected) - remove_column :lettings_logs, :manual_address_entry_selected, :boolean if column_exists?(:lettings_logs, :manual_address_entry_selected) - end - - def down - # Do nothing - end -end From f400207d09dbae6c2ea22837a93cbfd2892127bf Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Mon, 24 Mar 2025 17:15:49 +0000 Subject: [PATCH 21/36] Move update after save --- app/controllers/organisations_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 05130dc5a..5fec5dc55 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -90,8 +90,8 @@ class OrganisationsController < ApplicationController def create selected_rent_periods = rent_period_params[:rent_periods].compact_blank @organisation = Organisation.new(org_params) - @organisation.group = org_params[:group_member] ? helpers.assign_group_number(@organisation.id, org_params[:group_member_id]) : nil if @organisation.save + @organisation.update(group: helpers.assign_group_number(@organisation.id, org_params[:group_member_id])) if org_params[:group_member] OrganisationRentPeriod.transaction do selected_rent_periods.each { |period| OrganisationRentPeriod.create!(organisation: @organisation, rent_period: period) } end From 2762d20ca991ff1830a26425f4813946b80e24d0 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Mon, 24 Mar 2025 17:21:40 +0000 Subject: [PATCH 22/36] Move methods to controller --- app/controllers/organisations_controller.rb | 15 +++++++++++++-- app/helpers/organisations_helper.rb | 19 ------------------- 2 files changed, 13 insertions(+), 21 deletions(-) diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 5fec5dc55..d2546dd28 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -91,7 +91,7 @@ class OrganisationsController < ApplicationController selected_rent_periods = rent_period_params[:rent_periods].compact_blank @organisation = Organisation.new(org_params) if @organisation.save - @organisation.update(group: helpers.assign_group_number(@organisation.id, org_params[:group_member_id])) if org_params[:group_member] + @organisation.update(group: assign_group_number(@organisation.id, org_params[:group_member_id])) if org_params[:group_member] OrganisationRentPeriod.transaction do selected_rent_periods.each { |period| OrganisationRentPeriod.create!(organisation: @organisation, rent_period: period) } end @@ -129,7 +129,7 @@ class OrganisationsController < ApplicationController def update if (current_user.data_coordinator? && org_params[:active].nil?) || current_user.support? if org_params[:group_member] && org_params[:group_member_id] - @organisation.group = helpers.assign_group_number(@organisation.id, org_params[:group_member_id]) + @organisation.group = assign_group_number(@organisation.id, org_params[:group_member_id]) end if @organisation.update(org_params) case org_params[:active] @@ -390,6 +390,17 @@ private end end + def assign_group_number(current_org_id, selected_org_id) + selected_org = Organisation.find_by(id: selected_org_id) + if selected_org&.group.present? + selected_org.group + else + next_group_number = next_available_group_number + selected_org.update!(group_member: true, group_member_id: current_org_id, group: next_group_number) if selected_org + next_group_number + end + end + def next_available_group_number Organisation.maximum(:group).to_i + 1 end diff --git a/app/helpers/organisations_helper.rb b/app/helpers/organisations_helper.rb index c1cc08420..cbd4b5646 100644 --- a/app/helpers/organisations_helper.rb +++ b/app/helpers/organisations_helper.rb @@ -94,10 +94,6 @@ module OrganisationsHelper " (GROUP#{org.oldest_group_member&.group})" end - def group_organisation_options_group(org) - org.oldest_group_member&.group || next_available_group_number - end - def profit_status_options(provider_type = nil) null_option = [OpenStruct.new(id: "", name: "Select an option")] profit_statuses = Organisation::PROFIT_STATUS.map do |key, _value| @@ -113,19 +109,4 @@ module OrganisationsHelper null_option + profit_statuses end - - def assign_group_number(current_org_id, selected_org_id) - selected_org = Organisation.find_by(id: selected_org_id) - if selected_org&.group.present? - selected_org.group - else - next_group_number = next_available_group_number - selected_org.update!(group_member: true, group_member_id: current_org_id, group: next_group_number) if selected_org - next_group_number - end - end - - def next_available_group_number - Organisation.maximum(:group).to_i + 1 - end end From 82989e14ef74f805a3434c5871c72ab139571405 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Mon, 24 Mar 2025 17:29:02 +0000 Subject: [PATCH 23/36] Update xml export --- app/services/exports/organisation_export_service.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/services/exports/organisation_export_service.rb b/app/services/exports/organisation_export_service.rb index 8ceba93a9..da4c75af2 100644 --- a/app/services/exports/organisation_export_service.rb +++ b/app/services/exports/organisation_export_service.rb @@ -63,8 +63,7 @@ module Exports attribute_hash["provider_type"] = organisation.provider_type_before_type_cast attribute_hash["merge_date"] = organisation.merge_date&.iso8601 attribute_hash["available_from"] = organisation.available_from&.iso8601 - attribute_hash["profit_status"] = nil # will need update when we add the field to the org - attribute_hash["group"] = nil # will need update when we add the field to the org + attribute_hash["profit_status"] = organisation.profit_status_before_type_cast attribute_hash["status"] = organisation.status attribute_hash["active"] = attribute_hash["status"] == :active From 6b0cb329ab0e6790ae8fca3add7c61b1bff2fb57 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Mon, 24 Mar 2025 17:32:47 +0000 Subject: [PATCH 24/36] Lint --- app/controllers/organisations_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index d2546dd28..38c21d851 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -91,7 +91,7 @@ class OrganisationsController < ApplicationController selected_rent_periods = rent_period_params[:rent_periods].compact_blank @organisation = Organisation.new(org_params) if @organisation.save - @organisation.update(group: assign_group_number(@organisation.id, org_params[:group_member_id])) if org_params[:group_member] + @organisation.update!(group: assign_group_number(@organisation.id, org_params[:group_member_id])) if org_params[:group_member] OrganisationRentPeriod.transaction do selected_rent_periods.each { |period| OrganisationRentPeriod.create!(organisation: @organisation, rent_period: period) } end From 8e35c007b06bb381801f35ee96cddaab009491f0 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Mon, 24 Mar 2025 18:58:58 +0000 Subject: [PATCH 25/36] Update test xml file --- spec/fixtures/exports/organisation.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/fixtures/exports/organisation.xml b/spec/fixtures/exports/organisation.xml index f70ac2b7d..a9a511b9b 100644 --- a/spec/fixtures/exports/organisation.xml +++ b/spec/fixtures/exports/organisation.xml @@ -16,12 +16,12 @@ + + true {dsa_signed_at} {dpo_email} - - active From fa84e1336aa9363aa7ae2bdb2675a79ec0985ec4 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Tue, 25 Mar 2025 08:55:06 +0000 Subject: [PATCH 26/36] Update current user logic --- app/helpers/organisations_helper.rb | 8 +++--- app/views/organisations/show.html.erb | 2 +- spec/helpers/organisations_helper_spec.rb | 30 ++++++++++++++++++++--- 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/app/helpers/organisations_helper.rb b/app/helpers/organisations_helper.rb index cbd4b5646..544f3f243 100644 --- a/app/helpers/organisations_helper.rb +++ b/app/helpers/organisations_helper.rb @@ -9,7 +9,7 @@ module OrganisationsHelper end end - def display_organisation_attributes(organisation) + def display_organisation_attributes(user, organisation) attributes = [ { name: "Organisation ID", value: "ORG#{organisation.id}", editable: false }, { name: "Address", value: organisation.address_string, editable: true }, @@ -17,14 +17,14 @@ module OrganisationsHelper { name: "Registration number", value: organisation.housing_registration_no || "", editable: false }, { name: "Type of provider", value: organisation.display_provider_type, editable: false }, { name: "Owns housing stock", value: organisation.holds_own_stock ? "Yes" : "No", editable: false }, - { name: "Part of group", value: organisation.group_member ? "Yes" : "No", editable: current_user.support? }, + { name: "Part of group", value: organisation.group_member ? "Yes" : "No", editable: user.support? }, ] if organisation.group_member - attributes << { name: "Group number", value: "GROUP#{organisation.group}", editable: current_user.support? } + attributes << { name: "Group number", value: "GROUP#{organisation.group}", editable: user.support? } end - attributes << { name: "For profit", value: organisation.display_profit_status, editable: current_user.support? } + attributes << { name: "For profit", value: organisation.display_profit_status, editable: user.support? } attributes << { name: "Rent periods", value: organisation.rent_period_labels, editable: true, format: :bullet } attributes << { name: "Data Sharing Agreement" } attributes << { name: "Status", value: status_tag(organisation.status) + delete_organisation_text(organisation), editable: false } diff --git a/app/views/organisations/show.html.erb b/app/views/organisations/show.html.erb index 42c2482f7..e275769ee 100644 --- a/app/views/organisations/show.html.erb +++ b/app/views/organisations/show.html.erb @@ -15,7 +15,7 @@
<%= govuk_summary_list do |summary_list| %> <%= organisation_name_row(user: current_user, organisation: @organisation, summary_list:) %> - <% display_organisation_attributes(@organisation).each do |attr| %> + <% display_organisation_attributes(current_user, @organisation).each do |attr| %> <% if attr[:name] == "Data Sharing Agreement" %> <%= data_sharing_agreement_row(organisation: @organisation, user: current_user, summary_list:) %> <% else %> diff --git a/spec/helpers/organisations_helper_spec.rb b/spec/helpers/organisations_helper_spec.rb index ea0d9b158..2741d7f7a 100644 --- a/spec/helpers/organisations_helper_spec.rb +++ b/spec/helpers/organisations_helper_spec.rb @@ -3,10 +3,12 @@ require "rails_helper" RSpec.describe OrganisationsHelper do include TagHelper describe "display_organisation_attributes" do - let(:organisation) { create(:organisation, :la, :holds_own_stock, address_line1: "2 Marsham Street", address_line2: "London", postcode: "SW1P 4DF", housing_registration_no: 1234, organisation_rent_periods: []) } + let(:support_user) { create(:user, :support) } + let(:coordinator_user) { create(:user, :data_coordinator) } + let(:organisation) { create(:organisation, :la, :holds_own_stock, address_line1: "2 Marsham Street", address_line2: "London", postcode: "SW1P 4DF", housing_registration_no: 1234, organisation_rent_periods: [], group_member: true, group_member_id: 99, group: 1) } - it "has the correct values" do - expect(display_organisation_attributes(organisation)).to eq( + it "has the correct values and editable status for support users" do + expect(display_organisation_attributes(support_user, organisation)).to eq( [{ editable: false, name: "Organisation ID", value: "ORG#{organisation.id}" }, { editable: true, name: "Address", @@ -15,11 +17,33 @@ RSpec.describe OrganisationsHelper do { editable: false, name: "Registration number", value: "1234" }, { editable: false, name: "Type of provider", value: "Local authority" }, { editable: false, name: "Owns housing stock", value: "Yes" }, + { editable: true, name: "Part of group", value: "Yes"}, + { editable: true, name: "Group number", value: "GROUP1"}, + { editable: true, name: "For profit", value: ""}, { editable: true, format: :bullet, name: "Rent periods", value: [] }, { name: "Data Sharing Agreement" }, { editable: false, name: "Status", value: status_tag(organisation.status) }], ) end + + it "has the correct values and editable status for non-support users" do + expect(display_organisation_attributes(coordinator_user, organisation)).to eq( + [{ editable: false, name: "Organisation ID", value: "ORG#{organisation.id}" }, + { editable: true, + name: "Address", + value: "2 Marsham Street\nLondon\nSW1P 4DF" }, + { editable: true, name: "Telephone number", value: nil }, + { editable: false, name: "Registration number", value: "1234" }, + { editable: false, name: "Type of provider", value: "Local authority" }, + { editable: false, name: "Owns housing stock", value: "Yes" }, + { editable: false, name: "Part of group", value: "Yes"}, + { editable: false, name: "Group number", value: "GROUP1"}, + { editable: false, name: "For profit", value: ""}, + { editable: true, format: :bullet, name: "Rent periods", value: [] }, + { name: "Data Sharing Agreement" }, + { editable: false, name: "Status", value: status_tag(organisation.status) }], + ) + end end describe "rent_periods_with_checked_attr" do From b913438a64fbecb381b21ab2b8d65e6578f53a3c Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Tue, 25 Mar 2025 09:00:49 +0000 Subject: [PATCH 27/36] Lint --- spec/helpers/organisations_helper_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/helpers/organisations_helper_spec.rb b/spec/helpers/organisations_helper_spec.rb index 2741d7f7a..3893b03f2 100644 --- a/spec/helpers/organisations_helper_spec.rb +++ b/spec/helpers/organisations_helper_spec.rb @@ -17,9 +17,9 @@ RSpec.describe OrganisationsHelper do { editable: false, name: "Registration number", value: "1234" }, { editable: false, name: "Type of provider", value: "Local authority" }, { editable: false, name: "Owns housing stock", value: "Yes" }, - { editable: true, name: "Part of group", value: "Yes"}, - { editable: true, name: "Group number", value: "GROUP1"}, - { editable: true, name: "For profit", value: ""}, + { editable: true, name: "Part of group", value: "Yes" }, + { editable: true, name: "Group number", value: "GROUP1" }, + { editable: true, name: "For profit", value: "" }, { editable: true, format: :bullet, name: "Rent periods", value: [] }, { name: "Data Sharing Agreement" }, { editable: false, name: "Status", value: status_tag(organisation.status) }], @@ -36,9 +36,9 @@ RSpec.describe OrganisationsHelper do { editable: false, name: "Registration number", value: "1234" }, { editable: false, name: "Type of provider", value: "Local authority" }, { editable: false, name: "Owns housing stock", value: "Yes" }, - { editable: false, name: "Part of group", value: "Yes"}, - { editable: false, name: "Group number", value: "GROUP1"}, - { editable: false, name: "For profit", value: ""}, + { editable: false, name: "Part of group", value: "Yes" }, + { editable: false, name: "Group number", value: "GROUP1" }, + { editable: false, name: "For profit", value: "" }, { editable: true, format: :bullet, name: "Rent periods", value: [] }, { name: "Data Sharing Agreement" }, { editable: false, name: "Status", value: status_tag(organisation.status) }], From 34a9bea4d8250ca1467cc1808b903fd3da788818 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Tue, 25 Mar 2025 10:25:32 +0000 Subject: [PATCH 28/36] Lint --- spec/helpers/organisations_helper_spec.rb | 30 +++++++++++------------ 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/spec/helpers/organisations_helper_spec.rb b/spec/helpers/organisations_helper_spec.rb index 3893b03f2..ebdf73c67 100644 --- a/spec/helpers/organisations_helper_spec.rb +++ b/spec/helpers/organisations_helper_spec.rb @@ -28,21 +28,21 @@ RSpec.describe OrganisationsHelper do it "has the correct values and editable status for non-support users" do expect(display_organisation_attributes(coordinator_user, organisation)).to eq( - [{ editable: false, name: "Organisation ID", value: "ORG#{organisation.id}" }, - { editable: true, - name: "Address", - value: "2 Marsham Street\nLondon\nSW1P 4DF" }, - { editable: true, name: "Telephone number", value: nil }, - { editable: false, name: "Registration number", value: "1234" }, - { editable: false, name: "Type of provider", value: "Local authority" }, - { editable: false, name: "Owns housing stock", value: "Yes" }, - { editable: false, name: "Part of group", value: "Yes" }, - { editable: false, name: "Group number", value: "GROUP1" }, - { editable: false, name: "For profit", value: "" }, - { editable: true, format: :bullet, name: "Rent periods", value: [] }, - { name: "Data Sharing Agreement" }, - { editable: false, name: "Status", value: status_tag(organisation.status) }], - ) + [{ editable: false, name: "Organisation ID", value: "ORG#{organisation.id}" }, + { editable: true, + name: "Address", + value: "2 Marsham Street\nLondon\nSW1P 4DF" }, + { editable: true, name: "Telephone number", value: nil }, + { editable: false, name: "Registration number", value: "1234" }, + { editable: false, name: "Type of provider", value: "Local authority" }, + { editable: false, name: "Owns housing stock", value: "Yes" }, + { editable: false, name: "Part of group", value: "Yes" }, + { editable: false, name: "Group number", value: "GROUP1" }, + { editable: false, name: "For profit", value: "" }, + { editable: true, format: :bullet, name: "Rent periods", value: [] }, + { name: "Data Sharing Agreement" }, + { editable: false, name: "Status", value: status_tag(organisation.status) }], + ) end end From 566f7b5910036b2ce888740bab5c41e7e9dff3ce Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Tue, 25 Mar 2025 12:39:57 +0000 Subject: [PATCH 29/36] Reorder org js controller --- .../controllers/organisations_controller.js | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/app/frontend/controllers/organisations_controller.js b/app/frontend/controllers/organisations_controller.js index e3468d7ce..47df883a4 100644 --- a/app/frontend/controllers/organisations_controller.js +++ b/app/frontend/controllers/organisations_controller.js @@ -1,27 +1,27 @@ import { Controller } from '@hotwired/stimulus' -const profitStatusSelect = document.getElementById('organisation-profit-status-field') -const localAuthorityOption = profitStatusSelect.querySelector('option[value="local_authority"]') -const nonProfitOption = profitStatusSelect.querySelector('option[value="non_profit"]') -const profitOption = profitStatusSelect.querySelector('option[value="profit"]') export default class extends Controller { - updateProfitStatusOptions (event) { - const providerType = event.target.value + updateProfitStatusOptions (event) { + const profitStatusSelect = document.getElementById('organisation-profit-status-field') + if (!profitStatusSelect) return - if (profitStatusSelect) { - profitStatusSelect.disabled = false - localAuthorityOption.hidden = false - nonProfitOption.hidden = false - profitOption.hidden = false + const localAuthorityOption = profitStatusSelect.querySelector('option[value="local_authority"]') + const nonProfitOption = profitStatusSelect.querySelector('option[value="non_profit"]') + const profitOption = profitStatusSelect.querySelector('option[value="profit"]') + const providerType = event.target.value - if (providerType === 'LA') { - profitStatusSelect.value = 'local_authority' - nonProfitOption.hidden = true - profitOption.hidden = true - } else if (providerType === 'PRP') { - profitStatusSelect.value = '' - localAuthorityOption.hidden = true - } + profitStatusSelect.disabled = false + localAuthorityOption.hidden = false + nonProfitOption.hidden = false + profitOption.hidden = false + + if (providerType === 'LA') { + profitStatusSelect.value = 'local_authority' + nonProfitOption.hidden = true + profitOption.hidden = true + } else if (providerType === 'PRP') { + profitStatusSelect.value = '' + localAuthorityOption.hidden = true + } } - } } From 1cc3165916cc678d49a344040dab020f91199ab5 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Tue, 25 Mar 2025 12:42:52 +0000 Subject: [PATCH 30/36] Lint --- .../controllers/organisations_controller.js | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/app/frontend/controllers/organisations_controller.js b/app/frontend/controllers/organisations_controller.js index 47df883a4..060265f27 100644 --- a/app/frontend/controllers/organisations_controller.js +++ b/app/frontend/controllers/organisations_controller.js @@ -1,27 +1,27 @@ import { Controller } from '@hotwired/stimulus' export default class extends Controller { - updateProfitStatusOptions (event) { - const profitStatusSelect = document.getElementById('organisation-profit-status-field') - if (!profitStatusSelect) return + updateProfitStatusOptions (event) { + const profitStatusSelect = document.getElementById('organisation-profit-status-field') + if (!profitStatusSelect) return - const localAuthorityOption = profitStatusSelect.querySelector('option[value="local_authority"]') - const nonProfitOption = profitStatusSelect.querySelector('option[value="non_profit"]') - const profitOption = profitStatusSelect.querySelector('option[value="profit"]') - const providerType = event.target.value + const localAuthorityOption = profitStatusSelect.querySelector('option[value="local_authority"]') + const nonProfitOption = profitStatusSelect.querySelector('option[value="non_profit"]') + const profitOption = profitStatusSelect.querySelector('option[value="profit"]') + const providerType = event.target.value - profitStatusSelect.disabled = false - localAuthorityOption.hidden = false - nonProfitOption.hidden = false - profitOption.hidden = false + profitStatusSelect.disabled = false + localAuthorityOption.hidden = false + nonProfitOption.hidden = false + profitOption.hidden = false - if (providerType === 'LA') { - profitStatusSelect.value = 'local_authority' - nonProfitOption.hidden = true - profitOption.hidden = true - } else if (providerType === 'PRP') { - profitStatusSelect.value = '' - localAuthorityOption.hidden = true - } + if (providerType === 'LA') { + profitStatusSelect.value = 'local_authority' + nonProfitOption.hidden = true + profitOption.hidden = true + } else if (providerType === 'PRP') { + profitStatusSelect.value = '' + localAuthorityOption.hidden = true } + } } From 2cbb02e8bdc6e4a1d8609c4370ba67d6e00d6c2f Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Tue, 25 Mar 2025 13:07:51 +0000 Subject: [PATCH 31/36] Add tests to helper --- spec/helpers/organisations_helper_spec.rb | 32 +++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/spec/helpers/organisations_helper_spec.rb b/spec/helpers/organisations_helper_spec.rb index ebdf73c67..46af25be4 100644 --- a/spec/helpers/organisations_helper_spec.rb +++ b/spec/helpers/organisations_helper_spec.rb @@ -80,4 +80,36 @@ RSpec.describe OrganisationsHelper do end end end + + describe "#group_organisation_options_name" do + let(:older_org) { create(:organisation, group_member: true, group_member_id: 123, group: 9) } + let(:org) { create(:organisation, name: "Org1", group_member: true, group_member_id: older_org.id) } + + it "returns the organisation name with group text" do + allow(org).to receive(:oldest_group_member).and_return(older_org) + name = helper.group_organisation_options_name(org) + expect(name).to eq("Org1 (GROUP9)") + end + end + + describe "#profit_status_options" do + it "returns a list of profit statuses with a null option" do + options = helper.profit_status_options + expect(options.map(&:name)).to include("Select an option") + end + + context "when provider type is LA" do + it "returns only the local authority option" do + options = helper.profit_status_options("LA") + expect(options.map(&:id)).to eq(["", :local_authority]) + end + end + + context "when provider type is PRP" do + it "excludes the local authority option" do + options = helper.profit_status_options("PRP") + expect(options.map(&:id)).not_to include(:local_authority) + end + end + end end From 47840d0f673bfc957b9123f98bdd22dec6c9ecb8 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Tue, 25 Mar 2025 14:12:32 +0000 Subject: [PATCH 32/36] Some tests on the organisation pages --- spec/features/organisation_spec.rb | 38 ++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/spec/features/organisation_spec.rb b/spec/features/organisation_spec.rb index 336ae51af..eb11601c9 100644 --- a/spec/features/organisation_spec.rb +++ b/spec/features/organisation_spec.rb @@ -43,6 +43,11 @@ RSpec.describe "User Features" do expect(page).to have_current_path("/organisations/#{org_id}/details") end + it "does not allow coordinator users to edit their organisation's group and profit status" do + expect(page).to have_no_link("Change part of group") + expect(page).to have_no_link("Select profit status") + end + context "and the organisation does not hold housing stock" do before do organisation.update(holds_own_stock: false) @@ -365,5 +370,38 @@ RSpec.describe "User Features" do expect(page).not_to have_link("Schemes", href: "/schemes", count: 2) end end + + context "and is creating a new organisation" do + before do + visit("/organisations") + click_link("Create a new organisation") + end + + it "displays the new organisation form" do + expect(page).to have_content("Create a new organisation") + expect(page).to have_field("organisation[name]", type: "text") + expect(page).to have_field("organisation[address_line1]", type: "text") + expect(page).to have_field("organisation[address_line2]", type: "text") + expect(page).to have_field("organisation[postcode]", type: "text") + expect(page).to have_field("organisation[phone]") + expect(page).to have_field("organisation[housing_registration_no]", type: "text") + expect(page).to have_select("organisation[provider_type]") + expect(page).to have_field("organisation[holds_own_stock]", type: "radio") + expect(page).to have_field("organisation[group_member]", type: "radio") + expect(page).to have_select("organisation[profit_status]") + expect(page).to have_button("Create organisation") + end + end + + context "when viewing a specific organisation's details page" do + before do + visit("/organisations/#{org_id}/details") + end + + it "allows the support user to edit the organisation's group and profit status" do + expect(page).to have_link("Change part of group") + expect(page).to have_link("Select profit status") + end + end end end From c59465f790a6d8268d688966a14e43c32b31b3cb Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Wed, 16 Apr 2025 17:51:29 +0100 Subject: [PATCH 33/36] Fix label text for profit status question in forms --- app/views/organisations/edit.html.erb | 2 +- app/views/organisations/new.html.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/organisations/edit.html.erb b/app/views/organisations/edit.html.erb index c6649a30a..15a660966 100644 --- a/app/views/organisations/edit.html.erb +++ b/app/views/organisations/edit.html.erb @@ -56,7 +56,7 @@ profit_status_options(@organisation.provider_type), :id, :name, - label: { text: "Is the organisation for-profit?", size: "m" }, + label: { text: "Is the organisation for profit?", size: "m" }, options: { disabled: [""], selected: @organisation.profit_status || "" } %> <% end %> diff --git a/app/views/organisations/new.html.erb b/app/views/organisations/new.html.erb index d5aa33164..9752155ac 100644 --- a/app/views/organisations/new.html.erb +++ b/app/views/organisations/new.html.erb @@ -80,7 +80,7 @@ profit_status_options, :id, :name, - label: { text: "Is the organisation for-profit?", size: "m" }, + label: { text: "Is the organisation for profit?", size: "m" }, options: { disabled: [""], selected: @organisation.profit_status || "" } %> <%= f.govuk_check_boxes_fieldset :rent_periods, From aa8e60cb6c661442249fa681efbbee951f70d548 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Wed, 16 Apr 2025 18:25:41 +0100 Subject: [PATCH 34/36] Add CSV task to update organisation profit status and group member fields --- app/models/organisation.rb | 5 ++- ...ate_organisations_group_profit_status.rake | 29 ++++++++++++++ ...anisations_group_profit_status_invalid.csv | 2 + ...rganisations_group_profit_status_valid.csv | 2 + ..._organisations_group_profit_status_spec.rb | 40 +++++++++++++++++++ 5 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 lib/tasks/update_organisations_group_profit_status.rake create mode 100644 spec/fixtures/files/organisations_group_profit_status_invalid.csv create mode 100644 spec/fixtures/files/organisations_group_profit_status_valid.csv create mode 100644 spec/lib/tasks/update_organisations_group_profit_status_spec.rb diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 0dac3d530..6e38040f9 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -63,6 +63,8 @@ class Organisation < ApplicationRecord enum :profit_status, PROFIT_STATUS attribute :group_member, :boolean + attr_accessor :skip_group_member_validation + before_save :clear_group_member_fields_if_not_group_member alias_method :la?, :LA? @@ -70,8 +72,7 @@ class Organisation < ApplicationRecord validates :name, presence: { message: I18n.t("validations.organisation.name_missing") } validates :name, uniqueness: { case_sensitive: false, message: I18n.t("validations.organisation.name_not_unique") } validates :provider_type, presence: { message: I18n.t("validations.organisation.provider_type_missing") } - validates :group_member_id, presence: { message: I18n.t("validations.organisation.group_missing") }, if: :group_member? - + validates :group_member_id, presence: { message: I18n.t("validations.organisation.group_missing") }, if: -> { group_member? && !skip_group_member_validation } validate :validate_profit_status def self.find_by_id_on_multiple_fields(id) diff --git a/lib/tasks/update_organisations_group_profit_status.rake b/lib/tasks/update_organisations_group_profit_status.rake new file mode 100644 index 000000000..fb0718608 --- /dev/null +++ b/lib/tasks/update_organisations_group_profit_status.rake @@ -0,0 +1,29 @@ +namespace :data_update do + desc "Update organisations with data from a CSV file" + task :update_organisations, [:csv_path] => :environment do |_task, args| + require "csv" + + csv_path = args[:csv_path] + unless csv_path + Rails.logger.error "Please provide the path to the CSV file. Example: rake data_update:update_organisations[csv_path]" + exit + end + + CSV.foreach(csv_path, headers: true) do |row| + organisation = Organisation.find_by(id: row["id"].to_i) + if organisation + organisation.skip_group_member_validation = true + organisation.update!( + profit_status: row["profit_status"].to_i, + group_member: true, + group: row["group"].to_i, + ) + Rails.logger.info "Updated ORG#{row['id']}" + else + Rails.logger.warn "Organisation with ID #{row['id']} not found" + end + end + + Rails.logger.info "Organisation update task completed" + end +end diff --git a/spec/fixtures/files/organisations_group_profit_status_invalid.csv b/spec/fixtures/files/organisations_group_profit_status_invalid.csv new file mode 100644 index 000000000..648f9ecde --- /dev/null +++ b/spec/fixtures/files/organisations_group_profit_status_invalid.csv @@ -0,0 +1,2 @@ +id,profit_status,group +2000,2,4 diff --git a/spec/fixtures/files/organisations_group_profit_status_valid.csv b/spec/fixtures/files/organisations_group_profit_status_valid.csv new file mode 100644 index 000000000..c8a977db0 --- /dev/null +++ b/spec/fixtures/files/organisations_group_profit_status_valid.csv @@ -0,0 +1,2 @@ +id,profit_status,group +234,3,2 diff --git a/spec/lib/tasks/update_organisations_group_profit_status_spec.rb b/spec/lib/tasks/update_organisations_group_profit_status_spec.rb new file mode 100644 index 000000000..c2ab4a93a --- /dev/null +++ b/spec/lib/tasks/update_organisations_group_profit_status_spec.rb @@ -0,0 +1,40 @@ +require "rails_helper" +require "rake" + +RSpec.describe "data_update:update_organisations", type: :task do + let(:task) { Rake::Task["data_update:update_organisations"] } + + before do + Rake.application.rake_require("tasks/update_organisations_group_profit_status") + Rake::Task.define_task(:environment) + task.reenable + end + + context "when the CSV file is valid" do + let!(:organisation) { create(:organisation, id: 234) } + let(:csv_path) { Rails.root.join("spec/fixtures/files/organisations_group_profit_status_valid.csv") } + + it "updates the organisation fields" do + expect { + task.invoke(csv_path.to_s) + }.to change { organisation.reload.profit_status }.to("local_authority") + .and change { organisation.reload.group }.to(2) + end + end + + context "when the organisation is not found" do + let(:csv_path) { Rails.root.join("spec/fixtures/files/organisations_group_profit_status_invalid.csv") } + + it "logs a warning" do + expect(Rails.logger).to receive(:warn).with("Organisation with ID 2000 not found") + task.invoke(csv_path.to_s) + end + end + + context "when the CSV path is not provided" do + it "logs an error and exits" do + expect(Rails.logger).to receive(:error).with("Please provide the path to the CSV file. Example: rake data_update:update_organisations[csv_path]") + expect { task.invoke }.to raise_error(SystemExit) + end + end +end From 5cad998019df63473d4185eaa18246bbd3e64358 Mon Sep 17 00:00:00 2001 From: Samuel Date: Thu, 1 May 2025 14:50:41 +0100 Subject: [PATCH 35/36] only associate group on update if validation passes --- app/controllers/organisations_controller.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 38c21d851..d4e4f34fc 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -128,9 +128,6 @@ class OrganisationsController < ApplicationController def update if (current_user.data_coordinator? && org_params[:active].nil?) || current_user.support? - if org_params[:group_member] && org_params[:group_member_id] - @organisation.group = assign_group_number(@organisation.id, org_params[:group_member_id]) - end if @organisation.update(org_params) case org_params[:active] when "false" @@ -146,6 +143,9 @@ class OrganisationsController < ApplicationController end flash[:notice] = I18n.t("organisation.reactivated", organisation: @organisation.name) else + if org_params[:group_member] && org_params[:group_member_id] + @organisation.group = assign_group_number(@organisation.id, org_params[:group_member_id]) + end flash[:notice] = I18n.t("organisation.updated") end if rent_period_params[:rent_periods].present? From f725fcd1e9f558053536b297eeaefc78146bc0b7 Mon Sep 17 00:00:00 2001 From: Samuel Date: Thu, 1 May 2025 15:28:51 +0100 Subject: [PATCH 36/36] rearrange xml structure --- spec/fixtures/exports/organisation.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/fixtures/exports/organisation.xml b/spec/fixtures/exports/organisation.xml index a9a511b9b..f70ac2b7d 100644 --- a/spec/fixtures/exports/organisation.xml +++ b/spec/fixtures/exports/organisation.xml @@ -16,12 +16,12 @@ - - true {dsa_signed_at} {dpo_email} + + active