Browse Source
* 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 * lintpull/3059/head v0.5.11
Manny Dinssa
3 weeks ago
committed by
GitHub
30 changed files with 636 additions and 33 deletions
@ -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 |
||||
@ -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 |
||||
@ -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 %> |
||||
@ -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 %> |
||||
|
||||
<div class="govuk-grid-row"> |
||||
<div class="govuk-grid-column-two-thirds-from-desktop"> |
||||
<span class="govuk-caption-xl">Cancel scheduled name change </span> |
||||
<h1 class="govuk-heading-xl"> |
||||
<%= content_for(:title) %> |
||||
</h1> |
||||
|
||||
<%= govuk_warning_text(text: "You will not be able to undo this action.") %> |
||||
|
||||
<div class="govuk-inset-text"> |
||||
This name change would have updated the organisation name from |
||||
<strong><%= @organisation_name_change.previous_change.is_a?(OrganisationNameChange) ? @organisation_name_change.previous_change.name : @organisation_name_change.organisation.name %></strong> |
||||
to <strong><%= @organisation_name_change.name %></strong>. |
||||
</div> |
||||
|
||||
<div class="govuk-button-group"> |
||||
<%= 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 %> |
||||
</div> |
||||
</div> |
||||
</div> |
||||
@ -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| %> |
||||
<div class="govuk-grid-row"> |
||||
<div class="govuk-grid-column-two-thirds"> |
||||
<%= f.govuk_error_summary %> |
||||
|
||||
<h1 class="govuk-heading-l"> |
||||
<%= content_for(:title) %> |
||||
</h1> |
||||
|
||||
<%= f.govuk_text_field :name, autocomplete: "name", label: { text: "Enter new name", size: "m" }, value: @organisation.name %> |
||||
</div> |
||||
|
||||
<div class="govuk-grid-column-three-quarters"> |
||||
<%= render partial: "organisation_name_changes/name_history_list" %> |
||||
</div> |
||||
|
||||
<div class="govuk-grid-column-two-thirds"> |
||||
<%= 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 %> |
||||
|
||||
<div class="govuk-button-group"> |
||||
<%= f.govuk_submit "Save changes" %> |
||||
<%= govuk_button_link_to "Cancel", details_organisation_path(@organisation), secondary: true %> |
||||
</div> |
||||
</div> |
||||
</div> |
||||
<% end %> |
||||
@ -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 |
||||
@ -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 |
||||
@ -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 |
||||
@ -0,0 +1,27 @@
|
||||
<?xml version="1.0" encoding="UTF-8"?> |
||||
<forms> |
||||
<form> |
||||
<id>{id}</id> |
||||
<name>MHCLG New</name> |
||||
<phone/> |
||||
<provider_type>1</provider_type> |
||||
<address_line1>2 Marsham Street</address_line1> |
||||
<address_line2>London</address_line2> |
||||
<postcode>SW1P 4DF</postcode> |
||||
<holds_own_stock>true</holds_own_stock> |
||||
<active>true</active> |
||||
<housing_registration_no>1234</housing_registration_no> |
||||
<old_org_id/> |
||||
<old_visible_id/> |
||||
<merge_date/> |
||||
<absorbing_organisation_id/> |
||||
<available_from/> |
||||
<deleted_at/> |
||||
<dsa_signed>true</dsa_signed> |
||||
<dsa_signed_at>{dsa_signed_at}</dsa_signed_at> |
||||
<dpo_email>{dpo_email}</dpo_email> |
||||
<profit_status/> |
||||
<group/> |
||||
<status>active</status> |
||||
</form> |
||||
</forms> |
||||
@ -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 |
||||
Loading…
Reference in new issue