From a65cc59713e2c27e597ed7461f6c77354fc8ff27 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Wed, 3 Apr 2024 13:58:40 +0100
Subject: [PATCH 1/4] CLDC-2479 Allow deleting users (#2288)

* Add delete confirmation page

* Add status to the user

* Allow deleting user

* Refactor sign_in user in tests

* Add delete user button and update policy

* Update user policy

* Do not display deleted users as an answer option

* Update user table and details

* Do not show deleted users

* Disable delete user on production

* Update test
---
 app/controllers/organisations_controller.rb   |   4 +-
 app/controllers/users_controller.rb           |  13 +-
 app/helpers/tag_helper.rb                     |   2 +-
 app/helpers/user_helper.rb                    |   4 +
 .../form/lettings/questions/created_by_id.rb  |   6 +-
 .../form/sales/questions/created_by_id.rb     |   4 +-
 app/models/user.rb                            |  17 +-
 app/policies/user_policy.rb                   |  25 ++
 app/services/feature_toggle.rb                |   4 +
 app/views/users/_user_list.html.erb           |   6 +-
 app/views/users/delete_confirmation.html.erb  |  24 ++
 app/views/users/show.html.erb                 |  18 +-
 config/locales/en.yml                         |   1 +
 config/routes.rb                              |   2 +
 ...091927_add_discarded_at_column_to_users.rb |   5 +
 db/schema.rb                                  |   1 +
 spec/features/user_spec.rb                    |   8 +-
 .../lettings/questions/created_by_id_spec.rb  |  21 ++
 .../sales/questions/created_by_id_spec.rb     |  20 ++
 spec/models/user_spec.rb                      |   9 +
 spec/policies/user_policy_spec.rb             | 124 +++++++++
 spec/requests/users_controller_spec.rb        | 260 ++++++++++++------
 22 files changed, 465 insertions(+), 113 deletions(-)
 create mode 100644 app/views/users/delete_confirmation.html.erb
 create mode 100644 db/migrate/20240305091927_add_discarded_at_column_to_users.rb

diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb
index dc5a36c9b..4982c4287 100644
--- a/app/controllers/organisations_controller.rb
+++ b/app/controllers/organisations_controller.rb
@@ -46,14 +46,14 @@ class OrganisationsController < ApplicationController
   end
 
   def users
-    organisation_users = @organisation.users.sorted_by_organisation_and_role
+    organisation_users = @organisation.users.visible.sorted_by_organisation_and_role
     unpaginated_filtered_users = filter_manager.filtered_users(organisation_users, search_term, session_filters)
 
     respond_to do |format|
       format.html do
         @pagy, @users = pagy(unpaginated_filtered_users)
         @searched = search_term.presence
-        @total_count = @organisation.users.size
+        @total_count = @organisation.users.visible.size
         @filter_type = "users"
 
         if current_user.support?
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index a9344f0f7..6a76cb047 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -13,7 +13,7 @@ class UsersController < ApplicationController
   def index
     redirect_to users_organisation_path(current_user.organisation) unless current_user.support?
 
-    all_users = User.sorted_by_organisation_and_role
+    all_users = User.visible.sorted_by_organisation_and_role
     filtered_users = filter_manager.filtered_users(all_users, search_term, session_filters)
     @pagy, @users = pagy(filtered_users)
     @searched = search_term.presence
@@ -122,6 +122,16 @@ class UsersController < ApplicationController
     end
   end
 
+  def delete_confirmation
+    authorize @user
+  end
+
+  def delete
+    authorize @user
+    @user.discard!
+    redirect_to users_organisation_path(@user.organisation), notice: I18n.t("notification.user_deleted", name: @user.name)
+  end
+
 private
 
   def validate_attributes
@@ -197,6 +207,7 @@ private
     if action_name == "create"
       head :unauthorized and return unless current_user.data_coordinator? || current_user.support?
     else
+      render_not_found and return if @user.status == :deleted
       render_not_found and return unless (current_user.organisation == @user.organisation) || current_user.support?
       render_not_found and return if action_name == "edit_password" && current_user != @user
       render_not_found and return unless action_name == "show" ||
diff --git a/app/helpers/tag_helper.rb b/app/helpers/tag_helper.rb
index 3cb02b769..c389942dc 100644
--- a/app/helpers/tag_helper.rb
+++ b/app/helpers/tag_helper.rb
@@ -30,7 +30,7 @@ module TagHelper
     deactivated: "grey",
     deleted: "red",
     merged: "orange",
-    unconfirmed: "red",
+    unconfirmed: "blue",
   }.freeze
 
   def status_tag(status, classes = [])
diff --git a/app/helpers/user_helper.rb b/app/helpers/user_helper.rb
index 76fb78f57..6fcf6ca4c 100644
--- a/app/helpers/user_helper.rb
+++ b/app/helpers/user_helper.rb
@@ -10,4 +10,8 @@ module UserHelper
   def can_edit_org?(current_user)
     current_user.data_coordinator? || current_user.support?
   end
+
+  def delete_user_link(user)
+    govuk_button_link_to "Delete this user", delete_confirmation_user_path(user), warning: true
+  end
 end
diff --git a/app/models/form/lettings/questions/created_by_id.rb b/app/models/form/lettings/questions/created_by_id.rb
index 02176fb4d..b05f97e9f 100644
--- a/app/models/form/lettings/questions/created_by_id.rb
+++ b/app/models/form/lettings/questions/created_by_id.rb
@@ -22,15 +22,15 @@ class Form::Lettings::Questions::CreatedById < ::Form::Question
                [
                  (
                    if log.owning_organisation
-                     log.owning_organisation.absorbing_organisation.present? ? log.owning_organisation&.absorbing_organisation&.users : log.owning_organisation&.users
+                     log.owning_organisation.absorbing_organisation.present? ? log.owning_organisation&.absorbing_organisation&.users&.visible : log.owning_organisation&.users&.visible
                    end),
                  (
                    if log.managing_organisation
-                     log.managing_organisation.absorbing_organisation.present? ? log.managing_organisation&.absorbing_organisation&.users : log.managing_organisation.users
+                     log.managing_organisation.absorbing_organisation.present? ? log.managing_organisation&.absorbing_organisation&.users&.visible : log.managing_organisation.users&.visible
                    end),
                ].flatten
              else
-               current_user.organisation.users
+               current_user.organisation.users.visible
              end.uniq.compact
 
     users.each_with_object(ANSWER_OPTS.dup) do |user, hsh|
diff --git a/app/models/form/sales/questions/created_by_id.rb b/app/models/form/sales/questions/created_by_id.rb
index 725361ee4..6bcaed595 100644
--- a/app/models/form/sales/questions/created_by_id.rb
+++ b/app/models/form/sales/questions/created_by_id.rb
@@ -23,11 +23,11 @@ class Form::Sales::Questions::CreatedById < ::Form::Question
                [
                  (
                    if log.managing_organisation
-                     log.managing_organisation.absorbing_organisation.present? ? log.managing_organisation&.absorbing_organisation&.users : log.managing_organisation.users
+                     log.managing_organisation.absorbing_organisation.present? ? log.managing_organisation&.absorbing_organisation&.users&.visible : log.managing_organisation.users.visible
                    end),
                ].flatten
              else
-               log.managing_organisation.users
+               log.managing_organisation.users.visible
              end.uniq.compact
     users.each_with_object(ANSWER_OPTS.dup) do |user, hsh|
       hsh[user.id] = present_user(user)
diff --git a/app/models/user.rb b/app/models/user.rb
index 25599da56..6d13e8cd1 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -76,6 +76,7 @@ class User < ApplicationRecord
   scope :not_signed_in, -> { where(last_sign_in_at: nil, active: true) }
   scope :deactivated, -> { where(active: false) }
   scope :active_status, -> { where(active: true).where.not(last_sign_in_at: nil) }
+  scope :visible, -> { where(discarded_at: nil) }
 
   def lettings_logs
     if support?
@@ -238,13 +239,15 @@ class User < ApplicationRecord
   end
 
   def status
-    if active == false
-      :deactivated
-    elsif confirmed? == false
-      :unconfirmed
-    else
-      :active
-    end
+    return :deleted if discarded_at.present?
+    return :deactivated unless active
+    return :unconfirmed unless confirmed?
+
+    :active
+  end
+
+  def discard!
+    update!(discarded_at: Time.zone.now)
   end
 
 protected
diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb
index a4b1a3d5c..eb747266d 100644
--- a/app/policies/user_policy.rb
+++ b/app/policies/user_policy.rb
@@ -33,4 +33,29 @@ class UserPolicy
       (@current_user == @user || @current_user.data_coordinator? || @current_user.support?) && @user.active?
     end
   end
+
+  def delete_confirmation?
+    delete?
+  end
+
+  def delete?
+    return false unless current_user.support?
+    return false unless user.status == :deactivated
+
+    !has_any_logs_in_editable_collection_period && !has_signed_data_protection_agreement?
+  end
+
+private
+
+  def has_any_logs_in_editable_collection_period
+    editable_from_date = FormHandler.instance.earliest_open_for_editing_collection_start_date
+
+    LettingsLog.where(created_by_id: user.id).after_date(editable_from_date).or(LettingsLog.where(startdate: nil, created_by_id: user.id)).any?
+  end
+
+  def has_signed_data_protection_agreement?
+    return false unless user.is_dpo? && user.organisation.data_protection_confirmed?
+
+    user.organisation.data_protection_confirmation.data_protection_officer == user
+  end
 end
