From ca8929e338ffe369996f387945de818bc547aee9 Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Thu, 30 Jan 2025 18:00:24 -0500
Subject: [PATCH 01/18] Wip: client, notes and documents ability

---
 app/controllers/hub/clients_controller.rb     | 21 +++++++++++--------
 app/controllers/hub/ctc_clients_controller.rb |  2 +-
 app/controllers/hub/notes_controller.rb       |  2 +-
 app/lib/ability.rb                            | 21 +++++++++++++++----
 app/views/devise/invitations/new.html.erb     |  2 +-
 5 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/app/controllers/hub/clients_controller.rb b/app/controllers/hub/clients_controller.rb
index eac137bcc4..11f83a40c6 100644
--- a/app/controllers/hub/clients_controller.rb
+++ b/app/controllers/hub/clients_controller.rb
@@ -9,11 +9,16 @@ class ClientsController < Hub::BaseController
     before_action :setup_sortable_client, only: [:index]
     # need to use the presenter for :update bc it has ITIN applicant methods that are used in the form
     before_action :wrap_client_in_hub_presenter, only: [:show, :edit, :edit_take_action, :update, :update_take_action]
-    before_action :redirect_unless_client_is_hub_status_editable, only: [:edit, :edit_take_action, :update, :update_take_action]
+    before_action :redirect_if_not_authorized, only: [
+      :edit, :edit_take_action, :update, :update_take_action,
+      :edit_13614c_form_page1, :update_13614c_form_page1,
+      :edit_13614c_form_page2, :update_13614c_form_page2,
+      :edit_13614c_form_page3, :update_13614c_form_page3,
+      :edit_13614c_form_page4, :update_13614c_form_page4,
+      :edit_13614c_form_page5, :update_13614c_form_page5
+    ]
     layout "hub"
 
-    MAX_COUNT = 1000
-
     def index
       @page_title = I18n.t("hub.clients.index.title")
 
@@ -47,8 +52,6 @@ def destroy
     end
 
     def edit
-      return render "public_pages/page_not_found", status: 404 if @client.intake.is_ctc?
-
       @form = UpdateClientForm.from_client(@client)
     end
 
@@ -132,6 +135,10 @@ def edit_13614c_form_page5
       @form = Update13614cFormPage5.from_client(@client)
     end
 
+    def redirect_if_not_authorized
+      raise CanCan::AccessDenied if @client.nil? || cannot?(:update, @client) || @client.intake.is_ctc?
+    end
+
     def save_and_maybe_exit(save_button_clicked, path_to_13614c_page)
       if save_button_clicked == I18n.t("general.save")
         redirect_to path_to_13614c_page
@@ -268,10 +275,6 @@ def wrap_client_in_hub_presenter
       @client = HubClientPresenter.new(@client)
     end
 
-    def redirect_unless_client_is_hub_status_editable
-      redirect_to hub_client_path(id: @client.id) unless @client.hub_status_updatable
-    end
-
     class HubClientPresenter < SimpleDelegator
       attr_reader :intake
       attr_reader :archived
diff --git a/app/controllers/hub/ctc_clients_controller.rb b/app/controllers/hub/ctc_clients_controller.rb
index e6075e1d4e..48d617a53f 100644
--- a/app/controllers/hub/ctc_clients_controller.rb
+++ b/app/controllers/hub/ctc_clients_controller.rb
@@ -4,7 +4,7 @@ class CtcClientsController < Hub::BaseController
     layout "hub"
 
     def edit
-      return render "public_pages/page_not_found", status: 404 unless @client.intake.is_ctc?
+      raise CanCan::AccessDenied unless @client.intake.is_ctc?
 
       @is_dropoff = @client.tax_returns.any? { |tax_return| tax_return.service_type == "drop_off" }
       @form = UpdateCtcClientForm.from_client(@client)
diff --git a/app/controllers/hub/notes_controller.rb b/app/controllers/hub/notes_controller.rb
index 466d81d1e5..9b4214bae2 100644
--- a/app/controllers/hub/notes_controller.rb
+++ b/app/controllers/hub/notes_controller.rb
@@ -1,7 +1,7 @@
 module Hub
   class NotesController < Hub::BaseController
     load_and_authorize_resource :client
-    load_and_authorize_resource through: :client, only: [:create]
+    load_and_authorize_resource through: :client
     load_and_authorize_resource :user, parent: false, only: [:index]
     layout "hub"
 
diff --git a/app/lib/ability.rb b/app/lib/ability.rb
index 56cdf8392a..d55425e8c0 100644
--- a/app/lib/ability.rb
+++ b/app/lib/ability.rb
@@ -22,6 +22,7 @@ def initialize(user)
     if user.admin?
       # All admins who are also state file
       can :manage, :all
+      can :destroy, Client if user.admin?
 
       # Non-NJ staff cannot manage NJ EfileErrors, EfileSubmissions or FAQs
       cannot :manage, EfileError, service_type: "state_file_nj"
@@ -85,7 +86,9 @@ def initialize(user)
     ].freeze
 
     if user.role?(client_role_whitelist)
-      can :manage, Client, vita_partner: accessible_groups
+      can :read, Client, vita_partner: accessible_groups
+      # can only edit/update Intakes from the current product year
+      can [:create, :update], Client, intake: { product_year: Rails.configuration.product_year }, vita_partner: accessible_groups
     end
 
     if user.greeter?
@@ -114,15 +117,25 @@ def initialize(user)
 
     # Only admins can destroy clients
     cannot :destroy, Client unless user.admin?
