CLDC-3748 Check org name uniqueness (#2818)

* Check org name uniqueness

* Some tests

* Do not update org name

* Update some request specs

* Remove unnecessary updates

* Update error message

app/controllers/organisations_controller.rb

@@ -155,6 +155,7 @@ class OrganisationsController < ApplicationController
         end
         redirect_to details_organisation_path(@organisation)
       else
+        @used_rent_periods = @organisation.lettings_logs.pluck(:period).uniq.compact.map(&:to_s)
         @rent_periods = helpers.rent_periods_with_checked_attr(checked_periods: selected_rent_periods)
         render :edit, status: :unprocessable_entity
       end

app/models/organisation.rb

@@ -58,6 +58,7 @@ class Organisation < ApplicationRecord
   alias_method :la?, :LA?
 
   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") }
 
   def self.find_by_id_on_multiple_fields(id)

config/locales/en.yml

@@ -235,6 +235,7 @@ en:
     organisation:
       data_sharing_agreement_not_signed: "Your organisation must accept the Data Sharing Agreement before you can create any logs."
       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."
       stock_owner:
         blank: "You must choose a stock owner."

db/migrate/20241125153349_add_unique_index_to_org_name.rb

@@ -0,0 +1,5 @@
+class AddUniqueIndexToOrgName < ActiveRecord::Migration[7.0]
+  def change
+    add_index :organisations, :name, unique: true
+  end
+end

db/schema.rb

@@ -546,6 +546,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_12_04_100518) do
     t.datetime "discarded_at"
     t.datetime "schemes_deduplicated_at"
     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
   end
 

spec/fixtures/exports/general_needs_log.xml

@@ -147,10 +147,10 @@
     <duplicate_set_id/>
     <formid>{id}</formid>
     <owningorgid>{owning_org_id}</owningorgid>
-    <owningorgname>MHCLG</owningorgname>
+    <owningorgname>{owning_org_name}</owningorgname>
     <hcnum>1234</hcnum>
     <maningorgid>{managing_org_id}</maningorgid>
-    <maningorgname>MHCLG</maningorgname>
+    <maningorgname>{managing_org_name}</maningorgname>
     <manhcnum>1234</manhcnum>
     <createddate>2022-05-01T00:00:00+01:00</createddate>
     <uploaddate>2022-05-01T00:00:00+01:00</uploaddate>

spec/fixtures/exports/general_needs_log_23_24.xml

@@ -148,10 +148,10 @@
     <duplicate_set_id/>
     <formid>{id}</formid>
     <owningorgid>{owning_org_id}</owningorgid>
-    <owningorgname>MHCLG</owningorgname>
+    <owningorgname>{owning_org_name}</owningorgname>
     <hcnum>1234</hcnum>
     <maningorgid>{managing_org_id}</maningorgid>
-    <maningorgname>MHCLG</maningorgname>
+    <maningorgname>{managing_org_name}</maningorgname>
     <manhcnum>1234</manhcnum>
     <createddate>2023-04-03T00:00:00+01:00</createddate>
     <uploaddate>2023-04-03T00:00:00+01:00</uploaddate>

spec/fixtures/exports/general_needs_log_24_25.xml

@@ -161,10 +161,10 @@
     <la_as_entered>la as entered</la_as_entered>
     <formid>{id}</formid>
     <owningorgid>{owning_org_id}</owningorgid>
-    <owningorgname>MHCLG</owningorgname>
+    <owningorgname>{owning_org_name}</owningorgname>
     <hcnum>1234</hcnum>
     <maningorgid>{managing_org_id}</maningorgid>
-    <maningorgname>MHCLG</maningorgname>
+    <maningorgname>{managing_org_name}</maningorgname>
     <manhcnum>1234</manhcnum>
     <createddate>2024-04-03T00:00:00+01:00</createddate>
     <uploaddate>2024-04-03T00:00:00+01:00</uploaddate>

spec/fixtures/exports/organisation.xml

@@ -2,7 +2,7 @@
 <forms>
   <form>
     <id>{id}</id>
-    <name>MHCLG</name>
+    <name>{name}</name>
     <phone/>
     <provider_type>1</provider_type>
     <address_line1>2 Marsham Street</address_line1>

spec/fixtures/exports/supported_housing_logs.xml

