diff --git a/app/api/entities/minimal/minimal_unit_entity.rb b/app/api/entities/minimal/minimal_unit_entity.rb index 7e71b425b3..453c6c9e40 100644 --- a/app/api/entities/minimal/minimal_unit_entity.rb +++ b/app/api/entities/minimal/minimal_unit_entity.rb @@ -17,12 +17,14 @@ class MinimalUnitEntity < Grape::Entity with_options(format_with: :date_only) do expose :start_date expose :end_date + expose :last_teaching_date end expose :active expose :grade_values expose :grade_definitions expose :discuss_timeout_enabled, unless: :summary_only + expose :discuss_timeout_active?, as: :discuss_timeout_active, unless: :summary_only expose :discuss_timeout_expire_days, unless: :summary_only end end diff --git a/app/api/entities/teaching_period_entity.rb b/app/api/entities/teaching_period_entity.rb index b989a858f6..b105596bb7 100644 --- a/app/api/entities/teaching_period_entity.rb +++ b/app/api/entities/teaching_period_entity.rb @@ -5,6 +5,7 @@ class TeachingPeriodEntity < Grape::Entity expose :year expose :start_date expose :end_date + expose :last_teaching_date expose :active_until expose :active do |teaching_period, options| teaching_period.active? diff --git a/app/api/entities/unit_entity.rb b/app/api/entities/unit_entity.rb index 3c16ae474c..0eb96d96f6 100644 --- a/app/api/entities/unit_entity.rb +++ b/app/api/entities/unit_entity.rb @@ -33,6 +33,7 @@ def can_read_unit_config?(my_role) with_options(format_with: :date_only) do expose :start_date expose :end_date + expose :last_teaching_date expose :portfolio_auto_generation_date, unless: :summary_only, if: lambda { |unit, options| is_staff?(options[:my_role]) }, expose_nil: false end @@ -88,6 +89,7 @@ def can_read_unit_config?(my_role) expose :feedback_overflow_threshold_days, unless: :summary_only, if: lambda { |unit, options| is_staff?(options[:my_role]) } expose :discuss_timeout_enabled, unless: :summary_only + expose :discuss_timeout_active?, as: :discuss_timeout_active, unless: :summary_only expose :discuss_timeout_warning_days, unless: :summary_only, if: lambda { |unit, options| is_staff?(options[:my_role]) } expose :discuss_timeout_expire_days, unless: :summary_only diff --git a/app/api/teaching_periods_authenticated_api.rb b/app/api/teaching_periods_authenticated_api.rb index 4670cf9984..96ab5819e6 100644 --- a/app/api/teaching_periods_authenticated_api.rb +++ b/app/api/teaching_periods_authenticated_api.rb @@ -15,6 +15,7 @@ class TeachingPeriodsAuthenticatedApi < Grape::API requires :year, type: Integer, desc: 'The year of the teaching period' requires :start_date, type: Date, desc: 'The start date of the teaching period' requires :end_date, type: Date, desc: 'The end date of the teaching period' + optional :last_teaching_date, type: Date, desc: 'The final day of teaching, inclusive' requires :active_until, type: Date, desc: 'The teaching period will be active until this date' end end @@ -28,6 +29,7 @@ class TeachingPeriodsAuthenticatedApi < Grape::API :year, :start_date, :end_date, + :last_teaching_date, :active_until) result = TeachingPeriod.create!(teaching_period_parameters) @@ -47,6 +49,7 @@ class TeachingPeriodsAuthenticatedApi < Grape::API optional :year, type: Integer, desc: 'The year of the teaching period' optional :start_date, type: Date, desc: 'The start date of the teaching period' optional :end_date, type: Date, desc: 'The end date of the teaching period' + optional :last_teaching_date, type: Date, desc: 'The final day of teaching, inclusive' optional :active_until, type: Date, desc: 'The teaching period will be active until this date' end end @@ -61,6 +64,7 @@ class TeachingPeriodsAuthenticatedApi < Grape::API :year, :start_date, :end_date, + :last_teaching_date, :active_until) teaching_period.update!(teaching_period_parameters) diff --git a/app/api/units_api.rb b/app/api/units_api.rb index 108a144458..b9be1afdd9 100644 --- a/app/api/units_api.rb +++ b/app/api/units_api.rb @@ -14,7 +14,7 @@ class UnitsApi < Grape::API authenticated? if params[:unit] - for key in [:start_date, :end_date] do + for key in [:start_date, :end_date, :last_teaching_date] do if params[:unit][key].present? date_val = DateTime.parse(params[:unit][key]) params[:unit][key] = date_val @@ -77,6 +77,7 @@ class UnitsApi < Grape::API optional :teaching_period_id, type: Integer optional :start_date, type: Date optional :end_date, type: Date + optional :last_teaching_date, type: Date optional :main_convenor_id, type: Integer optional :auto_apply_extension_before_deadline, type: Boolean, desc: 'Indicates if extensions before the deadline should be automatically applied' optional :mark_late_submissions_as_assess_in_portfolio, type: Boolean, desc: 'Indicates if late submissions should be set Time Exceeded or Assess in Portfolio' @@ -106,6 +107,7 @@ class UnitsApi < Grape::API mutually_exclusive :teaching_period_id, :start_date mutually_exclusive :teaching_period_id, :end_date + mutually_exclusive :teaching_period_id, :last_teaching_date end end put '/units/:id' do @@ -120,6 +122,7 @@ class UnitsApi < Grape::API :description, :start_date, :end_date, + :last_teaching_date, :teaching_period_id, :active, :main_convenor_id, @@ -177,6 +180,7 @@ class UnitsApi < Grape::API optional :teaching_period_id, type: Integer optional :start_date, type: Date optional :end_date, type: Date + optional :last_teaching_date, type: Date optional :main_convenor_user_id, type: Integer optional :auto_apply_extension_before_deadline, type: Boolean, desc: 'Indicates if extensions before the deadline should be automatically applied', default: true optional :mark_late_submissions_as_assess_in_portfolio, type: Boolean, desc: 'Indicates if late submissions should be set to Time Exceeded or Assess in Portfolio', default: true @@ -203,6 +207,7 @@ class UnitsApi < Grape::API mutually_exclusive :teaching_period_id, :start_date mutually_exclusive :teaching_period_id, :end_date + mutually_exclusive :teaching_period_id, :last_teaching_date all_or_none_of :start_date, :end_date end end @@ -220,6 +225,7 @@ class UnitsApi < Grape::API :description, :start_date, :end_date, + :last_teaching_date, :auto_apply_extension_before_deadline, :mark_late_submissions_as_assess_in_portfolio, :send_notifications, @@ -288,10 +294,12 @@ class UnitsApi < Grape::API optional :teaching_period_id, type: Integer, desc: 'The teaching period to rollover to' optional :start_date, type: Date, desc: 'The start date of the new unit' optional :end_date, type: Date, desc: 'The end date of the new unit' + optional :last_teaching_date, type: Date, desc: 'The final day of teaching for a unit without a teaching period' optional :new_unit_code, type: String, desc: 'The unit code for the new unit' exactly_one_of :teaching_period_id, :start_date all_or_none_of :start_date, :end_date + mutually_exclusive :teaching_period_id, :last_teaching_date end post '/units/:id/rollover' do unit = Unit.find(params[:id]) @@ -306,7 +314,7 @@ class UnitsApi < Grape::API tp = TeachingPeriod.find(teaching_period_id) result = unit.rollover(tp, nil, nil, params[:new_unit_code]) else - result = unit.rollover(nil, params[:start_date], params[:end_date], params[:new_unit_code]) + result = unit.rollover(nil, params[:start_date], params[:end_date], params[:new_unit_code], params[:last_teaching_date]) end my_role = result.role_for(current_user) diff --git a/app/models/teaching_period.rb b/app/models/teaching_period.rb index 17e2c00d97..3cf78e94b5 100644 --- a/app/models/teaching_period.rb +++ b/app/models/teaching_period.rb @@ -13,9 +13,13 @@ class TeachingPeriod < ApplicationRecord inclusion: { in: 2000..2999, message: "%{value} is not a valid year" } validates :start_date, presence: true validates :end_date, presence: true + validates :last_teaching_date, presence: true validates :active_until, presence: true - validate :validate_end_date_after_start_date, :validate_active_until_after_end_date + before_validation :default_last_teaching_date, on: :create + before_validation :sync_last_teaching_date_with_end_date, on: :update + + validate :validate_end_date_after_start_date, :validate_last_teaching_date, :validate_active_until_after_end_date after_update :propogate_date_changes after_update :refresh_communication_schedule_caches, if: :saved_change_to_teaching_dates? @@ -190,11 +194,29 @@ def validate_end_date_after_start_date end end + def default_last_teaching_date + self.last_teaching_date ||= end_date&.to_date + end + + def sync_last_teaching_date_with_end_date + return unless will_save_change_to_end_date? && !will_save_change_to_last_teaching_date? + return unless last_teaching_date == end_date_change_to_be_saved.first&.to_date + + self.last_teaching_date = end_date&.to_date + end + + def validate_last_teaching_date + return if start_date.blank? || end_date.blank? || last_teaching_date.blank? + return if last_teaching_date.between?(start_date.to_date, end_date.to_date) + + errors.add(:last_teaching_date, 'must be between the start and end dates') + end + def propogate_date_changes - return unless saved_change_to_start_date? || saved_change_to_end_date? + return unless saved_change_to_start_date? || saved_change_to_end_date? || saved_change_to_last_teaching_date? units.each do |u| - u.update(start_date: self.start_date, end_date: self.end_date) + u.update!(start_date: start_date, end_date: end_date, last_teaching_date: last_teaching_date) end end diff --git a/app/models/unit.rb b/app/models/unit.rb index be15913ca2..4e44a6632b 100644 --- a/app/models/unit.rb +++ b/app/models/unit.rb @@ -211,7 +211,7 @@ def role_for(user) belongs_to :overseer_image, optional: true - validates :name, :description, :start_date, :end_date, presence: true + validates :name, :description, :start_date, :end_date, :last_teaching_date, presence: true validates :name, allowed_characters: { type: :unit_name } validates :code, allowed_characters: { type: :unit_code } @@ -237,7 +237,11 @@ def role_for(user) validate :warning_not_greater_than_overflow validate :discuss_timeout_warning_before_expiry + before_validation :default_last_teaching_date, on: :create + before_validation :sync_last_teaching_date_with_end_date, on: :update + validate :validate_end_date_after_start_date + validate :validate_last_teaching_date validate :ensure_teaching_period_dates_match, if: :has_teaching_period? validate :ensure_main_convenor_is_appropriate, if: :main_convenor_id_changed? @@ -259,6 +263,9 @@ def role_for(user) scope :within_teaching_dates, lambda { |now = Time.zone.now| set_active.where('units.end_date IS NULL OR units.end_date >= ?', now.to_date) } + scope :within_last_teaching_date, lambda { |now = Time.zone.now| + set_active.where('units.last_teaching_date >= ?', now.to_date) + } include UnitTiiModule @@ -287,13 +294,20 @@ def within_teaching_dates?(now = Time.zone.now) active? && (end_date.blank? || now.to_date <= end_date) end + def within_last_teaching_date?(now = Time.zone.now) + active? && now.to_date <= last_teaching_date + end + + def discuss_timeout_active? + discuss_timeout_enabled && within_last_teaching_date? + end + def self.notify_discuss_timeouts! - within_teaching_dates.find_each(&:notify_discuss_timeouts!) + within_last_teaching_date.find_each(&:notify_discuss_timeouts!) end def notify_discuss_timeouts! - return 0 unless discuss_timeout_enabled - return 0 unless within_teaching_dates? + return 0 unless discuss_timeout_active? discuss_timeout_tasks.find_each.sum do |task| notify_discuss_timeout_for(task) @@ -310,12 +324,15 @@ def discuss_timeout_tasks def notify_discuss_timeout_for(task, teaching_breaks: nil, now_time: Time.zone.now) return 0 if task.moved_to_discuss_at.blank? + return 0 if now_time.to_date > last_teaching_date + + expiry_at = task.discuss_timeout_expiry_at(discuss_timeout_expire_days, teaching_breaks: teaching_breaks) + return 0 if expiry_at.to_date >= last_teaching_date actor = task.project.tutor_for(task.task_definition) || main_convenor&.user return 0 if actor.blank? elapsed_days = task.discuss_timeout_elapsed_days(now_time, teaching_breaks: teaching_breaks) - expiry_at = task.discuss_timeout_expiry_at(discuss_timeout_expire_days, teaching_breaks: teaching_breaks) if now_time.in_time_zone(expiry_at.time_zone).to_date > expiry_at.to_date expire_discuss_timeout_task(task, actor) elsif elapsed_days >= discuss_timeout_warning_days @@ -456,6 +473,7 @@ def teaching_period=(tp) if tp.present? self[:start_date] = tp.start_date self[:end_date] = tp.end_date + self[:last_teaching_date] = tp.last_teaching_date self[:teaching_period_id] = tp.id end super(tp) @@ -524,6 +542,9 @@ def ensure_teaching_period_dates_match if self[:end_date] != teaching_period.end_date errors.add(:end_date, "should match teaching period date") end + if last_teaching_date != teaching_period.last_teaching_date + errors.add(:last_teaching_date, "should match teaching period date") + end end def ensure_main_convenor_is_appropriate @@ -624,13 +645,31 @@ def validate_end_date_after_start_date end end + def default_last_teaching_date + self.last_teaching_date ||= teaching_period&.last_teaching_date || end_date&.to_date + end + + def sync_last_teaching_date_with_end_date + return unless will_save_change_to_end_date? && !will_save_change_to_last_teaching_date? + return unless last_teaching_date == end_date_change_to_be_saved.first&.to_date + + self.last_teaching_date = end_date&.to_date + end + + def validate_last_teaching_date + return if start_date.blank? || end_date.blank? || last_teaching_date.blank? + return if last_teaching_date.between?(start_date.to_date, end_date.to_date) + + errors.add(:last_teaching_date, 'must be between the start and end dates') + end + def autogen_date_within_unit_active_period if [start_date, end_date, portfolio_auto_generation_date].all?(&:present?) && !(start_date < portfolio_auto_generation_date && portfolio_auto_generation_date <= end_date) errors.add(:portfolio_auto_generation_date, "should be after unit start date and before unit end date") end end - def rollover(teaching_period, start_date, end_date, new_code) + def rollover(teaching_period, start_date, end_date, new_code, last_teaching_date = nil) new_unit = self.dup new_unit.code = new_code if new_code.present? @@ -641,6 +680,7 @@ def rollover(teaching_period, start_date, end_date, new_code) new_unit.teaching_period = nil new_unit.start_date = start_date new_unit.end_date = end_date + new_unit.last_teaching_date = last_teaching_date || end_date&.to_date end # Clear archived @@ -3853,6 +3893,7 @@ def upload_batch_feedback_zip(user, task_definition, file, progress_callback: ni def send_weekly_status_emails(summary_stats) return unless send_notifications + return unless within_last_teaching_date? summary_stats[:unit] = self summary_stats[:tutorials] = {} diff --git a/app/sidekiq/send_discuss_timeout_email_job.rb b/app/sidekiq/send_discuss_timeout_email_job.rb index 76349e7835..96083642db 100644 --- a/app/sidekiq/send_discuss_timeout_email_job.rb +++ b/app/sidekiq/send_discuss_timeout_email_job.rb @@ -7,6 +7,7 @@ def perform(task_id, sender_id, notification_type, expiry_date = nil) task = Task.find_by(id: task_id) sender = User.find_by(id: sender_id) return if task.blank? || sender.blank? + return unless task.unit.discuss_timeout_active? return unless task.unit.send_notifications return unless task.project.student.receive_feedback_notifications diff --git a/db/migrate/20261023034838_add_last_teaching_date_to_teaching_periods_and_units.rb b/db/migrate/20261023034838_add_last_teaching_date_to_teaching_periods_and_units.rb new file mode 100644 index 0000000000..ba592251bb --- /dev/null +++ b/db/migrate/20261023034838_add_last_teaching_date_to_teaching_periods_and_units.rb @@ -0,0 +1,17 @@ +class AddLastTeachingDateToTeachingPeriodsAndUnits < ActiveRecord::Migration[8.0] + def up + add_column :teaching_periods, :last_teaching_date, :date + add_column :units, :last_teaching_date, :date + + execute 'UPDATE teaching_periods SET last_teaching_date = DATE(end_date)' + execute 'UPDATE units SET last_teaching_date = DATE(end_date)' + + change_column_null :teaching_periods, :last_teaching_date, false + change_column_null :units, :last_teaching_date, false + end + + def down + remove_column :units, :last_teaching_date + remove_column :teaching_periods, :last_teaching_date + end +end diff --git a/db/schema.rb b/db/schema.rb index f1ed428448..2aae972b40 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[8.0].define(version: 2026_10_15_040755) do +ActiveRecord::Schema[8.0].define(version: 2026_10_23_034838) do create_table "activity_types", charset: "utf8mb4", collation: "utf8mb4_general_ci", force: :cascade do |t| t.string "name", null: false t.string "abbreviation", null: false @@ -802,6 +802,7 @@ t.datetime "active_until", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.date "last_teaching_date", null: false t.index ["period", "year"], name: "index_teaching_periods_on_period_and_year", unique: true end @@ -1017,6 +1018,7 @@ t.boolean "discuss_timeout_enabled", default: false, null: false t.integer "discuss_timeout_warning_days", default: 7, null: false t.integer "discuss_timeout_expire_days", default: 14, null: false + t.date "last_teaching_date", null: false t.index ["draft_task_definition_id"], name: "index_units_on_draft_task_definition_id" t.index ["main_convenor_id"], name: "index_units_on_main_convenor_id" t.index ["overseer_image_id"], name: "index_units_on_overseer_image_id" diff --git a/test/api/teaching_period_api_test.rb b/test/api/teaching_period_api_test.rb index 46fcf1ddc4..386b8a6008 100644 --- a/test/api/teaching_period_api_test.rb +++ b/test/api/teaching_period_api_test.rb @@ -129,8 +129,10 @@ def test_put_teaching_period tp = FactoryBot.create(:teaching_period) # data to replace + replacement = FactoryBot.build(:teaching_period) + replacement.last_teaching_date = replacement.end_date.to_date data_to_put = { - teaching_period: FactoryBot.build(:teaching_period) + teaching_period: replacement } # Add username and auth_token to Header @@ -140,18 +142,20 @@ def test_put_teaching_period put_json "/api/teaching_periods/#{tp.id}", data_to_put # check if the POST get through - assert_equal 200, last_response.status + assert_equal 200, last_response.status, last_response_body # check if the details posted match as expected - response_keys = %w(period year start_date end_date active_until) + response_keys = %w[period year start_date end_date active_until] tp_updated = tp.reload assert_json_matches_model(tp_updated, last_response_body, response_keys) + assert_equal tp_updated.last_teaching_date.iso8601, last_response_body['last_teaching_date'] # check if the details in the replaced teaching period match as data set to replace assert_equal data_to_put[:teaching_period]['period'], tp_updated.period assert_equal data_to_put[:teaching_period]['active_until'].to_date, tp_updated.active_until.to_date assert_equal data_to_put[:teaching_period]['start_date'].to_date, tp_updated.start_date.to_date assert_equal data_to_put[:teaching_period]['end_date'].to_date, tp_updated.end_date.to_date + assert_equal data_to_put[:teaching_period]['last_teaching_date'], tp_updated.last_teaching_date end # Put teaching period using unauthorised account diff --git a/test/mailers/unit_mail_test.rb b/test/mailers/unit_mail_test.rb index 6bd08f862d..cae4fc4f91 100644 --- a/test/mailers/unit_mail_test.rb +++ b/test/mailers/unit_mail_test.rb @@ -132,6 +132,19 @@ def test_discuss_timeout_notifications_send_emails assert_includes ActionMailer::Base.deliveries.last.subject, 'Discussion deadline missed' end + def test_queued_discussion_email_is_not_sent_after_last_teaching_date + unit = FactoryBot.create(:unit, last_teaching_date: Time.zone.today) + project = unit.active_projects.first + task = project.task_for_task_definition(unit.task_definitions.first) + sender = unit.main_convenor_user + + travel_to (Time.zone.now + 1.day).beginning_of_day do + assert_emails 0 do + SendDiscussTimeoutEmailJob.new.perform(task.id, sender.id, 'approaching', Time.zone.today.iso8601) + end + end + end + def test_discuss_timeout_notifications_are_not_queued_for_unenrolled_students unit = FactoryBot.create(:unit) project = unit.active_projects.first diff --git a/test/models/discuss_timeout_test.rb b/test/models/discuss_timeout_test.rb index 93302b0c51..dfc60231ba 100644 --- a/test/models/discuss_timeout_test.rb +++ b/test/models/discuss_timeout_test.rb @@ -1,6 +1,66 @@ require 'test_helper' class DiscussTimeoutTest < ActiveSupport::TestCase + def test_discussion_timeout_stops_after_last_teaching_date + travel_to Time.zone.parse('2026-09-21 08:00:00') do + unit = FactoryBot.create( + :unit, + start_date: Time.zone.parse('2026-06-01 00:00:00'), + end_date: Time.zone.parse('2026-09-30 00:00:00'), + last_teaching_date: Date.new(2026, 9, 20), + discuss_timeout_enabled: true, + send_notifications: false + ) + task = unit.active_projects.first.task_for_task_definition(unit.task_definitions.first) + task.update!(task_status: TaskStatus.discuss, moved_to_discuss_at: Time.zone.parse('2026-09-01 12:00:00')) + + assert unit.within_teaching_dates? + assert_not unit.within_last_teaching_date? + assert_not_includes Unit.within_last_teaching_date, unit + assert_equal 0, unit.notify_discuss_timeouts! + assert_equal TaskStatus.discuss, task.reload.task_status + end + end + + def test_custom_unit_defaults_last_teaching_date_to_end_date + unit = FactoryBot.create(:unit, with_students: false) + + assert_equal unit.end_date.to_date, unit.last_teaching_date + end + + def test_deadlines_on_or_after_last_teaching_date_do_not_warn_or_expire + travel_to Time.zone.parse('2026-09-19 08:00:00') do + unit = FactoryBot.create( + :unit, + start_date: Time.zone.parse('2026-06-01 00:00:00'), + end_date: Time.zone.parse('2026-09-30 00:00:00'), + last_teaching_date: Date.new(2026, 9, 20), + discuss_timeout_enabled: true, + send_notifications: false + ) + project = unit.active_projects.first + tasks = unit.task_definitions.first(2).map { |definition| project.task_for_task_definition(definition) } + + tasks.zip([6, 7]).each do |task, day| + task.update!(task_status: TaskStatus.discuss) + task.update!(moved_to_discuss_at: Time.zone.local(2026, 9, day, 8, 0, 0)) + end + + assert_equal [Date.new(2026, 9, 20), Date.new(2026, 9, 21)], tasks.map(&:discuss_timeout_expiry_date) + assert_equal 0, unit.notify_discuss_timeouts! + assert(tasks.all? { |task| task.reload.notified_discuss_warning_at.nil? }) + assert(tasks.all? { |task| task.comments.where(content_type: DiscussTimeoutComment.warning).none? }) + + travel_to Time.zone.parse('2026-09-20 08:00:00') + assert_equal 0, unit.notify_discuss_timeouts! + + travel_to Time.zone.parse('2026-09-22 08:00:00') + assert_equal 0, unit.notify_discuss_timeout_for(tasks.first) + assert(tasks.all? { |task| task.reload.task_status == TaskStatus.discuss }) + assert(tasks.all? { |task| task.notified_discuss_expiry_at.nil? }) + end + end + def test_discussion_deadline_uses_breaks_for_the_students_campus teaching_period = FactoryBot.create( :teaching_period, @@ -79,15 +139,15 @@ def test_discussion_remains_open_through_the_entire_expiry_date end assert_equal 2, unit.notify_discuss_timeouts! - assert tasks.all? { |task| task.reload.task_status == TaskStatus.discuss } + assert(tasks.all? { |task| task.reload.task_status == TaskStatus.discuss }) travel_to Time.zone.parse('2026-10-22 23:59:59') assert_equal 0, unit.notify_discuss_timeouts! - assert tasks.all? { |task| task.reload.task_status == TaskStatus.discuss } + assert(tasks.all? { |task| task.reload.task_status == TaskStatus.discuss }) travel_to Time.zone.parse('2026-10-23 08:00:00') assert_equal 2, unit.notify_discuss_timeouts! - assert tasks.all? { |task| task.reload.task_status == TaskStatus.fix_and_resubmit } + assert(tasks.all? { |task| task.reload.task_status == TaskStatus.fix_and_resubmit }) end end diff --git a/test/models/teaching_period_test.rb b/test/models/teaching_period_test.rb index 66c1d0de9f..135aa7463b 100644 --- a/test/models/teaching_period_test.rb +++ b/test/models/teaching_period_test.rb @@ -2,6 +2,47 @@ class TeachingPeriodTest < ActiveSupport::TestCase + def test_last_teaching_date_defaults_to_end_date_and_propagates_to_units + teaching_period = FactoryBot.create(:teaching_period) + unit = FactoryBot.create(:unit, teaching_period: teaching_period, with_students: false) + + assert_equal teaching_period.end_date.to_date, teaching_period.last_teaching_date + assert_equal teaching_period.last_teaching_date, unit.last_teaching_date + + last_teaching_date = teaching_period.end_date.to_date - 1.week + teaching_period.update!(last_teaching_date: last_teaching_date) + + assert_equal last_teaching_date, unit.reload.last_teaching_date + assert_equal teaching_period.end_date, unit.end_date + end + + def test_last_teaching_date_must_be_within_the_teaching_period + teaching_period = FactoryBot.build(:teaching_period) + teaching_period.last_teaching_date = teaching_period.end_date.to_date + 1.day + + assert_not teaching_period.valid? + assert_includes teaching_period.errors.attribute_names, :last_teaching_date + end + + def test_default_last_teaching_date_follows_end_date_changes + teaching_period = FactoryBot.create(:teaching_period) + unit = FactoryBot.create(:unit, teaching_period: teaching_period, with_students: false) + original_end_date = teaching_period.end_date + earlier_end_date = original_end_date - 1.week + + teaching_period.update!(end_date: earlier_end_date) + + assert_equal earlier_end_date.to_date, teaching_period.last_teaching_date + assert_equal earlier_end_date.to_date, unit.reload.last_teaching_date + + custom_last_teaching_date = earlier_end_date.to_date - 1.week + teaching_period.update!(last_teaching_date: custom_last_teaching_date) + teaching_period.update!(end_date: original_end_date) + + assert_equal custom_last_teaching_date, teaching_period.last_teaching_date + assert_equal custom_last_teaching_date, unit.reload.last_teaching_date + end + # Ensure that at the start there are 3 teaching periods def test_check_periods_are_created assert_equal 3, TeachingPeriod.count, 'There are 3 teaching periods initially' diff --git a/test/models/unit_model_test.rb b/test/models/unit_model_test.rb index 8dd5613a09..e074c10796 100644 --- a/test/models/unit_model_test.rb +++ b/test/models/unit_model_test.rb @@ -17,6 +17,37 @@ class UnitModelTest < ActiveSupport::TestCase @unit.destroy end + def test_weekly_status_emails_stop_after_last_teaching_date + last_teaching_date = Time.zone.today + unit = FactoryBot.create( + :unit, + with_students: false, + end_date: Time.zone.now + 1.week, + last_teaching_date: last_teaching_date + ) + + travel_to Time.zone.parse("#{last_teaching_date + 1.day} 08:00:00") do + summary_stats = {} + unit.send_weekly_status_emails(summary_stats) + + assert_empty summary_stats + end + end + + def test_default_last_teaching_date_follows_end_date_changes + unit = FactoryBot.create(:unit, with_students: false, task_count: 0) + original_end_date = unit.end_date + earlier_end_date = original_end_date - 1.week + + unit.update!(end_date: earlier_end_date) + assert_equal earlier_end_date.to_date, unit.last_teaching_date + + custom_last_teaching_date = earlier_end_date.to_date - 1.week + unit.update!(last_teaching_date: custom_last_teaching_date) + unit.update!(end_date: original_end_date) + assert_equal custom_last_teaching_date, unit.last_teaching_date + end + def test_sync_unit import_settings = { replace_existing_campus: false, @@ -1156,6 +1187,7 @@ def test_change_unit_code_moves_files assert_equal @unit.code, unit2.code assert_in_delta start_date, unit2.start_date, 1.hour assert_in_delta end_date, unit2.end_date, 1.hour + assert_equal end_date.to_date, unit2.last_teaching_date unit2.destroy end @@ -1185,6 +1217,7 @@ def test_change_unit_code_moves_files assert_not_equal @unit.code, unit2.code assert_equal 'NEWCODE-1', unit2.code assert_equal tp, unit2.teaching_period + assert_equal tp.last_teaching_date, unit2.last_teaching_date unit2.task_definitions.each do |td| assert File.exist?(td.task_sheet), 'task sheet is absent'