-    can :manage, [
+
+    can [:create, :update, :destroy], [
+      Note,
+      Document,
+      TaxReturn
+    ], client: { intake: { product_year: Rails.configuration.product_year }, vita_partner: accessible_groups }
+
+    can [:read], [
+      Note,
       Document,
+      TaxReturn
+    ], client: { vita_partner: accessible_groups }
+
+    can :manage, [
       IncomingEmail,
       IncomingTextMessage,
-      Note,
       OutgoingEmail,
       OutgoingTextMessage,
       SystemNote,
-      TaxReturn,
     ], client: { vita_partner: accessible_groups }
 
     can :manage, TaxReturnSelection, tax_returns: { client: { vita_partner: accessible_groups } }
diff --git a/app/views/devise/invitations/new.html.erb b/app/views/devise/invitations/new.html.erb
index 676abdda8f..6f1569365a 100644
--- a/app/views/devise/invitations/new.html.erb
+++ b/app/views/devise/invitations/new.html.erb
@@ -53,7 +53,7 @@
       <%= f.hidden_field(:role, value: params[:role]) %>
 
       <div>
-        <%= f.submit t(".submit"), class: "button button--primary" %>
+        <%= f.submit t(".submit"), class: "button button--primary spacing-below-25" %>
       </div>
       <div>
         <%= link_to "Back", :back, class: "button button--secondary" %>

From 2ccc163455ddf88811b73c2c256c5d97eb17f05d Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Fri, 31 Jan 2025 12:45:58 -0500
Subject: [PATCH 02/18] flag / ability is failing for some reason argghhhh

---
 app/controllers/hub/clients_controller.rb       | 2 --
 app/views/hub/clients/index.html.erb            | 1 +
 spec/controllers/hub/clients_controller_spec.rb | 5 ++++-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/app/controllers/hub/clients_controller.rb b/app/controllers/hub/clients_controller.rb
index 11f83a40c6..7f98d9c197 100644
--- a/app/controllers/hub/clients_controller.rb
+++ b/app/controllers/hub/clients_controller.rb
@@ -20,8 +20,6 @@ class ClientsController < Hub::BaseController
     layout "hub"
 
     def index
-      @page_title = I18n.t("hub.clients.index.title")
-
       @clients = @client_sorter.filtered_and_sorted_clients.page(params[:page]).load
       @message_summaries = RecentMessageSummaryService.messages(@clients.map(&:id))
     end
diff --git a/app/views/hub/clients/index.html.erb b/app/views/hub/clients/index.html.erb
index ea301bf0ab..52cfb4154d 100644
--- a/app/views/hub/clients/index.html.erb
+++ b/app/views/hub/clients/index.html.erb
@@ -1,3 +1,4 @@
+<% @page_title = I18n.t("hub.clients.index.title") %>
 <% content_for :page_title, @page_title %>
 <% unless @hide_filters %>
   <% content_for :filters do %>
diff --git a/spec/controllers/hub/clients_controller_spec.rb b/spec/controllers/hub/clients_controller_spec.rb
index cc8a66f551..2d8a1a5598 100644
--- a/spec/controllers/hub/clients_controller_spec.rb
+++ b/spec/controllers/hub/clients_controller_spec.rb
@@ -865,11 +865,14 @@
     let(:params) do
       { id: client.id, client: { action: "set" } }
     end
-    let(:client) { create :client, vita_partner: organization }
+    let(:intake) { create :intake, client: create(:client, vita_partner: organization) }
+    let(:client) { intake.client }
     before { sign_in(user) }
 
     it "redirects to hub client path" do
       patch :flag, params: params
+      puts "***************"
+      puts client.intake.product_year
       expect(response).to redirect_to(hub_client_path(id: client.id))
     end
 

From 7f8593c2d98d28c7beb35ab52abb1d6aa255089a Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Tue, 4 Feb 2025 18:14:25 -0500
Subject: [PATCH 03/18] Add to ability file

---
 app/lib/ability.rb                              | 2 +-
 spec/controllers/hub/clients_controller_spec.rb | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/app/lib/ability.rb b/app/lib/ability.rb
index d55425e8c0..8343af6d6d 100644
--- a/app/lib/ability.rb
+++ b/app/lib/ability.rb
@@ -88,7 +88,7 @@ def initialize(user)
     if user.role?(client_role_whitelist)
       can :read, Client, vita_partner: accessible_groups
       # can only edit/update Intakes from the current product year
-      can [:create, :update], Client, intake: { product_year: Rails.configuration.product_year }, vita_partner: accessible_groups
+      can [:create, :update, :flag, :edit_take_action, :update_take_action, :toggle_field, :unlock], Client, intake: { product_year: Rails.configuration.product_year }, vita_partner: accessible_groups
     end
 
     if user.greeter?
diff --git a/spec/controllers/hub/clients_controller_spec.rb b/spec/controllers/hub/clients_controller_spec.rb
index 2d8a1a5598..4e40dbd7d1 100644
--- a/spec/controllers/hub/clients_controller_spec.rb
+++ b/spec/controllers/hub/clients_controller_spec.rb
@@ -871,8 +871,6 @@
 
     it "redirects to hub client path" do
       patch :flag, params: params
-      puts "***************"
-      puts client.intake.product_year
       expect(response).to redirect_to(hub_client_path(id: client.id))
     end
 

From 437921bfc43bb1b29a77ec9f53356601b00fb6bd Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Thu, 6 Feb 2025 12:41:35 -0500
Subject: [PATCH 04/18] wip: more stuff

---
 app/controllers/hub/clients_controller.rb     | 18 ++---
 app/lib/ability.rb                            | 18 +++--
 app/models/client.rb                          | 12 +++
 .../hub/clients_controller_spec.rb            |  4 +-
 spec/models/client_spec.rb                    | 74 +++++++++++++++++++
 5 files changed, 107 insertions(+), 19 deletions(-)

diff --git a/app/controllers/hub/clients_controller.rb b/app/controllers/hub/clients_controller.rb
index 7f98d9c197..bb36a4556d 100644
--- a/app/controllers/hub/clients_controller.rb
+++ b/app/controllers/hub/clients_controller.rb
@@ -133,10 +133,6 @@ def edit_13614c_form_page5
       @form = Update13614cFormPage5.from_client(@client)
     end
 
-    def redirect_if_not_authorized
-      raise CanCan::AccessDenied if @client.nil? || cannot?(:update, @client) || @client.intake.is_ctc?
-    end
-
     def save_and_maybe_exit(save_button_clicked, path_to_13614c_page)
       if save_button_clicked == I18n.t("general.save")
         redirect_to path_to_13614c_page
@@ -145,6 +141,11 @@ def save_and_maybe_exit(save_button_clicked, path_to_13614c_page)
       end
     end
 
+    def redirect_if_not_authorized
+      authorize! :hub_client_management, @client
+      # raise CanCan::AccessDenied if @client.nil? || @client.intake.is_ctc? || cannot?(:hub_client_management, @client)
+    end
+
     def update_13614c_form_page1
       @form = Update13614cFormPage1.new(@client, update_13614c_form_page1_params)
 
@@ -298,13 +299,8 @@ def initialize(client)
         @client = client
         __setobj__(client)
         @intake = client.intake
-        if @intake.present? && @intake.product_year != Rails.configuration.product_year
-          @archived = true
-        end
-        if @intake.blank?
-          @intake = Archived::Intake2021.find_by(client_id: @client.id)
-          @archived = true if @intake
-        end
+        @archived = client.has_archived_intake?
+        @intake = @archived ? client.archived_intake : client.intake
         # For a short while, we created Client records with no intake and/or moved which client the intake belonged to.
         if !@intake && @client.created_at < Date.parse('2022-04-15')
           @missing_intake = true
diff --git a/app/lib/ability.rb b/app/lib/ability.rb
index 8343af6d6d..a684aa66f1 100644
--- a/app/lib/ability.rb
+++ b/app/lib/ability.rb
@@ -8,12 +8,15 @@ def initialize(user)
     end
 
     # Custom actions
+    # Todo: Add new pages here
+    # Todo: maybe add some short of test if there are missing actions
     alias_action :flag, :toggle_field, :edit_take_action, :update_take_action,
-                 :unlock, :edit_13614c_form_page1, :edit_13614c_form_page2,
-                 :edit_13614c_form_page3, :save_and_maybe_exit,
+                 :unlock, :save_and_maybe_exit,
+                 :edit_13614c_form_page1, :edit_13614c_form_page2,
+                 :edit_13614c_form_page3, :edit_13614c_form_page4, :edit_13614c_form_page5,
                  :update_13614c_form_page1, :update_13614c_form_page2,
-                 :update_13614c_form_page3, :cancel_13614c,
-                 :resource_to_client_redirect,
+                 :update_13614c_form_page3, :update_13614c_form_page4, :update_13614c_form_page5,
+                 :cancel_13614c, :resource_to_client_redirect,
                  to: :hub_client_management
 
     accessible_groups = user.accessible_vita_partners
@@ -87,8 +90,11 @@ def initialize(user)
 
     if user.role?(client_role_whitelist)
       can :read, Client, vita_partner: accessible_groups
-      # can only edit/update Intakes from the current product year
-      can [:create, :update, :flag, :edit_take_action, :update_take_action, :toggle_field, :unlock], Client, intake: { product_year: Rails.configuration.product_year }, vita_partner: accessible_groups
+
+      client_management_actions = [:create, :update, :edit, :hub_client_management]
+      can client_management_actions, Client, vita_partner: accessible_groups
+      # Can only take actions on non-archived intakes
+      cannot client_management_actions, Client, &:has_archived_intake?
     end
 
     if user.greeter?
diff --git a/app/models/client.rb b/app/models/client.rb
index 2e8e4dec6e..358d82f1cb 100644
--- a/app/models/client.rb
+++ b/app/models/client.rb
@@ -267,6 +267,18 @@ def online_ctc?
     intake.is_ctc? && intake.tax_returns.any? { |tr| tr.service_type == "online_intake" }
   end
 
+  def has_archived_intake?
+    archived_intake.present?
+  end
+
+  def archived_intake
+    if intake.present? && intake.product_year != Rails.configuration.product_year
+      intake
+    elsif intake.blank?
+      Archived::Intake2021.find_by(client_id: self.id)
+    end
+  end
+
   def recaptcha_scores_average
     return efile_security_informations.last&.recaptcha_score unless recaptcha_scores.present?
 
diff --git a/spec/controllers/hub/clients_controller_spec.rb b/spec/controllers/hub/clients_controller_spec.rb
index 4e40dbd7d1..267ff6a29d 100644
--- a/spec/controllers/hub/clients_controller_spec.rb
+++ b/spec/controllers/hub/clients_controller_spec.rb
@@ -1181,10 +1181,10 @@
           create(:archived_2021_gyr_intake, client: client)
         end
 
-        it "redirects to the /show page for the client" do
+        it "redirects to Access Denied page" do
           get :edit_take_action, params: params
 
-          expect(response).to redirect_to(hub_client_path(id: client.id))
+          expect(response).to be_forbidden
         end
       end
 
diff --git a/spec/models/client_spec.rb b/spec/models/client_spec.rb
index ce8685ffb6..15ead06d51 100644
--- a/spec/models/client_spec.rb
+++ b/spec/models/client_spec.rb
@@ -708,6 +708,80 @@
     end
   end
 
+  describe "#has_archived_intake?" do
+    context "intake is blank" do
+      let(:client) { create :client, intake: nil }
+
+      context "there is an Archived::Intake2021 with a matching client id" do
+        let!(:archived_2021_intake) { create :archived_2021_gyr_intake, client: client }
+        it "returns true" do
+          expect(client.has_archived_intake?).to eq(true)
+        end
+      end
+
+      context "there is no matching archived intake" do
+        it "returns false" do
+          expect(client.has_archived_intake?).to eq(false)
+        end
+      end
+    end
+
+    context "intake is present" do
+      let(:intake) { create :intake, product_year: product_year }
+      let(:product_year) { Rails.configuration.product_year }
+
+      context "product year matches current product year" do
+        it "returns false" do
+          expect(intake.client.has_archived_intake?).to eq(false)
+        end
+      end
+
+      context "product year doesn't match current product year" do
+        let(:product_year) { (Rails.configuration.product_year.to_i - 1) }
+        it "returns true" do
+          expect(intake.client.has_archived_intake?).to eq(true)
+        end
+      end
+    end
+  end
+
+  describe "#archived_intake" do
+    context "intake is blank" do
+      let(:client) { create :client, intake: nil }
+
+      context "there is an Archived::Intake2021 with a matching client id" do
+        let!(:archived_2021_intake) { create :archived_2021_gyr_intake, client: client }
+        it "returns archived 2021 intake" do
+          expect(client.archived_intake).to eq(archived_2021_intake)
+        end
+      end
+
+      context "there is no matching archived intake" do
+        it "returns nil" do
+          expect(client.archived_intake).to eq(nil)
+        end
+      end
+    end
+
+    context "intake is present" do
+      let(:intake) { create :intake, product_year: product_year }
+      let(:product_year) { Rails.configuration.product_year }
+
+      context "product year matches current product year" do
+        it "returns nil" do
+          expect(intake.client.archived_intake).to eq(nil)
+        end
+      end
+
+      context "product year doesn't match current product year" do
+        let(:product_year) { (Rails.configuration.product_year.to_i - 1) }
+        it "returns intake with past product year" do
+          expect(intake.client.archived_intake).to eq(intake)
+        end
+      end
+    end
+  end
+
   describe "#request_doc_help" do
     let(:client) { create :client, intake: (build :intake) }
     let(:assigned_user_a) { create :user }

From 170f28a1ec5eebeb2fa81d7debfe02aee0100d1a Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Sun, 9 Feb 2025 23:33:48 -0500
Subject: [PATCH 05/18] Ability and specs

---
 app/controllers/hub/clients_controller.rb     |   6 +-
 app/lib/ability.rb                            |  35 +--
 .../hub/clients_controller_spec.rb            |  14 +-
 spec/lib/ability_spec.rb                      | 247 ++++++++++++++++--
 4 files changed, 252 insertions(+), 50 deletions(-)

diff --git a/app/controllers/hub/clients_controller.rb b/app/controllers/hub/clients_controller.rb
index bb36a4556d..2fa0b92984 100644
--- a/app/controllers/hub/clients_controller.rb
+++ b/app/controllers/hub/clients_controller.rb
@@ -5,7 +5,7 @@ class ClientsController < Hub::BaseController
 
     before_action :load_vita_partners, only: [:new, :create, :index]
     before_action :load_users, only: [:index]
-    load_and_authorize_resource except: [:new, :create, :resource_to_client_redirect]
+    load_and_authorize_resource except: [:new, :create, :resource_to_client_redirect] # why except new and create
     before_action :setup_sortable_client, only: [:index]
     # need to use the presenter for :update bc it has ITIN applicant methods that are used in the form
     before_action :wrap_client_in_hub_presenter, only: [:show, :edit, :edit_take_action, :update, :update_take_action]
@@ -142,8 +142,8 @@ def save_and_maybe_exit(save_button_clicked, path_to_13614c_page)
     end
 
     def redirect_if_not_authorized
-      authorize! :hub_client_management, @client
-      # raise CanCan::AccessDenied if @client.nil? || @client.intake.is_ctc? || cannot?(:hub_client_management, @client)
+      # todo: change name/back? redirects are happening based on ability in ability file
+      raise CanCan::AccessDenied if @client.nil? || @client.intake.is_ctc?
     end
 
     def update_13614c_form_page1
diff --git a/app/lib/ability.rb b/app/lib/ability.rb
index a684aa66f1..ed6bf4db2d 100644
--- a/app/lib/ability.rb
+++ b/app/lib/ability.rb
@@ -16,7 +16,7 @@ def initialize(user)
                  :edit_13614c_form_page3, :edit_13614c_form_page4, :edit_13614c_form_page5,
                  :update_13614c_form_page1, :update_13614c_form_page2,
                  :update_13614c_form_page3, :update_13614c_form_page4, :update_13614c_form_page5,
-                 :cancel_13614c, :resource_to_client_redirect,
+                 :cancel_13614c,
                  to: :hub_client_management
 
     accessible_groups = user.accessible_vita_partners
@@ -80,9 +80,9 @@ def initialize(user)
     can :read, Organization, id: accessible_groups.pluck(:id)
     can :read, Site, id: accessible_groups.pluck(:id)
 
-    # This was overly permissive. We should work out what the permissions should
-    # be for each role and reduce this check. As we need to modify this, please
-    # break out the role and specify permissions more granularly
+    # CLIENT CONTROLLER PERMISSIONS
+    # overly permissive, need to narrow permissions
+    # break out role and specify permissions when making modifications
     client_role_whitelist = [
       :client_success, :admin, :org_lead, :site_coordinator,
       :coalition_lead, :state_file_admin, :team_member
@@ -91,34 +91,25 @@ def initialize(user)
     if user.role?(client_role_whitelist)
       can :read, Client, vita_partner: accessible_groups
 
-      client_management_actions = [:create, :update, :edit, :hub_client_management]
-      can client_management_actions, Client, vita_partner: accessible_groups
-      # Can only take actions on non-archived intakes
-      cannot client_management_actions, Client, &:has_archived_intake?
+      can [:create, :update, :edit, :hub_client_management],
+          Client, vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year }
     end
 
     if user.greeter?
       can [:update, :read, :hub_client_management],
         Client,
         tax_returns: {
-          current_state: [
-            'intake_ready',
-            'intake_greeter_info_requested',
-            'intake_needs_doc_help',
-          ],
+          current_state: %w[intake_ready intake_greeter_info_requested intake_needs_doc_help],
         },
-        vita_partner: accessible_groups
+        vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year }
 
       can [:update, :read, :hub_client_management],
         Client,
         tax_returns: {
-          current_state: [
-            'file_not_filing',
-            'file_hold',
-          ],
+          current_state: %w[file_not_filing file_hold],
           assigned_user: user,
         },