diff --git a/app/services/feature_toggle.rb b/app/services/feature_toggle.rb
index f145b7132..1aaba1571 100644
--- a/app/services/feature_toggle.rb
+++ b/app/services/feature_toggle.rb
@@ -34,4 +34,8 @@ class FeatureToggle
   def self.delete_location_enabled?
     !Rails.env.production?
   end
+
+  def self.delete_user_enabled?
+    !Rails.env.production?
+  end
 end
diff --git a/app/views/users/_user_list.html.erb b/app/views/users/_user_list.html.erb
index 19e206b03..436c0def2 100644
--- a/app/views/users/_user_list.html.erb
+++ b/app/views/users/_user_list.html.erb
@@ -18,6 +18,9 @@
         <% row.with_cell(header: true, text: "Last logged in", html_attributes: {
           scope: "col",
         }) %>
+        <% row.with_cell(header: true, text: "Status", html_attributes: {
+          scope: "col",
+        }) %>
       <% end %>
     <% end %>
     <% users.each do |user| %>
@@ -50,7 +53,8 @@
             <% end %>
           <% end %>
           <% row.with_cell(text: simple_format(org_cell(user), {}, wrapper_tag: "div")) %>
-          <% row.with_cell(text: user.active? ? user.last_sign_in_at&.to_formatted_s(:govuk_date) : status_tag(user.status)) %>
+          <% row.with_cell(text: user.last_sign_in_at&.to_formatted_s(:govuk_date)) %>
+          <% row.with_cell(text: status_tag(user.status)) %>
           <%= govuk_link_to users_path(user) do %>
             <span class="govuk-visually-hidden">User </span><%= user.id %>
           <% end %>
diff --git a/app/views/users/delete_confirmation.html.erb b/app/views/users/delete_confirmation.html.erb
new file mode 100644
index 000000000..4c8653a78
--- /dev/null
+++ b/app/views/users/delete_confirmation.html.erb
@@ -0,0 +1,24 @@
+<% content_for :before_content do %>
+  <% content_for :title, "Are you sure you want to delete this user?" %>
+  <%= govuk_back_link(href: :back) %>
+<% end %>
+
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-two-thirds-from-desktop">
+    <span class="govuk-caption-xl">Delete <%= @user.name %></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-button-group">
+      <%= govuk_button_to(
+        "Delete this user",
+        delete_user_path(@user),
+        method: :delete,
+      ) %>
+      <%= govuk_button_link_to "Cancel", user_path(@user), html: { method: :get }, secondary: true %>
+    </div>
+  </div>
+</div>
diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb
index bf2877fbc..a93532e7b 100644
--- a/app/views/users/show.html.erb
+++ b/app/views/users/show.html.erb
@@ -112,11 +112,18 @@
               row.with_action
             end
           end %>
-
+      <%= summary_list.with_row do |row| %>
+          <% row.with_key { "Status" } %>
+          <% row.with_value do %>
+           <%= details_html({ name: "Status", value: status_tag(@user.status), id: "status" }) %>
+            <% if @user.status == :deactivated && current_user.support? && !UserPolicy.new(current_user, @user).delete? %>
+              <span class="app-!-colour-muted">This user was active in an open or editable collection year, and cannot be deleted.</span>
+          <% end %>
+          <% end %>
+        <% end %>
       <%= summary_list.with_row do |row|
-            row.with_key { "Status" }
-            row.with_value { status_tag(@user.status) }
-            row.with_action
+            row.with_key { "Last logged in" }
+            row.with_value { @user.last_sign_in_at&.to_formatted_s(:govuk_date) }
           end %>
     <% end %>
 
@@ -133,6 +140,9 @@
           </span>
         <% end %>
       <% end %>
+      <% if UserPolicy.new(current_user, @user).delete? && FeatureToggle.delete_user_enabled? %>
+        <%= delete_user_link(@user) %>
+      <% end %>
     </div>
   </div>
 </div>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index d95187a49..079967095 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -198,6 +198,7 @@ en:
       other: "There are %{count} sets of duplicate logs"
     location_deleted: "%{postcode} has been deleted."
     scheme_deleted: "%{service_name} has been deleted."
+    user_deleted: "%{name} has been deleted."
 
   validations:
     organisation:
diff --git a/config/routes.rb b/config/routes.rb
index f0f497291..79dcf0a04 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -129,6 +129,8 @@ Rails.application.routes.draw do
       get "deactivate", to: "users#deactivate"
       get "reactivate", to: "users#reactivate"
       post "resend-invite", to: "users#resend_invite"
+      get "delete-confirmation", to: "users#delete_confirmation"
+      delete "delete", to: "users#delete"
     end
   end
 
diff --git a/db/migrate/20240305091927_add_discarded_at_column_to_users.rb b/db/migrate/20240305091927_add_discarded_at_column_to_users.rb
new file mode 100644
index 000000000..469c4e145
--- /dev/null
+++ b/db/migrate/20240305091927_add_discarded_at_column_to_users.rb
@@ -0,0 +1,5 @@
+class AddDiscardedAtColumnToUsers < ActiveRecord::Migration[7.0]
+  def change
+    add_column :users, :discarded_at, :datetime
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index da9c4b0aa..cca8b7f5c 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -763,6 +763,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_03_19_122706) do
     t.datetime "confirmation_sent_at"
     t.string "unconfirmed_email"
     t.boolean "initial_confirmation_sent"
+    t.datetime "discarded_at"
     t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true
     t.index ["email"], name: "index_users_on_email", unique: true
     t.index ["encrypted_otp_secret_key"], name: "index_users_on_encrypted_otp_secret_key", unique: true
diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb
index 180a92aec..e3335c762 100644
--- a/spec/features/user_spec.rb
+++ b/spec/features/user_spec.rb
@@ -480,14 +480,14 @@ RSpec.describe "User Features" do
       it "allows to cancel user deactivation" do
         click_link("No – I’ve changed my mind")
         expect(page).to have_current_path("/users/#{other_user.id}")
-        expect(page).to have_no_content("This user has been deactivated.")
+        assert_selector ".govuk-tag", text: /Deactivated/, count: 0
         expect(page).to have_no_css(".govuk-notification-banner.govuk-notification-banner--success")
       end
 
       it "allows to deactivate the user" do
         click_button("I’m sure – deactivate this user")
         expect(page).to have_current_path("/users/#{other_user.id}")
-        expect(page).to have_content("Deactivated")
+        assert_selector ".govuk-tag", text: /Deactivated/, count: 1
         expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success")
       end
     end
@@ -517,7 +517,7 @@ RSpec.describe "User Features" do
       it "allows to cancel user reactivation" do
         click_link("No – I’ve changed my mind")
         expect(page).to have_current_path("/users/#{other_user.id}")
-        expect(page).to have_content("Deactivated")
+        assert_selector ".govuk-tag", text: /Deactivated/, count: 1
         expect(page).to have_no_css(".govuk-notification-banner.govuk-notification-banner--success")
       end
 
@@ -525,7 +525,7 @@ RSpec.describe "User Features" do
         expect(notify_client).to receive(:send_email).with(email_address: other_user.email, template_id: User::USER_REACTIVATED_TEMPLATE_ID, personalisation:).once
         click_button("I’m sure – reactivate this user")
         expect(page).to have_current_path("/users/#{other_user.id}")
-        expect(page).to have_no_content("This user has been deactivated.")
+        assert_selector ".govuk-tag", text: /Deactivated/, count: 0
         expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success")
       end
     end
diff --git a/spec/models/form/lettings/questions/created_by_id_spec.rb b/spec/models/form/lettings/questions/created_by_id_spec.rb
index c175997f6..0ff09c597 100644
--- a/spec/models/form/lettings/questions/created_by_id_spec.rb
+++ b/spec/models/form/lettings/questions/created_by_id_spec.rb
@@ -60,6 +60,17 @@ RSpec.describe Form::Lettings::Questions::CreatedById, type: :model do
       it "only displays users that belong to owning and managing organisations" do
         expect(question.displayed_answer_options(lettings_log, support_user)).to eq(expected_option_for_users(managing_org_user.organisation.users + owning_org_user.organisation.users))
       end
+
+      context "when organisation has deleted users" do
+        before do
+          create(:user, name: "Deleted user", discarded_at: Time.zone.yesterday, organisation: owning_org_user.organisation)
+          create(:user, name: "Deleted managing user", discarded_at: Time.zone.yesterday, organisation: managing_org_user.organisation)
+        end
+
+        it "does not display deleted users" do
+          expect(question.displayed_answer_options(lettings_log, support_user)).to eq(expected_option_for_users(managing_org_user.organisation.users.visible + owning_org_user.organisation.users.visible))
+        end
+      end
     end
   end
 
@@ -77,6 +88,16 @@ RSpec.describe Form::Lettings::Questions::CreatedById, type: :model do
       it "only displays users that belong user's org" do
         expect(question.displayed_answer_options(lettings_log, data_coordinator)).to eq(expected_option_for_users(data_coordinator.organisation.users))
       end
+
+      context "when organisation has deleted users" do
+        before do
+          create(:user, name: "Deleted user", discarded_at: Time.zone.yesterday, organisation: data_coordinator.organisation)
+        end
+
+        it "does not display deleted users" do
+          expect(question.displayed_answer_options(lettings_log, data_coordinator)).to eq(expected_option_for_users(data_coordinator.organisation.users.visible))
+        end
+      end
     end
   end
 end
