Merge pull request #1719 from DFE-Digital/stop-using-requestable-state

Stop using requestable state

app/controllers/personas_controller.rb

@@ -8,11 +8,6 @@ class PersonasController < ApplicationController
 
   def index
     @staff = Staff.all
-
-    @reference_requests =
-      ReferenceRequest.states.values.filter_map do |state|
-        ReferenceRequest.find_by(state:)
-      end
   end
 
   def eligible_sign_in

app/jobs/expire_requestable_job.rb

@@ -2,8 +2,12 @@
 
 class ExpireRequestableJob < ApplicationJob
   def perform(requestable)
-    if requestable.requested? && requestable.expires_at.present? &&
-         Time.zone.now > requestable.expires_at
+    if requestable.received? || requestable.expired? ||
+         !requestable.requested? || requestable.expires_at.nil?
+      return
+    end
+
+    if Time.zone.now > requestable.expires_at
       ExpireRequestable.call(requestable:, user: "Expirer")
     end
   end

app/jobs/expire_requestables_job.rb

@@ -2,8 +2,12 @@
 
 class ExpireRequestablesJob < ApplicationJob
   def perform(requestable_class_name)
-    requestable_class_name.constantize.requested.find_each do |requestable|
-      ExpireRequestableJob.perform_later(requestable)
-    end
+    requestable_class_name
+      .constantize
+      .requested
+      .where(expired_at: nil, received_at: nil)
+      .find_each do |requestable|
+        ExpireRequestableJob.perform_later(requestable)
+      end
   end
 end

app/lib/application_form_status_updater.rb

@@ -302,11 +302,15 @@ def overdue?(requestables:)
   end
 
   def waiting_on?(requestables:)
-    requestables.reject(&:reviewed?).any?(&:requested?)
+    requestables
+      .reject(&:reviewed?)
+      .reject(&:expired?)
+      .reject(&:received?)
+      .any?(&:requested?)
   end
 
   def received?(requestables:)
-    requestables.reject(&:reviewed?).any?(&:received?)
+    requestables.reject(&:reviewed?).reject(&:expired?).any?(&:received?)
   end
 
   def create_timeline_event(event_type:, **kwargs)

app/models/concerns/expirable.rb

@@ -9,6 +9,14 @@ def expires_at
     requested_at + expires_after
   end
 
+  def expired!
+    update!(expired_at: Time.zone.now)
+  end
+
+  def expired?
+    expired_at != nil
+  end
+
   def after_expired(user:)
     # implement logic after an expiration of this object
   end

app/models/concerns/requestable.rb

@@ -8,32 +8,32 @@ module Requestable
   included do
     belongs_to :assessment
 
-    enum :state,
-         { requested: "requested", received: "received", expired: "expired" },
-         default: "requested"
-
-    validates :state, presence: true, inclusion: { in: states.values }
-
-    validates :requested_at, presence: true, if: :requested?
-    validates :received_at, presence: true, if: :received?
-    validates :expired_at, presence: true, if: :expired?
     validates :reviewed_at, presence: true, unless: -> { passed.nil? }
 
-    scope :respondable, -> { not_received.merge(ApplicationForm.assessable) }
+    scope :requested, -> { where.not(requested_at: nil) }
+    scope :received, -> { where.not(received_at: nil) }
+    scope :respondable,
+          -> do
+            requested.where(received_at: nil).merge(ApplicationForm.assessable)
+          end
 
-    define_method :requested! do
-      update!(state: "requested", requested_at: Time.zone.now)
-    end
+    has_one :application_form, through: :assessment
+  end
 
-    define_method :received! do
-      update!(state: "received", received_at: Time.zone.now)
-    end
+  def requested!
+    update!(requested_at: Time.zone.now)
+  end
 
-    define_method :expired! do
-      update!(state: "expired", expired_at: Time.zone.now)
-    end
+  def requested?
+    requested_at != nil
+  end
 
-    has_one :application_form, through: :assessment
+  def received!
+    update!(received_at: Time.zone.now)
+  end
+
+  def received?
+    received_at != nil
   end
 
   def reviewed!(passed)
@@ -44,19 +44,25 @@ def reviewed?
     passed != nil
   end
 
-  def overdue?
-    expired? || (received? && expires_at.present? && received_at > expires_at)
-  end
-
   def failed
     return nil if passed.nil?
     passed == false
   end
 
   def status
-    return state if passed.nil?
-
-    passed ? "accepted" : "rejected"
+    if reviewed_at.present?
+      passed ? "accepted" : "rejected"
+    elsif received_at.present? && expired_at.present?
+      "received_and_overdue"
+    elsif expired_at.present?
+      "overdue"
+    elsif received_at.present?
+      "received"
+    elsif requested_at.present?
+      "waiting_on"
+    else
+      "not_started"
+    end
   end
 
   def after_requested(user:)

app/models/further_information_request.rb

@@ -9,7 +9,7 @@
 #  received_at                                 :datetime
 #  requested_at                                :datetime
 #  reviewed_at                                 :datetime
-#  state                                       :string           not null
+#  state                                       :string           default("requested"), not null
 #  working_days_assessment_started_to_creation :integer
 #  working_days_received_to_recommendation     :integer
 #  working_days_since_received                 :integer
@@ -32,9 +32,11 @@ class FurtherInformationRequest < ApplicationRecord
 
   scope :remindable,
         -> do