-        vita_partner: accessible_groups
+        vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year }
     end
 
     # Only admins can destroy clients
@@ -128,7 +119,7 @@ def initialize(user)
       Note,
       Document,
       TaxReturn
-    ], client: { intake: { product_year: Rails.configuration.product_year }, vita_partner: accessible_groups }
+    ], client: { vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year } }
 
     can [:read], [
       Note,
@@ -144,8 +135,8 @@ def initialize(user)
       SystemNote,
     ], client: { vita_partner: accessible_groups }
 
-    can :manage, TaxReturnSelection, tax_returns: { client: { vita_partner: accessible_groups } }
-    cannot :manage, TaxReturnSelection, tax_returns: { client: { vita_partner: VitaPartner.where.not(id: accessible_groups) }}
+    can :manage, TaxReturnSelection, tax_returns: { client: { vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year } } }
+    cannot :manage, TaxReturnSelection, tax_returns: { client: { vita_partner: VitaPartner.where.not(id: accessible_groups) } }
 
     can :manage, EfileSubmission, tax_return: { client: { vita_partner: accessible_groups } }
 
diff --git a/spec/controllers/hub/clients_controller_spec.rb b/spec/controllers/hub/clients_controller_spec.rb
index 267ff6a29d..387f61facb 100644
--- a/spec/controllers/hub/clients_controller_spec.rb
+++ b/spec/controllers/hub/clients_controller_spec.rb
@@ -938,10 +938,10 @@
           create(:archived_2021_gyr_intake, client: client)
         end
 
-        it "redirects to the /show page for the client" do
+        it "redirects to Access Denied page" do
           get :edit, params: params
 
-          expect(response).to redirect_to(hub_client_path(id: client.id))
+          expect(response).to be_forbidden
         end
       end
     end
@@ -1052,10 +1052,10 @@
           create(:archived_2021_gyr_intake, client: client)
         end
 
-        it "redirects to the /show page for the client" do
+        it "response is forbidden (403)" do
           post :update, params: { id: client.id }
 
-          expect(response).to redirect_to(hub_client_path(id: client.id))
+          expect(response).to be_forbidden
         end
       end
 
@@ -1299,13 +1299,11 @@
       end
 
       context "when the client is not hub updatable" do
-        before do
-          allow_any_instance_of(Hub::ClientsController::HubClientPresenter).to receive(:hub_status_updatable).and_return(false)
-        end
+        let(:intake) { build :ctc_intake, email_address: "gob@example.com", sms_phone_number: "+14155551212" }
 
         it "raises bad request" do
           post :update_take_action, params: params
-          expect(response).to redirect_to hub_client_path(id: client.id)
+          expect(response).to be_forbidden
         end
       end
 
diff --git a/spec/lib/ability_spec.rb b/spec/lib/ability_spec.rb
index 31927aa2ca..ba95c98b73 100644
--- a/spec/lib/ability_spec.rb
+++ b/spec/lib/ability_spec.rb
@@ -195,7 +195,8 @@
                 preferred_name: "George Sr.",
                 needs_help_2019: "yes",
                 needs_help_2018: "yes",
