From 9f6c1d55ef2a55dbfa661f9ff1ddedfb86cc39fd Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Thu, 17 Apr 2025 17:33:05 +0100 Subject: [PATCH] CLDC-3980: Add ability to change organisation's name with a startdate (#3058) * Separate change organisations name from organisation edit view * Remove change_name action from organisations_controller for unauthorized access handling * Add migration for organisation name changes * Implement organisation name change feature with history tracking * Update organisation label methods to accept date parameter for accurate historical representation * Rename change_name view to new and move it for clarity and consistency * Refactor organisation name change logic to ensure visibility and uniqueness of change dates * Refactor organisation name changes to use integer for change_type and date for change_date * Update validation error message for duplicate change dates in organisation name changes * Update validation logic for change dates to handle immediate changes * Rename change_date to startdate in organisation name changes and update related logic * Make change_type nullable in organisation name changes table * Remove debug logging for organisation name change parameters * Update conditional question data-info to use startdate instead of scheduled_date * Bug fix - ensure startdate is set only if not already defined for immediate changes * Fix immediate_change parameter type casting in organisation name change params * Refactor name_changes_with_dates method to streamline fetching and status assignment * Enhance layout in new.html.erb by adding additional grid columns for improved structure * Update status assignment logic in organisation.rb to reflect active state when no changes exist * Add validation to ensure start date is before organisation's merge date if present * Drop organisation_name_changes table if it exists * Remove organisation_name_changes table from schema * Add organisation_name_changes table * Lint fixes * Lint fixes * Add FactoryBot definition for organisation_name_change * Add tests for organisation name changes * Enhance validations and add tests for organisation name changes * Lint * Add tests for OrganisationNameChangesController actions * Add comments in OrganisationNameChange * Refactor organisation name change validations to use I18n for error messages * Fix name method to allow nil date parameter and default to current time * Update organisation name retrieval to support date parameter in exports * Remove change_type attribute from organisation name changes * Lint fix * Update organisation status logic to account for future start dates of the first change * Refactor name history display into a partial for better code organization * Add cancel functionality for scheduled name changes with confirmation page * lint * lint --- .../lettings_log_summary_component.html.erb | 4 +- .../sales_log_summary_component.html.erb | 4 +- app/controllers/merge_requests_controller.rb | 5 +- .../organisation_name_changes_controller.rb | 50 +++++++++ app/helpers/organisations_helper.rb | 2 +- app/helpers/tag_helper.rb | 4 + .../questions/managing_organisation.rb | 4 +- .../form/lettings/questions/stock_owner.rb | 12 +-- .../sales/questions/managing_organisation.rb | 2 +- .../sales/questions/owning_organisation_id.rb | 2 +- app/models/merge_request.rb | 2 +- app/models/organisation.rb | 34 +++++- app/models/organisation_name_change.rb | 100 ++++++++++++++++++ .../exports/lettings_log_export_service.rb | 4 +- .../exports/organisation_export_service.rb | 15 ++- .../exports/sales_log_export_service.rb | 4 +- .../_name_history_list.html.erb | 27 +++++ .../cancel_confirmation.html.erb | 30 ++++++ .../organisation_name_changes/new.html.erb | 49 +++++++++ app/views/organisations/edit.html.erb | 4 - config/locales/en.yml | 14 ++- config/routes.rb | 4 + ...111741_create_organisation_name_changes.rb | 14 +++ db/schema.rb | 13 ++- ...ganisation_name_changes_controller_spec.rb | 46 ++++++++ spec/factories/organisation_name_change.rb | 17 +++ .../exports/organisation_new_name.xml | 27 +++++ spec/models/organisation_name_change_spec.rb | 96 +++++++++++++++++ spec/models/organisation_spec.rb | 32 ++++++ .../organisation_export_service_spec.rb | 48 ++++++++- 30 files changed, 636 insertions(+), 33 deletions(-) create mode 100644 app/controllers/organisation_name_changes_controller.rb create mode 100644 app/models/organisation_name_change.rb create mode 100644 app/views/organisation_name_changes/_name_history_list.html.erb create mode 100644 app/views/organisation_name_changes/cancel_confirmation.html.erb create mode 100644 app/views/organisation_name_changes/new.html.erb create mode 100644 db/migrate/20250416111741_create_organisation_name_changes.rb create mode 100644 spec/controllers/organisation_name_changes_controller_spec.rb create mode 100644 spec/factories/organisation_name_change.rb create mode 100644 spec/fixtures/exports/organisation_new_name.xml create mode 100644 spec/models/organisation_name_change_spec.rb diff --git a/app/components/lettings_log_summary_component.html.erb b/app/components/lettings_log_summary_component.html.erb index 7686e3559..47978f1a8 100644 --- a/app/components/lettings_log_summary_component.html.erb +++ b/app/components/lettings_log_summary_component.html.erb @@ -33,13 +33,13 @@ <% if log.owning_organisation %>
Owned by
-
<%= log.owning_organisation&.label %>
+
<%= log.owning_organisation&.label(date: log.startdate) %>
<% end %> <% if log.managing_organisation %>
Managed by
-
<%= log.managing_organisation&.label %>
+
<%= log.managing_organisation&.label(date: log.startdate) %>
<% end %> diff --git a/app/components/sales_log_summary_component.html.erb b/app/components/sales_log_summary_component.html.erb index 1d376aa4a..7e392c7bb 100644 --- a/app/components/sales_log_summary_component.html.erb +++ b/app/components/sales_log_summary_component.html.erb @@ -26,13 +26,13 @@ <% if log.owning_organisation %>
Owned by
-
<%= log.owning_organisation&.label %>
+
<%= log.owning_organisation&.label(date: log.startdate) %>
<% end %> <% if log.managing_organisation %>
Reported by
-
<%= log.managing_organisation&.label %>
+
<%= log.managing_organisation&.label(date: log.startdate) %>
<% end %> diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index bbb9c8ee9..32f7b2d0c 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -105,8 +105,9 @@ private answer_options = { "" => "Select an option" } if current_user.support? - Organisation.all.pluck(:id, :name).each do |organisation| - answer_options[organisation[0]] = organisation[1] + Organisation.all.each do |organisation| + date = @merge_request.merge_date || Time.zone.today + answer_options[organisation.id] = organisation.name(date:) end end diff --git a/app/controllers/organisation_name_changes_controller.rb b/app/controllers/organisation_name_changes_controller.rb new file mode 100644 index 000000000..48cf785cc --- /dev/null +++ b/app/controllers/organisation_name_changes_controller.rb @@ -0,0 +1,50 @@ +class OrganisationNameChangesController < ApplicationController + before_action :set_organisation, only: %i[create change_name] + before_action :set_previous_name_changes, only: %i[create change_name] + + def create + @organisation_name_change = @organisation.organisation_name_changes.new(organisation_name_change_params) + + if @organisation_name_change.save + notice_message = @organisation_name_change.immediate_change ? "Name change saved successfully." : "Name change scheduled for #{@organisation_name_change.formatted_startdate}." + redirect_to organisation_path(@organisation), notice: notice_message + else + render :new, status: :unprocessable_entity + end + end + + def change_name + @organisation_name_change = OrganisationNameChange.new + render :new, layout: "application" + end + + def cancel_confirmation + @organisation_name_change = OrganisationNameChange.find(params[:change_id]) + render :cancel_confirmation, layout: "application" + end + + def cancel + @organisation_name_change = OrganisationNameChange.find(params[:change_id]) + if @organisation_name_change.update_column(:discarded_at, Time.zone.today) + redirect_to organisation_path(@organisation_name_change.organisation), notice: "The scheduled name change has been successfully cancelled." + else + redirect_to organisation_path(@organisation_name_change.organisation), notice: "Failed to cancel the scheduled name change." + end + end + +private + + def set_organisation + @organisation = Organisation.find(params[:id]) + end + + def set_previous_name_changes + @previous_name_changes = @organisation.name_changes_with_dates + end + + def organisation_name_change_params + params.require(:organisation_name_change).permit(:name, :startdate, :immediate_change).tap do |whitelisted| + whitelisted[:immediate_change] = ActiveModel::Type::Boolean.new.cast(whitelisted[:immediate_change]) + end + end +end diff --git a/app/helpers/organisations_helper.rb b/app/helpers/organisations_helper.rb index 19c77b357..1467d7c2e 100644 --- a/app/helpers/organisations_helper.rb +++ b/app/helpers/organisations_helper.rb @@ -30,7 +30,7 @@ module OrganisationsHelper if user.support? row.with_action( visually_hidden_text: organisation.name.humanize.downcase, - href: edit_organisation_path(organisation), + href: change_name_organisation_path(organisation), html_attributes: { "data-qa": "change-#{organisation.name.downcase}" }, ) else diff --git a/app/helpers/tag_helper.rb b/app/helpers/tag_helper.rb index 110198550..5e1d3422e 100644 --- a/app/helpers/tag_helper.rb +++ b/app/helpers/tag_helper.rb @@ -7,10 +7,12 @@ module TagHelper in_progress: "In progress", completed: "Completed", active: "Active", + inactive: "Inactive", incomplete: "Incomplete", deactivating_soon: "Deactivating soon", activating_soon: "Activating soon", reactivating_soon: "Reactivating soon", + scheduled: "Scheduled", deactivated: "Deactivated", deleted: "Deleted", merged: "Merged", @@ -35,10 +37,12 @@ module TagHelper in_progress: "blue", completed: "green", active: "green", + inactive: "grey", incomplete: "red", deactivating_soon: "yellow", activating_soon: "blue", reactivating_soon: "blue", + scheduled: "blue", deactivated: "grey", deleted: "red", merged: "orange", diff --git a/app/models/form/lettings/questions/managing_organisation.rb b/app/models/form/lettings/questions/managing_organisation.rb index e415837fe..9c766ebe0 100644 --- a/app/models/form/lettings/questions/managing_organisation.rb +++ b/app/models/form/lettings/questions/managing_organisation.rb @@ -15,7 +15,7 @@ class Form::Lettings::Questions::ManagingOrganisation < ::Form::Question return opts unless log if log.managing_organisation.present? - org_value = log.managing_organisation.label + org_value = log.managing_organisation.label(date: log.startdate) opts = opts.merge({ log.managing_organisation.id => org_value }) end @@ -74,7 +74,7 @@ class Form::Lettings::Questions::ManagingOrganisation < ::Form::Question organisation = Organisation.find_by(id: log.managing_organisation_id) return unless organisation - organisation.label + organisation.label(date: log.startdate) end private diff --git a/app/models/form/lettings/questions/stock_owner.rb b/app/models/form/lettings/questions/stock_owner.rb index 6fc418b62..466b08e3f 100644 --- a/app/models/form/lettings/questions/stock_owner.rb +++ b/app/models/form/lettings/questions/stock_owner.rb @@ -15,7 +15,7 @@ class Form::Lettings::Questions::StockOwner < ::Form::Question return answer_opts unless log if log.owning_organisation_id.present? - org_value = log.owning_organisation.label + org_value = log.owning_organisation.label(date: log.startdate) answer_opts[log.owning_organisation.id] = org_value end @@ -31,19 +31,19 @@ class Form::Lettings::Questions::StockOwner < ::Form::Question if user.support? Organisation.visible.filter_by_active.where(holds_own_stock: true).find_each do |org| if org.merge_date.present? - answer_opts[org.id] = "#{org.name} (inactive as of #{org.merge_date.to_fs(:govuk_date)})" if org.merge_date >= FormHandler.instance.start_date_of_earliest_open_for_editing_collection_period + answer_opts[org.id] = "#{org.name(date: org.merge_date)} (inactive as of #{org.merge_date.to_fs(:govuk_date)})" if org.merge_date >= FormHandler.instance.start_date_of_earliest_open_for_editing_collection_period elsif org.absorbed_organisations.merged_during_open_collection_period.exists? && org.available_from.present? - answer_opts[org.id] = "#{org.name} (active as of #{org.available_from.to_fs(:govuk_date)})" + answer_opts[org.id] = "#{org.name(date: org.available_from)} (active as of #{org.available_from.to_fs(:govuk_date)})" else - answer_opts[org.id] = org.name + answer_opts[org.id] = org.name(date: log.startdate) end end else user.organisation.stock_owners.visible.filter_by_active.each do |stock_owner| - answer_opts[stock_owner.id] = stock_owner.name + answer_opts[stock_owner.id] = stock_owner.name(date: log.startdate) end recently_absorbed_organisations.visible.each do |absorbed_org| - answer_opts[absorbed_org.id] = merged_organisation_label(absorbed_org.name, absorbed_org.merge_date) if absorbed_org.holds_own_stock? + answer_opts[absorbed_org.id] = merged_organisation_label(absorbed_org.name(date: log.startdate), absorbed_org.merge_date) if absorbed_org.holds_own_stock? end end diff --git a/app/models/form/sales/questions/managing_organisation.rb b/app/models/form/sales/questions/managing_organisation.rb index c5cf8f112..ffe143ab5 100644 --- a/app/models/form/sales/questions/managing_organisation.rb +++ b/app/models/form/sales/questions/managing_organisation.rb @@ -15,7 +15,7 @@ class Form::Sales::Questions::ManagingOrganisation < ::Form::Question return opts unless log if log.managing_organisation.present? - org_value = log.managing_organisation.label + org_value = log.managing_organisation.label(date: log.startdate) opts = opts.merge({ log.managing_organisation.id => org_value }) end diff --git a/app/models/form/sales/questions/owning_organisation_id.rb b/app/models/form/sales/questions/owning_organisation_id.rb index 6ff940865..d4ece8554 100644 --- a/app/models/form/sales/questions/owning_organisation_id.rb +++ b/app/models/form/sales/questions/owning_organisation_id.rb @@ -15,7 +15,7 @@ class Form::Sales::Questions::OwningOrganisationId < ::Form::Question return answer_opts unless log if log.owning_organisation_id.present? - org_value = log.owning_organisation.label + org_value = log.owning_organisation.label(date: log.startdate) answer_opts[log.owning_organisation.id] = org_value end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 47ffc84dc..725160afb 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -25,7 +25,7 @@ class MergeRequest < ApplicationRecord } def absorbing_organisation_name - absorbing_organisation&.name || "" + absorbing_organisation&.name(date: merge_date) || "" end def dpo_user diff --git a/app/models/organisation.rb b/app/models/organisation.rb index c3d0a8ca0..5fbce5716 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -8,6 +8,7 @@ class Organisation < ApplicationRecord has_many :parent_organisations, through: :parent_organisation_relationships has_many :child_organisation_relationships, foreign_key: :parent_organisation_id, class_name: "OrganisationRelationship" has_many :child_organisations, through: :child_organisation_relationships + has_many :organisation_name_changes, dependent: :destroy has_many :stock_owner_relationships, foreign_key: :child_organisation_id, class_name: "OrganisationRelationship" has_many :stock_owners, through: :stock_owner_relationships, source: :parent_organisation @@ -71,6 +72,12 @@ class Organisation < ApplicationRecord end end + def name(date: nil) + date ||= Time.zone.now + name_change = organisation_name_changes.visible.find { |change| change.includes_date?(date) } + name_change&.name || read_attribute(:name) + end + def can_be_managed_by?(organisation:) organisation == self || managing_agents.include?(organisation) end @@ -191,8 +198,31 @@ class Organisation < ApplicationRecord update!(discarded_at: Time.zone.now) end - def label - status == :deleted ? "#{name} (deleted)" : name + def label(date:) + date ||= Time.zone.now + status == :deleted ? "#{name(date:)} (deleted)" : name(date:) + end + + def name_changes_with_dates + changes = organisation_name_changes.visible.order(:startdate).map do |change| + { + id: change.id, + name: change.name, + start_date: change.startdate, + end_date: change.end_date, + status: change.status, + } + end + + changes.unshift( + id: nil, + name: self[:name], + start_date: created_at, + end_date: changes.first&.dig(:start_date)&.yesterday, + status: changes.empty? || Time.zone.now.to_date < changes.first[:start_date] ? "active" : "inactive", + ) + + changes end def has_visible_users? diff --git a/app/models/organisation_name_change.rb b/app/models/organisation_name_change.rb new file mode 100644 index 000000000..09b30efe3 --- /dev/null +++ b/app/models/organisation_name_change.rb @@ -0,0 +1,100 @@ +class OrganisationNameChange < ApplicationRecord + belongs_to :organisation + + scope :visible, -> { where(discarded_at: nil) } # We could add the ability to 'delete' scheduled name changes by using discarded_at + scope :before_date, ->(date) { where("startdate < ?", date) } + scope :after_date, ->(date) { where("startdate > ?", date) } + + validates :name, presence: { message: I18n.t("validations.organisation.name_changes.name.blank") } + validates :startdate, presence: { message: I18n.t("validations.organisation.name_changes.startdate.blank") }, unless: -> { immediate_change } + validate :startdate_must_be_after_last_change + validate :name_must_be_different_from_current + validate :startdate_must_be_unique_for_organisation + validate :startdate_must_be_before_merge_date + + attr_accessor :immediate_change + + before_validation :set_startdate_if_immediate + + has_paper_trail + + def status + if startdate > Time.zone.now.to_date + "scheduled" + elsif end_date.nil? || end_date >= Time.zone.now.to_date + "active" + else + "inactive" + end + end + + def includes_date?(date) + startdate <= date.to_date && (!next_change&.startdate || next_change.startdate > date.to_date) + end + + def next_change + organisation.organisation_name_changes.visible.where("startdate > ?", startdate).order(startdate: :asc).first + end + + def end_date + next_change&.startdate&.yesterday + end + + def previous_change + organisation.organisation_name_changes.visible.where("startdate < ?", startdate).order(startdate: :desc).first + end + + def active?(date = Time.zone.now) + includes_date?(date) + end + + def formatted_startdate(format = :govuk_date) + startdate.to_formatted_s(format) + end + +private + + def set_startdate_if_immediate + self.startdate ||= Time.zone.now if immediate_change + end + + def startdate_must_be_after_last_change + return if startdate.blank? + + last_startdate = organisation.organisation_name_changes + .visible + .where("startdate < ?", startdate) + .order(startdate: :desc) + .first&.startdate + + if last_startdate && startdate <= last_startdate + errors.add(:startdate, I18n.t("validations.organisation.name_changes.startdate.must_be_after_last_change", last_startdate:)) + end + end + + def startdate_must_be_unique_for_organisation + return if startdate.blank? + + if organisation.organisation_name_changes.visible.select(&:persisted?).any? { |record| record.startdate == startdate } + errors.add(:startdate, I18n.t("validations.organisation.name_changes.startdate.cannot_be_the_same_as_another_change")) unless immediate_change + errors.add(:immediate_change, I18n.t("validations.organisation.name_changes.immediate_change.cannot_be_the_same_as_another_change")) if immediate_change + end + end + + def name_must_be_different_from_current + return if name.blank? || startdate.blank? + + if name == organisation.name(date: startdate) + errors.add(:name, I18n.t("validations.organisation.name_changes.name.must_be_different")) + end + end + + def startdate_must_be_before_merge_date + return if startdate.blank? || organisation.merge_date.blank? + + if startdate >= organisation.merge_date + errors.add(:startdate, I18n.t("validations.organisation.name_changes.startdate.must_be_before_merge_date", merge_date: organisation.merge_date.to_formatted_s(:govuk_date))) unless immediate_change + errors.add(:immediate_change, I18n.t("validations.organisation.name_changes.immediate_change.must_be_before_merge_date", merge_date: organisation.merge_date.to_formatted_s(:govuk_date))) if immediate_change + end + end +end diff --git a/app/services/exports/lettings_log_export_service.rb b/app/services/exports/lettings_log_export_service.rb index c52bf8635..1ad44cbe4 100644 --- a/app/services/exports/lettings_log_export_service.rb +++ b/app/services/exports/lettings_log_export_service.rb @@ -57,12 +57,12 @@ module Exports # Organisation fields if lettings_log.owning_organisation attribute_hash["owningorgid"] = lettings_log.owning_organisation.old_visible_id || (lettings_log.owning_organisation.id + LOG_ID_OFFSET) - attribute_hash["owningorgname"] = lettings_log.owning_organisation.name + attribute_hash["owningorgname"] = lettings_log.owning_organisation.name(date: lettings_log.startdate) attribute_hash["hcnum"] = lettings_log.owning_organisation.housing_registration_no end if lettings_log.managing_organisation attribute_hash["maningorgid"] = lettings_log.managing_organisation.old_visible_id || (lettings_log.managing_organisation.id + LOG_ID_OFFSET) - attribute_hash["maningorgname"] = lettings_log.managing_organisation.name + attribute_hash["maningorgname"] = lettings_log.managing_organisation.name(date: lettings_log.startdate) attribute_hash["manhcnum"] = lettings_log.managing_organisation.housing_registration_no end diff --git a/app/services/exports/organisation_export_service.rb b/app/services/exports/organisation_export_service.rb index 8ceba93a9..7bceca840 100644 --- a/app/services/exports/organisation_export_service.rb +++ b/app/services/exports/organisation_export_service.rb @@ -28,10 +28,20 @@ module Exports def retrieve_resources(recent_export, full_update, _year) if !full_update && recent_export params = { from: recent_export.started_at, to: @start_time } - Organisation.where("(updated_at >= :from AND updated_at <= :to)", params) + + Organisation + .where(updated_at: params[:from]..params[:to]) + .or( + Organisation.where(id: OrganisationNameChange.where(created_at: params[:from]..params[:to]).select(:organisation_id)), + ) else params = { to: @start_time } - Organisation.where("updated_at <= :to", params) + + Organisation + .where("updated_at <= :to", params) + .or( + Organisation.where(id: OrganisationNameChange.where("created_at <= :to", params).select(:organisation_id)), + ) end end @@ -56,6 +66,7 @@ module Exports def apply_cds_transformation(organisation) attribute_hash = organisation.attributes + attribute_hash["name"] = organisation.name(date: Time.zone.now) attribute_hash["deleted_at"] = organisation.discarded_at&.iso8601 attribute_hash["dsa_signed"] = organisation.data_protection_confirmed? attribute_hash["dsa_signed_at"] = organisation.data_protection_confirmation&.signed_at&.iso8601 diff --git a/app/services/exports/sales_log_export_service.rb b/app/services/exports/sales_log_export_service.rb index fa806f6ab..9cac14207 100644 --- a/app/services/exports/sales_log_export_service.rb +++ b/app/services/exports/sales_log_export_service.rb @@ -57,9 +57,9 @@ module Exports attribute_hash["amendedbyid"] = sales_log.updated_by_id attribute_hash["owningorgid"] = sales_log.owning_organisation&.id - attribute_hash["owningorgname"] = sales_log.owning_organisation&.name + attribute_hash["owningorgname"] = sales_log.owning_organisation&.name(date: sales_log.saledate) attribute_hash["maningorgid"] = sales_log.managing_organisation&.id - attribute_hash["maningorgname"] = sales_log.managing_organisation&.name + attribute_hash["maningorgname"] = sales_log.managing_organisation&.name(date: sales_log.saledate) attribute_hash["creationmethod"] = sales_log.creation_method_before_type_cast attribute_hash["bulkuploadid"] = sales_log.bulk_upload_id diff --git a/app/views/organisation_name_changes/_name_history_list.html.erb b/app/views/organisation_name_changes/_name_history_list.html.erb new file mode 100644 index 000000000..dc7d604aa --- /dev/null +++ b/app/views/organisation_name_changes/_name_history_list.html.erb @@ -0,0 +1,27 @@ +<%= govuk_details(summary_text: "View name history") do %> + <%= govuk_table do |table| %> + <%= table.with_head do |head| %> + <% head.with_row do |row| %> + <% row.with_cell(header: true, text: "Name") %> + <% row.with_cell(header: true, text: "Start Date") %> + <% row.with_cell(header: true, text: "End Date") %> + <% row.with_cell(header: true, text: "Status") %> + <% end %> + <% end %> + <% @previous_name_changes.each do |change| %> + <%= table.with_body do |body| %> + <% body.with_row do |row| %> + <% row.with_cell(text: change[:name]) %> + <% row.with_cell(text: change[:start_date]&.to_formatted_s(:govuk_date)) %> + <% row.with_cell(text: change[:end_date]&.to_formatted_s(:govuk_date) || "None") %> + <% row.with_cell do %> + <%= status_tag(change[:status].to_sym, ["govuk-!-margin-right-2 govuk-!-margin-bottom-1"]) %> + <% if change[:status] == "scheduled" && change[:id].present? %> + <%= govuk_link_to "Cancel", cancel_name_change_confirmation_organisation_url(change_id: change[:id]), class: "app-red-link app-red-link---no-visited-state" %> + <% end %> + <% end %> + <% end %> + <% end %> + <% end %> + <% end %> +<% end %> diff --git a/app/views/organisation_name_changes/cancel_confirmation.html.erb b/app/views/organisation_name_changes/cancel_confirmation.html.erb new file mode 100644 index 000000000..442de1c41 --- /dev/null +++ b/app/views/organisation_name_changes/cancel_confirmation.html.erb @@ -0,0 +1,30 @@ +<% content_for :before_content do %> + <% content_for :title, "Are you sure you want to cancel this name change?" %> + <%= govuk_back_link(href: change_name_organisation_path(@organisation_name_change.organisation)) %> +<% end %> + +
+
+ Cancel scheduled name change +

