From 8b53aaeca5854c6c2ba02b1ad891e7d5949a83b1 Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Tue, 26 Sep 2023 10:56:53 +0100 Subject: [PATCH 1/5] Add requested_at column This adds a new column to the requestables for tracking when a requestable was requested. We need this as in the future with the new verification flow where the request happens in a separate step to the creation of the requestable. --- app/models/further_information_request.rb | 1 + app/models/professional_standing_request.rb | 1 + app/models/qualification_request.rb | 1 + app/models/reference_request.rb | 1 + config/analytics.yml | 90 ++++++++++--------- ...092809_add_requested_at_to_requestables.rb | 13 +++ db/schema.rb | 6 +- .../factories/further_information_requests.rb | 1 + .../professional_standing_requests.rb | 1 + spec/factories/qualification_requests.rb | 1 + spec/factories/reference_requests.rb | 1 + .../further_information_request_spec.rb | 1 + .../professional_standing_request_spec.rb | 1 + spec/models/qualification_request_spec.rb | 1 + spec/models/reference_request_spec.rb | 1 + 15 files changed, 77 insertions(+), 44 deletions(-) create mode 100644 db/migrate/20230926092809_add_requested_at_to_requestables.rb diff --git a/app/models/further_information_request.rb b/app/models/further_information_request.rb index 76da1017dc..56af555f8e 100644 --- a/app/models/further_information_request.rb +++ b/app/models/further_information_request.rb @@ -6,6 +6,7 @@ # failure_assessor_note :string default(""), not null # passed :boolean # received_at :datetime +# requested_at :datetime # reviewed_at :datetime # state :string not null # working_days_assessment_started_to_creation :integer diff --git a/app/models/professional_standing_request.rb b/app/models/professional_standing_request.rb index e52008a9a7..b7de1b703b 100644 --- a/app/models/professional_standing_request.rb +++ b/app/models/professional_standing_request.rb @@ -10,6 +10,7 @@ # passed :boolean # ready_for_review :boolean default(FALSE), not null # received_at :datetime +# requested_at :datetime # reviewed_at :datetime # state :string not null # created_at :datetime not null diff --git a/app/models/qualification_request.rb b/app/models/qualification_request.rb index 2b9f287bed..e9706b85bf 100644 --- a/app/models/qualification_request.rb +++ b/app/models/qualification_request.rb @@ -9,6 +9,7 @@ # location_note :text default(""), not null # passed :boolean # received_at :datetime +# requested_at :datetime # reviewed_at :datetime # state :string not null # created_at :datetime not null diff --git a/app/models/reference_request.rb b/app/models/reference_request.rb index 4869338b21..23e8198487 100644 --- a/app/models/reference_request.rb +++ b/app/models/reference_request.rb @@ -25,6 +25,7 @@ # received_at :datetime # reports_comment :text default(""), not null # reports_response :boolean +# requested_at :datetime # reviewed_at :datetime # satisfied_comment :text default(""), not null # satisfied_response :boolean diff --git a/config/analytics.yml b/config/analytics.yml index 07e9a844e8..32d4c58dc7 100644 --- a/config/analytics.yml +++ b/config/analytics.yml @@ -179,18 +179,19 @@ - created_at - updated_at :further_information_requests: + - assessment_id + - created_at + - failure_assessor_note - id + - passed - received_at + - requested_at + - reviewed_at - state - - created_at - updated_at - - assessment_id - - passed - - failure_assessor_note - - working_days_since_received - - working_days_received_to_recommendation - working_days_assessment_started_to_creation - - reviewed_at + - working_days_received_to_recommendation + - working_days_since_received :further_information_request_items: - id - information_type @@ -212,17 +213,18 @@ - created_at - updated_at :professional_standing_requests: - - id - assessment_id - - state - - received_at - - location_note - created_at - - updated_at - - reviewed_at - - passed - failure_assessor_note + - id + - location_note + - passed - ready_for_review + - received_at + - requested_at + - reviewed_at + - state + - updated_at :qualifications: - id - application_form_id @@ -236,48 +238,50 @@ - updated_at - part_of_university_degree :qualification_requests: - - id - assessment_id + - created_at + - failure_assessor_note + - id + - location_note + - passed - qualification_id - - state - received_at - - location_note - - created_at - - updated_at + - requested_at - reviewed_at - - passed - - failure_assessor_note + - state + - updated_at :reference_requests: - - id - - slug + - additional_information_response - assessment_id - - work_history_id - - state - - received_at - - contact_response - - contact_name - - contact_job + - children_comment + - children_response - contact_comment - - dates_response + - contact_job + - contact_name + - contact_response + - created_at - dates_comment - - hours_response + - dates_response + - failure_assessor_note - hours_comment - - children_response - - children_comment - - lessons_response + - hours_response + - id - lessons_comment - - reports_response - - reports_comment - - misconduct_response + - lessons_response - misconduct_comment - - satisfied_response - - satisfied_comment - - additional_information_response - - created_at - - updated_at + - misconduct_response - passed - - failure_assessor_note + - received_at + - reports_comment + - reports_response + - requested_at - reviewed_at + - satisfied_comment + - satisfied_response + - slug + - state + - updated_at + - work_history_id :regions: - application_form_skip_work_history - country_id diff --git a/db/migrate/20230926092809_add_requested_at_to_requestables.rb b/db/migrate/20230926092809_add_requested_at_to_requestables.rb new file mode 100644 index 0000000000..e3ccb58061 --- /dev/null +++ b/db/migrate/20230926092809_add_requested_at_to_requestables.rb @@ -0,0 +1,13 @@ +class AddRequestedAtToRequestables < ActiveRecord::Migration[7.0] + def change + add_column :further_information_requests, :requested_at, :datetime + add_column :professional_standing_requests, :requested_at, :datetime + add_column :qualification_requests, :requested_at, :datetime + add_column :reference_requests, :requested_at, :datetime + + FurtherInformationRequest.update_all("requested_at = created_at") + ProfessionalStandingRequest.update_all("requested_at = created_at") + QualificationRequest.update_all("requested_at = created_at") + ReferenceRequest.update_all("requested_at = created_at") + end +end diff --git a/db/schema.rb b/db/schema.rb index 1fe8ddfc16..a0c98f8a33 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_09_14_104107) do +ActiveRecord::Schema[7.0].define(version: 2023_09_26_092809) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -256,6 +256,7 @@ t.integer "working_days_since_received" t.integer "working_days_assessment_started_to_creation" t.datetime "reviewed_at" + t.datetime "requested_at" t.index ["assessment_id"], name: "index_further_information_requests_on_assessment_id" end @@ -280,6 +281,7 @@ t.boolean "passed" t.string "failure_assessor_note", default: "", null: false t.boolean "ready_for_review", default: false, null: false + t.datetime "requested_at" t.index ["assessment_id"], name: "index_professional_standing_requests_on_assessment_id" end @@ -294,6 +296,7 @@ t.datetime "reviewed_at", precision: nil t.boolean "passed" t.string "failure_assessor_note", default: "", null: false + t.datetime "requested_at" t.index ["assessment_id"], name: "index_qualification_requests_on_assessment_id" t.index ["qualification_id"], name: "index_qualification_requests_on_qualification_id" end @@ -342,6 +345,7 @@ t.boolean "satisfied_response" t.text "satisfied_comment", default: "", null: false t.string "failure_assessor_note", default: "", null: false + t.datetime "requested_at" t.index ["assessment_id"], name: "index_reference_requests_on_assessment_id" t.index ["slug"], name: "index_reference_requests_on_slug", unique: true t.index ["work_history_id"], name: "index_reference_requests_on_work_history_id" diff --git a/spec/factories/further_information_requests.rb b/spec/factories/further_information_requests.rb index 5f2981c2e4..7b45eac0b7 100644 --- a/spec/factories/further_information_requests.rb +++ b/spec/factories/further_information_requests.rb @@ -8,6 +8,7 @@ # failure_assessor_note :string default(""), not null # passed :boolean # received_at :datetime +# requested_at :datetime # reviewed_at :datetime # state :string not null # working_days_assessment_started_to_creation :integer diff --git a/spec/factories/professional_standing_requests.rb b/spec/factories/professional_standing_requests.rb index 3728da3e6f..42bf2f58bf 100644 --- a/spec/factories/professional_standing_requests.rb +++ b/spec/factories/professional_standing_requests.rb @@ -10,6 +10,7 @@ # passed :boolean # ready_for_review :boolean default(FALSE), not null # received_at :datetime +# requested_at :datetime # reviewed_at :datetime # state :string not null # created_at :datetime not null diff --git a/spec/factories/qualification_requests.rb b/spec/factories/qualification_requests.rb index 764c28cd85..1e6475774e 100644 --- a/spec/factories/qualification_requests.rb +++ b/spec/factories/qualification_requests.rb @@ -9,6 +9,7 @@ # location_note :text default(""), not null # passed :boolean # received_at :datetime +# requested_at :datetime # reviewed_at :datetime # state :string not null # created_at :datetime not null diff --git a/spec/factories/reference_requests.rb b/spec/factories/reference_requests.rb index 17dfe42da8..667f83f276 100644 --- a/spec/factories/reference_requests.rb +++ b/spec/factories/reference_requests.rb @@ -25,6 +25,7 @@ # received_at :datetime # reports_comment :text default(""), not null # reports_response :boolean +# requested_at :datetime # reviewed_at :datetime # satisfied_comment :text default(""), not null # satisfied_response :boolean diff --git a/spec/models/further_information_request_spec.rb b/spec/models/further_information_request_spec.rb index 66e9f5a534..0cafc0539e 100644 --- a/spec/models/further_information_request_spec.rb +++ b/spec/models/further_information_request_spec.rb @@ -8,6 +8,7 @@ # failure_assessor_note :string default(""), not null # passed :boolean # received_at :datetime +# requested_at :datetime # reviewed_at :datetime # state :string not null # working_days_assessment_started_to_creation :integer diff --git a/spec/models/professional_standing_request_spec.rb b/spec/models/professional_standing_request_spec.rb index 658ab055fe..1d2e5babbe 100644 --- a/spec/models/professional_standing_request_spec.rb +++ b/spec/models/professional_standing_request_spec.rb @@ -10,6 +10,7 @@ # passed :boolean # ready_for_review :boolean default(FALSE), not null # received_at :datetime +# requested_at :datetime # reviewed_at :datetime # state :string not null # created_at :datetime not null diff --git a/spec/models/qualification_request_spec.rb b/spec/models/qualification_request_spec.rb index 9e4bf5713e..5e4c65cf2c 100644 --- a/spec/models/qualification_request_spec.rb +++ b/spec/models/qualification_request_spec.rb @@ -9,6 +9,7 @@ # location_note :text default(""), not null # passed :boolean # received_at :datetime +# requested_at :datetime # reviewed_at :datetime # state :string not null # created_at :datetime not null diff --git a/spec/models/reference_request_spec.rb b/spec/models/reference_request_spec.rb index e52b3fb683..28933b6172 100644 --- a/spec/models/reference_request_spec.rb +++ b/spec/models/reference_request_spec.rb @@ -25,6 +25,7 @@ # received_at :datetime # reports_comment :text default(""), not null # reports_response :boolean +# requested_at :datetime # reviewed_at :datetime # satisfied_comment :text default(""), not null # satisfied_response :boolean From 66877b0b9151d95d0a60d3902351024e138e2a7c Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Tue, 26 Sep 2023 11:08:26 +0100 Subject: [PATCH 2/5] Set requested_at This updates the code to ensure the requested_at field is set in various places where it's needed. --- app/models/application_form.rb | 4 ++ app/models/concerns/expirable.rb | 4 +- app/models/concerns/requestable.rb | 7 +++- .../create_further_information_request.rb | 2 + app/services/submit_application_form.rb | 8 +++- app/services/verify_assessment.rb | 8 ++-- .../factories/further_information_requests.rb | 3 ++ .../professional_standing_requests.rb | 3 ++ spec/factories/qualification_requests.rb | 3 ++ spec/factories/reference_requests.rb | 3 ++ spec/jobs/expire_requestable_job_spec.rb | 42 +++++++++++-------- spec/mailers/teacher_mailer_spec.rb | 2 +- spec/services/send_reminder_email_spec.rb | 28 ++++++++++--- spec/support/shared_examples/requestable.rb | 16 +++++++ 14 files changed, 103 insertions(+), 30 deletions(-) diff --git a/app/models/application_form.rb b/app/models/application_form.rb index 34cd2f0d1b..c1564d0555 100644 --- a/app/models/application_form.rb +++ b/app/models/application_form.rb @@ -276,6 +276,10 @@ def expires_after draft? ? 6.months : nil end + def requested_at + created_at + end + private def build_documents diff --git a/app/models/concerns/expirable.rb b/app/models/concerns/expirable.rb index e7f881225a..dd3fa753e4 100644 --- a/app/models/concerns/expirable.rb +++ b/app/models/concerns/expirable.rb @@ -4,7 +4,9 @@ module Expirable extend ActiveSupport::Concern def expired_at - expires_after ? created_at + expires_after : nil + return nil if requested_at.nil? || expires_after.nil? + + requested_at + expires_after end def after_expired(user:) diff --git a/app/models/concerns/requestable.rb b/app/models/concerns/requestable.rb index 5066636385..1a28211eeb 100644 --- a/app/models/concerns/requestable.rb +++ b/app/models/concerns/requestable.rb @@ -14,13 +14,14 @@ module Requestable validates :state, presence: true, inclusion: { in: states.values } + validates :requested_at, presence: true, if: :requested? validates :received_at, presence: true, if: :received? validates :reviewed_at, presence: true, unless: -> { passed.nil? } scope :respondable, -> { not_received.merge(ApplicationForm.assessable) } define_method :requested! do - update!(state: "requested", received_at: nil) + update!(state: "requested", requested_at: Time.zone.now) end define_method :received! do @@ -53,6 +54,10 @@ def status passed ? "accepted" : "rejected" end + def after_requested(user:) + # implement logic after this requestable has been requested + end + def after_received(user:) # implement logic after this requestable has been received end diff --git a/app/services/create_further_information_request.rb b/app/services/create_further_information_request.rb index 1496c53300..ef14f3fdff 100644 --- a/app/services/create_further_information_request.rb +++ b/app/services/create_further_information_request.rb @@ -17,11 +17,13 @@ def call FurtherInformationRequestItemsFactory.call( assessment_sections: assessment.sections, ), + requested_at: Time.zone.now, ) ApplicationFormStatusUpdater.call(application_form:, user:) create_timeline_event(request) + request.after_requested(user:) request end diff --git a/app/services/submit_application_form.rb b/app/services/submit_application_form.rb index b3fb268cab..38f598502f 100644 --- a/app/services/submit_application_form.rb +++ b/app/services/submit_application_form.rb @@ -68,7 +68,11 @@ def call def create_professional_standing_request(assessment) return unless application_form.teaching_authority_provides_written_statement - requestable = ProfessionalStandingRequest.create!(assessment:) + requestable = + ProfessionalStandingRequest.create!( + assessment:, + requested_at: Time.zone.now, + ) TimelineEvent.create!( event_type: "requestable_requested", @@ -76,5 +80,7 @@ def create_professional_standing_request(assessment) creator: user, requestable:, ) + + requestable.after_requested(user:) end end diff --git a/app/services/verify_assessment.rb b/app/services/verify_assessment.rb index 40d26ce58a..9b6be33cb7 100644 --- a/app/services/verify_assessment.rb +++ b/app/services/verify_assessment.rb @@ -57,14 +57,14 @@ def create_professional_standing_request return if application_form.teaching_authority_provides_written_statement ProfessionalStandingRequest - .create!(assessment:) + .create!(assessment:, requested_at: Time.zone.now) .tap { |requestable| create_timeline_event(requestable) } end def create_qualification_requests qualifications.map do |qualification| QualificationRequest - .create!(assessment:, qualification:) + .create!(assessment:, qualification:, requested_at: Time.zone.now) .tap { |requestable| create_timeline_event(requestable) } end end @@ -72,7 +72,7 @@ def create_qualification_requests def create_reference_requests work_histories.map do |work_history| ReferenceRequest - .create!(assessment:, work_history:) + .create!(assessment:, work_history:, requested_at: Time.zone.now) .tap { |requestable| create_timeline_event(requestable) } end end @@ -84,6 +84,8 @@ def create_timeline_event(requestable) event_type: "requestable_requested", requestable:, ) + + requestable.after_requested(user:) end def send_reference_request_emails(reference_requests) diff --git a/spec/factories/further_information_requests.rb b/spec/factories/further_information_requests.rb index 7b45eac0b7..ec7d6d2f76 100644 --- a/spec/factories/further_information_requests.rb +++ b/spec/factories/further_information_requests.rb @@ -26,8 +26,11 @@ factory :further_information_request do association :assessment + requested + trait :requested do state { "requested" } + requested_at { Faker::Time.between(from: 1.month.ago, to: Time.zone.now) } end trait :received do diff --git a/spec/factories/professional_standing_requests.rb b/spec/factories/professional_standing_requests.rb index 42bf2f58bf..e00d74c071 100644 --- a/spec/factories/professional_standing_requests.rb +++ b/spec/factories/professional_standing_requests.rb @@ -29,8 +29,11 @@ factory :professional_standing_request do association :assessment + requested + trait :requested do state { "requested" } + requested_at { Faker::Time.between(from: 1.month.ago, to: Time.zone.now) } end trait :received do diff --git a/spec/factories/qualification_requests.rb b/spec/factories/qualification_requests.rb index 1e6475774e..02017275cc 100644 --- a/spec/factories/qualification_requests.rb +++ b/spec/factories/qualification_requests.rb @@ -32,8 +32,11 @@ association :assessment association :qualification, :completed + requested + trait :requested do state { "requested" } + requested_at { Faker::Time.between(from: 1.month.ago, to: Time.zone.now) } end trait :received do diff --git a/spec/factories/reference_requests.rb b/spec/factories/reference_requests.rb index 667f83f276..dea9577dfe 100644 --- a/spec/factories/reference_requests.rb +++ b/spec/factories/reference_requests.rb @@ -60,8 +60,11 @@ ) end + requested + trait :requested do state { "requested" } + requested_at { Faker::Time.between(from: 1.month.ago, to: Time.zone.now) } end trait :received do diff --git a/spec/jobs/expire_requestable_job_spec.rb b/spec/jobs/expire_requestable_job_spec.rb index 6e3c3d177a..1d683ee161 100644 --- a/spec/jobs/expire_requestable_job_spec.rb +++ b/spec/jobs/expire_requestable_job_spec.rb @@ -32,16 +32,16 @@ context "with requested FI request" do let(:requestable) do - create(:further_information_request, created_at:, assessment:) + create(:further_information_request, requested_at:, assessment:) end context "when less than six weeks old" do - let(:created_at) { (6.weeks - 1.hour).ago } + let(:requested_at) { (6.weeks - 1.hour).ago } it_behaves_like "not expired requestable" end context "when it is more than six weeks old" do - let(:created_at) { (6.weeks + 1.hour).ago } + let(:requested_at) { (6.weeks + 1.hour).ago } it_behaves_like "expired requestable" end @@ -56,12 +56,12 @@ let(:region) { create(:region, :in_country, country_code:) } context "when it is less than four weeks old" do - let(:created_at) { (4.weeks - 1.hour).ago } + let(:requested_at) { (4.weeks - 1.hour).ago } it_behaves_like "not expired requestable" end context "when it is more than four weeks old from #{country_code}" do - let(:created_at) { (4.weeks + 1.hour).ago } + let(:requested_at) { (4.weeks + 1.hour).ago } it_behaves_like "expired requestable" end end @@ -71,7 +71,11 @@ context "with any received FI request" do let(:requestable) do - create(:further_information_request, :received, created_at: 1.year.ago) + create( + :further_information_request, + :received, + requested_at: 1.year.ago, + ) end it_behaves_like "not expired requestable" @@ -79,7 +83,7 @@ context "with any expired FI request" do let(:requestable) do - create(:further_information_request, :expired, created_at: 1.year.ago) + create(:further_information_request, :expired, requested_at: 1.year.ago) end it_behaves_like "not expired requestable" @@ -93,17 +97,17 @@ end let(:requestable) do - create(:professional_standing_request, created_at:, assessment:) + create(:professional_standing_request, requested_at:, assessment:) end context "when less than 36 weeks old" do - let(:created_at) { (36.weeks - 1.hour).ago } + let(:requested_at) { (36.weeks - 1.hour).ago } it_behaves_like "not expired requestable" end context "when it is more than 36 weeks old" do - let(:created_at) { (36.weeks + 1.hour).ago } + let(:requested_at) { (36.weeks + 1.hour).ago } it_behaves_like "expired requestable" end @@ -114,7 +118,7 @@ create( :professional_standing_request, :received, - created_at: 1.year.ago, + requested_at: 1.year.ago, ) end @@ -123,23 +127,27 @@ context "with any expired professional standing request" do let(:requestable) do - create(:professional_standing_request, :expired, created_at: 1.year.ago) + create( + :professional_standing_request, + :expired, + requested_at: 1.year.ago, + ) end it_behaves_like "not expired requestable" end context "with a requested reference request" do - let(:requestable) { create(:reference_request, created_at:) } + let(:requestable) { create(:reference_request, requested_at:) } context "when less than six weeks old" do - let(:created_at) { (6.weeks - 1.hour).ago } + let(:requested_at) { (6.weeks - 1.hour).ago } it_behaves_like "not expired requestable" end context "when it is more than six weeks old" do - let(:created_at) { (6.weeks + 1.hour).ago } + let(:requested_at) { (6.weeks + 1.hour).ago } it_behaves_like "expired requestable" end @@ -147,7 +155,7 @@ context "with any received reference request" do let(:requestable) do - create(:reference_request, :received, created_at: 1.year.ago) + create(:reference_request, :received, requested_at: 1.year.ago) end it_behaves_like "not expired requestable" @@ -155,7 +163,7 @@ context "with any expired reference request" do let(:requestable) do - create(:reference_request, :expired, created_at: 1.year.ago) + create(:reference_request, :expired, requested_at: 1.year.ago) end it_behaves_like "not expired requestable" diff --git a/spec/mailers/teacher_mailer_spec.rb b/spec/mailers/teacher_mailer_spec.rb index 2ff5d7c1db..1b6ff6db02 100644 --- a/spec/mailers/teacher_mailer_spec.rb +++ b/spec/mailers/teacher_mailer_spec.rb @@ -302,7 +302,7 @@ end let(:further_information_request) do - create(:further_information_request, created_at: Date.new(2020, 1, 1)) + create(:further_information_request, requested_at: Date.new(2020, 1, 1)) end describe "#subject" do diff --git a/spec/services/send_reminder_email_spec.rb b/spec/services/send_reminder_email_spec.rb index fd84e4719b..532aeae457 100644 --- a/spec/services/send_reminder_email_spec.rb +++ b/spec/services/send_reminder_email_spec.rb @@ -191,7 +191,7 @@ let(:remindable) do create( :further_information_request, - created_at: further_information_requested_at, + requested_at: further_information_requested_at, assessment:, ) end @@ -230,12 +230,24 @@ end context "with a received FI request" do - let(:remindable) { create(:further_information_request, :received) } + let(:remindable) do + create( + :further_information_request, + :received, + requested_at: Time.zone.now, + ) + end include_examples "doesn't send an email" end context "with an expired FI request" do - let(:remindable) { create(:further_information_request, :expired) } + let(:remindable) do + create( + :further_information_request, + :expired, + requested_at: Time.zone.now, + ) + end include_examples "doesn't send an email" end @@ -250,7 +262,7 @@ let(:remindable) do create( :reference_request, - created_at: reference_requested_at, + requested_at: reference_requested_at, assessment:, ) end @@ -269,12 +281,16 @@ end context "with a received reference request" do - let(:remindable) { create(:reference_request, :received) } + let(:remindable) do + create(:reference_request, :received, requested_at: Time.zone.now) + end include_examples "doesn't send an email" end context "with an expired reference request" do - let(:remindable) { create(:reference_request, :expired) } + let(:remindable) do + create(:reference_request, :expired, requested_at: Time.zone.now) + end include_examples "doesn't send an email" end end diff --git a/spec/support/shared_examples/requestable.rb b/spec/support/shared_examples/requestable.rb index 5c9bade482..e3a8ef86c2 100644 --- a/spec/support/shared_examples/requestable.rb +++ b/spec/support/shared_examples/requestable.rb @@ -18,6 +18,12 @@ it { is_expected.to_not validate_presence_of(:received_at) } + context "when received" do + before { subject.state = "requested" } + + it { is_expected.to validate_presence_of(:requested_at) } + end + context "when received" do before { subject.state = "received" } @@ -31,6 +37,16 @@ end end + describe "#requested!" do + let(:call) { subject.requested! } + + it "sets the requested at date" do + freeze_time do + expect { call }.to change(subject, :requested_at).to(Time.zone.now) + end + end + end + describe "#received!" do let(:call) { subject.received! } From 9b8d496b35869d9ed5cb7f3a6bf2f93f25fc1da0 Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Tue, 26 Sep 2023 11:11:58 +0100 Subject: [PATCH 3/5] Rename expired_at This renames it to expires_at to better represent that it returns when the object _will_ expire, rather than when it actually expired. --- app/jobs/expire_requestable_job.rb | 4 ++-- app/models/concerns/expirable.rb | 2 +- app/models/concerns/requestable.rb | 2 +- app/services/send_reminder_email.rb | 4 ++-- .../preview_referee.html.erb | 2 +- app/views/referee_mailer/reference_reminder.text.erb | 2 +- app/views/referee_mailer/reference_requested.text.erb | 2 +- app/views/teacher_mailer/application_not_submitted.text.erb | 6 +++--- .../teacher_mailer/further_information_reminder.text.erb | 2 +- spec/support/shared_examples/expirable.rb | 6 +++--- 10 files changed, 16 insertions(+), 16 deletions(-) diff --git a/app/jobs/expire_requestable_job.rb b/app/jobs/expire_requestable_job.rb index b3f8634151..7651c9ca33 100644 --- a/app/jobs/expire_requestable_job.rb +++ b/app/jobs/expire_requestable_job.rb @@ -2,8 +2,8 @@ class ExpireRequestableJob < ApplicationJob def perform(requestable) - if requestable.requested? && requestable.expired_at.present? && - Time.zone.now > requestable.expired_at + if requestable.requested? && requestable.expires_at.present? && + Time.zone.now > requestable.expires_at ExpireRequestable.call(requestable:, user: "Expirer") end end diff --git a/app/models/concerns/expirable.rb b/app/models/concerns/expirable.rb index dd3fa753e4..92627eb65c 100644 --- a/app/models/concerns/expirable.rb +++ b/app/models/concerns/expirable.rb @@ -3,7 +3,7 @@ module Expirable extend ActiveSupport::Concern - def expired_at + def expires_at return nil if requested_at.nil? || expires_after.nil? requested_at + expires_after diff --git a/app/models/concerns/requestable.rb b/app/models/concerns/requestable.rb index 1a28211eeb..a97bb95c6c 100644 --- a/app/models/concerns/requestable.rb +++ b/app/models/concerns/requestable.rb @@ -40,7 +40,7 @@ def reviewed? end def overdue? - expired? || (received? && expired_at.present? && received_at > expired_at) + expired? || (received? && expires_at.present? && received_at > expires_at) end def failed diff --git a/app/services/send_reminder_email.rb b/app/services/send_reminder_email.rb index c2ceb09466..d6ed99779b 100644 --- a/app/services/send_reminder_email.rb +++ b/app/services/send_reminder_email.rb @@ -19,7 +19,7 @@ def call attr_reader :remindable def send_reminder? - return false unless remindable.expired_at + return false unless remindable.expires_at remindable.should_send_reminder_email?( days_until_expired, @@ -33,7 +33,7 @@ def number_of_reminders_sent def days_until_expired today = Time.zone.today - (remindable.expired_at.to_date - today).to_i + (remindable.expires_at.to_date - today).to_i end def send_email diff --git a/app/views/assessor_interface/assessment_recommendation_verify/preview_referee.html.erb b/app/views/assessor_interface/assessment_recommendation_verify/preview_referee.html.erb index a8d805a3cd..66009bc5a3 100644 --- a/app/views/assessor_interface/assessment_recommendation_verify/preview_referee.html.erb +++ b/app/views/assessor_interface/assessment_recommendation_verify/preview_referee.html.erb @@ -15,7 +15,7 @@ reference_request: OpenStruct.new( slug: "reference-request", work_history: @application_form.work_histories.first, - expired_at: Time.zone.now + 6.weeks, + expires_at: Time.zone.now + 6.weeks, assessment: @assessment, application_form: @application_form ), diff --git a/app/views/referee_mailer/reference_reminder.text.erb b/app/views/referee_mailer/reference_reminder.text.erb index f904f41ba5..235afe769a 100644 --- a/app/views/referee_mailer/reference_reminder.text.erb +++ b/app/views/referee_mailer/reference_reminder.text.erb @@ -4,7 +4,7 @@ We still need you to confirm some information about <%= application_form_full_na We recently contacted you to ask you to help us confirm that the information <%= application_form_full_name(@application_form) %> has provided about their work history and experience is accurate. -Please follow the link below and answer the questions by <%= @reference_request.expired_at.to_date.to_fs(:long_ordinal_uk) %>. If we do not receive your response by this date, it could affect the outcome for the applicant. +Please follow the link below and answer the questions by <%= @reference_request.expires_at.to_date.to_fs(:long_ordinal_uk) %>. If we do not receive your response by this date, it could affect the outcome for the applicant. <%= teacher_interface_reference_request_url(@reference_request.slug) %> diff --git a/app/views/referee_mailer/reference_requested.text.erb b/app/views/referee_mailer/reference_requested.text.erb index 212d489e6d..53bf469bf8 100644 --- a/app/views/referee_mailer/reference_requested.text.erb +++ b/app/views/referee_mailer/reference_requested.text.erb @@ -12,7 +12,7 @@ As part of the QTS application process, our assessors need to understand each ap # What we need you to do -Please follow the link below and answer the questions by <%= @reference_request.expired_at.to_date.to_fs(:long_ordinal_uk) %>. +Please follow the link below and answer the questions by <%= @reference_request.expires_at.to_date.to_fs(:long_ordinal_uk) %>. <%= teacher_interface_reference_request_url(@reference_request.slug) %> diff --git a/app/views/teacher_mailer/application_not_submitted.text.erb b/app/views/teacher_mailer/application_not_submitted.text.erb index c73be9a790..72df68acd3 100644 --- a/app/views/teacher_mailer/application_not_submitted.text.erb +++ b/app/views/teacher_mailer/application_not_submitted.text.erb @@ -4,11 +4,11 @@ Dear <%= application_form_full_name(@application_form) %> <% when 0 %> We’ve noticed that you have a draft application for qualified teacher status (QTS) that has not been submitted. -We need to let you know that if you do not complete and submit your application by <%= @application_form.expired_at.to_date.to_fs(:long_ordinal_uk) %> we’ll delete the application. +We need to let you know that if you do not complete and submit your application by <%= @application_form.expires_at.to_date.to_fs(:long_ordinal_uk) %> we’ll delete the application. <% when 1 %> We contacted you a week ago about your draft application for qualified teacher status (QTS). -If you do not complete and submit your application by <%= @application_form.expired_at.to_date.to_fs(:long_ordinal_uk) %> we’ll delete the application. +If you do not complete and submit your application by <%= @application_form.expires_at.to_date.to_fs(:long_ordinal_uk) %> we’ll delete the application. <% end %> # What happens if your application is deleted? @@ -19,7 +19,7 @@ Applications are deleted permanently, so if you still wanted to apply for QTS af # Your next steps -If you still want to apply for QTS you must do so before <%= @application_form.expired_at.to_date.to_fs(:long_ordinal_uk) %>. You can sign in to complete and submit your application using the link below: +If you still want to apply for QTS you must do so before <%= @application_form.expires_at.to_date.to_fs(:long_ordinal_uk) %>. You can sign in to complete and submit your application using the link below: <%= new_teacher_session_url %> diff --git a/app/views/teacher_mailer/further_information_reminder.text.erb b/app/views/teacher_mailer/further_information_reminder.text.erb index 699e27f989..44b74eda71 100644 --- a/app/views/teacher_mailer/further_information_reminder.text.erb +++ b/app/views/teacher_mailer/further_information_reminder.text.erb @@ -6,7 +6,7 @@ Dear <%= application_form_full_name(@application_form) %> # What happens next -You must respond to this request by <%= @further_information_request.expired_at.to_date.to_fs(:long_ordinal_uk) %> otherwise your QTS application will be declined. +You must respond to this request by <%= @further_information_request.expires_at.to_date.to_fs(:long_ordinal_uk) %> otherwise your QTS application will be declined. Once you respond with all of the information we’ve requested, the assessor will be able to continue reviewing your application. diff --git a/spec/support/shared_examples/expirable.rb b/spec/support/shared_examples/expirable.rb index 19bdc4ab3b..59b93e6206 100644 --- a/spec/support/shared_examples/expirable.rb +++ b/spec/support/shared_examples/expirable.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true RSpec.shared_examples "an expirable" do - describe "#expired_at" do - let(:expired_at) { subject.expired_at } + describe "#expires_at" do + let(:expires_at) { subject.expires_at } it "doesn't raise an error" do - expect { expired_at }.to_not raise_error + expect { expires_at }.to_not raise_error end end From 1ee6a9af1ef344f6f0e899e53bcecc02b4d7dc37 Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Tue, 26 Sep 2023 12:13:40 +0100 Subject: [PATCH 4/5] Add expired_at column This adds a new column to the requestables for tracking when a requestable was expired. This is because there can sometimes be a slight delay between when the requested is due to expire and when it actually has expired. --- app/models/further_information_request.rb | 1 + app/models/professional_standing_request.rb | 1 + app/models/qualification_request.rb | 1 + app/models/reference_request.rb | 1 + config/analytics.yml | 4 ++++ ...26101325_add_expired_at_to_requestables.rb | 24 +++++++++++++++++++ db/schema.rb | 6 ++++- .../factories/further_information_requests.rb | 1 + .../professional_standing_requests.rb | 1 + spec/factories/qualification_requests.rb | 1 + spec/factories/reference_requests.rb | 1 + .../further_information_request_spec.rb | 1 + .../professional_standing_request_spec.rb | 1 + spec/models/qualification_request_spec.rb | 1 + spec/models/reference_request_spec.rb | 1 + 15 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20230926101325_add_expired_at_to_requestables.rb diff --git a/app/models/further_information_request.rb b/app/models/further_information_request.rb index 56af555f8e..5314294dc4 100644 --- a/app/models/further_information_request.rb +++ b/app/models/further_information_request.rb @@ -3,6 +3,7 @@ # Table name: further_information_requests # # id :bigint not null, primary key +# expired_at :datetime # failure_assessor_note :string default(""), not null # passed :boolean # received_at :datetime diff --git a/app/models/professional_standing_request.rb b/app/models/professional_standing_request.rb index b7de1b703b..cb9cb36574 100644 --- a/app/models/professional_standing_request.rb +++ b/app/models/professional_standing_request.rb @@ -5,6 +5,7 @@ # Table name: professional_standing_requests # # id :bigint not null, primary key +# expired_at :datetime # failure_assessor_note :string default(""), not null # location_note :text default(""), not null # passed :boolean diff --git a/app/models/qualification_request.rb b/app/models/qualification_request.rb index e9706b85bf..93fa62f6f8 100644 --- a/app/models/qualification_request.rb +++ b/app/models/qualification_request.rb @@ -5,6 +5,7 @@ # Table name: qualification_requests # # id :bigint not null, primary key +# expired_at :datetime # failure_assessor_note :string default(""), not null # location_note :text default(""), not null # passed :boolean diff --git a/app/models/reference_request.rb b/app/models/reference_request.rb index 23e8198487..6360d57e6b 100644 --- a/app/models/reference_request.rb +++ b/app/models/reference_request.rb @@ -14,6 +14,7 @@ # contact_response :boolean # dates_comment :text default(""), not null # dates_response :boolean +# expired_at :datetime # failure_assessor_note :string default(""), not null # hours_comment :text default(""), not null # hours_response :boolean diff --git a/config/analytics.yml b/config/analytics.yml index 32d4c58dc7..ed537d58a6 100644 --- a/config/analytics.yml +++ b/config/analytics.yml @@ -181,6 +181,7 @@ :further_information_requests: - assessment_id - created_at + - expired_at - failure_assessor_note - id - passed @@ -215,6 +216,7 @@ :professional_standing_requests: - assessment_id - created_at + - expired_at - failure_assessor_note - id - location_note @@ -240,6 +242,7 @@ :qualification_requests: - assessment_id - created_at + - expired_at - failure_assessor_note - id - location_note @@ -262,6 +265,7 @@ - created_at - dates_comment - dates_response + - expired_at - failure_assessor_note - hours_comment - hours_response diff --git a/db/migrate/20230926101325_add_expired_at_to_requestables.rb b/db/migrate/20230926101325_add_expired_at_to_requestables.rb new file mode 100644 index 0000000000..feb7e75b57 --- /dev/null +++ b/db/migrate/20230926101325_add_expired_at_to_requestables.rb @@ -0,0 +1,24 @@ +class AddExpiredAtToRequestables < ActiveRecord::Migration[7.0] + def change + add_column :further_information_requests, :expired_at, :datetime + add_column :professional_standing_requests, :expired_at, :datetime + add_column :qualification_requests, :expired_at, :datetime + add_column :reference_requests, :expired_at, :datetime + + FurtherInformationRequest.expired.each do |requestable| + requestable.update!(expired_at: requestable.expires_at) + end + + ProfessionalStandingRequest.expired.each do |requestable| + requestable.update!(expired_at: requestable.expires_at) + end + + QualificationRequest.expired.each do |requestable| + requestable.update!(expired_at: requestable.expires_at) + end + + ReferenceRequest.expired.each do |requestable| + requestable.update!(expired_at: requestable.expires_at) + end + end +end diff --git a/db/schema.rb b/db/schema.rb index a0c98f8a33..be39721126 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_09_26_092809) do +ActiveRecord::Schema[7.0].define(version: 2023_09_26_101325) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -257,6 +257,7 @@ t.integer "working_days_assessment_started_to_creation" t.datetime "reviewed_at" t.datetime "requested_at" + t.datetime "expired_at" t.index ["assessment_id"], name: "index_further_information_requests_on_assessment_id" end @@ -282,6 +283,7 @@ t.string "failure_assessor_note", default: "", null: false t.boolean "ready_for_review", default: false, null: false t.datetime "requested_at" + t.datetime "expired_at" t.index ["assessment_id"], name: "index_professional_standing_requests_on_assessment_id" end @@ -297,6 +299,7 @@ t.boolean "passed" t.string "failure_assessor_note", default: "", null: false t.datetime "requested_at" + t.datetime "expired_at" t.index ["assessment_id"], name: "index_qualification_requests_on_assessment_id" t.index ["qualification_id"], name: "index_qualification_requests_on_qualification_id" end @@ -346,6 +349,7 @@ t.text "satisfied_comment", default: "", null: false t.string "failure_assessor_note", default: "", null: false t.datetime "requested_at" + t.datetime "expired_at" t.index ["assessment_id"], name: "index_reference_requests_on_assessment_id" t.index ["slug"], name: "index_reference_requests_on_slug", unique: true t.index ["work_history_id"], name: "index_reference_requests_on_work_history_id" diff --git a/spec/factories/further_information_requests.rb b/spec/factories/further_information_requests.rb index ec7d6d2f76..8c04ca21e9 100644 --- a/spec/factories/further_information_requests.rb +++ b/spec/factories/further_information_requests.rb @@ -5,6 +5,7 @@ # Table name: further_information_requests # # id :bigint not null, primary key +# expired_at :datetime # failure_assessor_note :string default(""), not null # passed :boolean # received_at :datetime diff --git a/spec/factories/professional_standing_requests.rb b/spec/factories/professional_standing_requests.rb index e00d74c071..5f23fbc89e 100644 --- a/spec/factories/professional_standing_requests.rb +++ b/spec/factories/professional_standing_requests.rb @@ -5,6 +5,7 @@ # Table name: professional_standing_requests # # id :bigint not null, primary key +# expired_at :datetime # failure_assessor_note :string default(""), not null # location_note :text default(""), not null # passed :boolean diff --git a/spec/factories/qualification_requests.rb b/spec/factories/qualification_requests.rb index 02017275cc..0883884e11 100644 --- a/spec/factories/qualification_requests.rb +++ b/spec/factories/qualification_requests.rb @@ -5,6 +5,7 @@ # Table name: qualification_requests # # id :bigint not null, primary key +# expired_at :datetime # failure_assessor_note :string default(""), not null # location_note :text default(""), not null # passed :boolean diff --git a/spec/factories/reference_requests.rb b/spec/factories/reference_requests.rb index dea9577dfe..7e6836a442 100644 --- a/spec/factories/reference_requests.rb +++ b/spec/factories/reference_requests.rb @@ -14,6 +14,7 @@ # contact_response :boolean # dates_comment :text default(""), not null # dates_response :boolean +# expired_at :datetime # failure_assessor_note :string default(""), not null # hours_comment :text default(""), not null # hours_response :boolean diff --git a/spec/models/further_information_request_spec.rb b/spec/models/further_information_request_spec.rb index 0cafc0539e..24d9e42aba 100644 --- a/spec/models/further_information_request_spec.rb +++ b/spec/models/further_information_request_spec.rb @@ -5,6 +5,7 @@ # Table name: further_information_requests # # id :bigint not null, primary key +# expired_at :datetime # failure_assessor_note :string default(""), not null # passed :boolean # received_at :datetime diff --git a/spec/models/professional_standing_request_spec.rb b/spec/models/professional_standing_request_spec.rb index 1d2e5babbe..e2555aaeef 100644 --- a/spec/models/professional_standing_request_spec.rb +++ b/spec/models/professional_standing_request_spec.rb @@ -5,6 +5,7 @@ # Table name: professional_standing_requests # # id :bigint not null, primary key +# expired_at :datetime # failure_assessor_note :string default(""), not null # location_note :text default(""), not null # passed :boolean diff --git a/spec/models/qualification_request_spec.rb b/spec/models/qualification_request_spec.rb index 5e4c65cf2c..63ab0fc5a6 100644 --- a/spec/models/qualification_request_spec.rb +++ b/spec/models/qualification_request_spec.rb @@ -5,6 +5,7 @@ # Table name: qualification_requests # # id :bigint not null, primary key +# expired_at :datetime # failure_assessor_note :string default(""), not null # location_note :text default(""), not null # passed :boolean diff --git a/spec/models/reference_request_spec.rb b/spec/models/reference_request_spec.rb index 28933b6172..aefa2db4a9 100644 --- a/spec/models/reference_request_spec.rb +++ b/spec/models/reference_request_spec.rb @@ -14,6 +14,7 @@ # contact_response :boolean # dates_comment :text default(""), not null # dates_response :boolean +# expired_at :datetime # failure_assessor_note :string default(""), not null # hours_comment :text default(""), not null # hours_response :boolean From 9741d16544dde0b715ae12a2187d68576f3a35f2 Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Tue, 26 Sep 2023 12:14:10 +0100 Subject: [PATCH 5/5] Use expired_at This updates the code to ensure the expired_at field is set in various places where it's needed. --- app/models/concerns/requestable.rb | 5 ++++ app/services/send_reminder_email.rb | 1 + .../factories/further_information_requests.rb | 1 + .../professional_standing_requests.rb | 5 ++++ spec/factories/qualification_requests.rb | 5 ++++ spec/factories/reference_requests.rb | 1 + spec/support/shared_examples/requestable.rb | 23 +++++++++++++++++++ 7 files changed, 41 insertions(+) diff --git a/app/models/concerns/requestable.rb b/app/models/concerns/requestable.rb index a97bb95c6c..e1618878f6 100644 --- a/app/models/concerns/requestable.rb +++ b/app/models/concerns/requestable.rb @@ -16,6 +16,7 @@ module Requestable 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) } @@ -28,6 +29,10 @@ module Requestable update!(state: "received", received_at: Time.zone.now) end + define_method :expired! do + update!(state: "expired", expired_at: Time.zone.now) + end + has_one :application_form, through: :assessment end diff --git a/app/services/send_reminder_email.rb b/app/services/send_reminder_email.rb index d6ed99779b..2878a84ee5 100644 --- a/app/services/send_reminder_email.rb +++ b/app/services/send_reminder_email.rb @@ -19,6 +19,7 @@ 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?( diff --git a/spec/factories/further_information_requests.rb b/spec/factories/further_information_requests.rb index 8c04ca21e9..4fe84ea880 100644 --- a/spec/factories/further_information_requests.rb +++ b/spec/factories/further_information_requests.rb @@ -41,6 +41,7 @@ trait :expired do state { "expired" } + expired_at { Faker::Time.between(from: 1.month.ago, to: Time.zone.now) } end trait :passed do diff --git a/spec/factories/professional_standing_requests.rb b/spec/factories/professional_standing_requests.rb index 5f23fbc89e..16acf1a0a0 100644 --- a/spec/factories/professional_standing_requests.rb +++ b/spec/factories/professional_standing_requests.rb @@ -43,6 +43,11 @@ receivable end + trait :expired do + state { "expired" } + expired_at { Faker::Time.between(from: 1.month.ago, to: Time.zone.now) } + end + trait :receivable do location_note { Faker::Lorem.sentence } end diff --git a/spec/factories/qualification_requests.rb b/spec/factories/qualification_requests.rb index 0883884e11..609a799af8 100644 --- a/spec/factories/qualification_requests.rb +++ b/spec/factories/qualification_requests.rb @@ -46,6 +46,11 @@ receivable end + trait :expired do + state { "expired" } + expired_at { Faker::Time.between(from: 1.month.ago, to: Time.zone.now) } + end + trait :receivable do location_note { Faker::Lorem.sentence } end diff --git a/spec/factories/reference_requests.rb b/spec/factories/reference_requests.rb index 7e6836a442..45577e3b2a 100644 --- a/spec/factories/reference_requests.rb +++ b/spec/factories/reference_requests.rb @@ -76,6 +76,7 @@ trait :expired do state { "expired" } + expired_at { Faker::Time.between(from: 1.month.ago, to: Time.zone.now) } end trait :passed do diff --git a/spec/support/shared_examples/requestable.rb b/spec/support/shared_examples/requestable.rb index e3a8ef86c2..cf3d6b19ce 100644 --- a/spec/support/shared_examples/requestable.rb +++ b/spec/support/shared_examples/requestable.rb @@ -17,6 +17,7 @@ it { is_expected.to validate_presence_of(:state) } it { is_expected.to_not validate_presence_of(:received_at) } + it { is_expected.to_not validate_presence_of(:expired_at) } context "when received" do before { subject.state = "requested" } @@ -30,6 +31,12 @@ it { is_expected.to validate_presence_of(:received_at) } end + context "when expired" do + before { subject.state = "expired" } + + it { is_expected.to validate_presence_of(:expired_at) } + end + context "when reviewed" do before { subject.passed = [true, false].sample } @@ -63,6 +70,22 @@ end end + describe "#expired!" do + let(:call) { subject.expired! } + + it "changes the state" do + expect { call }.to change(subject, :expired?).from(false).to(true) + end + + it "sets the received at date" do + freeze_time do + expect { call }.to change(subject, :expired_at).from(nil).to( + Time.zone.now, + ) + end + end + end + [true, false].each do |passed| describe "#reviewed!(#{passed})" do let(:call) { subject.reviewed!(passed) }