-                preferred_interview_language: "en", locale: "en"
+                preferred_interview_language: "en", locale: "en",
+                product_year: Rails.configuration.product_year
               ),
               tax_returns: [
                 build(
@@ -216,14 +217,66 @@
           it "can access all data for the client" do
             expect(subject.can?(:read, accessible_client)).to eq true
             expect(subject.can?(:update, accessible_client)).to eq true
-            expect(subject.can?(:manage, Document.new(client: accessible_client))).to eq true
-            expect(subject.can?(:manage, IncomingEmail.new(client: accessible_client))).to eq true
-            expect(subject.can?(:manage, IncomingTextMessage.new(client: accessible_client))).to eq true
-            expect(subject.can?(:manage, Note.new(client: accessible_client))).to eq true
-            expect(subject.can?(:manage, OutgoingEmail.new(client: accessible_client))).to eq true
-            expect(subject.can?(:manage, OutgoingTextMessage.new(client: accessible_client))).to eq true
-            expect(subject.can?(:manage, SystemNote.new(client: accessible_client))).to eq true
-            expect(subject.can?(:manage, TaxReturn.new(client: accessible_client))).to eq true
+            expect(subject.can?(:edit, accessible_client)).to eq true
+            expect(subject.can?(:flag, accessible_client)).to eq true
+            expect(subject.can?(:toggle_field, accessible_client)).to eq true
+            expect(subject.can?(:edit_take_action, accessible_client)).to eq true
+            expect(subject.can?(:update_take_action, accessible_client)).to eq true
+            expect(subject.can?(:unlock, accessible_client)).to eq true
+            expect(subject.can?(:save_and_maybe_exit, accessible_client)).to eq true
+
+            expect(subject.can?(:edit_13614c_form_page1, accessible_client)).to eq true
+            expect(subject.can?(:edit_13614c_form_page2, accessible_client)).to eq true
+            expect(subject.can?(:edit_13614c_form_page3, accessible_client)).to eq true
+            expect(subject.can?(:edit_13614c_form_page4, accessible_client)).to eq true
+            expect(subject.can?(:edit_13614c_form_page5, accessible_client)).to eq true
+            expect(subject.can?(:update_13614c_form_page1, accessible_client)).to eq true
+            expect(subject.can?(:update_13614c_form_page2, accessible_client)).to eq true
+            expect(subject.can?(:update_13614c_form_page3, accessible_client)).to eq true
+            expect(subject.can?(:update_13614c_form_page4, accessible_client)).to eq true
+            expect(subject.can?(:update_13614c_form_page5, accessible_client)).to eq true
+            expect(subject.can?(:cancel_13614c, accessible_client)).to eq true
+
+            expect(subject.can?(:read, Document.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, Document.new(client: accessible_client))).to eq true
+            expect(subject.can?(:update, Document.new(client: accessible_client))).to eq true
+            expect(subject.can?(:destroy, Document.new(client: accessible_client))).to eq true
+
+            expect(subject.can?(:read, IncomingEmail.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, IncomingEmail.new(client: accessible_client))).to eq true
+            expect(subject.can?(:update, IncomingEmail.new(client: accessible_client))).to eq true
+            expect(subject.can?(:destroy, IncomingEmail.new(client: accessible_client))).to eq true
+
+            expect(subject.can?(:read, IncomingTextMessage.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, IncomingTextMessage.new(client: accessible_client))).to eq true
+            expect(subject.can?(:update, IncomingTextMessage.new(client: accessible_client))).to eq true
+            expect(subject.can?(:destroy, IncomingTextMessage.new(client: accessible_client))).to eq true
+
+            expect(subject.can?(:read, Note.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, Note.new(client: accessible_client))).to eq true
+            expect(subject.can?(:update, Note.new(client: accessible_client))).to eq true
+            expect(subject.can?(:destroy, Note.new(client: accessible_client))).to eq true
+
+            expect(subject.can?(:read, OutgoingEmail.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, OutgoingEmail.new(client: accessible_client))).to eq true
+            expect(subject.can?(:update, OutgoingEmail.new(client: accessible_client))).to eq true
+            expect(subject.can?(:destroy, OutgoingEmail.new(client: accessible_client))).to eq true
+
+            expect(subject.can?(:read, OutgoingTextMessage.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, OutgoingTextMessage.new(client: accessible_client))).to eq true
+            expect(subject.can?(:update, OutgoingTextMessage.new(client: accessible_client))).to eq true
+            expect(subject.can?(:destroy, OutgoingTextMessage.new(client: accessible_client))).to eq true
+
+            expect(subject.can?(:read, SystemNote.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, SystemNote.new(client: accessible_client))).to eq true
+            expect(subject.can?(:update, SystemNote.new(client: accessible_client))).to eq true
+            expect(subject.can?(:destroy, SystemNote.new(client: accessible_client))).to eq true
+
+            expect(subject.can?(:read, TaxReturn.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, TaxReturn.new(client: accessible_client))).to eq true
+            expect(subject.can?(:update, TaxReturn.new(client: accessible_client))).to eq true
+            expect(subject.can?(:destroy, TaxReturn.new(client: accessible_client))).to eq true
+
             expect(subject.can?(:manage, TaxReturnSelection.create!(tax_returns: [build(:gyr_tax_return, client: accessible_client)]))).to eq true
           end
 
@@ -268,14 +321,66 @@
           it "can access all data for the client" do
             expect(subject.can?(:read, accessible_client)).to eq true
             expect(subject.can?(:update, accessible_client)).to eq true
-            expect(subject.can?(:manage, Document.new(client: accessible_client))).to eq true
-            expect(subject.can?(:manage, IncomingEmail.new(client: accessible_client))).to eq true
-            expect(subject.can?(:manage, IncomingTextMessage.new(client: accessible_client))).to eq true
-            expect(subject.can?(:manage, Note.new(client: accessible_client))).to eq true
-            expect(subject.can?(:manage, OutgoingEmail.new(client: accessible_client))).to eq true
-            expect(subject.can?(:manage, OutgoingTextMessage.new(client: accessible_client))).to eq true
-            expect(subject.can?(:manage, SystemNote.new(client: accessible_client))).to eq true
-            expect(subject.can?(:manage, TaxReturn.new(client: accessible_client))).to eq true
+            expect(subject.can?(:edit, accessible_client)).to eq true
+            expect(subject.can?(:flag, accessible_client)).to eq true
+            expect(subject.can?(:toggle_field, accessible_client)).to eq true
+            expect(subject.can?(:edit_take_action, accessible_client)).to eq true
+            expect(subject.can?(:update_take_action, accessible_client)).to eq true
+            expect(subject.can?(:unlock, accessible_client)).to eq true
+            expect(subject.can?(:save_and_maybe_exit, accessible_client)).to eq true
+
+            expect(subject.can?(:edit_13614c_form_page1, accessible_client)).to eq true
+            expect(subject.can?(:edit_13614c_form_page2, accessible_client)).to eq true
+            expect(subject.can?(:edit_13614c_form_page3, accessible_client)).to eq true
+            expect(subject.can?(:edit_13614c_form_page4, accessible_client)).to eq true
+            expect(subject.can?(:edit_13614c_form_page5, accessible_client)).to eq true
+            expect(subject.can?(:update_13614c_form_page1, accessible_client)).to eq true
+            expect(subject.can?(:update_13614c_form_page2, accessible_client)).to eq true
+            expect(subject.can?(:update_13614c_form_page3, accessible_client)).to eq true
+            expect(subject.can?(:update_13614c_form_page4, accessible_client)).to eq true
+            expect(subject.can?(:update_13614c_form_page5, accessible_client)).to eq true
+            expect(subject.can?(:cancel_13614c, accessible_client)).to eq true
+
+            expect(subject.can?(:read, Document.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, Document.new(client: accessible_client))).to eq true
+            expect(subject.can?(:update, Document.new(client: accessible_client))).to eq true
+            expect(subject.can?(:destroy, Document.new(client: accessible_client))).to eq true
+
+            expect(subject.can?(:read, IncomingEmail.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, IncomingEmail.new(client: accessible_client))).to eq true
+            expect(subject.can?(:update, IncomingEmail.new(client: accessible_client))).to eq true
+            expect(subject.can?(:destroy, IncomingEmail.new(client: accessible_client))).to eq true
+
+            expect(subject.can?(:read, IncomingTextMessage.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, IncomingTextMessage.new(client: accessible_client))).to eq true
+            expect(subject.can?(:update, IncomingTextMessage.new(client: accessible_client))).to eq true
+            expect(subject.can?(:destroy, IncomingTextMessage.new(client: accessible_client))).to eq true
+
+            expect(subject.can?(:read, Note.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, Note.new(client: accessible_client))).to eq true
+            expect(subject.can?(:update, Note.new(client: accessible_client))).to eq true
+            expect(subject.can?(:destroy, Note.new(client: accessible_client))).to eq true
+
+            expect(subject.can?(:read, OutgoingEmail.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, OutgoingEmail.new(client: accessible_client))).to eq true
+            expect(subject.can?(:update, OutgoingEmail.new(client: accessible_client))).to eq true
+            expect(subject.can?(:destroy, OutgoingEmail.new(client: accessible_client))).to eq true
+
+            expect(subject.can?(:read, OutgoingTextMessage.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, OutgoingTextMessage.new(client: accessible_client))).to eq true
+            expect(subject.can?(:update, OutgoingTextMessage.new(client: accessible_client))).to eq true
+            expect(subject.can?(:destroy, OutgoingTextMessage.new(client: accessible_client))).to eq true
+
+            expect(subject.can?(:read, SystemNote.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, SystemNote.new(client: accessible_client))).to eq true
+            expect(subject.can?(:update, SystemNote.new(client: accessible_client))).to eq true
+            expect(subject.can?(:destroy, SystemNote.new(client: accessible_client))).to eq true
+
+            expect(subject.can?(:read, TaxReturn.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, TaxReturn.new(client: accessible_client))).to eq true
+            expect(subject.can?(:update, TaxReturn.new(client: accessible_client))).to eq true
+            expect(subject.can?(:destroy, TaxReturn.new(client: accessible_client))).to eq true
+
             expect(subject.can?(:manage, TaxReturnSelection.create!(tax_returns: [build(:gyr_tax_return, client: accessible_client)]))).to eq true
           end
 
@@ -310,6 +415,113 @@
         end
       end
 
+      shared_examples :can_read_but_not_update_accessible_client_with_archived_intake do
+        context "when the user can access a particular site" do
+          let(:accessible_site) { create(:site) }
+          let(:accessible_client) do
+            create(
+              :client,
+              vita_partner: accessible_site,
+              intake: build(
+                :intake,
+                :filled_out,
+                preferred_name: "George Sr.",
+                needs_help_2019: "yes",
+                needs_help_2018: "yes",
+                preferred_interview_language: "en", locale: "en",
+                product_year: Rails.configuration.product_year - 2
+              ),
+              tax_returns: [
+                build(
+                  :tax_return,
+                  :intake_ready,
+                  year: 2019,
+                  service_type: "drop_off",
+                  filing_status: nil
+                ),
+              ]
+            )
+          end
+
+          before do
+            allow(user).to receive(:accessible_vita_partners).and_return(VitaPartner.where(id: accessible_site))
+          end
+
+          it "can access all data for the client" do
+            expect(subject.can?(:read, accessible_client)).to eq true
+            expect(subject.can?(:update, accessible_client)).to eq false
+            expect(subject.can?(:edit, accessible_client)).to eq false
+
+            expect(subject.can?(:flag, accessible_client)).to eq false
+            expect(subject.can?(:toggle_field, accessible_client)).to eq false
+            expect(subject.can?(:unlock, accessible_client)).to eq false
+
+            expect(subject.can?(:edit_take_action, accessible_client)).to eq false
+            expect(subject.can?(:update_take_action, accessible_client)).to eq false
+
+
+            expect(subject.can?(:edit_13614c_form_page1, accessible_client)).to eq false
+            expect(subject.can?(:edit_13614c_form_page2, accessible_client)).to eq false
+            expect(subject.can?(:edit_13614c_form_page3, accessible_client)).to eq false
+            expect(subject.can?(:edit_13614c_form_page4, accessible_client)).to eq false
+            expect(subject.can?(:edit_13614c_form_page5, accessible_client)).to eq false
+            expect(subject.can?(:update_13614c_form_page1, accessible_client)).to eq false
+            expect(subject.can?(:update_13614c_form_page2, accessible_client)).to eq false
+            expect(subject.can?(:update_13614c_form_page3, accessible_client)).to eq false
+            expect(subject.can?(:update_13614c_form_page4, accessible_client)).to eq false
+            expect(subject.can?(:update_13614c_form_page5, accessible_client)).to eq false
+            expect(subject.can?(:save_and_maybe_exit, accessible_client)).to eq false
+            expect(subject.can?(:cancel_13614c, accessible_client)).to eq false
+
+            expect(subject.can?(:read, Document.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, Document.new(client: accessible_client))).to eq false
+            expect(subject.can?(:update, Document.new(client: accessible_client))).to eq false
+            expect(subject.can?(:destroy, Document.new(client: accessible_client))).to eq false
+
+            expect(subject.can?(:read, IncomingEmail.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, IncomingEmail.new(client: accessible_client))).to eq true
+            expect(subject.can?(:update, IncomingEmail.new(client: accessible_client))).to eq true
+            expect(subject.can?(:destroy, IncomingEmail.new(client: accessible_client))).to eq true
+
+            expect(subject.can?(:read, IncomingTextMessage.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, IncomingTextMessage.new(client: accessible_client))).to eq true
+            expect(subject.can?(:update, IncomingTextMessage.new(client: accessible_client))).to eq true
+            expect(subject.can?(:destroy, IncomingTextMessage.new(client: accessible_client))).to eq true
+
+            expect(subject.can?(:read, Note.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, Note.new(client: accessible_client))).to eq false
+            expect(subject.can?(:update, Note.new(client: accessible_client))).to eq false
+            expect(subject.can?(:destroy, Note.new(client: accessible_client))).to eq false
+
+            expect(subject.can?(:read, OutgoingEmail.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, OutgoingEmail.new(client: accessible_client))).to eq true
+            expect(subject.can?(:update, OutgoingEmail.new(client: accessible_client))).to eq true
+            expect(subject.can?(:destroy, OutgoingEmail.new(client: accessible_client))).to eq true
+
+            expect(subject.can?(:read, OutgoingTextMessage.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, OutgoingTextMessage.new(client: accessible_client))).to eq true
+            expect(subject.can?(:update, OutgoingTextMessage.new(client: accessible_client))).to eq true
+            expect(subject.can?(:destroy, OutgoingTextMessage.new(client: accessible_client))).to eq true
+
+            expect(subject.can?(:read, SystemNote.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, SystemNote.new(client: accessible_client))).to eq true
+            expect(subject.can?(:update, SystemNote.new(client: accessible_client))).to eq true
+            expect(subject.can?(:destroy, SystemNote.new(client: accessible_client))).to eq true
+
+            expect(subject.can?(:read, TaxReturn.new(client: accessible_client))).to eq true
+            expect(subject.can?(:create, TaxReturn.new(client: accessible_client))).to eq false
+            expect(subject.can?(:update, TaxReturn.new(client: accessible_client))).to eq false
+            expect(subject.can?(:destroy, TaxReturn.new(client: accessible_client))).to eq false
+
+            expect(subject.can?(:manage, TaxReturnSelection.create!(tax_returns: [build(:gyr_tax_return, client: accessible_client)]))).to eq false
+          end
+
+          it "cannot delete a client" do
+            expect(subject.can?(:destroy, accessible_client)).to eq false
+          end
+        end
+      end
+
       context "users with valid non-admin roles" do
         context "a coalition lead" do
           let(:user) { create :coalition_lead_user }
@@ -345,6 +557,7 @@
           it_behaves_like :cannot_manage_inaccessible_client
           it_behaves_like :can_only_read_accessible_org_or_site
           it_behaves_like :cannot_manage_any_sites_or_orgs
+          it_behaves_like :can_read_but_not_update_accessible_client_with_archived_intake
         end
 
         context "a greeter" do

From 668ef3834d6e13f9dec8a64accfe530032bec524 Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Mon, 10 Feb 2025 00:24:45 -0500
Subject: [PATCH 06/18] Add back method

---
 app/controllers/hub/clients_controller.rb | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/app/controllers/hub/clients_controller.rb b/app/controllers/hub/clients_controller.rb
index 2fa0b92984..b16d656a43 100644
--- a/app/controllers/hub/clients_controller.rb
+++ b/app/controllers/hub/clients_controller.rb
@@ -5,18 +5,11 @@ class ClientsController < Hub::BaseController
 
     before_action :load_vita_partners, only: [:new, :create, :index]
     before_action :load_users, only: [:index]
-    load_and_authorize_resource except: [:new, :create, :resource_to_client_redirect] # why except new and create
+    load_and_authorize_resource except: [:new, :create, :resource_to_client_redirect]
     before_action :setup_sortable_client, only: [:index]
     # need to use the presenter for :update bc it has ITIN applicant methods that are used in the form
     before_action :wrap_client_in_hub_presenter, only: [:show, :edit, :edit_take_action, :update, :update_take_action]
-    before_action :redirect_if_not_authorized, only: [
-      :edit, :edit_take_action, :update, :update_take_action,
-      :edit_13614c_form_page1, :update_13614c_form_page1,
-      :edit_13614c_form_page2, :update_13614c_form_page2,
-      :edit_13614c_form_page3, :update_13614c_form_page3,
-      :edit_13614c_form_page4, :update_13614c_form_page4,
-      :edit_13614c_form_page5, :update_13614c_form_page5
-    ]
+    before_action :redirect_unless_client_is_hub_status_editable, only: [:edit, :edit_take_action, :update, :update_take_action]
     layout "hub"
 
     def index
@@ -274,6 +267,10 @@ def wrap_client_in_hub_presenter
       @client = HubClientPresenter.new(@client)
     end
 
+    def redirect_unless_client_is_hub_status_editable
+      redirect_to hub_client_path(id: @client.id) unless @client.hub_status_updatable
+    end
+
     class HubClientPresenter < SimpleDelegator
       attr_reader :intake
       attr_reader :archived

From 3dfd8f6bb4d1fa1beedc94d634036f39b96de42e Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Mon, 10 Feb 2025 00:31:20 -0500
Subject: [PATCH 07/18] Add changes

---
 app/controllers/hub/clients_controller.rb |  5 -----
 app/lib/ability.rb                        | 10 ++++------
 2 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/app/controllers/hub/clients_controller.rb b/app/controllers/hub/clients_controller.rb
index b16d656a43..2d57f7b037 100644
--- a/app/controllers/hub/clients_controller.rb
+++ b/app/controllers/hub/clients_controller.rb
@@ -134,11 +134,6 @@ def save_and_maybe_exit(save_button_clicked, path_to_13614c_page)
       end
     end
 
-    def redirect_if_not_authorized
-      # todo: change name/back? redirects are happening based on ability in ability file
-      raise CanCan::AccessDenied if @client.nil? || @client.intake.is_ctc?
-    end
-
     def update_13614c_form_page1
       @form = Update13614cFormPage1.new(@client, update_13614c_form_page1_params)
 
diff --git a/app/lib/ability.rb b/app/lib/ability.rb
index ed6bf4db2d..bb388d947d 100644
--- a/app/lib/ability.rb
+++ b/app/lib/ability.rb
@@ -8,15 +8,13 @@ def initialize(user)
     end
 
     # Custom actions
-    # Todo: Add new pages here
-    # Todo: maybe add some short of test if there are missing actions
-    alias_action :flag, :toggle_field, :edit_take_action, :update_take_action,
-                 :unlock, :save_and_maybe_exit,
+    alias_action :flag, :toggle_field, :unlock,
+                 :edit_take_action, :update_take_action,
                  :edit_13614c_form_page1, :edit_13614c_form_page2,
                  :edit_13614c_form_page3, :edit_13614c_form_page4, :edit_13614c_form_page5,
                  :update_13614c_form_page1, :update_13614c_form_page2,
                  :update_13614c_form_page3, :update_13614c_form_page4, :update_13614c_form_page5,
-                 :cancel_13614c,
+                 :cancel_13614c, :save_and_maybe_exit,
                  to: :hub_client_management
 
     accessible_groups = user.accessible_vita_partners
@@ -80,7 +78,7 @@ def initialize(user)
     can :read, Organization, id: accessible_groups.pluck(:id)
     can :read, Site, id: accessible_groups.pluck(:id)
 
-    # CLIENT CONTROLLER PERMISSIONS
+    # HUB CLIENT CONTROLLER PERMISSIONS
     # overly permissive, need to narrow permissions
     # break out role and specify permissions when making modifications
     client_role_whitelist = [

From fdd98d6e2f673a56f1aefd99b42815dbb75826a5 Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Mon, 10 Feb 2025 12:19:41 -0500
Subject: [PATCH 08/18] Add controller spec and unlock to ability

---
 app/controllers/hub/clients_controller.rb     |   2 -
 app/lib/ability.rb                            |   8 +-
 .../hub/clients_controller_spec.rb            | 261 +++++++++++++++++-
 3 files changed, 262 insertions(+), 9 deletions(-)

diff --git a/app/controllers/hub/clients_controller.rb b/app/controllers/hub/clients_controller.rb
index 2d57f7b037..f012746c0d 100644
--- a/app/controllers/hub/clients_controller.rb
+++ b/app/controllers/hub/clients_controller.rb
@@ -99,8 +99,6 @@ def update_take_action
     end
 
     def unlock
-      raise CanCan::AccessDenied unless current_user.admin? || current_user.org_lead? || current_user.site_coordinator?
-
       @client.unlock_access! if @client.access_locked?
       flash[:notice] = I18n.t("hub.clients.unlock.account_unlocked", name: @client.preferred_name)
       redirect_to(hub_client_path(id: @client))
diff --git a/app/lib/ability.rb b/app/lib/ability.rb
index bb388d947d..471a3258bc 100644
--- a/app/lib/ability.rb
+++ b/app/lib/ability.rb
@@ -7,8 +7,8 @@ def initialize(user)
       return
     end
 
-    # Custom actions
-    alias_action :flag, :toggle_field, :unlock,
+    # Custom client controller actions
+    alias_action :flag, :toggle_field,
                  :edit_take_action, :update_take_action,
                  :edit_13614c_form_page1, :edit_13614c_form_page2,
                  :edit_13614c_form_page3, :edit_13614c_form_page4, :edit_13614c_form_page5,
@@ -93,6 +93,10 @@ def initialize(user)
           Client, vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year }
     end
 
+    if user.role?([:admin, :org_lead, :site_coordinator])
+      can :unlock, Client, vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year }
+    end
+
     if user.greeter?
       can [:update, :read, :hub_client_management],
         Client,
diff --git a/spec/controllers/hub/clients_controller_spec.rb b/spec/controllers/hub/clients_controller_spec.rb
index 387f61facb..9718b51496 100644
--- a/spec/controllers/hub/clients_controller_spec.rb
+++ b/spec/controllers/hub/clients_controller_spec.rb
@@ -911,6 +911,19 @@
         )
       end
     end
