From 1b1c92d0acb998c5cee97786cf17c404ad521721 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Tue, 25 Jul 2023 10:59:32 +0100 Subject: [PATCH] CLDC-2572 Add merged status (#1787) * feat: add merge_date to org and display merged status * feat: prohibit users being added to merged orgs * feat: add migration * feat: update error message * feat: update tests * feat: add user validation test * refactor: linting * feat: update schema * feat: use orange tag for merged status * feat: merge with main * feat: update tests * feat: delete old org helper file * feat: update tests * refactor: lint --- app/helpers/organisation_helper.rb | 27 ------------- app/helpers/organisations_helper.rb | 40 +++++++++++++++++++ app/helpers/tag_helper.rb | 2 + app/models/organisation.rb | 22 +++++----- app/models/user.rb | 7 ++++ .../organisations/_organisation_list.html.erb | 4 ++ app/views/organisations/show.html.erb | 2 +- config/locales/en.yml | 1 + ...18151955_add_merge_date_to_organisation.rb | 5 +++ db/schema.rb | 29 +++++++------- spec/helpers/organisations_helper_spec.rb | 23 +++++++++++ spec/helpers/tag_helper_spec.rb | 1 + spec/models/organisation_spec.rb | 20 ---------- spec/models/user_spec.rb | 10 +++++ 14 files changed, 119 insertions(+), 74 deletions(-) delete mode 100644 app/helpers/organisation_helper.rb create mode 100644 app/helpers/organisations_helper.rb create mode 100644 db/migrate/20230718151955_add_merge_date_to_organisation.rb create mode 100644 spec/helpers/organisations_helper_spec.rb diff --git a/app/helpers/organisation_helper.rb b/app/helpers/organisation_helper.rb deleted file mode 100644 index 7c42bf358..000000000 --- a/app/helpers/organisation_helper.rb +++ /dev/null @@ -1,27 +0,0 @@ -module OrganisationHelper - def organisation_header(path, user, current_organisation) - if path == "/organisations" - "Organisations" - elsif user.organisation_id == current_organisation.id - "Your organisation" - else - current_organisation.name - end - end - - def organisation_name_row(user:, organisation:, summary_list:) - summary_list.row do |row| - row.key { "Name" } - row.value { organisation.name } - if user.support? - row.action( - visually_hidden_text: organisation.name.humanize.downcase, - href: edit_organisation_path(organisation), - html_attributes: { "data-qa": "change-#{organisation.name.downcase}" }, - ) - else - row.action - end - end - end -end diff --git a/app/helpers/organisations_helper.rb b/app/helpers/organisations_helper.rb new file mode 100644 index 000000000..850742afc --- /dev/null +++ b/app/helpers/organisations_helper.rb @@ -0,0 +1,40 @@ +module OrganisationsHelper + def organisation_header(path, user, current_organisation) + if path == "/organisations" + "Organisations" + elsif user.organisation_id == current_organisation.id + "Your organisation" + else + current_organisation.name + end + end + + def display_organisation_attributes(organisation) + [ + { 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: "Type of provider", value: organisation.display_provider_type, editable: false }, + { name: "Registration number", value: organisation.housing_registration_no || "", editable: false }, + { name: "Rent periods", value: organisation.rent_period_labels, editable: false, format: :bullet }, + { name: "Owns housing stock", value: organisation.holds_own_stock ? "Yes" : "No", editable: false }, + { name: "Status", value: status_tag(organisation.status), editable: false }, + ] + end + + def organisation_name_row(user:, organisation:, summary_list:) + summary_list.row do |row| + row.key { "Name" } + row.value { organisation.name } + if user.support? + row.action( + visually_hidden_text: organisation.name.humanize.downcase, + href: edit_organisation_path(organisation), + html_attributes: { "data-qa": "change-#{organisation.name.downcase}" }, + ) + else + row.action + end + end + end +end diff --git a/app/helpers/tag_helper.rb b/app/helpers/tag_helper.rb index 5a7498b78..e2a3ec92d 100644 --- a/app/helpers/tag_helper.rb +++ b/app/helpers/tag_helper.rb @@ -13,6 +13,7 @@ module TagHelper reactivating_soon: "Reactivating soon", deactivated: "Deactivated", deleted: "Deleted", + merged: "Merged", }.freeze COLOUR = { @@ -27,6 +28,7 @@ module TagHelper reactivating_soon: "blue", deactivated: "grey", deleted: "red", + merged: "orange", }.freeze def status_tag(status, classes = []) diff --git a/app/models/organisation.rb b/app/models/organisation.rb index b4317c3e0..c050fef06 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -102,18 +102,6 @@ class Organisation < ApplicationRecord DISPLAY_PROVIDER_TYPE[provider_type.to_sym] end - def display_organisation_attributes - [ - { name: "Organisation ID", value: "ORG#{id}", editable: false }, - { name: "Address", value: address_string, editable: true }, - { name: "Telephone number", value: phone, editable: true }, - { name: "Type of provider", value: display_provider_type, editable: false }, - { name: "Registration number", value: housing_registration_no || "", editable: false }, - { name: "Rent periods", value: rent_period_labels, editable: false, format: :bullet }, - { name: "Owns housing stock", value: holds_own_stock ? "Yes" : "No", editable: false }, - ] - end - def has_managing_agents? managing_agents.count.positive? end @@ -121,4 +109,14 @@ class Organisation < ApplicationRecord def has_stock_owners? stock_owners.count.positive? end + + def status + @status ||= status_at(Time.zone.now) + end + + def status_at(date) + return :merged if merge_date.present? && merge_date < date + + :active + end end diff --git a/app/models/user.rb b/app/models/user.rb index f9e34a652..4e762c9a1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -24,6 +24,7 @@ class User < ApplicationRecord after_validation :send_data_protection_confirmation_reminder, if: :is_dpo_changed? validates :organisation_id, presence: true + validate :organisation_not_merged has_paper_trail ignore: %w[last_sign_in_at current_sign_in_at @@ -185,6 +186,12 @@ protected private + def organisation_not_merged + if organisation&.merge_date.present? && organisation.merge_date < Time.zone.now + errors.add :organisation_id, I18n.t("validations.organisation.merged") + end + end + def send_data_protection_confirmation_reminder return unless persisted? return unless is_dpo? diff --git a/app/views/organisations/_organisation_list.html.erb b/app/views/organisations/_organisation_list.html.erb index 389dff6b7..6a68433be 100644 --- a/app/views/organisations/_organisation_list.html.erb +++ b/app/views/organisations/_organisation_list.html.erb @@ -14,6 +14,9 @@ <% row.cell(header: true, text: "Type", html_attributes: { scope: "col", }) %> + <% row.cell(header: true, text: "Status", html_attributes: { + scope: "col", + }) %> <% end %> <% end %> <% @organisations.each do |organisation| %> @@ -26,6 +29,7 @@ <% end %> <% row.cell(text: organisation.housing_registration_no) %> <% row.cell(text: organisation.display_provider_type) %> + <% row.cell(text: status_tag(organisation.status)) %> <% end %> <% end %> <% end %> diff --git a/app/views/organisations/show.html.erb b/app/views/organisations/show.html.erb index 87020120d..cc504c9f5 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:) %> - <% @organisation.display_organisation_attributes.each do |attr| %> + <% display_organisation_attributes(@organisation).each do |attr| %> <% if can_edit_org?(current_user) && attr[:editable] %> <%= summary_list.row do |row| %> <% row.key { attr[:name] } %> diff --git a/config/locales/en.yml b/config/locales/en.yml index f4d968d44..07142821d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -198,6 +198,7 @@ en: managing_agent: blank: "You must choose a managing agent" already_added: "You have already added this managing agent" + merged: "That organisation has already been merged. Select a different organisation." not_answered: "You must answer %{question}" invalid_option: "Enter a valid value for %{question}" invalid_number: "Enter a number for %{question}" diff --git a/db/migrate/20230718151955_add_merge_date_to_organisation.rb b/db/migrate/20230718151955_add_merge_date_to_organisation.rb new file mode 100644 index 000000000..7a7bac420 --- /dev/null +++ b/db/migrate/20230718151955_add_merge_date_to_organisation.rb @@ -0,0 +1,5 @@ +class AddMergeDateToOrganisation < ActiveRecord::Migration[7.0] + def change + add_column :organisations, :merge_date, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index 81d233874..75282b2bd 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.0].define(version: 2023_07_13_140231) do +ActiveRecord::Schema[7.0].define(version: 2023_07_18_151955) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -192,14 +192,14 @@ ActiveRecord::Schema[7.0].define(version: 2023_07_13_140231) 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" @@ -347,7 +347,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_07_13_140231) do t.string "old_id" t.string "old_visible_id" t.string "mobility_type" - t.datetime "startdate", precision: nil + t.datetime "startdate" t.string "location_admin_district" t.boolean "confirmed" t.index ["old_id"], name: "index_locations_on_old_id", unique: true @@ -433,6 +433,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_07_13_140231) do t.integer "unspecified_units" t.string "old_org_id" t.string "old_visible_id" + t.datetime "merge_date" t.index ["old_visible_id"], name: "index_organisations_on_old_visible_id", unique: true end @@ -518,7 +519,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_07_13_140231) do t.integer "stairbought" t.integer "stairowned" t.decimal "mrent", precision: 10, scale: 2 - t.datetime "exdate", precision: nil + t.datetime "exdate" t.integer "exday" t.integer "exmonth" t.integer "exyear" @@ -554,7 +555,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_07_13_140231) do t.integer "wchair" t.integer "income2_value_check" t.integer "armedforcesspouse" - t.datetime "hodate", precision: nil + t.datetime "hodate" t.integer "hoday" t.integer "homonth" t.integer "hoyear" @@ -607,9 +608,9 @@ ActiveRecord::Schema[7.0].define(version: 2023_07_13_140231) do t.integer "discounted_sale_value_check" t.integer "student_not_child_value_check" t.integer "percentage_discount_value_check" + t.integer "combined_income_value_check" t.integer "buyer_livein_value_check" t.integer "status_cache", default: 0, null: false - t.integer "combined_income_value_check" t.datetime "discarded_at" t.integer "stairowned_value_check" t.integer "creation_method", default: 1 @@ -662,8 +663,8 @@ ActiveRecord::Schema[7.0].define(version: 2023_07_13_140231) 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" @@ -671,7 +672,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_07_13_140231) do t.string "phone" t.integer "failed_attempts", default: 0 t.string "unlock_token" - t.datetime "locked_at", precision: nil + t.datetime "locked_at" t.boolean "is_dpo", default: false t.boolean "is_key_contact", default: false t.integer "second_factor_attempts_count", default: 0 @@ -679,12 +680,12 @@ ActiveRecord::Schema[7.0].define(version: 2023_07_13_140231) do t.string "encrypted_otp_secret_key_iv" t.string "encrypted_otp_secret_key_salt" t.string "direct_otp" - t.datetime "direct_otp_sent_at", precision: nil + t.datetime "direct_otp_sent_at" t.datetime "totp_timestamp", precision: nil t.boolean "active", default: true t.string "confirmation_token" - t.datetime "confirmed_at", precision: nil - t.datetime "confirmation_sent_at", precision: nil + t.datetime "confirmed_at" + t.datetime "confirmation_sent_at" t.string "unconfirmed_email" t.boolean "initial_confirmation_sent" t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true diff --git a/spec/helpers/organisations_helper_spec.rb b/spec/helpers/organisations_helper_spec.rb new file mode 100644 index 000000000..f8afb3e04 --- /dev/null +++ b/spec/helpers/organisations_helper_spec.rb @@ -0,0 +1,23 @@ +require "rails_helper" + +RSpec.describe OrganisationsHelper do + include TagHelper + describe "display_organisation_attributes" do + let(:organisation) { create(:organisation) } + + it "does not include data protection agreement" do + expect(display_organisation_attributes(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: "Type of provider", value: "Local authority" }, + { editable: false, name: "Registration number", value: "1234" }, + { editable: false, format: :bullet, name: "Rent periods", value: %w[All] }, + { editable: false, name: "Owns housing stock", value: "Yes" }, + { editable: false, name: "Status", value: status_tag(organisation.status) }], + ) + end + end +end diff --git a/spec/helpers/tag_helper_spec.rb b/spec/helpers/tag_helper_spec.rb index 3373737de..231323278 100644 --- a/spec/helpers/tag_helper_spec.rb +++ b/spec/helpers/tag_helper_spec.rb @@ -20,6 +20,7 @@ RSpec.describe TagHelper do expect(status_tag("activating_soon", "app-tag--small")).to eq("Activating soon") expect(status_tag("reactivating_soon", "app-tag--small")).to eq("Reactivating soon") expect(status_tag("deactivated", "app-tag--small")).to eq("Deactivated") + expect(status_tag("merged", "app-tag--small")).to eq("Merged") end end end diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb index c6179b34f..ad9034d35 100644 --- a/spec/models/organisation_spec.rb +++ b/spec/models/organisation_spec.rb @@ -230,24 +230,4 @@ RSpec.describe Organisation, type: :model do end end end - - describe "display_organisation_attributes" do - let(:organisation) { create(:organisation) } - - it "does not include data protection agreement" do - expect(organisation.display_organisation_attributes).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: "Type of provider", value: "Local authority" }, - { editable: false, name: "Registration number", value: "1234" }, - { editable: false, format: :bullet, name: "Rent periods", value: %w[All] }, - { editable: false, name: "Owns housing stock", value: "Yes" }, - ], - ) - end - end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d13d648a1..2bd862a90 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -334,6 +334,16 @@ RSpec.describe User, type: :model do .to raise_error(ActiveRecord::RecordInvalid, error_message) end end + + context "when a user is added to a merged organisation" do + let(:merged_organisation) { create(:organisation, merge_date: Time.zone.yesterday) } + let(:error_message) { "Validation failed: Organisation #{I18n.t('validations.organisation.merged')}" } + + it "validates organisation merge status" do + expect { create(:user, organisation: merged_organisation) } + .to raise_error(ActiveRecord::RecordInvalid, error_message) + end + end end describe "delete" do