-          requested.joins(assessment: :application_form).merge(
-            ApplicationForm.assessable,
-          )
+          where
+            .not(requested_at: nil)
+            .where(expired_at: nil)
+            .joins(assessment: :application_form)
+            .merge(ApplicationForm.assessable)
         end
 
   FOUR_WEEK_COUNTRY_CODES = %w[AU CA GI NZ US].freeze

app/models/professional_standing_request.rb

@@ -13,7 +13,7 @@
 #  received_at           :datetime
 #  requested_at          :datetime
 #  reviewed_at           :datetime
-#  state                 :string           not null
+#  state                 :string           default("requested"), not null
 #  created_at            :datetime         not null
 #  updated_at            :datetime         not null
 #  assessment_id         :bigint           not null

app/models/qualification_request.rb

@@ -12,7 +12,7 @@
 #  received_at           :datetime
 #  requested_at          :datetime
 #  reviewed_at           :datetime
-#  state                 :string           not null
+#  state                 :string           default("requested"), not null
 #  created_at            :datetime         not null
 #  updated_at            :datetime         not null
 #  assessment_id         :bigint           not null

app/models/reference_request.rb

@@ -31,7 +31,7 @@
 #  satisfied_comment               :text             default(""), not null
 #  satisfied_response              :boolean
 #  slug                            :string           not null
-#  state                           :string           not null
+#  state                           :string           default("requested"), not null
 #  created_at                      :datetime         not null
 #  updated_at                      :datetime         not null
 #  assessment_id                   :bigint           not null
@@ -58,9 +58,11 @@ class ReferenceRequest < ApplicationRecord
 
   scope :remindable,
         -> do
-          requested.joins(assessment: :application_form).merge(
-            ApplicationForm.assessable,
-          )
+          where
+            .not(requested_at: nil)
+            .where(expired_at: nil)
+            .joins(assessment: :application_form)
+            .merge(ApplicationForm.assessable)
         end
 
   with_options if: :received? do

app/services/create_further_information_request.rb

@@ -11,21 +11,20 @@ def initialize(assessment:, user:)
   def call
     further_information_request =
       ActiveRecord::Base.transaction do
-        request =
-          assessment.further_information_requests.create!(
+        requestable =
+          FurtherInformationRequest.create!(
+            assessment:,
             items:
               FurtherInformationRequestItemsFactory.call(
                 assessment_sections: assessment.sections,
               ),
-            requested_at: Time.zone.now,
           )
 
-        ApplicationFormStatusUpdater.call(application_form:, user:)
+        RequestRequestable.call(requestable:, user:)
 
-        create_timeline_event(request)
-        request.after_requested(user:)
+        ApplicationFormStatusUpdater.call(application_form:, user:)
 
-        request
+        requestable
       end
 
     TeacherMailer.with(teacher:).further_information_requested.deliver_later
@@ -44,13 +43,4 @@ def application_form
   def teacher
     @teacher ||= application_form.teacher
   end
-
-  def create_timeline_event(further_information_request)
-    TimelineEvent.create!(
-      application_form:,
-      creator: user,
-      event_type: "requestable_requested",
-      requestable: further_information_request,
-    )
-  end
 end

app/services/request_requestable.rb

@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+class RequestRequestable
+  include ServicePattern
+
+  def initialize(requestable:, user:)
+    @requestable = requestable
+    @user = user
+  end
+
+  def call
+    raise AlreadyRequested if requestable.requested?
+
+    ActiveRecord::Base.transaction do
+      requestable.requested!
+      create_timeline_event
+      ApplicationFormStatusUpdater.call(application_form:, user:)
+    end
+
+    requestable.after_requested(user:)
+  end
+
+  class AlreadyRequested < StandardError
+  end
+
+  private
+
+  attr_reader :requestable, :user
+
+  delegate :application_form, to: :requestable
+
+  def create_timeline_event
+    creator = user.is_a?(String) ? nil : user
+    creator_name = user.is_a?(String) ? user : ""
+
+    TimelineEvent.create!(
+      application_form:,
+      creator:,
+      creator_name:,
+      event_type: "requestable_requested",
+      requestable:,
+    )
+  end
+end

app/services/send_reminder_email.rb

@@ -19,7 +19,6 @@ def call
   attr_reader :remindable
 
   def send_reminder?
-    return false if remindable.try(:expired_at).present?
     return false unless remindable.expires_at
 
     remindable.should_send_reminder_email?(

app/services/submit_application_form.rb

@@ -18,15 +18,15 @@ def call
         region.requires_preliminary_check
       application_form.submitted_at = Time.zone.now
 
+      ApplicationFormStatusUpdater.call(application_form:, user:)
+
       assessment = AssessmentFactory.call(application_form:)
 
       create_professional_standing_request(assessment)
 
       if application_form.reduced_evidence_accepted
         UpdateAssessmentInductionRequired.call(assessment:)
       end
-
-      ApplicationFormStatusUpdater.call(application_form:, user:)
     end
 
     TeacherMailer
@@ -68,19 +68,8 @@ def call
   def create_professional_standing_request(assessment)
     return unless application_form.teaching_authority_provides_written_statement
 
-    requestable =
-      ProfessionalStandingRequest.create!(
-        assessment:,
-        requested_at: Time.zone.now,
-      )
-
-    TimelineEvent.create!(
-      event_type: "requestable_requested",
-      application_form:,
-      creator: user,
-      requestable:,
-    )
+    requestable = ProfessionalStandingRequest.create!(assessment:)
 
-    requestable.after_requested(user:)
+    RequestRequestable.call(requestable:, user:)
   end
 end