+
+    context "with a client with an archived intake" do
+      before do
+        client.intake.destroy!
+        create(:intake, client: client, product_year: Rails.configuration.product_year - 1)
+      end
+
+      it "response is forbidden (403)" do
+        patch :flag, params: params
+
+        expect(response).to be_forbidden
+      end
+    end
   end
 
   describe "#edit" do
@@ -1154,6 +1167,11 @@
           delete :destroy, params: params
         end.not_to change(Client, :count)
       end
+
+      it "redirects to access denied page" do
+        delete :destroy, params: params
+        expect(response).to be_forbidden
+      end
     end
   end
 
@@ -1320,6 +1338,19 @@
           expect(response).to redirect_to hub_client_path(id: client.id)
         end
       end
+
+      context "with a client with an archived intake" do
+        before do
+          client.intake.destroy!
+          create(:intake, client: client, product_year: Rails.configuration.product_year - 1)
+        end
+
+        it "response is forbidden (403)" do
+          post :update_take_action, params: params
+
+          expect(response).to be_forbidden
+        end
+      end
     end
   end
 
@@ -1381,6 +1412,18 @@
         expect(response).to redirect_to(hub_client_path(id: client))
         expect(flash[:notice]).to eq "Unlocked #{client.preferred_name}'s account."
       end
+
+      context "with an archived intake" do
+        before do
+          client.intake.update(product_year: Rails.configuration.product_year - 2)
+        end
+
+        it "response is forbidden (403)" do
+          patch :unlock, params: params
+          expect(client.reload.access_locked?).to eq true
+          expect(response).to be_forbidden
+        end
+      end
     end
 
     context "as a organization lead user" do
@@ -1842,6 +1885,18 @@
             end
           end
         end
+
+        context "with a client with an archived intake" do
+          before do
+            client.intake.update(product_year: Rails.configuration.product_year - 1)
+          end
+
+          it "response is forbidden (403)" do
+            put :update_13614c_form_page1, params: params
+
+            expect(response).to be_forbidden
+          end
+        end
       end
     end
 
@@ -1975,6 +2030,18 @@
           client.reload
           expect(client.intake.job_count).to eq 3
         end
+
+        context "with a client with an archived intake" do
+          before do
+            client.intake.update(product_year: Rails.configuration.product_year - 1)
+          end
+
+          it "response is forbidden (403)" do
+            put :update_13614c_form_page2, params: params
+
+            expect(response).to be_forbidden
+          end
+        end
       end
     end
 
@@ -2027,6 +2094,18 @@
           client.reload
           expect(client.intake.tax_credit_disallowed_year).to eq 2001
         end
+
+        context "with a client with an archived intake" do
+          before do
+            client.intake.update(product_year: Rails.configuration.product_year - 1)
+          end
+
+          it "response is forbidden (403)" do
+            put :update_13614c_form_page3, params: params
+
+            expect(response).to be_forbidden
+          end
+        end
       end
     end
 
@@ -2099,6 +2178,18 @@
           client.reload
           expect(client.intake.demographic_english_conversation).to eq "well"
         end
+
+        context "with a client with an archived intake" do
+          before do
+            client.intake.update(product_year: Rails.configuration.product_year - 1)
+          end
+
+          it "response is forbidden (403)" do
+            put :update_13614c_form_page4, params: params
+
+            expect(response).to be_forbidden
+          end
+        end
       end
     end
 
@@ -2150,6 +2241,171 @@
           client.reload
           expect(client.intake.additional_notes_comments).to eq 'Call me Ishmael.'
         end
+
+        context "with a client with an archived intake" do
+          before do
+            client.intake.update(product_year: Rails.configuration.product_year - 1)
+          end
+
+          it "response is forbidden (403)" do
+            put :update_13614c_form_page5, params: params
+
+            expect(response).to be_forbidden
+          end
+        end
+      end
+    end
+  end
+
+  context "editing 13614c" do
+    let(:client) { create :client, vita_partner: organization, intake: intake }
+
+    let(:intake) { build :intake, :with_contact_info, preferred_interview_language: "en", ever_married: "yes", dependents: [build(:dependent), build(:dependent)] }
+    let(:first_dependent) { intake.dependents.first }
+    let(:params) { { id: client } }
+
+    describe "#edit_13614c_form_page1" do
+      it_behaves_like :a_get_action_for_authenticated_users_only, action: :edit_13614c_form_page1
+
+      context "with a signed in user" do
+        let(:user) { create(:user, role: create(:organization_lead_role, organization: organization)) }
+
+        before do
+          sign_in user
+        end
+
+        it "renders edit 13614c page 1" do
+            get :edit_13614c_form_page1, params: params
+            expect(response).to be_ok
+        end
+
+        context "with a client with an archived intake" do
+          before do
+            client.intake.update(product_year: Rails.configuration.product_year - 1)
+          end
+
+          it "response is forbidden (403)" do
+            put :update_13614c_form_page1, params: params
+
+            expect(response).to be_forbidden
+          end
+        end
+      end
+    end
+
+    describe "#edit_13614c_form_page2" do
+      it_behaves_like :a_get_action_for_authenticated_users_only, action: :edit_13614c_form_page2
+
+      context "with a signed in user" do
+        let(:user) { create(:user, role: create(:organization_lead_role, organization: organization)) }
+
+        before do
+          sign_in user
+        end
+
+        it "renders edit 13614c page 2" do
+          get :edit_13614c_form_page2, params: params
+          expect(response).to be_ok
+        end
+
+        context "with a client with an archived intake" do
+          before do
+            client.intake.update(product_year: Rails.configuration.product_year - 1)
+          end
+
+          it "response is forbidden (403)" do
+            put :update_13614c_form_page2, params: params
+
+            expect(response).to be_forbidden
+          end
+        end
+      end
+    end
+
+    describe "#edit_13614c_form_page3" do
+      it_behaves_like :a_get_action_for_authenticated_users_only, action: :edit_13614c_form_page3
+
+      context "with a signed in user" do
+        let(:user) { create(:user, role: create(:organization_lead_role, organization: organization)) }
+
+        before do
+          sign_in user
+        end
+
+        it "renders edit 13614c page 3" do
+          get :edit_13614c_form_page3, params: params
+          expect(response).to be_ok
+        end
+
+        context "with a client with an archived intake" do
+          before do
+            client.intake.update(product_year: Rails.configuration.product_year - 1)
+          end
+
+          it "response is forbidden (403)" do
+            put :update_13614c_form_page3, params: params
+
+            expect(response).to be_forbidden
+          end
+        end
+      end
+    end
+
+    describe "#edit_13614c_form_page4" do
+      it_behaves_like :a_get_action_for_authenticated_users_only, action: :edit_13614c_form_page4
+
+      context "with a signed in user" do
+        let(:user) { create(:user, role: create(:organization_lead_role, organization: organization)) }
+
+        before do
+          sign_in user
+        end
+
+        it "renders edit 13614c page 4" do
+          get :edit_13614c_form_page4, params: params
+          expect(response).to be_ok
+        end
+
+        context "with a client with an archived intake" do
+          before do
+            client.intake.update(product_year: Rails.configuration.product_year - 1)
+          end
+
+          it "response is forbidden (403)" do
+            put :update_13614c_form_page4, params: params
+
+            expect(response).to be_forbidden
+          end
+        end
+      end
+    end
+
+    describe "#edit_13614c_form_page5" do
+      it_behaves_like :a_get_action_for_authenticated_users_only, action: :edit_13614c_form_page5
+
+      context "with a signed in user" do
+        let(:user) { create(:user, role: create(:organization_lead_role, organization: organization)) }
+
+        before do
+          sign_in user
+        end
+
+        it "renders edit 13614c page 5" do
+          get :edit_13614c_form_page5, params: params
+          expect(response).to be_ok
+        end
+
+        context "with a client with an archived intake" do
+          before do
+            client.intake.update(product_year: Rails.configuration.product_year - 1)
+          end
+
+          it "response is forbidden (403)" do
+            put :update_13614c_form_page5, params: params
+
+            expect(response).to be_forbidden
+          end
+        end
       end
     end
   end
@@ -2195,7 +2451,6 @@
             create(:client, **good_client_params)
 
             get :index
-            # puts Client.where(filterable_product_year: 2024).first
             expect(assigns(:clients)).not_to be_empty
           end
 
@@ -2261,7 +2516,6 @@
             create(:client, **good_client_params)
 
             get :index
-            # puts Client.where(filterable_product_year: 2024).first
             expect(assigns(:clients)).to be_empty
           end
 
@@ -2329,7 +2583,6 @@
             create(:client, **good_client_params)
 
             get :index
-            # puts Client.where(filterable_product_year: 2024).first
             expect(assigns(:clients)).to be_empty
           end
 