diff --git a/spec/models/form/sales/questions/created_by_id_spec.rb b/spec/models/form/sales/questions/created_by_id_spec.rb
index 7645f45f0..742694f1d 100644
--- a/spec/models/form/sales/questions/created_by_id_spec.rb
+++ b/spec/models/form/sales/questions/created_by_id_spec.rb
@@ -57,6 +57,16 @@ RSpec.describe Form::Sales::Questions::CreatedById, type: :model do
       it "only displays users that belong to the managing organisation" do
         expect(question.displayed_answer_options(sales_log, support_user)).to eq(expected_option_for_users(owning_org_user.organisation.users))
       end
+
+      context "when organisation has deleted users" do
+        before do
+          create(:user, name: "Deleted user", discarded_at: Time.zone.yesterday, organisation: owning_org_user.organisation)
+        end
+
+        it "does not display deleted users" do
+          expect(question.displayed_answer_options(sales_log, support_user)).to eq(expected_option_for_users(owning_org_user.organisation.users.visible))
+        end
+      end
     end
   end
 
@@ -78,6 +88,16 @@ RSpec.describe Form::Sales::Questions::CreatedById, type: :model do
       it "only displays users that belong to managing organisation" do
         expect(question.displayed_answer_options(sales_log, data_coordinator)).to eq(expected_option_for_users(owning_org_user.organisation.users))
       end
+
+      context "when organisation has deleted users" do
+        before do
+          create(:user, name: "Deleted user", discarded_at: Time.zone.yesterday, organisation: owning_org_user.organisation)
+        end
+
+        it "does not display deleted users" do
+          expect(question.displayed_answer_options(sales_log, data_coordinator)).to eq(expected_option_for_users(owning_org_user.organisation.users.visible))
+        end
+      end
     end
   end
 end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 774a7893b..f00aa1c7c 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -517,5 +517,14 @@ RSpec.describe User, type: :model do
 
       expect(user.status).to eq(:active)
     end
+
+    context "when the user is deleted" do
+      let(:user) { create(:user, discarded_at: Time.zone.yesterday) }
+
+      it "returns the status of the user" do
+        user.destroy!
+        expect(user.status).to eq(:deleted)
+      end
+    end
   end
 end
diff --git a/spec/policies/user_policy_spec.rb b/spec/policies/user_policy_spec.rb
index ec84fbceb..be0de12cb 100644
--- a/spec/policies/user_policy_spec.rb
+++ b/spec/policies/user_policy_spec.rb
@@ -99,5 +99,129 @@ RSpec.describe UserPolicy do
       expect(policy).to permit(support, data_provider)
     end
   end
+
+  permissions :delete? do
+    context "with active user" do
+      let(:user) { create(:user, last_sign_in_at: Time.zone.yesterday) }
+
+      it "does not allow deleting a user as a provider" do
+        expect(user.status).to eq(:active)
+        expect(policy).not_to permit(data_provider, user)
+      end
+
+      it "does not allow allows deleting a user as a coordinator" do
+        expect(policy).not_to permit(data_coordinator, user)
+      end
+
+      it "does not allow deleting a user as a support user" do
+        expect(policy).not_to permit(support, user)
+      end
+    end
+
+    context "with unconfirmed user" do
+      let(:user) { create(:user) }
+
+      before do
+        user.confirmed_at = nil
+        user.save!(validate: false)
+      end
+
+      it "does not allow deleting a user as a provider" do
+        expect(user.status).to eq(:unconfirmed)
+        expect(policy).not_to permit(data_provider, user)
+      end
+
+      it "does not allow allows deleting a user as a coordinator" do
+        expect(policy).not_to permit(data_coordinator, user)
+      end
+
+      it "does not allow deleting a user as a support user" do
+        expect(policy).not_to permit(support, user)
+      end
+    end
+
+    context "with deactivated user" do
+      let(:user) { create(:user, active: false) }
+
+      before do
+        Timecop.freeze(Time.utc(2024, 4, 10))
+        log = create(:lettings_log, owning_organisation: user.organisation, created_by: user)
+        log.startdate = Time.zone.local(2022, 10, 10)
+        log.save!(validate: false)
+      end
+
+      after do
+        Timecop.unfreeze
+      end
+
+      context "and associated logs in editable collection period" do
+        before do
+          create(:lettings_log, :sh, owning_organisation: user.organisation, created_by: user, startdate: Time.zone.local(2024, 4, 9))
+        end
+
+        it "does not allow deleting a user as a provider" do
+          expect(policy).not_to permit(data_provider, user)
+        end
+
+        it "does not allow allows deleting a user as a coordinator" do
+          expect(policy).not_to permit(data_coordinator, user)
+        end
+
+        it "does not allow deleting a user as a support user" do
+          expect(policy).not_to permit(support, user)
+        end
+      end
+
+      context "and no associated logs in editable collection period" do
+        it "does not allow deleting a user as a provider" do
+          expect(policy).not_to permit(data_provider, user)
+        end
+
+        it "does not allow allows deleting a user as a coordinator" do
+          expect(policy).not_to permit(data_coordinator, user)
+        end
+
+        it "allows deleting a user as a support user" do
+          expect(policy).to permit(support, user)
+        end
+      end
+
+      context "and user is the DPO that has signed the agreement" do
+        let(:user) { create(:user, active: false, is_dpo: true) }
+
+        before do
+          user.organisation.data_protection_confirmation.update!(data_protection_officer: user)
+        end
+
+        it "does not allow deleting a user as a provider" do
+          expect(policy).not_to permit(data_provider, user)
+        end
+
+        it "does not allow allows deleting a user as a coordinator" do
+          expect(policy).not_to permit(data_coordinator, user)
+        end
+
+        it "does not allow deleting a user as a support user" do
+          expect(policy).not_to permit(support, user)
+        end
+      end
+
+      context "and user is the DPO that hasn't signed the agreement" do
+        let(:user) { create(:user, active: false, is_dpo: true) }
+
+        it "does not allow deleting a user as a provider" do
+          expect(policy).not_to permit(data_provider, user)
+        end
+
+        it "does not allow allows deleting a user as a coordinator" do
+          expect(policy).not_to permit(data_coordinator, user)
+        end
+
+        it "allows deleting a user as a support user" do
+          expect(policy).to permit(support, user)
+        end
+      end
+    end
+  end
 end
 # rubocop:enable RSpec/RepeatedExample
diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb
index ddc457f6c..f73136948 100644
--- a/spec/requests/users_controller_spec.rb
+++ b/spec/requests/users_controller_spec.rb
@@ -103,13 +103,30 @@ RSpec.describe UsersController, type: :request do
         expect(response).to redirect_to(new_user_session_path)
       end
     end
+
+    describe "#delete-confirmation" do
+      it "redirects to the sign in page" do
+        get "/users/#{user.id}/delete-confirmation"
+        expect(response).to redirect_to("/account/sign-in")
+      end
+    end
+
+    describe "#delete" do
+      it "redirects to the sign in page" do
+        delete "/users/#{user.id}/delete"
+        expect(response).to redirect_to("/account/sign-in")
+      end
+    end
   end
 
   context "when user is signed in as a data provider" do
+    before do
+      sign_in user
+    end
+
     describe "#show" do
       context "when the current user matches the user ID" do
         before do
-          sign_in user
           get "/users/#{user.id}", headers:, params: {}
         end
 
@@ -155,7 +172,6 @@ RSpec.describe UsersController, type: :request do
         let(:user) { create(:user, role: nil) }
 
         before do
-          sign_in user
           get "/users/#{user.id}", headers:, params: {}
         end
 
@@ -166,7 +182,6 @@ RSpec.describe UsersController, type: :request do
 
       context "when the current user does not match the user ID" do
         before do
-          sign_in user
           get "/users/#{other_user.id}", headers:, params: {}
         end
 
@@ -223,7 +238,6 @@ RSpec.describe UsersController, type: :request do
     describe "#edit" do
       context "when the current user matches the user ID" do
         before do
-          sign_in user
           get "/users/#{user.id}/edit", headers:, params: {}
         end
 
@@ -242,7 +256,6 @@ RSpec.describe UsersController, type: :request do
 
       context "when the current user does not match the user ID" do
         before do
-          sign_in user
           get "/users/#{other_user.id}/edit", headers:, params: {}
         end
 
@@ -255,7 +268,6 @@ RSpec.describe UsersController, type: :request do
     describe "#edit_password" do
       context "when the current user matches the user ID" do
         before do
-          sign_in user
           get "/account/edit/password", headers:, params: {}
         end
 
@@ -270,7 +282,6 @@ RSpec.describe UsersController, type: :request do
 
       context "when the current user does not match the user ID" do
         before do
-          sign_in user
           get "/users/#{other_user.id}/edit", headers:, params: {}
         end
 
@@ -283,7 +294,6 @@ RSpec.describe UsersController, type: :request do
     describe "#update" do
       context "when the current user matches the user ID" do
         before do
-          sign_in user
           patch "/users/#{user.id}", headers:, params:
         end
 
@@ -313,7 +323,6 @@ RSpec.describe UsersController, type: :request do
 
       context "when the update fails to persist" do
         before do
-          sign_in user
           allow(User).to receive(:find_by).and_return(user)
           allow(user).to receive(:update).and_return(false)
           patch "/users/#{user.id}", headers:, params:
@@ -328,7 +337,6 @@ RSpec.describe UsersController, type: :request do
         let(:params) { { id: other_user.id, user: { name: new_name } } }
 
         before do