+ <%= content_for(:title) %> +

+ + <%= govuk_warning_text(text: "You will not be able to undo this action.") %> + +
+ This name change would have updated the organisation name from + <%= @organisation_name_change.previous_change.is_a?(OrganisationNameChange) ? @organisation_name_change.previous_change.name : @organisation_name_change.organisation.name %> + to <%= @organisation_name_change.name %>. +
+ +
+ <%= govuk_button_to( + "Cancel", + cancel_name_change_organisation_path(@organisation_name_change), + method: :delete, + ) %> + <%= govuk_button_link_to "Back", change_name_organisation_path(@organisation_name_change.organisation), secondary: true %> +
+
+
diff --git a/app/views/organisation_name_changes/new.html.erb b/app/views/organisation_name_changes/new.html.erb new file mode 100644 index 000000000..075d70eb4 --- /dev/null +++ b/app/views/organisation_name_changes/new.html.erb @@ -0,0 +1,49 @@ +<% content_for :title, "Change #{@organisation.name}’s name" %> + +<% content_for :before_content do %> + <%= govuk_back_link(href: details_organisation_path(@organisation)) %> +<% end %> + +<%= form_for(@organisation_name_change, url: change_name_organisation_path(@organisation), html: { method: :post }) do |f| %> +
+
+ <%= f.govuk_error_summary %> + +

