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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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 705c896b2ca4d4035900b0844f786056e79cf411 Mon Sep 17 00:00:00 2001
From: Samuel
Date: Thu, 15 May 2025 17:44:15 +0100
Subject: [PATCH 36/39] Update import raketask to use provided csv
looking into it I couldn't see a use for skip_group_member_validation. it temporarily disables validation but since it's not saved to database it'll make the organisation invalid in the future
opted instead to make all properties valid when importing
---
app/models/organisation.rb | 3 +-
...5_organisation_profit_status_and_group.csv | 898 ++++++++++++++++++
...ate_organisations_group_profit_status.rake | 24 +-
3 files changed, 919 insertions(+), 6 deletions(-)
create mode 100644 config/csv/organisations_structure/20250515_organisation_profit_status_and_group.csv
diff --git a/app/models/organisation.rb b/app/models/organisation.rb
index 6e38040f9..8af785e22 100644
--- a/app/models/organisation.rb
+++ b/app/models/organisation.rb
@@ -63,7 +63,6 @@ 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
@@ -72,7 +71,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? && !skip_group_member_validation }
+ 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)
diff --git a/config/csv/organisations_structure/20250515_organisation_profit_status_and_group.csv b/config/csv/organisations_structure/20250515_organisation_profit_status_and_group.csv
new file mode 100644
index 000000000..971038dfe
--- /dev/null
+++ b/config/csv/organisations_structure/20250515_organisation_profit_status_and_group.csv
@@ -0,0 +1,898 @@
+id,provider_type,profit_status,group
+701,PRP,Non-profit,
+950,PRP,Non-profit,
+808,PRP,Non-profit,2
+952,PRP,Non-profit,2
+815,PRP,Non-profit,2
+311,PRP,Non-profit,
+610,PRP,Non-profit,
+743,PRP,Non-profit,
+750,PRP,Non-profit,
+642,PRP,Profit,
+181,PRP,Non-profit,
+568,PRP,Non-profit,
+954,PRP,Non-profit,
+994,PRP,Non-profit,
+763,PRP,Non-profit,
+878,PRP,Non-profit,
+961,PRP,Non-profit,
+908,LA,Non-profit,
+723,PRP,Non-profit,
+198,PRP,Non-profit,
+895,PRP,Non-profit,
+420,LA,Local authority,
+470,PRP,Non-profit,
+541,PRP,Non-profit,
+479,PRP,Profit,
+82,PRP,Non-profit,
+857,PRP,Non-profit,
+807,PRP,Non-profit,
+83,PRP,Non-profit,
+39,PRP,Non-profit,
+707,PRP,Non-profit,
+1006,PRP,Local authority,
+765,PRP,Non-profit,
+649,PRP,Non-profit,
+805,PRP,Non-profit,
+960,PRP,Non-profit,
+985,PRP,Non-profit,
+531,PRP,Non-profit,
+84,PRP,Non-profit,
+629,PRP,Non-profit,
+199,PRP,Non-profit,
+273,LA,Local authority,
+827,PRP,Profit,
+442,LA,Local authority,
+409,LA,Local authority,
+772,PRP,Non-profit,
+314,PRP,Non-profit,
+240,PRP,Non-profit,
+624,PRP,Non-profit,
+918,PRP,Non-profit,4
+888,PRP,Non-profit,4
+955,PRP,Non-profit,4
+907,PRP,Non-profit,
+464,PRP,Profit,
+468,PRP,Non-profit,
+450,LA,Local authority,
+790,PRP,Local authority,
+874,PRP,Non-profit,
+739,PRP,Non-profit,
+556,PRP,Non-profit,
+923,PRP,Non-profit,
+200,PRP,Non-profit,
+534,LA,Local authority,
+806,LA,Local authority,
+668,LA,Local authority,
+934,PRP,Non-profit,
+85,PRP,Non-profit,
+2,PRP,Non-profit,
+905,PRP,Non-profit,
+524,PRP,Non-profit,
+791,PRP,Non-profit,
+86,PRP,Non-profit,
+201,PRP,Non-profit,
+640,PRP,Non-profit,
+315,PRP,Non-profit,
+323,PRP,Non-profit,
+693,LA,Local authority,
+496,PRP,Non-profit,
+481,PRP,Non-profit,
+579,PRP,Non-profit,
+322,LA,Local authority,
+298,PRP,Local authority,
+4,LA,Local authority,
+490,PRP,Non-profit,
+438,LA,Non-profit,
+87,PRP,Non-profit,
+88,PRP,Non-profit,
+527,LA,Local authority,
+877,PRP,Local authority,
+818,PRP,Non-profit,
+670,PRP,Non-profit,
+766,PRP,Non-profit,
+639,PRP,Non-profit,
+940,PRP,Non-profit,
+626,PRP,Non-profit,
+460,LA,Local authority,
+713,PRP,Non-profit,
+89,PRP,Non-profit,
+609,PRP,Non-profit,
+422,LA,Local authority,
+90,PRP,Non-profit,
+529,LA,Local authority,
+999,PRP,Non-profit,
+462,PRP,Non-profit,
+914,PRP,Non-profit,
+182,PRP,Non-profit,
+933,PRP,Non-profit,
+241,PRP,Non-profit,5
+959,PRP,Non-profit,5
+692,PRP,Non-profit,
+465,PRP,Non-profit,
+518,LA,Local authority,
+25,PRP,Non-profit,
+34,LA,Local authority,
+800,PRP,Local authority,
+202,PRP,Non-profit,
+544,PRP,Non-profit,
+953,PRP,Non-profit,
+637,PRP,Non-profit,
+461,LA,Local authority,
+880,PRP,Local authority,
+40,PRP,Non-profit,
+401,LA,Local authority,
+1015,PRP,Local authority,
+429,LA,Local authority,
+983,PRP,Non-profit,
+652,PRP,Non-profit,
+279,LA,Local authority,
+275,PRP,Non-profit,
+665,PRP,Non-profit,
+704,PRP,Non-profit,
+734,PRP,Non-profit,
+408,LA,Local authority,
+854,PRP,Non-profit,
+700,PRP,Non-profit,
+733,PRP,Non-profit,
+870,PRP,Non-profit,
+23,PRP,Non-profit,
+430,LA,Local authority,
+831,PRP,Non-profit,
+844,PRP,Non-profit,
+302,LA,Local authority,
+286,PRP,Non-profit,
+41,PRP,Non-profit,
+284,LA,Local authority,
+493,PRP,Non-profit,
+798,LA,Local authority,
+335,LA,Local authority,
+91,PRP,Non-profit,
+661,PRP,Non-profit,
+850,PRP,Non-profit,
+66,PRP,Non-profit,
+31,PRP,Non-profit,
+191,PRP,Non-profit,
+558,PRP,Non-profit,
+511,PRP,Non-profit,
+721,LA,Local authority,
+307,LA,Local authority,
+911,PRP,Non-profit,
+454,LA,Local authority,
+981,PRP,Non-profit,
+319,LA,Local authority,
+404,LA,Local authority,
+92,LA,Local authority,
+659,PRP,Non-profit,
+747,PRP,Non-profit,
+5,PRP,Non-profit,
+316,PRP,Non-profit,
+292,LA,Local authority,
+42,PRP,Non-profit,
+242,PRP,Non-profit,
+931,PRP,Non-profit,
+759,PRP,Non-profit,
+612,PRP,Non-profit,
+658,PRP,Profit,
+243,PRP,Non-profit,
+1007,PRP,Non-profit,
+232,PRP,Non-profit,
+44,PRP,Non-profit,
+43,PRP,Non-profit,
+203,PRP,Non-profit,
+497,LA,Local authority,
+514,PRP,Non-profit,
+24,PRP,Non-profit,
+204,PRP,Non-profit,
+635,PRP,Non-profit,
+813,PRP,Non-profit,
+865,PRP,Non-profit,
+715,PRP,Non-profit,
+93,LA,Local authority,
+817,PRP,Non-profit,
+557,PRP,Non-profit,
+746,PRP,Non-profit,
+282,PRP,Non-profit,
+618,PRP,Non-profit,
+663,PRP,Non-profit,
+821,PRP,Non-profit,
+741,PRP,Non-profit,
+403,LA,Local authority,
+78,PRP,Non-profit,
+415,LA,Local authority,
+405,LA,Local authority,
+977,PRP,Non-profit,
+675,PRP,Non-profit,
+525,LA,Local authority,
+519,PRP,Non-profit,
+94,PRP,Non-profit,
+67,PRP,Non-profit,
+485,PRP,Non-profit,
+930,PRP,Non-profit,
+95,PRP,Non-profit,
+656,PRP,Local authority,
+288,LA,Local authority,
+192,LA,Local authority,
+96,PRP,Non-profit,
+205,PRP,Non-profit,
+97,PRP,Non-profit,
+413,LA,Local authority,
+6,PRP,Non-profit,
+760,LA,Local authority,
+447,LA,Local authority,
+280,LA,Local authority,
+300,PRP,Local authority,
+317,LA,Local authority,
+512,PRP,Non-profit,
+98,PRP,Non-profit,
+70,PRP,Non-profit,
+99,PRP,Non-profit,
+29,PRP,Non-profit,
+100,PRP,Non-profit,
+1004,PRP,Non-profit,
+101,PRP,Non-profit,
+1011,LA,Local authority,
+722,PRP,Non-profit,
+559,PRP,Non-profit,
+206,PRP,Non-profit,
+495,PRP,Non-profit,
+207,PRP,Non-profit,
+102,LA,Local authority,
+912,PRP,Non-profit,
+561,PRP,Non-profit,
+103,PRP,Non-profit,
+644,PRP,Non-profit,
+452,LA,Local authority,
+608,PRP,Non-profit,
+472,PRP,Non-profit,
+466,PRP,Non-profit,
+208,PRP,Non-profit,
+580,PRP,Non-profit,
+105,PRP,Non-profit,
+835,PRP,Non-profit,
+970,LA,Local authority,
+276,PRP,Non-profit,
+486,PRP,Non-profit,
+951,PRP,Non-profit,
+1003,PRP,Non-profit,
+797,PRP,Non-profit,
+548,PRP,Profit,
+414,LA,Local authority,
+748,PRP,Non-profit,
+872,PRP,Non-profit,
+777,PRP,Non-profit,
+16,PRP,Non-profit,
+263,PRP,Non-profit,
+245,PRP,Non-profit,
+630,PRP,Profit,
+515,PRP,Non-profit,3
+244,PRP,Non-profit,3
+968,PRP,Non-profit,3
+634,PRP,Non-profit,
+538,PRP,Non-profit,
+521,LA,Local authority,
+728,PRP,Non-profit,
+799,PRP,Non-profit,
+246,PRP,Non-profit,
+889,PRP,Non-profit,
+614,PRP,Non-profit,
+893,PRP,Non-profit,
+106,PRP,Non-profit,
+247,PRP,Non-profit,
+424,LA,Local authority,
+45,PRP,Non-profit,
+209,PRP,Profit,
+716,PRP,Non-profit,
+657,PRP,Non-profit,
+833,PRP,Non-profit,
+494,LA,Local authority,
+473,PRP,Non-profit,
+1012,PRP,Non-profit,
+28,PRP,Non-profit,
+856,PRP,Non-profit,
+572,PRP,Non-profit,
+969,PRP,Non-profit,6
+643,PRP,Non-profit,
+418,LA,Local authority,
+210,PRP,Non-profit,
+551,PRP,Profit,
+564,PRP,Non-profit,
+476,PRP,Non-profit,
+788,PRP,Non-profit,
+46,PRP,Non-profit,
+881,PRP,Non-profit,
+419,LA,Local authority,
+653,PRP,Non-profit,
+972,PRP,Non-profit,
+814,PRP,Non-profit,
+327,LA,Local authority,
+502,PRP,Non-profit,
+108,PRP,Non-profit,
+921,PRP,Non-profit,
+957,PRP,Non-profit,
+731,PRP,Non-profit,
+21,PRP,Profit,
+65,PRP,Non-profit,
+109,PRP,Non-profit,
+47,PRP,Non-profit,
+847,PRP,Non-profit,
+860,PRP,Non-profit,
+265,PRP,Profit,
+287,LA,Local authority,
+980,PRP,Non-profit,
+824,PRP,Non-profit,
+546,PRP,Non-profit,
+211,PRP,Non-profit,
+416,LA,Local authority,
+785,PRP,Local authority,
+578,PRP,Non-profit,
+71,PRP,Non-profit,
+48,PRP,Non-profit,
+672,PRP,Non-profit,
+684,PRP,Non-profit,
+650,PRP,Non-profit,
+500,PRP,Non-profit,
+332,LA,Non-profit,
+266,PRP,Non-profit,
+702,PRP,Non-profit,
+752,PRP,Non-profit,
+110,PRP,Non-profit,
+694,PRP,Non-profit,
+819,PRP,Non-profit,
+620,PRP,Non-profit,
+1010,PRP,Non-profit,
+111,PRP,Non-profit,
+853,PRP,Non-profit,7
+732,PRP,Non-profit,
+112,PRP,Non-profit,
+755,PRP,Non-profit,
+12,PRP,Non-profit,
+655,PRP,Non-profit,
+49,PRP,Non-profit,
+574,PRP,Non-profit,
+113,PRP,Non-profit,
+823,PRP,Non-profit,
+775,PRP,Non-profit,
+706,PRP,Non-profit,
+780,PRP,Non-profit,
+822,PRP,Non-profit,
+264,PRP,Non-profit,
+887,PRP,Non-profit,
+114,PRP,Non-profit,
+115,PRP,Non-profit,
+212,PRP,Non-profit,
+290,LA,Local authority,
+982,PRP,Local authority,
+72,PRP,Non-profit,
+575,PRP,Non-profit,
+778,PRP,Non-profit,
+477,PRP,Non-profit,
+183,PRP,Non-profit,
+767,PRP,Non-profit,
+837,PRP,Non-profit,
+272,PRP,Non-profit,
+248,PRP,Non-profit,
+990,PRP,Non-profit,
+736,PRP,Non-profit,
+988,PRP,Non-profit,
+939,PRP,Non-profit,
+116,PRP,Non-profit,
+1017,PRP,Non-profit,
+526,PRP,Non-profit,
+214,PRP,Non-profit,
+520,LA,Local authority,
+117,PRP,Non-profit,
+583,PRP,Non-profit,7
+118,LA,Local authority,
+50,PRP,Non-profit,
+709,PRP,Non-profit,
+26,PRP,Non-profit,
+11,PRP,Non-profit,
+781,PRP,Non-profit,
+421,LA,Local authority,
+119,PRP,Non-profit,
+73,PRP,Non-profit,
+926,PRP,Non-profit,
+51,PRP,Non-profit,
+1001,PRP,Non-profit,
+848,PRP,Non-profit,
+679,PRP,Non-profit,
+328,PRP,Non-profit,
+428,LA,Local authority,
+971,PRP,Local authority,
+892,PRP,Non-profit,
+553,PRP,Non-profit,
+121,PRP,Non-profit,
+758,PRP,Non-profit,
+1019,PRP,Profit,
+735,PRP,Profit,
+1020,PRP,Profit,
+435,LA,Local authority,
+678,PRP,Local authority,
+691,PRP,Local authority,
+329,LA,Local authority,
+836,PRP,Non-profit,
+120,PRP,Non-profit,
+250,PRP,Non-profit,
+14,PRP,Non-profit,
+927,LA,Local authority,
+74,PRP,Non-profit,
+122,PRP,Non-profit,
+816,PRP,Non-profit,
+184,PRP,Non-profit,
+185,PRP,Non-profit,
+978,PRP,Non-profit,
+52,PRP,Non-profit,
+621,PRP,Non-profit,
+333,LA,Local authority,
+193,LA,Local authority,
+308,LA,Local authority,
+436,LA,Local authority,
+965,LA,Local authority,
+310,LA,Local authority,
+517,LA,Local authority,
+535,LA,Local authority,
+123,LA,Local authority,
+79,LA,Local authority,
+492,LA,Local authority,
+859,LA,Local authority,
+262,LA,Local authority,
+498,LA,Local authority,
+633,LA,Local authority,
+437,LA,Local authority,
+443,LA,Local authority,
+124,LA,Local authority,
+523,LA,Local authority,
+278,LA,Local authority,
+309,LA,Local authority,
+194,LA,Local authority,
+761,PRP,Non-profit,
+186,PRP,Non-profit,
+811,PRP,Non-profit,
+216,PRP,Non-profit,
+425,LA,Local authority,
+148,PRP,Non-profit,
+17,PRP,Non-profit,
+942,PRP,Profit,
+261,PRP,Non-profit,
+251,PRP,Non-profit,
+919,LA,Local authority,
+289,LA,Local authority,
+787,PRP,Non-profit,
+631,PRP,Non-profit,
+771,PRP,Non-profit,
+217,PRP,Non-profit,
+68,PRP,Non-profit,
+904,PRP,Profit,
+126,LA,Local authority,
+445,LA,Local authority,
+963,PRP,Local authority,
+573,PRP,Non-profit,5
+127,PRP,Non-profit,
+613,PRP,Non-profit,
+632,PRP,Non-profit,
+730,PRP,Non-profit,
+509,LA,Local authority,
+417,LA,Local authority,
+616,PRP,Non-profit,
+997,PRP,Non-profit,
+689,LA,Local authority,
+862,PRP,Non-profit,
+682,PRP,Non-profit,
+964,PRP,Non-profit,
+681,PRP,Non-profit,
+796,PRP,Non-profit,
+719,PRP,Non-profit,
+505,PRP,Non-profit,
+487,PRP,Non-profit,
+128,PRP,Non-profit,
+932,PRP,Non-profit,
+846,PRP,Non-profit,
+998,PRP,Non-profit,
+129,PRP,Non-profit,
+130,PRP,Non-profit,
+867,PRP,Non-profit,
+407,LA,Local authority,
+625,PRP,Non-profit,
+131,PRP,Non-profit,
+549,PRP,Non-profit,
+915,PRP,Non-profit,
+132,PRP,Non-profit,
+710,PRP,Profit,
+439,LA,Local authority,
+325,LA,Local authority,
+53,PRP,Non-profit,
+738,PRP,Non-profit,
+727,PRP,Non-profit,
+906,PRP,Non-profit,
+690,PRP,Non-profit,
+469,PRP,Non-profit,
+296,LA,Local authority,
+793,PRP,Non-profit,
+336,LA,Local authority,
+218,PRP,Non-profit,
+133,LA,Local authority,
+973,PRP,Local authority,
+885,PRP,Non-profit,
+536,LA,Local authority,
+80,LA,Local authority,
+440,LA,Local authority,
+456,LA,Local authority,
+789,PRP,Local authority,
+297,PRP,Local authority,
+677,PRP,Non-profit,
+277,PRP,Local authority,
+38,PRP,Local authority,
+830,PRP,Non-profit,
+301,LA,Local authority,
+75,PRP,Non-profit,
+770,PRP,Non-profit,
+686,PRP,Non-profit,
+537,LA,Local authority,
+533,PRP,Non-profit,
+698,PRP,Non-profit,
+717,PRP,Non-profit,
+69,PRP,Non-profit,
+239,PRP,Non-profit,
+948,PRP,Non-profit,
+444,LA,Local authority,
+402,LA,Local authority,
+986,PRP,Non-profit,
+252,PRP,Non-profit,
+253,PRP,Non-profit,
+219,PRP,Non-profit,
+820,LA,Local authority,
+254,PRP,Non-profit,
+324,PRP,Non-profit,
+845,PRP,Non-profit,
+705,PRP,Non-profit,
+554,PRP,Non-profit,
+255,PRP,Non-profit,
+712,PRP,Non-profit,
+651,PRP,Non-profit,
+699,PRP,Non-profit,
+987,PRP,Non-profit,
+779,PRP,Non-profit,
+886,PRP,Non-profit,
+195,LA,Local authority,
+869,PRP,Non-profit,
+683,PRP,Non-profit,
+917,PRP,Non-profit,
+688,PRP,Non-profit,
+654,PRP,Non-profit,
+326,PRP,Non-profit,
+555,PRP,Profit,
+696,PRP,Non-profit,
+782,PRP,Non-profit,
+839,PRP,Non-profit,
+946,PRP,Profit,
+220,PRP,Non-profit,
+221,PRP,Non-profit,
+1000,PRP,Non-profit,7
+222,PRP,Non-profit,
+294,PRP,Profit,
+863,PRP,Non-profit,
+562,PRP,Non-profit,
+223,PRP,Local authority,
+135,PRP,Non-profit,
+669,PRP,Non-profit,
+136,PRP,Non-profit,
+137,PRP,Non-profit,
+611,PRP,Profit,
+891,PRP,Profit,
+864,PRP,Profit,
+989,PRP,Non-profit,1
+724,PRP,Non-profit,1
+680,PRP,Non-profit,1
+187,PRP,Non-profit,
+224,PRP,Non-profit,
+501,PRP,Non-profit,
+499,PRP,Non-profit,
+225,PRP,Non-profit,
+852,PRP,Non-profit,
+913,PRP,Non-profit,
+451,LA,Local authority,
+20,PRP,Non-profit,
+958,PRP,Non-profit,
+674,PRP,Profit,
+10,PRP,Non-profit,
+774,PRP,Non-profit,
+54,PRP,Non-profit,
+55,PRP,Non-profit,
+944,PRP,Non-profit,
+627,PRP,Non-profit,
+949,PRP,Non-profit,
+1005,PRP,Non-profit,
+920,PRP,Non-profit,
+138,PRP,Non-profit,
+139,PRP,Non-profit,
+729,PRP,Non-profit,
+1018,PRP,Non-profit,
+530,LA,Local authority,
+1016,PRP,Non-profit,
+140,PRP,Non-profit,
+426,LA,Local authority,
+226,PRP,Non-profit,
+812,PRP,Non-profit,
+809,PRP,Non-profit,
+545,LA,Local authority,
+873,PRP,Profit,6
+764,PRP,Profit,6
+141,PRP,Non-profit,
+884,PRP,Non-profit,
+142,PRP,Non-profit,
+1013,PRP,Non-profit,
+330,PRP,Non-profit,
+482,PRP,Non-profit,
+615,PRP,Non-profit,
+227,PRP,Non-profit,
+459,LA,Local authority,
+305,LA,Local authority,
+648,PRP,Local authority,
+607,PRP,Non-profit,
+803,LA,Local authority,
+423,LA,Local authority,
+448,LA,Local authority,
+507,LA,Local authority,
+412,LA,Local authority,
+834,PRP,Local authority,
+979,PRP,Non-profit,
+306,PRP,Non-profit,
+563,PRP,Non-profit,
+143,PRP,Non-profit,
+646,PRP,Non-profit,
+695,PRP,Non-profit,
+188,PRP,Non-profit,
+334,PRP,Profit,
+993,PRP,Profit,
+30,PRP,Non-profit,
+883,PRP,Non-profit,
+776,PRP,Non-profit,
+619,PRP,Non-profit,
+714,PRP,Non-profit,
+144,PRP,Non-profit,
+196,LA,Local authority,
+516,PRP,Non-profit,
+664,PRP,Non-profit,
+744,PRP,Non-profit,
+467,PRP,Non-profit,
+189,PRP,Non-profit,
+483,PRP,Non-profit,
+506,PRP,Non-profit,
+996,PRP,Non-profit,
+751,LA,Local authority,
+567,PRP,Non-profit,
+560,PRP,Non-profit,
+56,PRP,Non-profit,
+299,PRP,Non-profit,
+37,LA,Local authority,
+36,PRP,Non-profit,
+35,PRP,Non-profit,
+145,PRP,Non-profit,
+754,PRP,Profit,
+749,PRP,Profit,
+941,PRP,Non-profit,
+33,PRP,Non-profit,
+331,LA,Local authority,
+956,PRP,Non-profit,
+27,PRP,Non-profit,
+916,PRP,Non-profit,
+882,PRP,Non-profit,
+992,PRP,Non-profit,
+925,PRP,Non-profit,
+966,LA,Local authority,
+910,PRP,Non-profit,
+312,LA,Local authority,
+146,LA,Local authority,
+478,LA,Local authority,
+228,PRP,Non-profit,
+453,LA,Local authority,
+539,LA,Local authority,
+147,PRP,Non-profit,
+256,PRP,Non-profit,
+976,LA,Local authority,
+285,LA,Local authority,
+318,PRP,Local authority,
+565,PRP,Non-profit,
+427,LA,Local authority,
+922,PRP,Non-profit,
+641,PRP,Non-profit,
+455,LA,Local authority,
+57,PRP,Non-profit,
+938,PRP,Non-profit,
+623,PRP,Non-profit,
+840,PRP,Non-profit,
+257,PRP,Non-profit,
+569,PRP,Non-profit,
+552,LA,Local authority,
+876,PRP,Local authority,
+19,PRP,Non-profit,
+58,PRP,Non-profit,
+841,PRP,Non-profit,
+293,PRP,Profit,
+433,LA,Local authority,
+149,PRP,Non-profit,
+150,PRP,Non-profit,
+843,PRP,Non-profit,
+281,PRP,Non-profit,
+991,PRP,Non-profit,
+929,PRP,Non-profit,
+795,PRP,Non-profit,
+742,PRP,Non-profit,
+151,PRP,Non-profit,
+1002,PRP,Non-profit,
+685,PRP,Non-profit,
+291,PRP,Non-profit,
+152,PRP,Non-profit,
+153,LA,Local authority,
+463,PRP,Non-profit,
+491,LA,Local authority,
+532,LA,Local authority,
+871,PRP,Local authority,
+229,PRP,Non-profit,
+666,PRP,Non-profit,
+829,PRP,Non-profit,
+697,PRP,Non-profit,
+936,PRP,Profit,
+570,PRP,Local authority,
+410,LA,Local authority,
+855,PRP,Non-profit,
+295,LA,Local authority,
+155,PRP,Non-profit,
+304,PRP,Non-profit,
+156,PRP,Non-profit,
+1008,PRP,Non-profit,
+875,PRP,Non-profit,
+446,LA,Local authority,
+879,PRP,Local authority,
+786,PRP,Non-profit,4
+434,LA,Local authority,
+157,PRP,Non-profit,
+441,LA,Local authority,
+230,PRP,Non-profit,
+628,PRP,Non-profit,
+828,PRP,Non-profit,
+231,PRP,Non-profit,
+158,PRP,Non-profit,
+489,PRP,Non-profit,
+1021,LA,Local authority,
+159,PRP,Non-profit,
+825,PRP,Non-profit,
+708,PRP,Non-profit,
+81,LA,Local authority,
+671,PRP,Non-profit,
+197,PRP,Non-profit,
+810,PRP,Non-profit,
+962,PRP,Non-profit,
+160,PRP,Non-profit,
+510,PRP,Non-profit,
+22,PRP,Non-profit,
+258,PRP,Non-profit,
+984,PRP,Non-profit,
+622,PRP,Non-profit,
+104,PRP,Non-profit,
+943,PRP,Non-profit,
+638,PRP,Non-profit,
+107,PRP,Non-profit,
+769,PRP,Non-profit,
+660,PRP,Non-profit,
+161,PRP,Non-profit,
+162,PRP,Non-profit,
+1014,PRP,Non-profit,
+667,PRP,Non-profit,
+673,PRP,Non-profit,
+134,PRP,Non-profit,
+480,PRP,Non-profit,
+896,PRP,Non-profit,
+890,PRP,Non-profit,
+937,PRP,Non-profit,
+995,PRP,Non-profit,
+543,PRP,Non-profit,
+76,PRP,Non-profit,
+794,PRP,Non-profit,
+267,PRP,Non-profit,
+725,PRP,Non-profit,
+59,PRP,Non-profit,
+522,PRP,Non-profit,
+945,PRP,Non-profit,
+163,PRP,Non-profit,
+897,PRP,Non-profit,
+636,PRP,Non-profit,
+475,PRP,Non-profit,
+768,LA,Local authority,
+645,PRP,Non-profit,
+726,PRP,Non-profit,
+259,PRP,Non-profit,
+547,PRP,Non-profit,
+566,PRP,Non-profit,
+550,PRP,Non-profit,
+861,PRP,Non-profit,
+832,PRP,Non-profit,
+508,PRP,Non-profit,
+233,PRP,Non-profit,
+234,PRP,Non-profit,
+488,PRP,Non-profit,
+773,PRP,Non-profit,
+164,PRP,Non-profit,
+740,PRP,Non-profit,
+804,PRP,Non-profit,
+647,PRP,Non-profit,
+32,PRP,Non-profit,
+851,PRP,Non-profit,
+703,PRP,Non-profit,
+826,PRP,Non-profit,
+577,PRP,Non-profit,
+432,LA,Local authority,
+571,PRP,Local authority,
+909,PRP,Non-profit,
+581,PRP,Non-profit,
+617,PRP,Non-profit,
+504,PRP,Profit,
+801,PRP,Non-profit,
+935,PRP,Non-profit,
+320,PRP,Non-profit,
+260,PRP,Non-profit,
+9,PRP,Non-profit,
+165,PRP,Non-profit,
+757,PRP,Non-profit,
+166,PRP,Non-profit,
+431,LA,Local authority,
+235,PRP,Non-profit,
+236,PRP,Non-profit,
+167,PRP,Non-profit,
+237,PRP,Non-profit,
+313,LA,Local authority,
+838,PRP,Local authority,
+457,LA,Local authority,
+474,PRP,Non-profit,
+60,PRP,Non-profit,
+168,PRP,Non-profit,
+406,LA,Local authority,
+61,PRP,Non-profit,
+858,PRP,Non-profit,
+484,PRP,Non-profit,
+169,LA,Local authority,
+967,PRP,Local authority,
+274,LA,Local authority,
+8,PRP,Non-profit,
+792,PRP,Non-profit,
+974,LA,Local authority,
+62,PRP,Non-profit,
+190,PRP,Non-profit,
+18,PRP,Non-profit,
+77,PRP,Non-profit,
+458,LA,Local authority,
+975,PRP,Non-profit,
+449,LA,Local authority,
+411,LA,Local authority,
+13,PRP,Non-profit,
+171,PRP,Non-profit,
+172,LA,Local authority,
+283,LA,Local authority,
+737,LA,Local authority,
+718,PRP,Local authority,
+753,PRP,Local authority,
+842,PRP,Non-profit,
+173,PRP,Non-profit,
+174,PRP,Non-profit,
+784,PRP,Non-profit,
+238,PRP,Non-profit,
+720,PRP,Non-profit,
+576,PRP,Non-profit,
+176,PRP,Non-profit,
+1009,PRP,Non-profit,
+762,PRP,Non-profit,
+125,PRP,Non-profit,
+154,PRP,Non-profit,
+947,PRP,Non-profit,
+177,PRP,Non-profit,
+178,PRP,Non-profit,
+179,PRP,Non-profit,
+180,PRP,Non-profit,
+584,PRP,Non-profit,
+662,PRP,Non-profit,
+849,PRP,Non-profit,
+582,PRP,Non-profit,
+1022,PRP,Non-profit,
+868,PRP,Non-profit,
+903,PRP,Profit,
diff --git a/lib/tasks/update_organisations_group_profit_status.rake b/lib/tasks/update_organisations_group_profit_status.rake
index fb0718608..f87c5f948 100644
--- a/lib/tasks/update_organisations_group_profit_status.rake
+++ b/lib/tasks/update_organisations_group_profit_status.rake
@@ -12,12 +12,20 @@ namespace :data_update do
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,
+ profit_status: map_profit_status(row["profit_status"]),
)
+ # not all orgs have a group
+ if row["group"].present?
+ organisation.update!(
+ group_member: true,
+ group: row["group"].to_i,
+ # normally set to the ID of the other organisation you picked on the group form
+ # we can't set that here so we default to its own org ID
+ group_member_id: organisation.id,
+ )
+ end
+
Rails.logger.info "Updated ORG#{row['id']}"
else
Rails.logger.warn "Organisation with ID #{row['id']} not found"
@@ -27,3 +35,11 @@ namespace :data_update do
Rails.logger.info "Organisation update task completed"
end
end
+
+def map_profit_status(profit_status)
+ return :non_profit if profit_status == "Non-profit"
+ return :profit if profit_status == "Profit"
+ return :local_authority if profit_status == "Local authority"
+
+ nil
+end
From 2542e0987c10660543ad24f85b3e38317efe1d93 Mon Sep 17 00:00:00 2001
From: Samuel
Date: Fri, 16 May 2025 17:10:45 +0100
Subject: [PATCH 37/39] Update tests to check more cases of organisation import
---
...anisations_group_profit_status_invalid.csv | 4 +--
...rganisations_group_profit_status_valid.csv | 6 ++--
..._organisations_group_profit_status_spec.rb | 31 ++++++++++++++++---
3 files changed, 33 insertions(+), 8 deletions(-)
diff --git a/spec/fixtures/files/organisations_group_profit_status_invalid.csv b/spec/fixtures/files/organisations_group_profit_status_invalid.csv
index 648f9ecde..679cb506d 100644
--- a/spec/fixtures/files/organisations_group_profit_status_invalid.csv
+++ b/spec/fixtures/files/organisations_group_profit_status_invalid.csv
@@ -1,2 +1,2 @@
-id,profit_status,group
-2000,2,4
+id,provider_type,profit_status,group
+2000,Profit,4
diff --git a/spec/fixtures/files/organisations_group_profit_status_valid.csv b/spec/fixtures/files/organisations_group_profit_status_valid.csv
index c8a977db0..35f57678c 100644
--- a/spec/fixtures/files/organisations_group_profit_status_valid.csv
+++ b/spec/fixtures/files/organisations_group_profit_status_valid.csv
@@ -1,2 +1,4 @@
-id,profit_status,group
-234,3,2
+id,provider_type,profit_status,group
+234,LA,Local authority,2
+750,PRP,Non-profit,2
+642,PRP,Profit,
diff --git a/spec/lib/tasks/update_organisations_group_profit_status_spec.rb b/spec/lib/tasks/update_organisations_group_profit_status_spec.rb
index c2ab4a93a..674ac1b4d 100644
--- a/spec/lib/tasks/update_organisations_group_profit_status_spec.rb
+++ b/spec/lib/tasks/update_organisations_group_profit_status_spec.rb
@@ -11,14 +11,37 @@ RSpec.describe "data_update:update_organisations", type: :task do
end
context "when the CSV file is valid" do
- let!(:organisation) { create(:organisation, id: 234) }
+ let!(:organisation_la) { create(:organisation, id: 234, provider_type: "LA") }
+ let!(:organisation_prp_non_profit) { create(:organisation, id: 750, provider_type: "PRP") }
+ let!(:organisation_prp_profit) { create(:organisation, id: 642, provider_type: "PRP") }
let(:csv_path) { Rails.root.join("spec/fixtures/files/organisations_group_profit_status_valid.csv") }
- it "updates the organisation fields" do
+ it "updates an organisation profit status field" do
expect {
task.invoke(csv_path.to_s)
- }.to change { organisation.reload.profit_status }.to("local_authority")
- .and change { organisation.reload.group }.to(2)
+ }
+ .to change { organisation_la.reload.profit_status }.to("local_authority")
+ .and change { organisation_prp_non_profit.reload.profit_status }.to("non_profit")
+ .and change { organisation_prp_profit.reload.profit_status }.to("profit")
+ end
+
+ it "updates an organisation group fields" do
+ task.invoke(csv_path.to_s)
+ organisation_la.reload
+ organisation_prp_non_profit.reload
+ organisation_prp_profit.reload
+
+ expect(organisation_la.group).to eq(2)
+ expect(organisation_la.group_member).to be_truthy
+ expect(organisation_la.group_member_id).to eq(organisation_la.id)
+
+ expect(organisation_prp_non_profit.group).to eq(2)
+ expect(organisation_prp_non_profit.group_member).to be_truthy
+ expect(organisation_prp_non_profit.group_member_id).to eq(organisation_prp_non_profit.id)
+
+ expect(organisation_prp_profit.group).to be_nil
+ expect(organisation_prp_profit.group_member).to be_falsy
+ expect(organisation_prp_profit.group_member_id).to be_nil
end
end
From 7a6131e5bcde51b796c0b91e48e95a46eeb34800 Mon Sep 17 00:00:00 2001
From: Samuel
Date: Mon, 19 May 2025 15:01:33 +0100
Subject: [PATCH 38/39] Make XML tests check unordered file contents
stops tests flakily failing on the order of cols which is not important in xml
---
.../lettings_log_export_service_spec.rb | 20 ++++++++---------
.../organisation_export_service_spec.rb | 10 ++++-----
.../exports/sales_log_export_service_spec.rb | 16 +++++++-------
.../exports/user_export_service_spec.rb | 10 ++++-----
spec/spec_helper.rb | 22 +++++++++++++++++++
5 files changed, 50 insertions(+), 28 deletions(-)
diff --git a/spec/services/exports/lettings_log_export_service_spec.rb b/spec/services/exports/lettings_log_export_service_spec.rb
index 600590e71..c2197c027 100644
--- a/spec/services/exports/lettings_log_export_service_spec.rb
+++ b/spec/services/exports/lettings_log_export_service_spec.rb
@@ -102,7 +102,7 @@ RSpec.describe Exports::LettingsLogExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_manifest_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_lettings_logs
@@ -113,7 +113,7 @@ RSpec.describe Exports::LettingsLogExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_data_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_lettings_logs
@@ -143,7 +143,7 @@ RSpec.describe Exports::LettingsLogExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_data_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_lettings_logs
@@ -188,7 +188,7 @@ RSpec.describe Exports::LettingsLogExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_data_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_lettings_logs
@@ -271,7 +271,7 @@ RSpec.describe Exports::LettingsLogExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_manifest_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_lettings_logs
@@ -390,7 +390,7 @@ RSpec.describe Exports::LettingsLogExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_manifest_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
expect(export_service.export_xml_lettings_logs).to eq({ expected_zip_filename.gsub(".zip", "") => start_time })
@@ -410,7 +410,7 @@ RSpec.describe Exports::LettingsLogExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_data_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_lettings_logs
@@ -441,7 +441,7 @@ RSpec.describe Exports::LettingsLogExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_data_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_lettings_logs
@@ -473,7 +473,7 @@ RSpec.describe Exports::LettingsLogExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_data_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_lettings_logs
@@ -504,7 +504,7 @@ RSpec.describe Exports::LettingsLogExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_data_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_lettings_logs(full_update: true)
end
diff --git a/spec/services/exports/organisation_export_service_spec.rb b/spec/services/exports/organisation_export_service_spec.rb
index 6ef66161a..03474de78 100644
--- a/spec/services/exports/organisation_export_service_spec.rb
+++ b/spec/services/exports/organisation_export_service_spec.rb
@@ -64,7 +64,7 @@ RSpec.describe Exports::OrganisationExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_manifest_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_organisations
@@ -75,7 +75,7 @@ RSpec.describe Exports::OrganisationExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_data_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_organisations
@@ -99,7 +99,7 @@ RSpec.describe Exports::OrganisationExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_data_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_organisations
@@ -118,7 +118,7 @@ RSpec.describe Exports::OrganisationExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_manifest_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_organisations
@@ -231,7 +231,7 @@ RSpec.describe Exports::OrganisationExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_manifest_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
expect(export_service.export_xml_organisations).to eq({ expected_zip_filename.gsub(".zip", "") => start_time })
diff --git a/spec/services/exports/sales_log_export_service_spec.rb b/spec/services/exports/sales_log_export_service_spec.rb
index c11c13712..15894ae3d 100644
--- a/spec/services/exports/sales_log_export_service_spec.rb
+++ b/spec/services/exports/sales_log_export_service_spec.rb
@@ -87,7 +87,7 @@ RSpec.describe Exports::SalesLogExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_manifest_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_sales_logs
@@ -98,7 +98,7 @@ RSpec.describe Exports::SalesLogExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_data_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_sales_logs
@@ -183,7 +183,7 @@ RSpec.describe Exports::SalesLogExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_manifest_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_sales_logs
@@ -304,7 +304,7 @@ RSpec.describe Exports::SalesLogExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_manifest_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
expect(export_service.export_xml_sales_logs).to eq({ expected_zip_filename.gsub(".zip", "") => start_time })
@@ -324,7 +324,7 @@ RSpec.describe Exports::SalesLogExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_data_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_sales_logs
@@ -355,7 +355,7 @@ RSpec.describe Exports::SalesLogExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_data_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_sales_logs(full_update: true, collection_year: 2024)
@@ -377,7 +377,7 @@ RSpec.describe Exports::SalesLogExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_data_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_sales_logs
@@ -414,7 +414,7 @@ RSpec.describe Exports::SalesLogExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_data_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_sales_logs
diff --git a/spec/services/exports/user_export_service_spec.rb b/spec/services/exports/user_export_service_spec.rb
index 93ba124d9..bafbc7f77 100644
--- a/spec/services/exports/user_export_service_spec.rb
+++ b/spec/services/exports/user_export_service_spec.rb
@@ -64,7 +64,7 @@ RSpec.describe Exports::UserExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_manifest_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_users
@@ -75,7 +75,7 @@ RSpec.describe Exports::UserExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_data_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_users
@@ -97,7 +97,7 @@ RSpec.describe Exports::UserExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_manifest_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_users
@@ -210,7 +210,7 @@ RSpec.describe Exports::UserExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_manifest_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
expect(export_service.export_xml_users).to eq({ expected_zip_filename.gsub(".zip", "") => start_time })
@@ -229,7 +229,7 @@ RSpec.describe Exports::UserExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_manifest_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
expect(export_service.export_xml_users).to eq({ expected_zip_filename.gsub(".zip", "") => start_time })
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 9677a128c..3a4a7daed 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -126,3 +126,25 @@ RSpec.configure do |config|
end
RSpec::Matchers.define_negated_matcher :not_change, :change
+
+def fix_xml_content_order(xml_file_content)
+ # split the content up my lines, sort them, then join together again
+ # this ensures the order of the output elements doesn't matter for the tests
+ # as the tests care about the content of the XML, not the order
+ xml_file_content.split("\n").sort.join("\n")
+end
+
+# write a matcher that checks if two strings are equal after apply fix_xml_content_order to both
+RSpec::Matchers.define :have_same_xml_contents_as do |expected|
+ match do |actual|
+ fix_xml_content_order(actual) == fix_xml_content_order(expected)
+ end
+
+ failure_message do |actual|
+ "expected that unordered #{fix_xml_content_order(actual)} would be equal to unordered #{fix_xml_content_order(expected)}"
+ end
+
+ failure_message_when_negated do |actual|
+ "expected that unordered #{fix_xml_content_order(actual)} would not be equal to unordered #{fix_xml_content_order(expected)}"
+ end
+end
From 6604fd56eb415902427debcefb48b300c66bc83a Mon Sep 17 00:00:00 2001
From: Samuel
Date: Mon, 19 May 2025 16:47:15 +0100
Subject: [PATCH 39/39] Fix final failing tests
---
spec/services/exports/organisation_export_service_spec.rb | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/spec/services/exports/organisation_export_service_spec.rb b/spec/services/exports/organisation_export_service_spec.rb
index 69be45c54..623cb359e 100644
--- a/spec/services/exports/organisation_export_service_spec.rb
+++ b/spec/services/exports/organisation_export_service_spec.rb
@@ -121,7 +121,7 @@ RSpec.describe Exports::OrganisationExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_data_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_organisations
@@ -145,7 +145,7 @@ RSpec.describe Exports::OrganisationExportService do
expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) do |_, content|
entry = Zip::File.open_buffer(content).find_entry(expected_data_filename)
expect(entry).not_to be_nil
- expect(entry.get_input_stream.read).to eq(expected_content)
+ expect(entry.get_input_stream.read).to have_same_xml_contents_as(expected_content)
end
export_service.export_xml_organisations