-          sign_in user
           patch "/users/#{other_user.id}", headers:, params:
         end
 
@@ -345,7 +353,6 @@ RSpec.describe UsersController, type: :request do
         end
 
         before do
-          sign_in user
           patch "/users/#{user.id}", headers:, params:
         end
 
@@ -368,10 +375,6 @@ RSpec.describe UsersController, type: :request do
       end
       let(:request) { post "/users/", headers:, params: }
 
-      before do
-        sign_in user
-      end
-
       it "does not invite a new user" do
         expect { request }.not_to change(User, :count)
       end
@@ -381,17 +384,37 @@ RSpec.describe UsersController, type: :request do
         expect(response).to have_http_status(:unauthorized)
       end
     end
+
+    describe "#delete-confirmation" do
+      before do
+        get "/users/#{user.id}/delete-confirmation"
+      end
+
+      it "returns 401 unauthorized" do
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+
+    describe "#delete" do
+      before do
+        delete "/users/#{user.id}/delete"
+      end
+
+      it "returns 401 unauthorized" do
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
   end
 
   context "when user is signed in as a data coordinator" do
     let(:user) { create(:user, :data_coordinator, email: "coordinator@example.com", organisation: create(:organisation, :without_dpc)) }
     let!(:other_user) { create(:user, organisation: user.organisation, name: "filter name", email: "filter@example.com") }
 
-    describe "#index" do
-      before do
-        sign_in user
-      end
+    before do
+      sign_in user
+    end
 
+    describe "#index" do
       context "when there are no url params" do
         before do
           get "/users", headers:, params: {}
@@ -532,7 +555,6 @@ RSpec.describe UsersController, type: :request do
       let(:user) { create(:user) }
 
       before do
-        sign_in user
         get "/users", headers:, params: {}
       end
 
@@ -544,7 +566,6 @@ RSpec.describe UsersController, type: :request do
     describe "#show" do
       context "when the current user matches the user ID" do
         before do
-          sign_in user
           get "/users/#{user.id}", headers:, params: {}
         end
 
@@ -579,12 +600,15 @@ RSpec.describe UsersController, type: :request do
           it "does not allow resending invitation emails" do
             expect(page).not_to have_button("Resend invite link")
           end
+
+          it "does not allow deleting the the user" do
+            expect(page).not_to have_link("Delete this user", href: "/users/#{user.id}/delete-confirmation")
+          end
         end
       end
 
       context "when the current user does not match the user ID" do
         before do
-          sign_in user
           get "/users/#{other_user.id}", headers:, params: {}
         end
 
@@ -622,7 +646,7 @@ RSpec.describe UsersController, type: :request do
             end
 
             it "shows if user is not active" do
-              expect(page).to have_content("Deactivated")
+              assert_select ".govuk-tag", text: /Deactivated/, count: 1
             end
 
             it "allows reactivating the user" do
@@ -652,7 +676,6 @@ RSpec.describe UsersController, type: :request do
     describe "#edit" do
       context "when the current user matches the user ID" do
         before do
-          sign_in user
           get "/users/#{user.id}/edit", headers:, params: {}
         end
 
@@ -673,7 +696,6 @@ RSpec.describe UsersController, type: :request do
 
       context "when the current user does not match the user ID" do
         before do
-          sign_in user
           get "/users/#{other_user.id}/edit", headers:, params: {}
         end
 
@@ -706,7 +728,6 @@ RSpec.describe UsersController, type: :request do
     describe "#edit_password" do
       context "when the current user matches the user ID" do
         before do
-          sign_in user
           get "/account/edit/password", headers:, params: {}
         end
 
@@ -720,10 +741,6 @@ RSpec.describe UsersController, type: :request do
       end
 
       context "when the current user does not match the user ID" do