@@ -2395,7 +2648,6 @@
             create(:client, **good_client_params)
 
             get :index
-            # puts Client.where(filterable_product_year: 2024).first
             expect(assigns(:clients)).to be_empty
           end
 
@@ -2461,7 +2713,6 @@
             create(:client, **good_client_params)
 
             get :index
-            # puts Client.where(filterable_product_year: 2024).first
             expect(assigns(:clients)).not_to be_empty
           end
 

From 5260645cebbb27a9766f4f18d41ff043279d1100 Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Mon, 10 Feb 2025 13:47:23 -0500
Subject: [PATCH 09/18] ddd

---
 app/controllers/hub/clients_controller.rb | 2 ++
 app/lib/ability.rb                        | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/controllers/hub/clients_controller.rb b/app/controllers/hub/clients_controller.rb
index f012746c0d..04af4d921f 100644
--- a/app/controllers/hub/clients_controller.rb
+++ b/app/controllers/hub/clients_controller.rb
@@ -43,6 +43,8 @@ def destroy
     end
 
     def edit
+      raise CanCan::AccessDenied if @client.intake.is_ctc?
+
       @form = UpdateClientForm.from_client(@client)
     end
 
diff --git a/app/lib/ability.rb b/app/lib/ability.rb
index 471a3258bc..5a2fe57b23 100644
--- a/app/lib/ability.rb
+++ b/app/lib/ability.rb
@@ -117,7 +117,7 @@ def initialize(user)
     # Only admins can destroy clients
     cannot :destroy, Client unless user.admin?
 
-    can [:create, :update, :destroy], [
+    can [:create, :update, :edit, :destroy], [
       Note,
       Document,
       TaxReturn

From 17164c563ba4b4a67e57e3cc359300deb9e2d437 Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Mon, 10 Feb 2025 23:29:09 -0500
Subject: [PATCH 10/18] break out document ability and add more tests for
 controllers and tax return

---
 .../tax_returns/certifications_controller.rb  |  2 +
 app/controllers/hub/tax_returns_controller.rb |  8 +-
 app/lib/ability.rb                            | 14 +--
 .../certifications_controller_spec.rb         | 28 +++++-
 .../hub/tax_returns_controller_spec.rb        | 86 ++++++++++++++++++-
 5 files changed, 120 insertions(+), 18 deletions(-)

diff --git a/app/controllers/hub/tax_returns/certifications_controller.rb b/app/controllers/hub/tax_returns/certifications_controller.rb
index 16da2599e6..93c5f4d07a 100644
--- a/app/controllers/hub/tax_returns/certifications_controller.rb
+++ b/app/controllers/hub/tax_returns/certifications_controller.rb
@@ -10,6 +10,8 @@ def update
         redirect_to next_path || hub_client_path(id: @tax_return.client.id)
       end
 
+      private
+
       def tax_return_params
         params.permit(:certification_level)
       end
diff --git a/app/controllers/hub/tax_returns_controller.rb b/app/controllers/hub/tax_returns_controller.rb
index d3be431c57..43ab71868b 100644
--- a/app/controllers/hub/tax_returns_controller.rb
+++ b/app/controllers/hub/tax_returns_controller.rb
@@ -1,10 +1,8 @@
 module Hub
   class TaxReturnsController < Hub::BaseController
     include TaxReturnAssignableUsers
+    load_and_authorize_resource :client
     load_and_authorize_resource except: [:new, :create]
-    # on new/create, authorize through client but initialize tax return object
-    before_action :load_client, only: [:new, :create]
-    authorize_resource :client, parent: false, only: [:new, :create]
     before_action :load_assignable_users, except: [:show]
     before_action :load_and_authorize_assignee, only: [:update, :create]
 
@@ -59,10 +57,6 @@ def update
 
     private
 
-    def load_client
-      @client = Client.accessible_to_user(current_user).find(params[:client_id])
-    end
-
     def load_assignable_users
       @client ||= @tax_return.client
       @assignable_users = assignable_users(@client, [current_user, @tax_return&.assigned_user].compact)
diff --git a/app/lib/ability.rb b/app/lib/ability.rb
index 5a2fe57b23..b66858e543 100644
--- a/app/lib/ability.rb
+++ b/app/lib/ability.rb
@@ -89,7 +89,7 @@ def initialize(user)
     if user.role?(client_role_whitelist)
       can :read, Client, vita_partner: accessible_groups
 
-      can [:create, :update, :edit, :hub_client_management],
+      can [:create, :update, :hub_client_management],
           Client, vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year }
     end
 
@@ -117,17 +117,19 @@ def initialize(user)
     # Only admins can destroy clients
     cannot :destroy, Client unless user.admin?
 
-    can [:create, :update, :edit, :destroy], [
+    can [:read], [
       Note,
       Document,
       TaxReturn
-    ], client: { vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year } }
+    ], client: { vita_partner: accessible_groups }
 
-    can [:read], [
+    can [:create, :update, :destroy], [
       Note,
-      Document,
       TaxReturn
-    ], client: { vita_partner: accessible_groups }
+    ], client: { vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year } }
+
+    can [:create, :update, :destroy, :archived, :confirm],
+        Document, client: { vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year } }
 
     can :manage, [
       IncomingEmail,
diff --git a/spec/controllers/hub/tax_returns/certifications_controller_spec.rb b/spec/controllers/hub/tax_returns/certifications_controller_spec.rb
index 8b0698d415..45da41f0a4 100644
--- a/spec/controllers/hub/tax_returns/certifications_controller_spec.rb
+++ b/spec/controllers/hub/tax_returns/certifications_controller_spec.rb
@@ -1,9 +1,13 @@
 require 'rails_helper'
 
 RSpec.describe Hub::TaxReturns::CertificationsController do
+  let(:user) { create :organization_lead_user }
+  let!(:unauthorized_org_lead) { create :organization_lead_user }
+  let(:intake){ create :intake, product_year: product_year, client: create(:client, :with_gyr_return, vita_partner: user.role.organization)}
+  let(:product_year) { Rails.configuration.product_year }
+  let(:tax_return) { intake.client.tax_returns.first }
+
   describe "#update" do
-    let(:user) { create :organization_lead_user }
-    let(:tax_return) { create :gyr_tax_return, client: (create :client, vita_partner: user.role.organization) }
     let(:next_path) { "/next/path" }
     let(:params) { { id: tax_return.id, certification_level: "foreign_student", next: next_path } }
 
@@ -20,6 +24,7 @@
           tax_return.reload
         }.to change(tax_return, :certification_level).to('foreign_student')
       end
+
       context "redirecting on success" do
         context "with next param" do
           it "redirects to referring path without params" do
@@ -45,6 +50,25 @@
           end
         end
       end
+
+      context "with an archived intake" do
+        let(:product_year) { Rails.configuration.product_year - 1 }
+        it "response is forbidden (403)" do
+          patch :update, params: params
+          expect(response).to be_forbidden
+        end
+      end
+    end
+
+    context "with an unauthorized user" do
+      before do
+        sign_in unauthorized_org_lead
+      end
+
+      it "is not allowed to access the page" do
+        patch :update, params: params
+        expect(response).to be_forbidden
+      end
     end
   end
 end
diff --git a/spec/controllers/hub/tax_returns_controller_spec.rb b/spec/controllers/hub/tax_returns_controller_spec.rb
index f8014fe3ec..6019e184b0 100644
--- a/spec/controllers/hub/tax_returns_controller_spec.rb
+++ b/spec/controllers/hub/tax_returns_controller_spec.rb
@@ -43,9 +43,8 @@
       end
 
       it "is not allowed to access the page" do
-        expect do
-          get :new, params: params
-        end.to raise_error(ActiveRecord::RecordNotFound)
+        get :new, params: params
+        expect(response).to be_forbidden
       end
     end
 
@@ -63,6 +62,19 @@
         expect(assigns(:form).tax_return_years).to eq [2018]
         expect(assigns(:form).remaining_years).to eq(MultiTenantService.gyr.filing_years(fake_time) - [2018])
       end
+
+      context "with an archived intake" do
+        let(:user) { team_member }
+
+        before do
+          client.intake.update(product_year: Rails.configuration.product_year - 2)
+        end
+
+        it "response is forbidden (403)" do
+          get :new, params: params
+          expect(response).to be_forbidden
+        end
+      end
     end
   end
 
@@ -158,6 +170,30 @@
           expect(response).to be_forbidden
         end
       end
+
+      context "with an archived intake" do
+        let(:user) { team_member }
+
+        before do
+          client.intake.update(product_year: Rails.configuration.product_year - 2)
+        end
+
+        it "response is forbidden (403)" do
+          post :create, params: params
+          expect(response).to be_forbidden
+        end
+      end
+    end
+
+    context "an unauthorized user" do
+      before do
+        sign_in unauthorized_team_member
+      end
+
+      it "is not allowed to access the page" do
+        post :create, params: params
+        expect(response).to be_forbidden
+      end
     end
   end
 
@@ -216,6 +252,28 @@
           expect(assigns(:assignable_users)).not_to include suspended_user
         end
       end
+
+      context "with an archived intake" do
+        before do
+          client.intake.update(product_year: Rails.configuration.product_year - 2)
+        end
+
+        it "response is forbidden (403)" do
+          get :edit, params: params
+          expect(response).to be_forbidden
+        end
+      end
+    end
+
+    context "an unauthorized user" do
+      before do
+        sign_in unauthorized_team_member
+      end
+
+      it "is not allowed to access the page" do
+        get :edit, params: params
+        expect(response).to be_forbidden
+      end
     end
   end
 
@@ -328,6 +386,28 @@
           expect(response).to be_forbidden
         end
       end
+
+      context "with an archived intake" do
+        before do
+          client.intake.update(product_year: Rails.configuration.product_year - 2)
+        end
+
+        it "response is forbidden (403)" do
+          put :update, params: params
+          expect(response).to be_forbidden
+        end
+      end
+    end
+
+    context "an unauthorized user" do
+      before do
+        sign_in unauthorized_team_member
+      end
+
+      it "is not allowed to access the page" do
+        put :update, params: params
+        expect(response).to be_forbidden
+      end
     end
   end
 end

From f518d9d48b41f51de655a4afe383180d64dd216c Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Mon, 10 Feb 2025 23:33:03 -0500
Subject: [PATCH 11/18] Remove unneccessary line

---
 app/lib/ability.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/lib/ability.rb b/app/lib/ability.rb
index b66858e543..4ae65ca8c9 100644
--- a/app/lib/ability.rb
+++ b/app/lib/ability.rb
@@ -140,7 +140,6 @@ def initialize(user)
     ], client: { vita_partner: accessible_groups }
 
     can :manage, TaxReturnSelection, tax_returns: { client: { vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year } } }
-    cannot :manage, TaxReturnSelection, tax_returns: { client: { vita_partner: VitaPartner.where.not(id: accessible_groups) } }
 
     can :manage, EfileSubmission, tax_return: { client: { vita_partner: accessible_groups } }
 

From 4c9e37c7a311f66ffaa30d9ad99e1c7d9eca378a Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Tue, 11 Feb 2025 00:05:43 -0500
Subject: [PATCH 12/18] Add ability changes

---
 app/lib/ability.rb | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/app/lib/ability.rb b/app/lib/ability.rb
index 4ae65ca8c9..528a782bde 100644
--- a/app/lib/ability.rb
+++ b/app/lib/ability.rb
@@ -23,7 +23,6 @@ def initialize(user)
     if user.admin?
       # All admins who are also state file
       can :manage, :all
-      can :destroy, Client if user.admin?
 
       # Non-NJ staff cannot manage NJ EfileErrors, EfileSubmissions or FAQs
       cannot :manage, EfileError, service_type: "state_file_nj"