+ <%= content_for(:title) %> +

+ + <%= f.govuk_text_field :name, autocomplete: "name", label: { text: "Enter new name", size: "m" }, value: @organisation.name %> +
+ +
+ <%= render partial: "organisation_name_changes/name_history_list" %> +
+ +
+ <%= f.govuk_radio_buttons_fieldset :immediate_change, + legend: { text: "Does this change take effect starting today?", size: "m" } do %> + <%= f.govuk_radio_button :immediate_change, "true", label: { text: "Yes" } %> + <%= f.govuk_radio_button :immediate_change, "false", + label: { text: "No" }, + "data-controller": "conditional-question", + "data-action": "click->conditional-question#displayConditional", + "data-info": { conditional_questions: { startdate: [false] } }.to_json do %> + <%= render partial: "components/date_picker", locals: { + resource: @organisation_name_change, + question_id: :startdate, + legend: { text: "Set start date", size: "m" }, + resource_type: "organisation_name_change", + hint: "For example, 13/9/2025", + f:, + } %> + <% end %> + <% end %> + +
+ <%= f.govuk_submit "Save changes" %> + <%= govuk_button_link_to "Cancel", details_organisation_path(@organisation), secondary: true %> +
+
+
+<% end %> diff --git a/app/views/organisations/edit.html.erb b/app/views/organisations/edit.html.erb index b0bd05bf6..cf9033180 100644 --- a/app/views/organisations/edit.html.erb +++ b/app/views/organisations/edit.html.erb @@ -11,10 +11,6 @@ <%= content_for(:title) %> - <% if current_user.support? %> - <%= f.govuk_text_field :name, autocomplete: "name", label: { size: "m" } %> - <% end %> - <%= f.govuk_text_field :address_line1, label: { text: "Address line 1", size: "m" }, autocomplete: "address-line1" %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 086a26923..bcdd1cc88 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -229,12 +229,23 @@ en: already_added: "You have already added this managing agent." merged: "That organisation has already been merged. Select a different organisation." scheme_duplicates_not_resolved: "You must resolve all duplicates or indicate that there are no duplicates" + name_changes: + name: + blank: "New name is required and cannot be left blank." + must_be_different: "New name must be different from the current name on the change date." + startdate: + blank: "Start date must be provided unless this is an immediate change." + must_be_after_last_change: "Start date must be after the last change date (%{last_startdate})." + cannot_be_the_same_as_another_change: "Start date cannot be the same as another name change." + must_be_before_merge_date: "Start date must be earlier than the organisation's merge date (%{merge_date}). You cannot make changes to the name of an organisation after it has merged." + immediate_change: + cannot_be_the_same_as_another_change: "Start date cannot be the same as another name change." + must_be_before_merge_date: "Start date must be earlier than the organisation's merge date (%{merge_date}). You cannot make changes to the name of an organisation after it has merged." not_answered: "You must answer %{question}" not_number: "%{field} must be a number." invalid_option: "Enter a valid value for %{question}" invalid_number: "Enter a number for %{question}" no_address_found: "We could not find this address. Check the address data in your CSV file is correct and complete, or select the correct address using the CORE site." - date: outside_collection_window: "Enter a date within the %{year_combo} collection year, which is between 1st April %{start_year} and 31st March %{end_year}." postcode: "Enter a postcode in the correct format, for example AA1 1AA." @@ -361,6 +372,7 @@ en: before_deactivation: "This location was deactivated on %{date}. The reactivation date must be on or after deactivation date." deactivation: during_deactivated_period: "The location is already deactivated during this date, please enter a different date." + merge_request: organisation_part_of_another_merge: "This organisation is part of another merge - select a different one." organisation_part_of_another_incomplete_merge: "Another merge request records %{organisation} as merging into %{absorbing_organisation} on %{merge_date}. Select another organisation or remove this organisation from the other merge request." diff --git a/config/routes.rb b/config/routes.rb index 6b1b6458b..753d2a119 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -156,6 +156,10 @@ Rails.application.routes.draw do get "details", to: "organisations#details" get "data-sharing-agreement", to: "organisations#data_sharing_agreement" post "data-sharing-agreement", to: "organisations#confirm_data_sharing_agreement" + get "change-name", to: "organisation_name_changes#change_name", as: "change_name" + post "change-name", to: "organisation_name_changes#create" + get "cancel-name-change/:change_id", to: "organisation_name_changes#cancel_confirmation", as: "cancel_name_change_confirmation" + delete "cancel-name-change/:change_id", to: "organisation_name_changes#cancel", as: "cancel_name_change" get "users", to: "organisations#users" get "lettings-logs", to: "organisations#lettings_logs" diff --git a/db/migrate/20250416111741_create_organisation_name_changes.rb b/db/migrate/20250416111741_create_organisation_name_changes.rb new file mode 100644 index 000000000..f6caf8562 --- /dev/null +++ b/db/migrate/20250416111741_create_organisation_name_changes.rb @@ -0,0 +1,14 @@ +class CreateOrganisationNameChanges < ActiveRecord::Migration[7.0] + def change + create_table :organisation_name_changes do |t| + t.references :organisation, null: false, foreign_key: true + t.string :name, null: false + t.date :startdate, null: false + t.date :discarded_at + + t.timestamps + end + + add_index :organisation_name_changes, %i[organisation_id startdate discarded_at], unique: true, name: "index_org_name_changes_on_org_id_startdate_discarded_at" + end +end diff --git a/db/schema.rb b/db/schema.rb index e29560461..833703534 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_03_05_092900) do +ActiveRecord::Schema[7.2].define(version: 2025_04_16_111741) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -497,6 +497,17 @@ ActiveRecord::Schema[7.2].define(version: 2025_03_05_092900) do t.boolean "show_additional_page" end + create_table "organisation_name_changes", force: :cascade do |t| + t.bigint "organisation_id", null: false + t.string "name", null: false + t.date "startdate", null: false + t.date "discarded_at" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["organisation_id", "startdate"], name: "index_org_name_changes_on_org_id_and_startdate", unique: true + t.index ["organisation_id"], name: "index_organisation_name_changes_on_organisation_id" + end + create_table "organisation_relationships", force: :cascade do |t| t.integer "child_organisation_id" t.integer "parent_organisation_id" diff --git a/spec/controllers/organisation_name_changes_controller_spec.rb b/spec/controllers/organisation_name_changes_controller_spec.rb new file mode 100644 index 000000000..dbfd6d3f2 --- /dev/null +++ b/spec/controllers/organisation_name_changes_controller_spec.rb @@ -0,0 +1,46 @@ +require "rails_helper" + +RSpec.describe OrganisationNameChangesController, type: :controller do + let(:organisation) { create(:organisation) } + + describe "GET #change_name" do + it "assigns previous name changes" do + create(:organisation_name_change, organisation:, name: "Old Name", startdate: 1.day.ago) + get :change_name, params: { id: organisation.id } + expect(controller.instance_variable_get(:@previous_name_changes)).to eq(organisation.name_changes_with_dates) + end + end + + describe "POST #create" do + let(:params) do + { + organisation_name_change: { + name: "New Name", + startdate: 1.day.from_now, + immediate_change: false, + }, + } + end + + it "creates a new organisation name change with valid params" do + expect { + post :create, params: { id: organisation.id }.merge(params) + }.to change(OrganisationNameChange, :count).by(1) + + expect(response).to redirect_to(organisation_path(organisation)) + expect(flash[:notice]).to eq("Name change scheduled for #{1.day.from_now.to_date.to_formatted_s(:govuk_date)}.") + end + + it "creates an immediate name change when immediate_change is true" do + params[:organisation_name_change][:immediate_change] = true + params[:organisation_name_change].delete(:startdate) + + expect { + post :create, params: { id: organisation.id }.merge(params) + }.to change(OrganisationNameChange, :count).by(1) + + expect(response).to redirect_to(organisation_path(organisation)) + expect(flash[:notice]).to eq("Name change saved successfully.") + end + end +end diff --git a/spec/factories/organisation_name_change.rb b/spec/factories/organisation_name_change.rb new file mode 100644 index 000000000..1d90e2069 --- /dev/null +++ b/spec/factories/organisation_name_change.rb @@ -0,0 +1,17 @@ +FactoryBot.define do + factory :organisation_name_change do + association :organisation + name { "#{Faker::Name.name} Housing Org" } + immediate_change { true } + startdate { Time.zone.tomorrow } + + trait :future_change do + immediate_change { false } + startdate { 5.days.from_now } + end + + trait :merge_change do + change_type { :merge } + end + end +end diff --git a/spec/fixtures/exports/organisation_new_name.xml b/spec/fixtures/exports/organisation_new_name.xml new file mode 100644 index 000000000..d9ffedab0 --- /dev/null +++ b/spec/fixtures/exports/organisation_new_name.xml @@ -0,0 +1,27 @@ + + +
+ {id} + MHCLG New + + 1 + 2 Marsham Street + London + SW1P 4DF + true + true + 1234 + + + + + + + true + {dsa_signed_at} + {dpo_email} + + + active + +
 diff --git a/spec/models/organisation_name_change_spec.rb b/spec/models/organisation_name_change_spec.rb new file mode 100644 index 000000000..db32874bd --- /dev/null +++ b/spec/models/organisation_name_change_spec.rb @@ -0,0 +1,96 @@ +require "rails_helper" + +RSpec.describe OrganisationNameChange, type: :model do + let(:organisation) { create(:organisation) } + + describe "validations" do + it "is valid with valid attributes" do + name_change = build(:organisation_name_change, organisation:) + expect(name_change).to be_valid + end + + it "is invalid without a name" do + name_change = build(:organisation_name_change, organisation:, name: nil) + expect(name_change).not_to be_valid + expect(name_change.errors[:name]).to include(I18n.t("validations.organisation.name_changes.name.blank")) + end + + it "is invalid without a startdate if not immediate" do + name_change = build(:organisation_name_change, organisation:, startdate: nil, immediate_change: false) + expect(name_change).not_to be_valid + expect(name_change.errors[:startdate]).to include(I18n.t("validations.organisation.name_changes.startdate.blank")) + end + + it "is invalid if startdate is not unique for the organisation" do + create(:organisation_name_change, organisation:, startdate: Time.zone.tomorrow) + name_change = build(:organisation_name_change, organisation:, immediate_change: false, startdate: Time.zone.tomorrow) + expect(name_change).not_to be_valid + expect(name_change.errors[:startdate]).to include(I18n.t("validations.organisation.name_changes.startdate.cannot_be_the_same_as_another_change")) + end + + it "is invalid if name is the same as the current name on the change date" do + create(:organisation_name_change, organisation:, name: "New Name", startdate: 1.day.ago) + name_change = build(:organisation_name_change, organisation:, name: "New Name", startdate: Time.zone.now) + expect(name_change).not_to be_valid + expect(name_change.errors[:name]).to include(I18n.t("validations.organisation.name_changes.name.must_be_different")) + end + + it "is invalid if startdate is after the organisation's merge date" do + organisation.update!(merge_date: Time.zone.now) + name_change = build(:organisation_name_change, organisation:, immediate_change: false, startdate: Time.zone.tomorrow) + expect(name_change).not_to be_valid + expect(name_change.errors[:startdate]).to include(I18n.t("validations.organisation.name_changes.startdate.must_be_before_merge_date", merge_date: organisation.merge_date.to_formatted_s(:govuk_date))) + end + end + + describe "scopes" do + let!(:visible_change) { create(:organisation_name_change, :future_change, organisation:) } + let!(:discarded_change) { create(:organisation_name_change, organisation:, discarded_at: Time.zone.now) } + + it "returns only visible changes" do + expect(described_class.visible).to include(visible_change) + expect(described_class.visible).not_to include(discarded_change) + end + + it "returns changes before a specific date" do + name_change = create(:organisation_name_change, organisation:, startdate: 1.day.ago) + expect(described_class.before_date(Time.zone.now)).to include(name_change) + end + + it "returns changes after a specific date" do + name_change = create(:organisation_name_change, organisation:, startdate: 2.days.from_now) + expect(described_class.after_date(Time.zone.now)).to include(name_change) + end + end + + describe "#status" do + it "returns 'scheduled' if the startdate is in the future" do + name_change = build(:organisation_name_change, organisation:, startdate: 1.day.from_now) + expect(name_change.status).to eq("scheduled") + end + + it "returns 'active' if the startdate is today or in the past and end_date is nil or in the future" do + name_change = build(:organisation_name_change, organisation:, startdate: 1.day.ago) + expect(name_change.status).to eq("active") + end + + it "returns 'inactive' if the end_date is in the past" do + name_change = create(:organisation_name_change, organisation:, startdate: 2.days.ago) + allow(name_change).to receive(:end_date).and_return(1.day.ago) + expect(name_change.status).to eq("inactive") + end + end + + describe "#includes_date?" do + it "returns true if the date is within the change period" do + name_change = create(:organisation_name_change, organisation:, startdate: 1.day.ago) + expect(name_change.includes_date?(Time.zone.now)).to be true + end + + it "returns false if the date is outside the change period" do + name_change = create(:organisation_name_change, organisation:, startdate: 2.days.ago) + create(:organisation_name_change, organisation:, startdate: 1.day.from_now) + expect(name_change.includes_date?(2.days.from_now)).to be false + end + end +end diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb index b117feef7..402aa57c7 100644 --- a/spec/models/organisation_spec.rb +++ b/spec/models/organisation_spec.rb @@ -332,4 +332,36 @@ RSpec.describe Organisation, type: :model do end end end + + describe "organisation name changes" do + let(:organisation) { create(:organisation, name: "Original Name") } + + before do + create(:organisation_name_change, organisation:, name: "New Name", immediate_change: false, startdate: 1.day.ago) + end + + context "when checking the name before the change" do + it "returns the original name" do + expect(organisation.name(date: 2.days.ago)).to eq("Original Name") + end + end + + context "when checking the name day of the change" do + it "returns the new name" do + expect(organisation.name(date: Time.zone.now)).to eq("New Name") + end + end + + context "when checking the name after the change" do + it "returns the new name" do + expect(organisation.name(date: 5.days.from_now)).to eq("New Name") + end + end + + context "when there is no name change for the given date" do + it "returns the current name" do + expect(organisation.name(date: 10.years.ago)).to eq("Original Name") + end + end + end end diff --git a/spec/services/exports/organisation_export_service_spec.rb b/spec/services/exports/organisation_export_service_spec.rb index 6ef66161a..69d350351 100644 --- a/spec/services/exports/organisation_export_service_spec.rb +++ b/spec/services/exports/organisation_export_service_spec.rb @@ -6,6 +6,7 @@ RSpec.describe Exports::OrganisationExportService do let(:storage_service) { instance_double(Storage::S3Service) } let(:xml_export_file) { File.open("spec/fixtures/exports/organisation.xml", "r:UTF-8") } + let(:new_name_xml_export_file) { File.open("spec/fixtures/exports/organisation_new_name.xml", "r:UTF-8") } let(:local_manifest_file) { File.open("spec/fixtures/exports/manifest.xml", "r:UTF-8") } let(:expected_zip_filename) { "organisations_2024_2025_apr_mar_f0001_inc0001.zip" } @@ -16,7 +17,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!(/\{name\}/, organisation.name(date: start_time)) export_template.sub!(/\{dsa_signed_at\}/, organisation.data_protection_confirmation&.signed_at&.iso8601) export_template.sub!(/\{dpo_email\}/, organisation.data_protection_confirmation&.data_protection_officer_email) end @@ -107,6 +108,51 @@ RSpec.describe Exports::OrganisationExportService do end end + context "and one organisation with a name change is available for export" do + let!(:organisation) { create(:organisation, name: "MHCLG", address_line1: "2 Marsham Street", address_line2: "London", postcode: "SW1P 4DF", housing_registration_no: "1234") } + + before do + create(:organisation_name_change, organisation:, name: "MHCLG New", startdate: Time.zone.local(2022, 5, 1)) + end + + it "generates an XML export file with the expected content within the ZIP file" do + expected_content = replace_entity_ids(organisation, new_name_xml_export_file.read) + + 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) + end + + export_service.export_xml_organisations + end + + it "returns the list with correct archive" do + expect(export_service.export_xml_organisations).to eq({ expected_zip_filename.gsub(".zip", "") => start_time }) + end + + context "and the organisation is merged" do + let(:expected_content) { replace_entity_ids(organisation, new_name_xml_export_file.read) } + + before do + organisation.update!(merge_date: Time.zone.yesterday) + expected_content.sub!("true", "false") + expected_content.sub!("", "#{organisation.merge_date.iso8601}") + expected_content.sub!("active", "merged") + end + + it "generates an XML export file with the expected content within the ZIP file" 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) + end + + export_service.export_xml_organisations + end + end + end + context "and multiple organisations are available for export" do before do create(:organisation)