-        before do
-          sign_in user
-        end
-
         it "there is no route" do
           expect {
             get "/users/#{other_user.id}/password/edit", headers:, params: {}
@@ -735,7 +752,6 @@ RSpec.describe UsersController, type: :request do
     describe "#update" do
       context "when the current user matches the user ID" do
         before do
-          sign_in user
           patch "/users/#{user.id}", headers:, params:
         end
 
@@ -770,7 +786,6 @@ RSpec.describe UsersController, type: :request do
           end
 
           before do
-            sign_in user
             patch "/users/#{user.id}", headers:, params:
           end
 
@@ -782,10 +797,6 @@ RSpec.describe UsersController, type: :request do
       end
 
       context "when the current user does not match the user ID" do
-        before do
-          sign_in user
-        end
-
         context "when the user is part of the same organisation as the current user" do
           it "updates the user" do
             expect { patch "/users/#{other_user.id}", headers:, params: }
@@ -871,7 +882,6 @@ RSpec.describe UsersController, type: :request do
             let(:params) { { id: other_user.id, user: { name: new_name } } }
 
             before do
-              sign_in user
               patch "/users/#{other_user.id}", headers:, params:
             end
 
@@ -884,7 +894,6 @@ RSpec.describe UsersController, type: :request do
 
       context "when the update fails to persist" do
         before do
-          sign_in user
           allow(User).to receive(:find_by).and_return(user)
           allow(user).to receive(:update).and_return(false)
           patch "/users/#{user.id}", headers:, params:
@@ -905,7 +914,6 @@ RSpec.describe UsersController, type: :request do
         end
 
         before do
-          sign_in user
           patch "/users/#{user.id}", headers:, params:
         end
 
@@ -977,10 +985,6 @@ RSpec.describe UsersController, type: :request do
       end
       let(:request) { post "/users/", headers:, params: }
 
-      before do
-        sign_in user
-      end
-
       it "invites a new user" do
         expect { request }.to change(User, :count).by(1)
       end
@@ -1102,10 +1106,6 @@ RSpec.describe UsersController, type: :request do
     end
 
     describe "#new" do
-      before do
-        sign_in user
-      end
-
       it "cannot assign support role to the new user" do
         get "/users/new"
         expect(page).not_to have_field("user-role-support-field")
@@ -1113,10 +1113,6 @@ RSpec.describe UsersController, type: :request do
     end
 
     describe "#deactivate" do
-      before do
-        sign_in user
-      end
-
       context "when the current user matches the user ID" do
         before do
           get "/users/#{user.id}/deactivate", headers:, params: {}
@@ -1143,10 +1139,6 @@ RSpec.describe UsersController, type: :request do
     end
 
     describe "#reactivate" do
-      before do
-        sign_in user
-      end
-
       context "when the current user does not match the user ID" do
         before do
           other_user.update!(active: false)
@@ -1162,6 +1154,26 @@ RSpec.describe UsersController, type: :request do
         end
       end
     end
+
+    describe "#delete-confirmation" do
+      before do
+        get "/users/#{user.id}/delete-confirmation"
+      end
+
+      it "returns 401 unauthorized" do
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+
+    describe "#delete" do
+      before do
+        delete "/users/#{user.id}/delete"
+      end
+
+      it "returns 401 unauthorized" do
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
   end
 
   context "when user is signed in as a support user" do
@@ -1170,15 +1182,15 @@ RSpec.describe UsersController, type: :request do
 
     before do
       allow(user).to receive(:need_two_factor_authentication?).and_return(false)
+      sign_in user
     end
 
     describe "#index" do
       let!(:other_user) { create(:user, organisation: user.organisation, name: "User 2", email: "other@example.com") }
-      let!(:inactive_user) { create(:user, organisation: user.organisation, active: false, name: "User 3", email: "inactive@example.com") }
+      let!(:inactive_user) { create(:user, organisation: user.organisation, active: false, name: "User 3", email: "inactive@example.com", last_sign_in_at: Time.zone.local(2022, 10, 10)) }
       let!(:other_org_user) { create(:user, name: "User 4", email: "otherorg@otherexample.com", organisation: create(:organisation, :without_dpc)) }
 
       before do
-        sign_in user
         get "/users", headers:, params: {}
       end
 
@@ -1189,7 +1201,11 @@ RSpec.describe UsersController, type: :request do
         expect(page).to have_content(other_org_user.name)
       end
 
-      it "shows last logged in as deactivated for inactive users" do
+      it "shows last logged in date for all users" do
+        expect(page).to have_content("10 October 2022")
+      end
+
+      it "shows status tag as deactivated for inactive users" do
         expect(page).to have_content("Deactivated")
       end
 
@@ -1326,7 +1342,6 @@ RSpec.describe UsersController, type: :request do
 
       before do
         create_list(:user, 25)
-        sign_in user
       end
 
       context "when there is no search param" do
@@ -1371,7 +1386,6 @@ RSpec.describe UsersController, type: :request do
     describe "#show" do
       context "when the current user matches the user ID" do
         before do
-          sign_in user
           get "/users/#{user.id}", headers:, params: {}
         end
 
@@ -1396,7 +1410,6 @@ RSpec.describe UsersController, type: :request do
 
       context "when the current user does not match the user ID" do
         before do
-          sign_in user
           get "/users/#{other_user.id}", headers:, params: {}
         end
 
@@ -1427,6 +1440,10 @@ RSpec.describe UsersController, type: :request do
             expect(page).to have_link("Deactivate user", href: "/users/#{other_user.id}/deactivate")
           end
 
+          it "does not alow deleting the the user" do
+            expect(page).not_to have_link("Delete this user", href: "/users/#{other_user.id}/delete-confirmation")
+          end
+
           context "when user never logged in" do
             before do
               other_user.update!(last_sign_in_at: nil)
@@ -1458,6 +1475,10 @@ RSpec.describe UsersController, type: :request do
             it "allows you to resend invitation emails" do
               expect(page).to have_button("Resend invite link")
             end
+
+            it "does not allow deleting the the user" do
+              expect(page).not_to have_link("Delete this user", href: "/users/#{other_user.id}/delete-confirmation")
+            end
           end
 
           context "when user is deactivated" do
@@ -1467,12 +1488,39 @@ RSpec.describe UsersController, type: :request do
             end
 
             it "shows if user is not active" do
-              expect(page).to have_content("Deactivated")
+              assert_select ".govuk-tag", text: /Deactivated/, count: 1
             end
 
             it "allows reactivating the user" do
               expect(page).to have_link("Reactivate user", href: "/users/#{other_user.id}/reactivate")
             end
+
+            it "allows deleting the the user" do
+              expect(page).to have_link("Delete this user", href: "/users/#{other_user.id}/delete-confirmation")
+            end
+
+            it "does not render informative text about deleting the user" do
+              expect(response).to have_http_status(:ok)
+              expect(page).not_to have_content("This user was active in an open or editable collection year, and cannot be deleted.")
+            end
+
+            context "and has associated logs in editable collection period" do
+              before do
+                create(:data_protection_confirmation, organisation: other_user.organisation, confirmed: true)
+                create(:lettings_log, owning_organisation: other_user.organisation, created_by: other_user)
+                get "/users/#{other_user.id}"
+              end
+
+              it "does not render delete this user" do
+                expect(response).to have_http_status(:ok)
+                expect(page).not_to have_link("Delete this user", href: "/users/#{user.id}/delete-confirmation")
+              end
+
+              it "adds informative text about deleting the user" do
+                expect(response).to have_http_status(:ok)
+                expect(page).to have_content("This user was active in an open or editable collection year, and cannot be deleted.")
+              end
+            end
           end
         end
 
@@ -1503,7 +1551,6 @@ RSpec.describe UsersController, type: :request do
     describe "#edit" do
       context "when the current user matches the user ID" do
         before do
-          sign_in user
           get "/users/#{user.id}/edit", headers:, params: {}
         end
 
@@ -1525,7 +1572,6 @@ RSpec.describe UsersController, type: :request do
 
       context "when the current user does not match the user ID" do
         before do
-          sign_in user
           get "/users/#{other_user.id}/edit", headers:, params: {}
         end
 
@@ -1581,7 +1627,6 @@ RSpec.describe UsersController, type: :request do
     describe "#edit_password" do
       context "when the current user matches the user ID" do
         before do
-          sign_in user
           get "/account/edit/password", headers:, params: {}
         end
 
@@ -1595,10 +1640,6 @@ RSpec.describe UsersController, type: :request do
       end
 
       context "when the current user does not match the user ID" do
-        before do
-          sign_in user
-        end
-
         it "there is no route" do
           expect {
             get "/users/#{other_user.id}/password/edit", headers:, params: {}
@@ -1611,10 +1652,6 @@ RSpec.describe UsersController, type: :request do
       context "when the current user matches the user ID" do
         let(:request) { patch "/users/#{user.id}", headers:, params: }
 
-        before do
-          sign_in user
-        end
-
         it "updates the user" do
           request
           user.reload
@@ -1726,7 +1763,6 @@ RSpec.describe UsersController, type: :request do
           end
 
           before do
-            sign_in user
             patch "/users/#{user.id}", headers:, params:
           end
 
@@ -1738,10 +1774,6 @@ RSpec.describe UsersController, type: :request do
       end
 
       context "when the current user does not match the user ID" do
-        before do
-          sign_in user
-        end
-
         context "when the user is part of the same organisation as the current user" do
           it "updates the user" do
             expect { patch "/users/#{other_user.id}", headers:, params: }
@@ -1796,10 +1828,6 @@ RSpec.describe UsersController, type: :request do
             let(:other_user) { create(:user) }
             let(:params) { { id: other_user.id, user: { name: new_name } } }
 
-            before do
-              sign_in user
-            end
-
             it "updates the user" do
               expect { patch "/users/#{other_user.id}", headers:, params: }
                 .to change { other_user.reload.name }.from(other_user.name).to(new_name)
@@ -1886,7 +1914,6 @@ RSpec.describe UsersController, type: :request do
 
       context "when the update fails to persist" do
         before do
-          sign_in user
           allow(User).to receive(:find_by).and_return(user)
           allow(user).to receive(:update).and_return(false)
           patch "/users/#{user.id}", headers:, params:
@@ -1914,10 +1941,6 @@ RSpec.describe UsersController, type: :request do
       end
       let(:request) { post "/users/", headers:, params: }
 
-      before do
-        sign_in user
-      end
-
       it "invites a new user" do
         expect { request }.to change(User, :count).by(1)
       end
@@ -1990,7 +2013,6 @@ RSpec.describe UsersController, type: :request do
 
     describe "#new" do
       before do
-        sign_in user
         create(:organisation, name: "other org")
       end
 
@@ -2018,6 +2040,68 @@ RSpec.describe UsersController, type: :request do
         end
       end
     end
+
+    describe "#delete-confirmation" do
+      let(:other_user) { create(:user, active: false) }
+
+      before do
+        get "/users/#{other_user.id}/delete-confirmation"
+      end
+
+      it "shows the correct title" do
+        expect(page.find("h1").text).to include "Are you sure you want to delete this user?"
+      end
+
+      it "shows a warning to the user" do
+        expect(page).to have_selector(".govuk-warning-text", text: "You will not be able to undo this action")
+      end
+
+      it "shows a button to delete the selected user" do
+        expect(page).to have_selector("form.button_to button", text: "Delete this user")
+      end
+
+      it "the delete user button submits the correct data to the correct path" do
+        form_containing_button = page.find("form.button_to")
+
+        expect(form_containing_button[:action]).to eq delete_user_path(other_user)
+        expect(form_containing_button).to have_field "_method", type: :hidden, with: "delete"
+      end
+
+      it "shows a cancel link with the correct style" do
+        expect(page).to have_selector("a.govuk-button--secondary", text: "Cancel")
+      end
+
+      it "shows cancel link that links back to the user page" do
+        expect(page).to have_link(text: "Cancel", href: user_path(other_user))
+      end
+    end
+
+    describe "#delete" do
+      let(:other_user) { create(:user, name: "User to be deleted", active: false) }
+
+      before do
+        delete "/users/#{other_user.id}/delete"
+      end
+
+      it "deletes the user" do
+        other_user.reload
+        expect(other_user.status).to eq(:deleted)
+        expect(other_user.discarded_at).not_to be nil
+      end
+
+      it "redirects to the users list and displays a notice that the user has been deleted" do
+        expect(response).to redirect_to users_organisation_path(other_user.organisation)
+        follow_redirect!
+        expect(page).to have_selector(".govuk-notification-banner--success")
+        expect(page).to have_selector(".govuk-notification-banner--success", text: "User to be deleted has been deleted.")
+      end
+
+      it "does not display the deleted user" do
+        expect(response).to redirect_to users_organisation_path(other_user.organisation)
+        follow_redirect!
+        expect(page).not_to have_link("User to be deleted")
+      end
+    end
   end
 
   describe "title link" do

From f06bcaa5e6ff425f84814c4814679478c6a6c185 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Wed, 3 Apr 2024 16:00:59 +0100
Subject: [PATCH 2/4] Bump express from 4.18.2 to 4.19.2 (#2364)

Bumps [express](https://github.com/expressjs/express) from 4.18.2 to 4.19.2.
- [Release notes](https://github.com/expressjs/express/releases)
- [Changelog](https://github.com/expressjs/express/blob/master/History.md)
- [Commits](https://github.com/expressjs/express/compare/4.18.2...4.19.2)

---
updated-dependencies:
- dependency-name: express
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
---
 yarn.lock | 54 ++++++++++++++++++------------------------------------
 1 file changed, 18 insertions(+), 36 deletions(-)

diff --git a/yarn.lock b/yarn.lock
index 24ea564fe..e67ab3514 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -1771,25 +1771,7 @@ bl@^4.1.0:
     inherits "^2.0.4"
     readable-stream "^3.4.0"
 
-body-parser@1.20.1:
-  version "1.20.1"
-  resolved "https://registry.yarnpkg.com/body-parser/-/body-parser-1.20.1.tgz#b1812a8912c195cd371a3ee5e66faa2338a5c668"
-  integrity sha512-jWi7abTbYwajOytWCQc37VulmWiRae5RyTpaCyDcS5/lMdtwSz5lOpDE67srw/HYe35f1z3fDQw+3txg7gNtWw==
-  dependencies:
-    bytes "3.1.2"
-    content-type "~1.0.4"
-    debug "2.6.9"
-    depd "2.0.0"
-    destroy "1.2.0"
-    http-errors "2.0.0"
-    iconv-lite "0.4.24"
-    on-finished "2.4.1"
-    qs "6.11.0"
-    raw-body "2.5.1"
-    type-is "~1.6.18"
-    unpipe "1.0.0"
-
-body-parser@^1.20.2:
+body-parser@1.20.2, body-parser@^1.20.2:
   version "1.20.2"
   resolved "https://registry.yarnpkg.com/body-parser/-/body-parser-1.20.2.tgz#6feb0e21c4724d06de7ff38da36dad4f57a747fd"
   integrity sha512-ml9pReCu3M61kGlqoTm2umSXTlRTuGTx0bfYj+uIUKKYycG5NtSbeetV3faSU6R7ajOPw0g/J1PvK4qNy7s5bA==
@@ -2177,10 +2159,10 @@ cookie@0.4.2, cookie@~0.4.1:
   resolved "https://registry.yarnpkg.com/cookie/-/cookie-0.4.2.tgz#0e41f24de5ecf317947c82fc789e06a884824432"
   integrity sha512-aSWTXFzaKWkvHO1Ny/s+ePFpvKsPnjc551iI41v3ny/ow6tBG5Vd+FuqGNhh1LxOmVzOlGUriIlOaokOvhaStA==
 
-cookie@0.5.0:
-  version "0.5.0"
-  resolved "https://registry.yarnpkg.com/cookie/-/cookie-0.5.0.tgz#d1f5d71adec6558c58f389987c366aa47e994f8b"
-  integrity sha512-YZ3GUyn/o8gfKJlnlX7g7xq4gyO6OSuhGPKaaGssGB2qgDUS0gPgtTvoyZLTt9Ab6dC4hfc9dV5arkvc/OCmrw==
+cookie@0.6.0:
+  version "0.6.0"
+  resolved "https://registry.yarnpkg.com/cookie/-/cookie-0.6.0.tgz#2798b04b071b0ecbff0dbb62a505a8efa4e19051"
+  integrity sha512-U71cyTamuh1CRNCfpGY6to28lxvNwPG4Guz/EVjgf3Jmzv0vlDp1atT9eS5dDjMYHucpHbWns6Lwf3BKz6svdw==
 
 copy-webpack-plugin@^10.2.4:
   version "10.2.4"
@@ -2855,16 +2837,16 @@ express-session@^1.17.3:
     uid-safe "~2.1.5"
 
 express@^4.18.2:
-  version "4.18.2"
-  resolved "https://registry.yarnpkg.com/express/-/express-4.18.2.tgz#3fabe08296e930c796c19e3c516979386ba9fd59"
-  integrity sha512-5/PsL6iGPdfQ/lKM1UuielYgv3BUoJfz1aUwU9vHZ+J7gyvwdQXFEBIEIaxeGf0GIcreATNyBExtalisDbuMqQ==
+  version "4.19.2"
+  resolved "https://registry.yarnpkg.com/express/-/express-4.19.2.tgz#e25437827a3aa7f2a827bc8171bbbb664a356465"
+  integrity sha512-5T6nhjsT+EOMzuck8JjBHARTHfMht0POzlA60WV2pMD3gyXw2LZnZ+ueGdNxG+0calOJcWKbpFcuzLZ91YWq9Q==
   dependencies:
     accepts "~1.3.8"
     array-flatten "1.1.1"
-    body-parser "1.20.1"
+    body-parser "1.20.2"
     content-disposition "0.5.4"
     content-type "~1.0.4"
-    cookie "0.5.0"
+    cookie "0.6.0"
     cookie-signature "1.0.6"
     debug "2.6.9"
     depd "2.0.0"
@@ -4664,20 +4646,20 @@ range-parser@~1.2.0, range-parser@~1.2.1:
   resolved "https://registry.yarnpkg.com/range-parser/-/range-parser-1.2.1.tgz#3cf37023d199e1c24d1a55b84800c2f3e6468031"
   integrity sha512-Hrgsx+orqoygnmhFbKaHE6c296J+HTAQXoxEF6gNupROmmGJRoyzfG3ccAveqCBrwr/2yxQ5BVd/GTl5agOwSg==
 
-raw-body@2.5.1, raw-body@^2.3.2:
-  version "2.5.1"
-  resolved "https://registry.yarnpkg.com/raw-body/-/raw-body-2.5.1.tgz#fe1b1628b181b700215e5fd42389f98b71392857"
-  integrity sha512-qqJBtEyVgS0ZmPGdCFPWJ3FreoqvG4MVQln/kCgF7Olq95IbOp0/BWyMwbdtn4VTvkM8Y7khCQ2Xgk/tcrCXig==
+raw-body@2.5.2:
+  version "2.5.2"
+  resolved "https://registry.yarnpkg.com/raw-body/-/raw-body-2.5.2.tgz#99febd83b90e08975087e8f1f9419a149366b68a"
+  integrity sha512-8zGqypfENjCIqGhgXToC8aB2r7YrBX+AQAfIPs/Mlk+BtPTztOvTS01NRW/3Eh60J+a48lt8qsCzirQ6loCVfA==
   dependencies:
     bytes "3.1.2"
     http-errors "2.0.0"
     iconv-lite "0.4.24"
     unpipe "1.0.0"
 
-raw-body@2.5.2:
-  version "2.5.2"
-  resolved "https://registry.yarnpkg.com/raw-body/-/raw-body-2.5.2.tgz#99febd83b90e08975087e8f1f9419a149366b68a"
-  integrity sha512-8zGqypfENjCIqGhgXToC8aB2r7YrBX+AQAfIPs/Mlk+BtPTztOvTS01NRW/3Eh60J+a48lt8qsCzirQ6loCVfA==
+raw-body@^2.3.2:
+  version "2.5.1"
+  resolved "https://registry.yarnpkg.com/raw-body/-/raw-body-2.5.1.tgz#fe1b1628b181b700215e5fd42389f98b71392857"
+  integrity sha512-qqJBtEyVgS0ZmPGdCFPWJ3FreoqvG4MVQln/kCgF7Olq95IbOp0/BWyMwbdtn4VTvkM8Y7khCQ2Xgk/tcrCXig==
   dependencies:
     bytes "3.1.2"
     http-errors "2.0.0"

From 73cef27a0cbcc1536440285315f3f7092f742c68 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Wed, 3 Apr 2024 17:09:41 +0100
Subject: [PATCH 3/4] Restore form date validations (#2365)

---
 app/services/feature_toggle.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/services/feature_toggle.rb b/app/services/feature_toggle.rb
index 1aaba1571..987a087ac 100644
--- a/app/services/feature_toggle.rb
+++ b/app/services/feature_toggle.rb
@@ -1,6 +1,6 @@
 class FeatureToggle
   def self.allow_future_form_use?
-    Rails.env.development? || Rails.env.review? || Rails.env.staging?
+    false
   end
 
   def self.bulk_upload_duplicate_log_check_enabled?

From 378388ebc5d4bb893a051c78528c3af2d03063db Mon Sep 17 00:00:00 2001
From: Robert Sullivan <Robert.Sullivan@Softwire.com>
Date: Thu, 4 Apr 2024 15:35:45 +0100
Subject: [PATCH 4/4] CLDC-3156: Update 2024 referral options (#2366)

* CLDC-3156: Update 2024 referral options

* CLDC-3156: Add tests for referral question

* CLDC-3156: Update other referral questions
---
 .../form/lettings/questions/referral.rb       | 123 ++++++++++-----
 .../form/lettings/questions/referral_prp.rb   | 141 ++++++++++++------
 .../questions/referral_supported_housing.rb   | 132 ++++++++++------
 .../referral_supported_housing_prp.rb         | 112 +++++++++++---
 .../form/lettings/questions/referral_spec.rb  | 101 +++++++++++++
 5 files changed, 463 insertions(+), 146 deletions(-)
 create mode 100644 spec/models/form/lettings/questions/referral_spec.rb

diff --git a/app/models/form/lettings/questions/referral.rb b/app/models/form/lettings/questions/referral.rb
index 5bf45a187..feb0082f4 100644
--- a/app/models/form/lettings/questions/referral.rb
+++ b/app/models/form/lettings/questions/referral.rb
@@ -7,49 +7,92 @@ class Form::Lettings::Questions::Referral < ::Form::Question
     @type = "radio"
     @check_answers_card_number = 0
     @hint_text = "You told us that you are a local authority and that the needs type is general needs. We have removed some options because of this."
-    @answer_options = ANSWER_OPTIONS
     @question_number = QUESTION_NUMBER_FROM_YEAR[form.start_date.year] || QUESTION_NUMBER_FROM_YEAR[QUESTION_NUMBER_FROM_YEAR.keys.max]
   end
 
-  ANSWER_OPTIONS = {
-    "1" => {
-      "value" => "Internal transfer",
-      "hint" => "Where the tenant has moved to another social property owned by the same landlord.",
-    },
-    "2" => {
-      "value" => "Tenant applied directly (no referral or nomination)",
-    },
-    "8" => {
-      "value" => "Re-located through official housing mobility scheme",
-    },
-    "10" => {
-      "value" => "Other social landlord",
-    },
-    "9" => {
-      "value" => "Community learning disability team",
-    },
-    "14" => {
-      "value" => "Community mental health team",
-    },
-    "15" => {
-      "value" => "Health service",
-    },
-    "12" => {
-      "value" => "Police, probation or prison",
-    },
-    "7" => {
-      "value" => "Voluntary agency",
-    },
-    "13" => {
-      "value" => "Youth offending team",
-    },
-    "17" => {
-      "value" => "Children’s Social Care",
-    },
-    "16" => {
-      "value" => "Other",
-    },
-  }.freeze
+  def answer_options
+    if form.start_year_after_2024?
+      {
+        "1" => {
+          "value" => "Internal transfer",
+          "hint" => "Where the tenant has moved to another social property owned by the same landlord.",
+        },
+        "2" => {
+          "value" => "Tenant applied directly (no referral or nomination)",
+        },
+        "8" => {
+          "value" => "Re-located through official housing mobility scheme",
+        },
+        "10" => {
+          "value" => "Other social landlord",
+        },
+        "9" => {
+          "value" => "Community learning disability team",
+        },
+        "14" => {
+          "value" => "Community mental health team",
+        },
+        "15" => {
+          "value" => "Health service",
+        },
+        "18" => {
+          "value" => "Police, probation, prison or youth offending team – tenant had custodial sentence",
+        },
+        "19" => {
+          "value" => "Police, probation, prison or youth offending team – no custodial sentence",
+        },
+        "7" => {
+          "value" => "Voluntary agency",
+        },
+        "17" => {
+          "value" => "Children’s Social Care",
+        },
+        "16" => {
+          "value" => "Other",
+        },
+      }.freeze
+    else
+      {
+        "1" => {
+          "value" => "Internal transfer",
+          "hint" => "Where the tenant has moved to another social property owned by the same landlord.",
+        },
+        "2" => {
+          "value" => "Tenant applied directly (no referral or nomination)",
+        },
+        "8" => {
+          "value" => "Re-located through official housing mobility scheme",
+        },
+        "10" => {
+          "value" => "Other social landlord",
+        },
+        "9" => {
+          "value" => "Community learning disability team",
+        },
+        "14" => {
+          "value" => "Community mental health team",
+        },
+        "15" => {
+          "value" => "Health service",
+        },
+        "12" => {
+          "value" => "Police, probation or prison",
+        },
+        "7" => {
+          "value" => "Voluntary agency",
+        },
+        "13" => {
+          "value" => "Youth offending team",
+        },
+        "17" => {
+          "value" => "Children’s Social Care",
+        },
+        "16" => {
+          "value" => "Other",
+        },
+      }.freeze
+    end
+  end
 
   QUESTION_NUMBER_FROM_YEAR = { 2023 => 85, 2024 => 84 }.freeze
 end
diff --git a/app/models/form/lettings/questions/referral_prp.rb b/app/models/form/lettings/questions/referral_prp.rb
index ee676e81e..c5549ce49 100644
--- a/app/models/form/lettings/questions/referral_prp.rb
+++ b/app/models/form/lettings/questions/referral_prp.rb
@@ -7,55 +7,104 @@ class Form::Lettings::Questions::ReferralPrp < ::Form::Question
     @type = "radio"
     @check_answers_card_number = 0
     @hint_text = "You told us that the needs type is general needs. We have removed some options because of this."
-    @answer_options = ANSWER_OPTIONS
     @question_number = QUESTION_NUMBER_FROM_YEAR[form.start_date.year] || QUESTION_NUMBER_FROM_YEAR[QUESTION_NUMBER_FROM_YEAR.keys.max]
   end
 
-  ANSWER_OPTIONS = {
-    "1" => {
-      "value" => "Internal transfer",
-      "hint" => "Where the tenant has moved to another social property owned by the same landlord.",
-    },
-    "2" => {
-      "value" => "Tenant applied directly (no nomination)",
-    },
-    "3" => {
-      "value" => "Nominated by a local housing authority",
-    },
-    "4" => {
-      "value" => "Referred by local authority housing department",
-    },
-    "8" => {
-      "value" => "Re-located through official housing mobility scheme",
-    },
-    "10" => {
-      "value" => "Other social landlord",
-    },
-    "9" => {
-      "value" => "Community learning disability team",
-    },
-    "14" => {
-      "value" => "Community mental health team",
-    },
-    "15" => {
-      "value" => "Health service",
-    },
-    "12" => {
-      "value" => "Police, probation or prison",
-    },
-    "7" => {
-      "value" => "Voluntary agency",
-    },
-    "13" => {
-      "value" => "Youth offending team",
-    },
-    "17" => {
-      "value" => "Children’s Social Care",
-    },
-    "16" => {
-      "value" => "Other",
-    },
-  }.freeze
+  def answer_options
+    if form.start_year_after_2024?
+      {
+        "1" => {
+          "value" => "Internal transfer",
+          "hint" => "Where the tenant has moved to another social property owned by the same landlord.",
+        },
+        "2" => {
+          "value" => "Tenant applied directly (no nomination)",
+        },
+        "3" => {
+          "value" => "Nominated by a local housing authority",
+        },
+        "4" => {
+          "value" => "Referred by local authority housing department",
+        },
+        "8" => {
+          "value" => "Re-located through official housing mobility scheme",
+        },
+        "10" => {
+          "value" => "Other social landlord",
+        },
+        "9" => {
+          "value" => "Community learning disability team",
+        },
+        "14" => {
+          "value" => "Community mental health team",
+        },
+        "15" => {
+          "value" => "Health service",
+        },
+        "18" => {
+          "value" => "Police, probation, prison or youth offending team – tenant had custodial sentence",
+        },
+        "19" => {
+          "value" => "Police, probation, prison or youth offending team – no custodial sentence",
+        },
+        "7" => {
+          "value" => "Voluntary agency",
+        },
+        "17" => {
+          "value" => "Children’s Social Care",
+        },
+        "16" => {
+          "value" => "Other",
+        },
+      }.freeze
+    else
+      {
+        "1" => {
+          "value" => "Internal transfer",
+          "hint" => "Where the tenant has moved to another social property owned by the same landlord.",
+        },
+        "2" => {
+          "value" => "Tenant applied directly (no nomination)",
+        },
+        "3" => {
+          "value" => "Nominated by a local housing authority",
+        },
+        "4" => {
+          "value" => "Referred by local authority housing department",
+        },
+        "8" => {
+          "value" => "Re-located through official housing mobility scheme",
+        },
+        "10" => {
+          "value" => "Other social landlord",
+        },
+        "9" => {
+          "value" => "Community learning disability team",
+        },
+        "14" => {
+          "value" => "Community mental health team",
+        },
+        "15" => {
+          "value" => "Health service",
+        },
+        "12" => {
+          "value" => "Police, probation or prison",
+        },
+        "7" => {
+          "value" => "Voluntary agency",
+        },
+        "13" => {
+          "value" => "Youth offending team",
+        },
+        "17" => {
+          "value" => "Children’s Social Care",
+        },
+        "16" => {
+          "value" => "Other",
+        },
+      }.freeze
+    end
+  end
 
   QUESTION_NUMBER_FROM_YEAR = { 2023 => 85, 2024 => 84 }.freeze
 end
diff --git a/app/models/form/lettings/questions/referral_supported_housing.rb b/app/models/form/lettings/questions/referral_supported_housing.rb
index a4d0403d6..642d2eb16 100644
--- a/app/models/form/lettings/questions/referral_supported_housing.rb
+++ b/app/models/form/lettings/questions/referral_supported_housing.rb
@@ -7,52 +7,98 @@ class Form::Lettings::Questions::ReferralSupportedHousing < ::Form::Question
     @type = "radio"
     @check_answers_card_number = 0
     @hint_text = "You told us that you are a local authority. We have removed some options because of this."
-    @answer_options = ANSWER_OPTIONS
     @question_number = QUESTION_NUMBER_FROM_YEAR[form.start_date.year] || QUESTION_NUMBER_FROM_YEAR[QUESTION_NUMBER_FROM_YEAR.keys.max]
   end
 
-  ANSWER_OPTIONS = {
-    "1" => {
-      "value" => "Internal transfer",
-      "hint" => "Where the tenant has moved to another social property owned by the same landlord.",
-    },
-    "2" => {
-      "value" => "Tenant applied directly (no referral)",
-    },
-    "3" => {
-      "value" => "Referred by local authority housing department",
-    },
-    "8" => {
-      "value" => "Re-located through official housing mobility scheme",
-    },
-    "10" => {
-      "value" => "Other social landlord",
-    },
-    "9" => {
-      "value" => "Community learning disability team",
-    },
-    "14" => {
-      "value" => "Community mental health team",
-    },
-    "15" => {
-      "value" => "Health service",
-    },
-    "12" => {
-      "value" => "Police, probation or prison",
-    },
-    "7" => {
-      "value" => "Voluntary agency",
-    },
-    "13" => {
-      "value" => "Youth offending team",
-    },
-    "17" => {
-      "value" => "Children’s Social Care",
-    },
-    "16" => {
-      "value" => "Other",
-    },
-  }.freeze
+  def answer_options
+    if form.start_year_after_2024?
+      {
+        "1" => {
+          "value" => "Internal transfer",
+          "hint" => "Where the tenant has moved to another social property owned by the same landlord.",
+        },
+        "2" => {
+          "value" => "Tenant applied directly (no referral)",
+        },
+        "3" => {
+          "value" => "Nominated by a local housing authority",
+        },
+        "8" => {
+          "value" => "Re-located through official housing mobility scheme",
+        },
+        "10" => {
+          "value" => "Other social landlord",
+        },
+        "9" => {
+          "value" => "Community learning disability team",
+        },
+        "14" => {
+          "value" => "Community mental health team",
+        },
+        "15" => {
+          "value" => "Health service",
+        },
+        "18" => {
+          "value" => "Police, probation, prison or youth offending team – tenant had custodial sentence",
+        },
+        "19" => {
+          "value" => "Police, probation, prison or youth offending team – no custodial sentence",
+        },
+        "7" => {
+          "value" => "Voluntary agency",
+        },
+        "17" => {
+          "value" => "Children’s Social Care",
+        },
+        "16" => {
+          "value" => "Other",
+        },
+      }.freeze
+    else
+      {
+        "1" => {
+          "value" => "Internal transfer",
+          "hint" => "Where the tenant has moved to another social property owned by the same landlord.",
+        },
+        "2" => {
+          "value" => "Tenant applied directly (no referral)",
+        },
+        "3" => {
+          "value" => "Nominated by a local housing authority",
+        },
+        "8" => {
+          "value" => "Re-located through official housing mobility scheme",
+        },
+        "10" => {
+          "value" => "Other social landlord",
+        },
+        "9" => {
+          "value" => "Community learning disability team",
+        },
+        "14" => {
+          "value" => "Community mental health team",
+        },
+        "15" => {
+          "value" => "Health service",
+        },
+        "12" => {
+          "value" => "Police, probation or prison",
+        },
+        "7" => {
+          "value" => "Voluntary agency",
+        },
+        "13" => {
+          "value" => "Youth offending team",
+        },
+        "17" => {
+          "value" => "Children’s Social Care",
+        },
+        "16" => {
+          "value" => "Other",
+        },
+      }.freeze
+    end
+  end
 
   QUESTION_NUMBER_FROM_YEAR = { 2023 => 85, 2024 => 84 }.freeze
 end
diff --git a/app/models/form/lettings/questions/referral_supported_housing_prp.rb b/app/models/form/lettings/questions/referral_supported_housing_prp.rb
index b7b937fa3..ffdd7eafb 100644
--- a/app/models/form/lettings/questions/referral_supported_housing_prp.rb
+++ b/app/models/form/lettings/questions/referral_supported_housing_prp.rb
@@ -7,26 +7,104 @@ class Form::Lettings::Questions::ReferralSupportedHousingPrp < ::Form::Question
     @type = "radio"
     @check_answers_card_number = 0
     @hint_text = ""
-    @answer_options = ANSWER_OPTIONS
     @question_number = QUESTION_NUMBER_FROM_YEAR[form.start_date.year] || QUESTION_NUMBER_FROM_YEAR[QUESTION_NUMBER_FROM_YEAR.keys.max]
   end
 
-  ANSWER_OPTIONS = {
-    "1" => { "value" => "Internal transfer", "hint" => "Where the tenant has moved to another social property owned by the same landlord." },
-    "2" => { "value" => "Tenant applied directly (no referral or nomination)" },
-    "3" => { "value" => "Nominated by a local housing authority" },
-    "4" => { "value" => "Referred by local authority housing department" },
-    "8" => { "value" => "Re-located through official housing mobility scheme" },
-    "10" => { "value" => "Other social landlord" },
-    "9" => { "value" => "Community learning disability team" },
-    "14" => { "value" => "Community mental health team" },
-    "15" => { "value" => "Health service" },
-    "12" => { "value" => "Police, probation or prison" },
-    "7" => { "value" => "Voluntary agency" },
-    "13" => { "value" => "Youth offending team" },
-    "17" => { "value" => "Children’s Social Care" },
-    "16" => { "value" => "Other" },
-  }.freeze
+  def answer_options
+    if form.start_year_after_2024?
+      {
+        "1" => {
+          "value" => "Internal transfer",
+          "hint" => "Where the tenant has moved to another social property owned by the same landlord.",
+        },
+        "2" => {
+          "value" => "Tenant applied directly (no referral or nomination)",
+        },
+        "3" => {
+          "value" => "Nominated by a local housing authority",
+        },
+        "4" => {
+          "value" => "Referred by local authority housing department",
+        },
+        "8" => {
+          "value" => "Re-located through official housing mobility scheme",
+        },
+        "10" => {
+          "value" => "Other social landlord",
+        },
+        "9" => {
+          "value" => "Community learning disability team",
+        },
+        "14" => {
+          "value" => "Community mental health team",
+        },
+        "15" => {
+          "value" => "Health service",
+        },
+        "18" => {
+          "value" => "Police, probation, prison or youth offending team – tenant had custodial sentence",
+        },
+        "19" => {
+          "value" => "Police, probation, prison or youth offending team – no custodial sentence",
+        },
+        "7" => {
+          "value" => "Voluntary agency",
+        },
+        "17" => {
+          "value" => "Children’s Social Care",
+        },
+        "16" => {
+          "value" => "Other",
+        },
+      }.freeze
+    else
+      {
+        "1" => {
+          "value" => "Internal transfer",
+          "hint" => "Where the tenant has moved to another social property owned by the same landlord.",
+        },
+        "2" => {
+          "value" => "Tenant applied directly (no referral or nomination)",
+        },
+        "3" => {
+          "value" => "Nominated by a local housing authority",
+        },
+        "4" => {
+          "value" => "Referred by local authority housing department",
+        },
+        "8" => {
+          "value" => "Re-located through official housing mobility scheme",
+        },
+        "10" => {
+          "value" => "Other social landlord",
+        },
+        "9" => {
+          "value" => "Community learning disability team",
+        },
+        "14" => {
+          "value" => "Community mental health team",
+        },
+        "15" => {
+          "value" => "Health service",
+        },
+        "12" => {
+          "value" => "Police, probation or prison",
+        },
+        "7" => {
+          "value" => "Voluntary agency",
+        },
+        "13" => {
+          "value" => "Youth offending team",
+        },
+        "17" => {
+          "value" => "Children’s Social Care",
+        },
+        "16" => {
+          "value" => "Other",
+        },
+      }.freeze
+    end
+  end
 
   QUESTION_NUMBER_FROM_YEAR = { 2023 => 85, 2024 => 84 }.freeze
 end
diff --git a/spec/models/form/lettings/questions/referral_spec.rb b/spec/models/form/lettings/questions/referral_spec.rb
new file mode 100644
index 000000000..5c7bc1735
--- /dev/null
+++ b/spec/models/form/lettings/questions/referral_spec.rb
@@ -0,0 +1,101 @@
+require "rails_helper"
+
+RSpec.describe Form::Lettings::Questions::Referral, type: :model do
+  subject(:question) { described_class.new(question_id, question_definition, page) }
+
+  let(:question_id) { nil }
+  let(:question_definition) { nil }
+  let(:page) { instance_double(Form::Page) }
+  let(:subsection) { instance_double(Form::Subsection) }
+  let(:form) { instance_double(Form, start_date: Time.zone.local(2023, 4, 1)) }
+
+  before do
+    allow(form).to receive(:start_year_after_2024?).and_return(false)
+    allow(page).to receive(:subsection).and_return(subsection)
+    allow(subsection).to receive(:form).and_return(form)
+  end
+
+  it "has correct page" do
+    expect(question.page).to eq(page)
+  end
+
+  it "has the correct id" do
+    expect(question.id).to eq("referral")
+  end
+
+  it "has the correct header" do
+    expect(question.header).to eq("What was the source of referral for this letting?")
+  end
+
+  it "has the correct check_answer_label" do
+    expect(question.check_answer_label).to eq("Source of referral for letting")
+  end
+
+  it "has the correct type" do
+    expect(question.type).to eq("radio")
+  end
+
+  it "has the correct check_answers_card_number" do
+    expect(question.check_answers_card_number).to eq(0)
+  end
+
+  it "has the correct hint" do
+    expect(question.hint_text).to eq("You told us that you are a local authority and that the needs type is general needs. We have removed some options because of this.")
+  end
+
+  it "is not marked as derived" do
+    expect(question).not_to be_derived(nil)
+  end
+
+  context "with 2023/24 form" do
+    it "has the correct answer_options" do
+      expect(question.answer_options).to eq({
+        "1" => { "value" => "Internal transfer", "hint" => "Where the tenant has moved to another social property owned by the same landlord." },
+        "2" => { "value" => "Tenant applied directly (no referral or nomination)" },
+        "8" => { "value" => "Re-located through official housing mobility scheme" },
+        "10" => { "value" => "Other social landlord" },
+        "9" => { "value" => "Community learning disability team" },
+        "14" => { "value" => "Community mental health team" },
+        "15" => { "value" => "Health service" },
+        "12" => { "value" => "Police, probation or prison" },
+        "7" => { "value" => "Voluntary agency" },
+        "13" => { "value" => "Youth offending team" },
+        "17" => { "value" => "Children’s Social Care" },
+        "16" => { "value" => "Other" },
+      })
+    end
+
+    it "has the correct question number" do
+      expect(question.question_number).to eq(85)
+    end
+  end
+
+  context "with 2024/25 form" do
+    let(:form) { instance_double(Form, start_date: Time.zone.local(2024, 4, 1)) }
+
+    before do
+      allow(form).to receive(:start_year_after_2024?).and_return(true)
+    end
+
+    it "has the correct answer_options" do
+      expect(question.answer_options).to eq({
+        "1" => { "value" => "Internal transfer", "hint" => "Where the tenant has moved to another social property owned by the same landlord." },
+        "2" => { "value" => "Tenant applied directly (no referral or nomination)" },
+        "8" => { "value" => "Re-located through official housing mobility scheme" },
+        "10" => { "value" => "Other social landlord" },
+        "9" => { "value" => "Community learning disability team" },
+        "14" => { "value" => "Community mental health team" },
+        "15" => { "value" => "Health service" },
+        "18" => { "value" => "Police, probation, prison or youth offending team – tenant had custodial sentence" },
+        "19" => { "value" => "Police, probation, prison or youth offending team – no custodial sentence" },
+        "7" => { "value" => "Voluntary agency" },
+        "17" => { "value" => "Children’s Social Care" },
+        "16" => { "value" => "Other" },
+      })
+    end
+
+    it "has the correct question number" do
+      expect(question.question_number).to eq(84)
+    end
+  end
+end