@@ -146,10 +146,10 @@
     <duplicate_set_id/>
     <formid>{id}</formid>
     <owningorgid>{owning_org_id}</owningorgid>
-    <owningorgname>MHCLG</owningorgname>
+    <owningorgname>{owning_org_name}</owningorgname>
     <hcnum>1234</hcnum>
     <maningorgid>{managing_org_id}</maningorgid>
-    <maningorgname>MHCLG</maningorgname>
+    <maningorgname>{managing_org_name}</maningorgname>
     <manhcnum>1234</manhcnum>
     <createddate>2022-05-01T00:00:00+01:00</createddate>
     <uploaddate>2022-05-01T00:00:00+01:00</uploaddate>

spec/fixtures/exports/user.xml

@@ -12,6 +12,6 @@
     <is_dpo>false</is_dpo>
     <is_key_contact>false</is_key_contact>
     <active>true</active>
-    <organisation_name>MHCLG</organisation_name>
+    <organisation_name>{organisation_name}</organisation_name>
   </form>
 </forms>

spec/models/form/lettings/questions/managing_organisation_spec.rb

@@ -185,7 +185,7 @@ RSpec.describe Form::Lettings::Questions::ManagingOrganisation, type: :model do
     context "when organisation has merged" do
       let(:absorbing_org) { create(:organisation, name: "Absorbing org", holds_own_stock: true) }
       let!(:merged_org) { create(:organisation, name: "Merged org", holds_own_stock: false) }
-      let!(:merged_deleted_org) { create(:organisation, name: "Merged org", holds_own_stock: false, discarded_at: Time.zone.yesterday) }
+      let!(:merged_deleted_org) { create(:organisation, name: "Merged org 2", holds_own_stock: false, discarded_at: Time.zone.yesterday) }
       let(:user) { create(:user, :data_coordinator, organisation: absorbing_org) }
 
       let(:log) do

spec/models/organisation_spec.rb

@@ -19,6 +19,12 @@ RSpec.describe Organisation, type: :model do
         .to raise_error(ActiveRecord::RecordInvalid, "Validation failed: Provider type #{I18n.t('validations.organisation.provider_type_missing')}")
     end
 
+    it "validates uniqueness of name" do
+      org = build(:organisation, name: organisation.name.downcase)
+      org.valid?
+      expect(org.errors[:name]).to include(I18n.t("validations.organisation.name_not_unique"))
+    end
+
     context "with parent/child associations", :aggregate_failures do
       let!(:child_organisation) { create(:organisation, name: "MHCLG Child") }
       let!(:grandchild_organisation) { create(:organisation, name: "MHCLG Grandchild") }

spec/requests/organisation_relationships_controller_spec.rb

@@ -58,7 +58,7 @@ RSpec.describe OrganisationRelationshipsController, type: :request do
 
           context "when adding a stock owner" do
             let!(:active_organisation) { FactoryBot.create(:organisation, name: "Active Org", active: true) }
-            let!(:inactive_organisation) { FactoryBot.create(:organisation, name: "Inactive LTD", active: false) }
+            let!(:inactive_organisation) { FactoryBot.create(:organisation, name: "Inactive LTD 2", active: false) }
 
             before do
               get "/organisations/#{organisation.id}/stock-owners/add", headers:, params: {}
@@ -115,7 +115,7 @@ RSpec.describe OrganisationRelationshipsController, type: :request do
           let!(:managing_agent) { FactoryBot.create(:organisation) }
           let!(:other_org_managing_agent) { FactoryBot.create(:organisation, name: "Foobar LTD") }
           let!(:inactive_managing_agent) { FactoryBot.create(:organisation, name: "Inactive LTD", active: false) }
-          let!(:other_organisation) { FactoryBot.create(:organisation, name: "Foobar LTD") }
+          let!(:other_organisation) { FactoryBot.create(:organisation, name: "Foobar LTD 3") }
 
           before do
             FactoryBot.create(:organisation_relationship, parent_organisation: organisation, child_organisation: managing_agent)
@@ -316,7 +316,7 @@ RSpec.describe OrganisationRelationshipsController, type: :request do
         context "with an organisation that the user belongs to" do
           let!(:stock_owner) { FactoryBot.create(:organisation) }
           let!(:other_org_stock_owner) { FactoryBot.create(:organisation, name: "Foobar LTD") }