@@ -46,7 +45,7 @@ def initialize(user)
           %w[state_file unfilled state_file_az state_file_ny state_file_md state_file_nc state_file_id].include?(error.service_type)
         end
       end
-      unless user.email.include?("@codeforamerica.org")
+      unless user.email.downcase.include?("@codeforamerica.org")
         cannot :manage, :flipper_dashboard
       end
       return
@@ -72,7 +71,7 @@ def initialize(user)
     can :manage, User, id: user.id
 
     # Anyone can read info about users that they can access
-    can :read, User, id: user.accessible_users.pluck(:id)
+    can :read, User, id: user.accessible_users.ids
 
     # Anyone can read info about an organization or site they can access
     can :read, Organization, id: accessible_groups.pluck(:id)
@@ -98,20 +97,19 @@ def initialize(user)
     end
 
     if user.greeter?
-      can [:update, :read, :hub_client_management],
-        Client,
-        tax_returns: {
-          current_state: %w[intake_ready intake_greeter_info_requested intake_needs_doc_help],
-        },
-        vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year }
-
-      can [:update, :read, :hub_client_management],
-        Client,
-        tax_returns: {
-          current_state: %w[file_not_filing file_hold],
-          assigned_user: user,
-        },
-        vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year }
+      general_states = %w[intake_ready intake_greeter_info_requested intake_needs_doc_help]
+      assigned_states = %w[file_not_filing file_hold]
+
+      can :read, Client, tax_returns: { current_state: general_states }, vita_partner: accessible_groups
+      can :read, Client, tax_returns: { current_state: assigned_states, assigned_user: user }, vita_partner: accessible_groups
+
+      can [:update, :hub_client_management], Client,
+          tax_returns: { current_state: general_states },
+          vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year }
+
+      can [:update, :hub_client_management], Client,
+          tax_returns: { current_state: assigned_states, assigned_user: user },
+          vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year }
     end
 
     # Only admins can destroy clients
@@ -166,7 +164,7 @@ def initialize(user)
       can :read, Coalition, id: user.role.coalition_id
 
       # Coalition leads can view and edit users who are coalition leads, organization leads, site coordinators, and team members in their coalition
-      can :manage, User, id: user.accessible_users.pluck(:id)
+      can :manage, User, id: user.accessible_users.ids
 
       # Coalition leads can create coalition leads, organization leads, site coordinators, and team members in their coalition
       can :manage, CoalitionLeadRole, coalition: user.role.coalition
@@ -178,7 +176,7 @@ def initialize(user)
     if user.org_lead?
 
       # Organization leads can view and edit users who are organization leads, site coordinators, and team members in their coalition
-      can :manage, User, id: user.accessible_users.pluck(:id)
+      can :manage, User, id: user.accessible_users.ids
 
       # Organization leads can create organization leads, site coordinators, and team members in their org
       can :manage, OrganizationLeadRole, organization: user.role.organization

From 6c19e18245869b4aa1d2e6a2465af18bda1b7127 Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Tue, 11 Feb 2025 00:37:26 -0500
Subject: [PATCH 13/18] Add changes

---
 app/lib/ability.rb       | 6 +++---
 spec/lib/ability_spec.rb | 3 +--
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/app/lib/ability.rb b/app/lib/ability.rb
index 528a782bde..303558bbd0 100644
--- a/app/lib/ability.rb
+++ b/app/lib/ability.rb
@@ -71,7 +71,7 @@ def initialize(user)
     can :manage, User, id: user.id
 
     # Anyone can read info about users that they can access
-    can :read, User, id: user.accessible_users.ids
+    can :read, User, id: user.accessible_users.pluck(:id)
 
     # Anyone can read info about an organization or site they can access
     can :read, Organization, id: accessible_groups.pluck(:id)
@@ -164,7 +164,7 @@ def initialize(user)
       can :read, Coalition, id: user.role.coalition_id
 
       # Coalition leads can view and edit users who are coalition leads, organization leads, site coordinators, and team members in their coalition
-      can :manage, User, id: user.accessible_users.ids
+      can :manage, User, id: user.accessible_users.pluck(:id)
 
       # Coalition leads can create coalition leads, organization leads, site coordinators, and team members in their coalition
       can :manage, CoalitionLeadRole, coalition: user.role.coalition
@@ -176,7 +176,7 @@ def initialize(user)
     if user.org_lead?
 
       # Organization leads can view and edit users who are organization leads, site coordinators, and team members in their coalition
-      can :manage, User, id: user.accessible_users.ids
+      can :manage, User, id: user.accessible_users.pluck(:id)
 
       # Organization leads can create organization leads, site coordinators, and team members in their org
       can :manage, OrganizationLeadRole, organization: user.role.organization
diff --git a/spec/lib/ability_spec.rb b/spec/lib/ability_spec.rb
index ba95c98b73..f0e16c627a 100644
--- a/spec/lib/ability_spec.rb
+++ b/spec/lib/ability_spec.rb
@@ -222,7 +222,6 @@
             expect(subject.can?(:toggle_field, accessible_client)).to eq true
             expect(subject.can?(:edit_take_action, accessible_client)).to eq true
             expect(subject.can?(:update_take_action, accessible_client)).to eq true
-            expect(subject.can?(:unlock, accessible_client)).to eq true
             expect(subject.can?(:save_and_maybe_exit, accessible_client)).to eq true
 
             expect(subject.can?(:edit_13614c_form_page1, accessible_client)).to eq true
@@ -326,7 +325,7 @@
             expect(subject.can?(:toggle_field, accessible_client)).to eq true
             expect(subject.can?(:edit_take_action, accessible_client)).to eq true
             expect(subject.can?(:update_take_action, accessible_client)).to eq true
-            expect(subject.can?(:unlock, accessible_client)).to eq true
+            expect(subject.can?(:unlock, accessible_client)).to eq false
             expect(subject.can?(:save_and_maybe_exit, accessible_client)).to eq true
 
             expect(subject.can?(:edit_13614c_form_page1, accessible_client)).to eq true

From b9fc5061b813041e8d61a7a3aeacf144ef03675e Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Tue, 11 Feb 2025 01:21:02 -0500
Subject: [PATCH 14/18] Add changes

---
 app/lib/ability.rb                                     |  4 ++++
 .../bulk_actions/base_bulk_actions_controller_spec.rb  | 10 ++++++++++
 2 files changed, 14 insertions(+)

diff --git a/app/lib/ability.rb b/app/lib/ability.rb
index 303558bbd0..cc387d7d2f 100644
--- a/app/lib/ability.rb
+++ b/app/lib/ability.rb
@@ -138,6 +138,10 @@ def initialize(user)
     ], client: { vita_partner: accessible_groups }
 
     can :manage, TaxReturnSelection, tax_returns: { client: { vita_partner: accessible_groups, intake: { product_year: Rails.configuration.product_year } } }
+    cannot :manage, TaxReturnSelection, tax_returns: { client: { vita_partner: VitaPartner.where.not(id: accessible_groups) } }
+    cannot :manage, TaxReturnSelection do |selection|
+      selection.tax_returns.any? { |tax_return| tax_return.client.has_archived_intake? }
+    end
 
     can :manage, EfileSubmission, tax_return: { client: { vita_partner: accessible_groups } }
 
diff --git a/spec/controllers/hub/bulk_actions/base_bulk_actions_controller_spec.rb b/spec/controllers/hub/bulk_actions/base_bulk_actions_controller_spec.rb
index 4e9c80f57a..4b896273fe 100644
--- a/spec/controllers/hub/bulk_actions/base_bulk_actions_controller_spec.rb
+++ b/spec/controllers/hub/bulk_actions/base_bulk_actions_controller_spec.rb
@@ -62,6 +62,16 @@ def edit
           end
         end
 
+        context "with a tax return selection connected to an archived intake" do
+          let!(:tax_return_selection) { create :tax_return_selection, tax_returns: [tax_return_1, tax_return_2, tax_return_3] }
+          let(:tax_return_3) { create :tax_return, year: 2021, client: create(:client, intake: build(:intake, product_year: Rails.configuration.product_year - 2)) }
+
+          it "response is forbidden (403)" do
+            get :edit, params: params
+            expect(response).to be_forbidden
+          end
+        end
+
         context "with only clients who don't have sufficient contact info" do
           let(:intake) { build :intake, email_notification_opt_in: "yes", email_address: nil, sms_notification_opt_in: "yes", sms_phone_number: nil }
 

From 3dc0a6d210d009c98a41d1cf94d0f74c6523f5bf Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Tue, 11 Feb 2025 01:41:50 -0500
Subject: [PATCH 15/18] Add tests

---
 .../hub/ctc_clients_controller_spec.rb        | 52 +++++++++++++------
 1 file changed, 35 insertions(+), 17 deletions(-)

diff --git a/spec/controllers/hub/ctc_clients_controller_spec.rb b/spec/controllers/hub/ctc_clients_controller_spec.rb
index 671adaed42..d6d503cbfa 100644
--- a/spec/controllers/hub/ctc_clients_controller_spec.rb
+++ b/spec/controllers/hub/ctc_clients_controller_spec.rb
@@ -5,7 +5,7 @@
   let(:user) { create(:user, role: create(:organization_lead_role, organization: organization), timezone: "America/Los_Angeles") }
 
   describe "#edit" do
-    let(:client) { create :client, :with_ctc_return, intake: (build :ctc_intake), vita_partner: organization }
+    let(:client) { create :client, :with_ctc_return, intake: (build :ctc_intake, product_year: Rails.configuration.product_year), vita_partner: organization }
     let(:params) {
       { id: client.id }
     }
@@ -21,13 +21,22 @@
         expect(response).to be_ok
         expect(assigns(:form)).to be_an_instance_of Hub::UpdateCtcClientForm
       end
+
+      context "with an archived intake" do
+        let(:client) { create :client, :with_ctc_return, intake: (build :ctc_intake), vita_partner: organization }
+
+        it "response is forbidden (403)" do
+          get :edit, params: params
+          expect(response).to be_forbidden
+        end
+      end
     end
   end
 
   describe "#update" do
     let!(:client) { create :client, :with_ctc_return, intake: intake, vita_partner: organization }
 
-    let(:intake) { build :ctc_intake, :filled_out_ctc, :with_contact_info, :with_ssns, :with_dependents, email_address: "cher@example.com", primary_last_name: "Cherimoya" }
+    let(:intake) { build :ctc_intake, :filled_out_ctc, :with_contact_info, :with_ssns, :with_dependents, email_address: "cher@example.com", primary_last_name: "Cherimoya", product_year: Rails.configuration.product_year }
     let(:first_dependent) { intake.dependents.first }
     let!(:params) do
       {
@@ -107,21 +116,21 @@
         expect(system_note.client).to eq(client)
         expect(system_note.user).to eq(user)
         expect(system_note.data['changes']).to match({
-          "spouse_ssn" => ["[REDACTED]", "[REDACTED]"],
-          "primary_ssn" => ["[REDACTED]", "[REDACTED]"],
-          "email_address" => ["cher@example.com", "san@mateo.com"],
-          "primary_tin_type" => ["ssn", nil],
-          "spouse_last_name" => ["Hesse", "Diego"],
-          "primary_last_name" => ["Cherimoya", "Mateo"],
-          "spouse_birth_date" => ["1929-09-02", "1980-01-11"],
-          "spouse_first_name" => ["Eva", "San"],
-          "primary_first_name" => ["Cher", "San"],
-          "eip1_amount_received" => [1000, 9000],
-          "spouse_email_address" => ["eva@hesse.com", "san@diego.com"],
-          "spouse_last_four_ssn" => ["[REDACTED]", "[REDACTED]"],
-          "primary_last_four_ssn" => ["[REDACTED]", "[REDACTED]"],
-          "preferred_interview_language" => ["en", nil],
-        })
+                                                       "spouse_ssn" => ["[REDACTED]", "[REDACTED]"],
+                                                       "primary_ssn" => ["[REDACTED]", "[REDACTED]"],
+                                                       "email_address" => ["cher@example.com", "san@mateo.com"],
+                                                       "primary_tin_type" => ["ssn", nil],
+                                                       "spouse_last_name" => ["Hesse", "Diego"],
+                                                       "primary_last_name" => ["Cherimoya", "Mateo"],
+                                                       "spouse_birth_date" => ["1929-09-02", "1980-01-11"],
+                                                       "spouse_first_name" => ["Eva", "San"],
+                                                       "primary_first_name" => ["Cher", "San"],
+                                                       "eip1_amount_received" => [1000, 9000],
+                                                       "spouse_email_address" => ["eva@hesse.com", "san@diego.com"],
+                                                       "spouse_last_four_ssn" => ["[REDACTED]", "[REDACTED]"],
+                                                       "primary_last_four_ssn" => ["[REDACTED]", "[REDACTED]"],
+                                                       "preferred_interview_language" => ["en", nil],
+                                                     })
       end
 
       context "when the client's email address has changed" do