-          let!(:other_organisation) { FactoryBot.create(:organisation, name: "Foobar LTD") }
+          let!(:other_organisation) { FactoryBot.create(:organisation, name: "Foobar LTD 2") }
 
           before do
             FactoryBot.create(:organisation_relationship, child_organisation: organisation, parent_organisation: stock_owner)
@@ -452,7 +452,7 @@ RSpec.describe OrganisationRelationshipsController, type: :request do
         context "with an organisation that the user belongs to" do
           let!(:managing_agent) { FactoryBot.create(:organisation) }
           let!(:other_org_managing_agent) { FactoryBot.create(:organisation, name: "Foobar LTD") }
-          let!(:other_organisation) { FactoryBot.create(:organisation, name: "Foobar LTD") }
+          let!(:other_organisation) { FactoryBot.create(:organisation, name: "Foobar LTD 5") }
 
           before do
             FactoryBot.create(:organisation_relationship, parent_organisation: organisation, child_organisation: managing_agent)

spec/requests/organisations_controller_spec.rb

@@ -1555,7 +1555,10 @@ RSpec.describe OrganisationsController, type: :request do
           let(:total_organisations_count) { Organisation.all.count }
 
           before do
-            create_list(:organisation, 25)
+            build_list(:organisation, 25) do |organisation, index|
+              organisation.name = "Organisation #{index}"
+              organisation.save!
+            end
             get "/organisations"
           end
 
@@ -1644,7 +1647,10 @@ RSpec.describe OrganisationsController, type: :request do
               let(:search_param) { "MHCLG" }
 
               before do
-                create_list(:organisation, 27, name: "MHCLG")
+                build_list(:organisation, 27) do |organisation, index|
+                  organisation.name = "MHCLG #{index}"
+                  organisation.save!
+                end
                 get "/organisations?search=#{search_param}"
               end
 

spec/services/exports/lettings_log_export_service_spec.rb

@@ -21,7 +21,9 @@ RSpec.describe Exports::LettingsLogExportService do
   def replace_entity_ids(lettings_log, export_template)
     export_template.sub!(/\{id\}/, (lettings_log["id"] + Exports::LettingsLogExportService::LOG_ID_OFFSET).to_s)
     export_template.sub!(/\{owning_org_id\}/, (lettings_log["owning_organisation_id"] + Exports::LettingsLogExportService::LOG_ID_OFFSET).to_s)
+    export_template.sub!(/\{owning_org_name\}/, lettings_log.owning_organisation.name)
     export_template.sub!(/\{managing_org_id\}/, (lettings_log["managing_organisation_id"] + Exports::LettingsLogExportService::LOG_ID_OFFSET).to_s)
+    export_template.sub!(/\{managing_org_name\}/, lettings_log.managing_organisation.name)
     export_template.sub!(/\{location_id\}/, (lettings_log["location_id"]).to_s) if lettings_log.needstype == 2
     export_template.sub!(/\{scheme_id\}/, (lettings_log["scheme_id"]).to_s) if lettings_log.needstype == 2
     export_template.sub!(/\{log_id\}/, lettings_log["id"].to_s)

spec/services/exports/organisation_export_service_spec.rb

@@ -16,6 +16,7 @@ RSpec.describe Exports::OrganisationExportService do
 
   def replace_entity_ids(organisation, export_template)
     export_template.sub!(/\{id\}/, organisation["id"].to_s)
+    export_template.sub!(/\{name\}/, organisation["name"])
     export_template.sub!(/\{dsa_signed_at\}/, organisation.data_protection_confirmation&.signed_at.to_s)
     export_template.sub!(/\{dpo_email\}/, organisation.data_protection_confirmation&.data_protection_officer_email)
   end

spec/services/exports/user_export_service_spec.rb

@@ -17,6 +17,7 @@ RSpec.describe Exports::UserExportService do
   def replace_entity_ids(user, export_template)
     export_template.sub!(/\{id\}/, user["id"].to_s)
     export_template.sub!(/\{organisation_id\}/, user["organisation_id"].to_s)
+    export_template.sub!(/\{organisation_name\}/, user.organisation.name)
     export_template.sub!(/\{email\}/, user["email"].to_s)
   end