@@ -214,6 +223,15 @@
           expect(flash[:alert]).to eq "Please fix indicated errors before continuing."
         end
       end
+
+      context "with an archived intake" do
+        let(:client) { create :client, :with_ctc_return, intake: (build :ctc_intake), vita_partner: organization }
+
+        it "response is forbidden (403)" do
+          post :update, params: params
+          expect(response).to be_forbidden
+        end
+      end
     end
   end
 end

From 26f21266242bd5ac3252361f1e8de7078dad3d26 Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Tue, 11 Feb 2025 01:43:55 -0500
Subject: [PATCH 16/18] remove change

---
 .../hub/ctc_clients_controller_spec.rb        | 30 +++++++++----------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/spec/controllers/hub/ctc_clients_controller_spec.rb b/spec/controllers/hub/ctc_clients_controller_spec.rb
index d6d503cbfa..21c6d3c6ab 100644
--- a/spec/controllers/hub/ctc_clients_controller_spec.rb
+++ b/spec/controllers/hub/ctc_clients_controller_spec.rb
@@ -116,21 +116,21 @@
         expect(system_note.client).to eq(client)
         expect(system_note.user).to eq(user)
         expect(system_note.data['changes']).to match({
-                                                       "spouse_ssn" => ["[REDACTED]", "[REDACTED]"],
-                                                       "primary_ssn" => ["[REDACTED]", "[REDACTED]"],
-                                                       "email_address" => ["cher@example.com", "san@mateo.com"],
-                                                       "primary_tin_type" => ["ssn", nil],
-                                                       "spouse_last_name" => ["Hesse", "Diego"],
-                                                       "primary_last_name" => ["Cherimoya", "Mateo"],
-                                                       "spouse_birth_date" => ["1929-09-02", "1980-01-11"],
-                                                       "spouse_first_name" => ["Eva", "San"],
-                                                       "primary_first_name" => ["Cher", "San"],
-                                                       "eip1_amount_received" => [1000, 9000],
-                                                       "spouse_email_address" => ["eva@hesse.com", "san@diego.com"],
-                                                       "spouse_last_four_ssn" => ["[REDACTED]", "[REDACTED]"],
-                                                       "primary_last_four_ssn" => ["[REDACTED]", "[REDACTED]"],
-                                                       "preferred_interview_language" => ["en", nil],
-                                                     })
+          "spouse_ssn" => ["[REDACTED]", "[REDACTED]"],
+          "primary_ssn" => ["[REDACTED]", "[REDACTED]"],
+          "email_address" => ["cher@example.com", "san@mateo.com"],
+          "primary_tin_type" => ["ssn", nil],
+          "spouse_last_name" => ["Hesse", "Diego"],
+          "primary_last_name" => ["Cherimoya", "Mateo"],
+          "spouse_birth_date" => ["1929-09-02", "1980-01-11"],
+          "spouse_first_name" => ["Eva", "San"],
+          "primary_first_name" => ["Cher", "San"],
+          "eip1_amount_received" => [1000, 9000],
+          "spouse_email_address" => ["eva@hesse.com", "san@diego.com"],
+          "spouse_last_four_ssn" => ["[REDACTED]", "[REDACTED]"],
+          "primary_last_four_ssn" => ["[REDACTED]", "[REDACTED]"],
+          "preferred_interview_language" => ["en", nil],
+       })
       end
 
       context "when the client's email address has changed" do

From 0d50985c1789389b188ab9d56b9ab46f7d50f994 Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Tue, 11 Feb 2025 11:31:49 -0500
Subject: [PATCH 17/18] Add page title back

---
 app/controllers/hub/clients_controller.rb | 1 +
 app/views/hub/clients/index.html.erb      | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/controllers/hub/clients_controller.rb b/app/controllers/hub/clients_controller.rb
index 04af4d921f..41cd2dbffa 100644
--- a/app/controllers/hub/clients_controller.rb
+++ b/app/controllers/hub/clients_controller.rb
@@ -13,6 +13,7 @@ class ClientsController < Hub::BaseController
     layout "hub"
 
     def index
+      @page_title = I18n.t("hub.clients.index.title")
       @clients = @client_sorter.filtered_and_sorted_clients.page(params[:page]).load
       @message_summaries = RecentMessageSummaryService.messages(@clients.map(&:id))
     end
diff --git a/app/views/hub/clients/index.html.erb b/app/views/hub/clients/index.html.erb
index 52cfb4154d..ea301bf0ab 100644
--- a/app/views/hub/clients/index.html.erb
+++ b/app/views/hub/clients/index.html.erb
@@ -1,4 +1,3 @@
-<% @page_title = I18n.t("hub.clients.index.title") %>
 <% content_for :page_title, @page_title %>
 <% unless @hide_filters %>
   <% content_for :filters do %>

From c38ea4da58a5574be30435c532c06079f7892935 Mon Sep 17 00:00:00 2001
From: Em Barnard-Shao <ebarnard@codeforamerica.org>
Date: Tue, 11 Feb 2025 13:18:14 -0500
Subject: [PATCH 18/18] Add changes

---
 ...nge_assignee_and_status_controller_spec.rb | 24 ++++++++++++++-
 .../change_organization_controller_spec.rb    | 30 ++++++++++++++++---
 .../send_a_message_controller_spec.rb         | 13 +++++++-
 3 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/spec/controllers/hub/bulk_actions/change_assignee_and_status_controller_spec.rb b/spec/controllers/hub/bulk_actions/change_assignee_and_status_controller_spec.rb
index aec77b3020..999c25fe05 100644
--- a/spec/controllers/hub/bulk_actions/change_assignee_and_status_controller_spec.rb
+++ b/spec/controllers/hub/bulk_actions/change_assignee_and_status_controller_spec.rb
@@ -1,7 +1,7 @@
 require 'rails_helper'
 
 RSpec.describe Hub::BulkActions::ChangeAssigneeAndStatusController do
-  let(:client) { create :client, vita_partner: site, intake: build(:intake) }
+  let(:client) { create :client, vita_partner: site, intake: build(:intake, product_year: Rails.configuration.product_year) }
   let(:site) { create :site }
   let(:organization) { create :organization }
 
@@ -36,6 +36,17 @@
         expect(assigns(:assignable_users)).to match_array [team_member, site_coordinator]
         expect(assigns(:assignable_users)).not_to include inaccessible_user
       end
+
+      context "with an archived intake" do
+        before do
+          client.intake.update(product_year: Rails.configuration.product_year - 2)
+        end
+
+        it "response is forbidden (403)" do
+          get :edit, params: params
+          expect(response).to be_forbidden
+        end
+      end
     end
 
     context "an unauthorized user" do
@@ -131,6 +142,17 @@
           end.not_to have_enqueued_job(BulkActionJob)
         end
       end
+
+      context "with an archived intake" do
+        before do
+          client.intake.update(product_year: Rails.configuration.product_year - 2)
+        end
+
+        it "response is forbidden (403)" do
+          put :update, params: params
+          expect(response).to be_forbidden
+        end
+      end
     end
   end
 end
diff --git a/spec/controllers/hub/bulk_actions/change_organization_controller_spec.rb b/spec/controllers/hub/bulk_actions/change_organization_controller_spec.rb
index 18df46ec7c..0495034453 100644
--- a/spec/controllers/hub/bulk_actions/change_organization_controller_spec.rb
+++ b/spec/controllers/hub/bulk_actions/change_organization_controller_spec.rb
@@ -2,10 +2,10 @@
 
 RSpec.describe Hub::BulkActions::ChangeOrganizationController do
   let(:organization) { create :organization }
-  let(:client) { create :client, vita_partner: organization }
-  let(:tax_return_1) { create :tax_return, client: client, year: 2020 }
-  let(:tax_return_2) { create :tax_return, client: client, year: 2019 }
-  let(:tax_return_3) { create :tax_return, client: client, year: 2018 }
+  let(:intake){ create :intake, client: create(:client, vita_partner: organization), product_year: Rails.configuration.product_year }
+  let(:tax_return_1) { create :tax_return, client: intake.client, year: 2020 }
+  let(:tax_return_2) { create :tax_return, client: intake.client, year: 2019 }
+  let(:tax_return_3) { create :tax_return, client: intake.client, year: 2018 }
   let!(:tax_return_selection) { create :tax_return_selection, tax_returns: [tax_return_1, tax_return_2, tax_return_3] }
   let(:user) { create :organization_lead_user, organization: organization }
 
@@ -29,6 +29,17 @@
           expect(assigns(:vita_partners)).to match_array [organization, site, other_site]
         end
       end
+
+      context "with an archived intake" do
+        before do
+          intake.update(product_year: Rails.configuration.product_year - 2)
+        end
+
+        it "response is forbidden (403)" do
+          get :edit, params: params
+          expect(response).to be_forbidden
+        end
+      end
     end
   end
 
@@ -81,6 +92,17 @@
           }.not_to have_enqueued_job
         end
       end
+
+      context "with an archived intake" do
+        before do
+          intake.update(product_year: Rails.configuration.product_year - 2)
+        end
+
+        it "response is forbidden (403)" do
+          put :update, params: params
+          expect(response).to be_forbidden
+        end
+      end
     end
   end
 end
diff --git a/spec/controllers/hub/bulk_actions/send_a_message_controller_spec.rb b/spec/controllers/hub/bulk_actions/send_a_message_controller_spec.rb
index 58cb8e1c96..2de6f78aef 100644
--- a/spec/controllers/hub/bulk_actions/send_a_message_controller_spec.rb
+++ b/spec/controllers/hub/bulk_actions/send_a_message_controller_spec.rb
@@ -2,7 +2,7 @@
 
 RSpec.describe Hub::BulkActions::SendAMessageController do
   let(:organization) { create :organization }
-  let(:client) { create :client, vita_partner: organization }
+  let(:client) { create :client, vita_partner: organization, intake: build(:intake, product_year: Rails.configuration.product_year) }
   let(:tax_return_1) { create :tax_return, client: client, year: 2020 }
   let(:tax_return_2) { create :tax_return, client: client, year: 2019 }
   let(:tax_return_3) { create :tax_return, client: client, year: 2018 }
@@ -69,6 +69,17 @@
           }.not_to have_enqueued_job
         end
       end
+
+      context "with an archived intake" do
+        before do
+          client.intake.update(product_year: Rails.configuration.product_year - 2)
+        end
+
+        it "response is forbidden (403)" do
+          put :update, params: params
+          expect(response).to be_forbidden
+        end
+      end
     end
   end
 end