From cae19ab67922837373b9cae7a2a2c610b53bf99f Mon Sep 17 00:00:00 2001 From: Daniel Rafailov Date: Mon, 11 May 2026 23:21:25 -0400 Subject: [PATCH 1/8] added full test coverage for the MarksGraderController#randomly_assign method --- .../marks_graders_controller_spec.rb | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/spec/controllers/marks_graders_controller_spec.rb b/spec/controllers/marks_graders_controller_spec.rb index 2a590ec2b5..3837b9ac82 100644 --- a/spec/controllers/marks_graders_controller_spec.rb +++ b/spec/controllers/marks_graders_controller_spec.rb @@ -376,4 +376,87 @@ end end end + + describe '#randomly_assign' do + before do + allow(GradeEntryStudent).to receive(:randomly_assign_tas) + end + + context 'when students and graders are selected' do + it 'calls GradeEntryStudent `randomly_assign_tas` and returns a success response' do + student = create(:student) + grade_entry_student = grade_entry_form.grade_entry_students.find_or_create_by(role: student) + grade_entry_student_ta = create(:grade_entry_student_ta, grade_entry_student: grade_entry_student) + + expect(GradeEntryStudent).to receive(:randomly_assign_tas).with( + [grade_entry_student.id.to_s], + [grade_entry_student_ta.id.to_s], + grade_entry_form + ) + + post_as instructor, :randomly_assign, params: { + course_id: course.id, + grade_entry_form_id: grade_entry_form.id, + students: [grade_entry_student.id], + graders: [grade_entry_student_ta.id] + } + + expect(response).to have_http_status(:ok) + end + end + + context 'when students are not selected' do + it 'returns bad request and sets a flash error' do + post_as instructor, :randomly_assign, params: { + course_id: course.id, + grade_entry_form_id: grade_entry_form.id, + students: [], + graders: [1] + } + + expect(response).to have_http_status(:bad_request) + expect(flash[:error]).to have_message(I18n.t('groups.select_a_student')) + end + end + + context 'when graders are not selected' do + it 'returns bad request and sets flash error' do + post_as instructor, :randomly_assign, params: { + course_id: course.id, + grade_entry_form_id: grade_entry_form.id, + students: [1], + graders: [] + } + + expect(response).to have_http_status(:bad_request) + expect(flash[:error]).to have_message(I18n.t('graders.select_a_grader')) + end + end + + context 'when students parameter is missing' do + it 'returns bad request and sets flash error' do + post_as instructor, :randomly_assign, params: { + course_id: course.id, + grade_entry_form_id: grade_entry_form.id, + graders: [1] + } + + expect(response).to have_http_status(:bad_request) + expect(flash[:error]).to have_message(I18n.t('groups.select_a_student')) + end + end + + context 'when graders parameter is missing' do + it 'returns bad request and sets flash error' do + post_as instructor, :randomly_assign, params: { + course_id: course.id, + grade_entry_form_id: grade_entry_form.id, + students: [1] + } + + expect(response).to have_http_status(:bad_request) + expect(flash[:error]).to have_message(I18n.t('graders.select_a_grader')) + end + end + end end From 68e16a0b0d42acc21780dac0e1046bc5464fa1a1 Mon Sep 17 00:00:00 2001 From: Daniel Rafailov Date: Mon, 11 May 2026 23:44:44 -0400 Subject: [PATCH 2/8] added my name to the contributors file --- doc/markus-contributors.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/markus-contributors.txt b/doc/markus-contributors.txt index 40bce9dcac..cae371ff80 100644 --- a/doc/markus-contributors.txt +++ b/doc/markus-contributors.txt @@ -57,6 +57,7 @@ Clément Delafargue Clément Schiano Danesh Dadachanji Daniel Dervishi +Daniel Rafailov Daniel St. Jules Daniyal Liaqat Daryn Lam From 4b10fc9b9136af7ef5b60f02682a8e9fd03754d8 Mon Sep 17 00:00:00 2001 From: Daniel Rafailov Date: Tue, 12 May 2026 12:13:02 -0400 Subject: [PATCH 3/8] Added relevant changes to the changelog --- Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Changelog.md b/Changelog.md index eb59a11fc5..b92eb2d09e 100644 --- a/Changelog.md +++ b/Changelog.md @@ -7,6 +7,7 @@ ### 🚨 Breaking changes ### ✨ New features and improvements +- Added tests for `MarksGradersController` to achieve full test coverage for `randomly_assign` (#7947) - Improve admin user list loading time by replacing ActiveRecord instantiation with direct column extraction (#7897) - Provide suggestions for partial student matching scans (#7760) - Allow inactive students to join groups (#7757) From 349a73d56cb3878821ffbbbf74e06856206b7443 Mon Sep 17 00:00:00 2001 From: Naragod Date: Thu, 14 May 2026 16:46:38 -0400 Subject: [PATCH 4/8] Included due date in LTI assessment sync (#7872) --- Changelog.md | 1 + app/helpers/lti_helper.rb | 4 ++- app/models/assessment.rb | 16 +++++++++ spec/helpers/lti_helper_spec.rb | 63 +++++++++++++++++++++++++++++++++ spec/models/assessment_spec.rb | 31 ++++++++++++++++ 5 files changed, 114 insertions(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index b92eb2d09e..85d291daba 100644 --- a/Changelog.md +++ b/Changelog.md @@ -17,6 +17,7 @@ - Add JS autotester example (#7866) - Return structured JSON from grade entry forms API show endpoint with optional student filter and CSV export (#7886) - Added term-based suffixes to course names created via LTI to ensure uniqueness across academic years (#7881) +- Sync due date when creating or updating LTI gradebook line items, and re-sync automatically when an assessment is edited (#7872) ### 🐛 Bug fixes - Prevent "No rows found" message from displaying in tables when data is loading (#7790) diff --git a/app/helpers/lti_helper.rb b/app/helpers/lti_helper.rb index ba18ca1294..92ce231787 100644 --- a/app/helpers/lti_helper.rb +++ b/app/helpers/lti_helper.rb @@ -190,6 +190,7 @@ def create_or_update_lti_assessment(lti_deployment, assessment) resourceId: assessment.short_identifier, scoreMaximum: assessment.max_mark.to_f } + payload[:endDateTime] = assessment.due_date.iso8601 if assessment.due_date.present? auth_data = lti_deployment.lti_client.get_oauth_token([LtiDeployment::LTI_SCOPES[:ags_lineitem]]) lineitem_service = lti_deployment.lti_services.find_by!(service_type: 'agslineitem') lineitem_uri = URI(lineitem_service.url) @@ -199,7 +200,8 @@ def create_or_update_lti_assessment(lti_deployment, assessment) else req = Net::HTTP::Post.new(lineitem_uri) end - req.set_form_data(payload) + req.content_type = 'application/vnd.ims.lis.v2.lineitem+json' + req.body = payload.to_json res = lti_deployment.send_lti_request!(req, lineitem_uri, auth_data, [LtiDeployment::LTI_SCOPES[:ags_lineitem]]) line_item_data = JSON.parse(res.body) line_item.update!(lti_line_item_id: line_item_data['id']) diff --git a/app/models/assessment.rb b/app/models/assessment.rb index a060c80386..f01834018b 100644 --- a/app/models/assessment.rb +++ b/app/models/assessment.rb @@ -50,6 +50,8 @@ class Assessment < ApplicationRecord has_many :lti_line_items, dependent: :destroy + after_update_commit :sync_lti_line_items, if: :lti_sync_needed? + # Call custom validator in order to validate the :due_date attribute # date: true maps to DateValidator (custom_name: true maps to CustomNameValidator) # Look in lib/validators/* for more info @@ -161,4 +163,18 @@ def results_standard_deviation DescriptiveStatistics.standard_deviation(marks) end end + + private + + # Re-pushes line item metadata to any LMS that already has this assessment linked, + # so changes in MarkUs (notably due_date) propagate without manual re-trigger. + def sync_lti_line_items + deployment_ids = lti_line_items.distinct.pluck(:lti_deployment_id) + LtiLineItemJob.perform_later(deployment_ids, self) + end + + def lti_sync_needed? + return false unless lti_line_items.exists? + saved_change_to_due_date? || saved_change_to_description? + end end diff --git a/spec/helpers/lti_helper_spec.rb b/spec/helpers/lti_helper_spec.rb index 211aa18f8e..2d3206273e 100644 --- a/spec/helpers/lti_helper_spec.rb +++ b/spec/helpers/lti_helper_spec.rb @@ -887,6 +887,42 @@ expect(LtiLineItem.count).to eq(1) end + it 'sends JSON body with correct LTI AGS Content-Type' do + allow(lti_deployment).to receive(:send_lti_request!) do |req, *| + expect(req.content_type).to eq('application/vnd.ims.lis.v2.lineitem+json') + body = JSON.parse(req.body) + expect(body).to include( + 'label' => assessment.description, + 'resourceId' => assessment.short_identifier, + 'scoreMaximum' => assessment.max_mark.to_f + ) + OpenStruct.new(body: { id: 'https://test.example.com/lineitems/1' }.to_json) + end + subject + end + + it 'includes endDateTime in the request payload' do + allow(lti_deployment).to receive(:send_lti_request!) do |req, *| + body = JSON.parse(req.body) + expect(body).to include('endDateTime' => assessment.due_date.iso8601) + OpenStruct.new(body: { id: 'https://test.example.com/lineitems/1' }.to_json) + end + subject + end + + context 'when the assessment has no due date' do + let(:assessment) { create(:grade_entry_form, due_date: nil) } + + it 'does not include endDateTime in the payload' do + allow(lti_deployment).to receive(:send_lti_request!) do |req, *| + body = JSON.parse(req.body) + expect(body).not_to have_key('endDateTime') + OpenStruct.new(body: { id: 'https://test.example.com/lineitems/1' }.to_json) + end + subject + end + end + context 'when a line item already exists' do before { create(:lti_line_item, lti_deployment: lti_deployment, assessment: assessment) } @@ -898,6 +934,33 @@ subject expect(LtiLineItem.first.lti_line_item_id).to eq('https://test.example.com/lineitems/1') end + + it 'sends a PUT request with JSON body and endDateTime' do + allow(lti_deployment).to receive(:send_lti_request!) do |req, *| + expect(req).to be_a(Net::HTTP::Put) + expect(req.content_type).to eq('application/vnd.ims.lis.v2.lineitem+json') + body = JSON.parse(req.body) + expect(body).to include('endDateTime' => assessment.due_date.iso8601) + OpenStruct.new(body: { id: 'https://test.example.com/lineitems/1' }.to_json) + end + subject + end + end + + context 'when updating a line item without a due date' do + let(:assessment) { create(:grade_entry_form, due_date: nil) } + + before { create(:lti_line_item, lti_deployment: lti_deployment, assessment: assessment) } + + it 'sends a PUT request without endDateTime' do + allow(lti_deployment).to receive(:send_lti_request!) do |req, *| + expect(req).to be_a(Net::HTTP::Put) + body = JSON.parse(req.body) + expect(body).not_to have_key('endDateTime') + OpenStruct.new(body: { id: 'https://test.example.com/lineitems/1' }.to_json) + end + subject + end end end end diff --git a/spec/models/assessment_spec.rb b/spec/models/assessment_spec.rb index 6290c59c1f..7ced24ca93 100644 --- a/spec/models/assessment_spec.rb +++ b/spec/models/assessment_spec.rb @@ -56,4 +56,35 @@ expect(assessment.errors[:visible_until]).to include('must be after visible_on') end end + + describe 'LTI line item resync on update' do + let(:assignment) { create(:assignment) } + let(:lti_deployment) { create(:lti_deployment, course: assignment.course) } + + context 'when no LTI line item exists for the assessment' do + it 'does not enqueue a sync job on due_date change' do + expect(LtiLineItemJob).not_to receive(:perform_later) + assignment.update!(due_date: 2.days.from_now) + end + end + + context 'when an LTI line item exists for the assessment' do + before { create(:lti_line_item, lti_deployment: lti_deployment, assessment: assignment) } + + it 'enqueues a sync job when due_date changes' do + expect(LtiLineItemJob).to receive(:perform_later).with([lti_deployment.id], assignment) + assignment.update!(due_date: 3.days.from_now) + end + + it 'enqueues a sync job when description changes' do + expect(LtiLineItemJob).to receive(:perform_later).with([lti_deployment.id], assignment) + assignment.update!(description: 'updated description') + end + + it 'does not enqueue when only an unrelated attribute changes' do + expect(LtiLineItemJob).not_to receive(:perform_later) + assignment.update!(message: 'unrelated change') + end + end + end end From 7e60231358736d8cf4dc0d29aaebc8c4e7796bf0 Mon Sep 17 00:00:00 2001 From: Naragod Date: Thu, 14 May 2026 16:49:40 -0400 Subject: [PATCH 5/8] Added rake task to backfill start and end course dates (#7925) --- Changelog.md | 1 + app/controllers/admin/courses_controller.rb | 2 +- lib/tasks/populate_course_dates.rake | 95 ++++++++++ .../admin/courses_controller_spec.rb | 19 ++ spec/lib/tasks/populate_course_dates_spec.rb | 179 ++++++++++++++++++ 5 files changed, 295 insertions(+), 1 deletion(-) create mode 100644 lib/tasks/populate_course_dates.rake create mode 100644 spec/lib/tasks/populate_course_dates_spec.rb diff --git a/Changelog.md b/Changelog.md index 85d291daba..9d8eddfc1a 100644 --- a/Changelog.md +++ b/Changelog.md @@ -17,6 +17,7 @@ - Add JS autotester example (#7866) - Return structured JSON from grade entry forms API show endpoint with optional student filter and CSV export (#7886) - Added term-based suffixes to course names created via LTI to ensure uniqueness across academic years (#7881) +- Added `db:populate_course_dates` rake task to backfill `start_at`/`end_at` for existing courses, and permit those fields when creating courses through the admin UI (#7925) - Sync due date when creating or updating LTI gradebook line items, and re-sync automatically when an assessment is edited (#7872) ### 🐛 Bug fixes diff --git a/app/controllers/admin/courses_controller.rb b/app/controllers/admin/courses_controller.rb index c03101f2dd..00f6810b4c 100644 --- a/app/controllers/admin/courses_controller.rb +++ b/app/controllers/admin/courses_controller.rb @@ -86,7 +86,7 @@ def destroy_lti_deployment private def course_create_params - params.require(:course).permit(:name, :is_hidden, :display_name, :max_file_size) + params.require(:course).permit(:name, :is_hidden, :display_name, :max_file_size, :start_at, :end_at) end def course_update_params diff --git a/lib/tasks/populate_course_dates.rake b/lib/tasks/populate_course_dates.rake new file mode 100644 index 0000000000..621b46ded2 --- /dev/null +++ b/lib/tasks/populate_course_dates.rake @@ -0,0 +1,95 @@ +namespace :db do + desc 'Populate courses.start_at and courses.end_at based on naming patterns' + task populate_course_dates: :environment do + PopulateCourseDates.run(dry_run: ENV['DRY_RUN'] == '1') + end +end + +module PopulateCourseDates + DEMO_OR_SANDBOX = /demo|sandbox/i + YEAR_MONTH_IN_NAME = /-(\d{4})-(01|05|09)\b/ + UOFT_SESSION_CODE = /(?-20s] %-32s %s', action: action, name: course.name, detail: detail) + end + + puts '[populate_course_dates] tally:' + tally.sort.each { |k, v| puts " #{k}: #{v}" } + end + + def process(course, dry_run:) + return [:skipped_demo_sandbox, ''] if demo_or_sandbox?(course) + + parsed = parse(course) + return [:skipped_unparseable, "name=#{course.name.inspect} display=#{course.display_name.inspect}"] if parsed.nil? + + start_at, end_at = parsed + apply(course, start_at, end_at, dry_run: dry_run) + end + + def demo_or_sandbox?(course) + course.name.to_s.match?(DEMO_OR_SANDBOX) || course.display_name.to_s.match?(DEMO_OR_SANDBOX) + end + + def parse(course) + bucket, year = bucket_and_year(course) + return if bucket.nil? + + year_long = course.name.to_s.match?(YEAR_LONG) + range_for(bucket, year, year_long: year_long) + end + + def bucket_and_year(course) + if (m = course.name.to_s.match(YEAR_MONTH_IN_NAME)) + [m[2], m[1].to_i] + elsif (m = "#{course.name} #{course.display_name}".match(UOFT_SESSION_CODE)) + [format('%02d', m[2].to_i), m[1].to_i] + end + end + + def range_for(bucket, year, year_long:) + case bucket + when '01' + start_year = year_long ? year - 1 : year + [start_of(start_year, year_long ? 9 : 1), end_of(year, 4)] + when '05' + [start_of(year, 5), end_of(year, 8)] + when '09' + end_year = year_long ? year + 1 : year + [start_of(year, 9), end_of(end_year, year_long ? 4 : 12)] + end + end + + def start_of(year, month) + Time.zone.local(year, month, 1, 0, 0, 0) + end + + def end_of(year, month) + Time.zone.local(year, month, 1).end_of_month.change(hour: 23, min: 59, sec: 59) + end + + def apply(course, start_at, end_at, dry_run:) + return [:skipped_already_set, ''] if course.start_at && course.end_at + + new_start = course.start_at || start_at + new_end = course.end_at || end_at + detail = "start=#{new_start.to_date} end=#{new_end.to_date}" + return [:would_update, detail] if dry_run + + course.update(start_at: new_start, end_at: new_end) + return [:invalid, course.errors.full_messages.join('; ')] if course.errors.any? + + [:updated, detail] + end +end diff --git a/spec/controllers/admin/courses_controller_spec.rb b/spec/controllers/admin/courses_controller_spec.rb index e2afc4eb80..30ad2f4665 100644 --- a/spec/controllers/admin/courses_controller_spec.rb +++ b/spec/controllers/admin/courses_controller_spec.rb @@ -201,6 +201,25 @@ } } end + + it 'persists start_at and end_at when provided' do + new_start_at = Time.zone.parse('2026-01-01') + new_end_at = Time.zone.parse('2026-04-30') + + post_as admin, :create, params: { + course: { + name: 'CSC207', + display_name: 'Software Design', + is_hidden: true, + start_at: new_start_at, + end_at: new_end_at + } + } + + created_course = Course.find_by(name: 'CSC207') + expect(created_course.start_at).to be_within(1.second).of(new_start_at) + expect(created_course.end_at).to be_within(1.second).of(new_end_at) + end end describe '#edit' do diff --git a/spec/lib/tasks/populate_course_dates_spec.rb b/spec/lib/tasks/populate_course_dates_spec.rb new file mode 100644 index 0000000000..85e6b8b433 --- /dev/null +++ b/spec/lib/tasks/populate_course_dates_spec.rb @@ -0,0 +1,179 @@ +require 'rails_helper' +require 'rake' + +RSpec.describe 'db:populate_course_dates', type: :task do + before do + Rake.application.rake_require('tasks/populate_course_dates') unless Rake::Task.task_defined?('db:populate_course_dates') + Rake::Task.define_task(:environment) + task.reenable + end + + let(:task) { Rake::Task['db:populate_course_dates'] } + + def make_course(name, display_name = nil, **attrs) + create(:course, name: name, display_name: display_name || name, **attrs) + end + + def expect_dates(course, start_date, end_date) + course.reload + expect(course.start_at.to_date.iso8601).to eq(start_date) + expect(course.end_at.to_date.iso8601).to eq(end_date) + end + + describe 'numeric name suffix (Pattern A)' do + it 'parses Fall: csc110-2024-09 -> Sep 1 .. Dec 31 of the same year' do + c = make_course('csc110-2024-09', 'Foundations of Computer Science I') + task.invoke + expect_dates(c, '2024-09-01', '2024-12-31') + end + + it 'parses Winter: csc111-2026-01 -> Jan 1 .. Apr 30 of the same year' do + c = make_course('csc111-2026-01', 'Foundations of Computer Science II') + task.invoke + expect_dates(c, '2026-01-01', '2026-04-30') + end + + it 'parses Summer: csc384-2025-05 -> May 1 .. Aug 31 of the same year' do + c = make_course('csc384-2025-05', 'Introduction to Artificial Intelligence') + task.invoke + expect_dates(c, '2025-05-01', '2025-08-31') + end + end + + describe 'UofT 5-digit session code (Pattern B)' do + it 'reads the session from display_name when name has none' do + c = make_course('CSC369H1-S', 'Operating Systems (20261)') + task.invoke + expect_dates(c, '2026-01-01', '2026-04-30') + end + + it 'reads the session embedded in display_name with surrounding text' do + c = make_course('JSC370H1-S-LEC0101', 'JSC370H1 S LEC0101 20261:Data Science II') + task.invoke + expect_dates(c, '2026-01-01', '2026-04-30') + end + + it 'trusts the session code over the F/S/Y registrar suffix' do + c = make_course('MRKUS100H-F-LEC0101-20251', 'MarkUs Jan 2025') + task.invoke + expect_dates(c, '2025-01-01', '2025-04-30') + end + end + + describe 'year-long courses (H1Y)' do + it 'spans Sep (Y-1) -> Apr Y for a Winter session code' do + c = make_course('CSC998H1Y-20251', 'CSC998H1Y 20251') + task.invoke + expect_dates(c, '2024-09-01', '2025-04-30') + end + + it 'spans Sep Y -> Apr (Y+1) for a Fall session code' do + c = make_course('CSC999H1Y-20259', 'CSC999H1Y 20259') + task.invoke + expect_dates(c, '2025-09-01', '2026-04-30') + end + end + + describe 'demo and sandbox exclusions' do + it 'skips when name matches /sandbox/' do + c = make_course('ds-sandbox', 'Data Science Sandbox') + task.invoke + c.reload + expect(c.start_at).to be_nil + expect(c.end_at).to be_nil + end + + it 'skips when display_name matches /demo/ even though name has a parseable date' do + c = make_course('fakedemo', 'Fake Demo Course 2024-09') + task.invoke + c.reload + expect(c.start_at).to be_nil + expect(c.end_at).to be_nil + end + + it 'skips when only display_name matches /sandbox/' do + c = make_course('GRE101H1-F-LEC0101', "GRE101:Ryan's fourth sandbox") + task.invoke + c.reload + expect(c.start_at).to be_nil + expect(c.end_at).to be_nil + end + end + + describe 'idempotency and partial population' do + it 'leaves a fully-populated course untouched' do + original_start = Time.zone.parse('2020-01-01') + original_end = Time.zone.parse('2020-04-30') + c = make_course('csc110-2024-09', start_at: original_start, end_at: original_end) + task.invoke + c.reload + expect(c.start_at).to be_within(1.second).of(original_start) + expect(c.end_at).to be_within(1.second).of(original_end) + end + + it 'fills only the missing field when one is already set' do + preset_start = Time.zone.parse('2024-08-15') + c = make_course('csc110-2024-09', start_at: preset_start) + task.invoke + c.reload + expect(c.start_at).to be_within(1.second).of(preset_start) + expect(c.end_at.to_date.iso8601).to eq('2024-12-31') + end + + it 'leaves dates unchanged on a second run' do + c = make_course('csc110-2024-09') + task.invoke + c.reload + original_start, original_end = c.start_at, c.end_at + + task.reenable + task.invoke + c.reload + expect(c.start_at).to eq(original_start) + expect(c.end_at).to eq(original_end) + end + end + + describe 'validation failures' do + it 'skips and logs without raising when the existing date conflicts with the computed one' do + preset_start = Time.zone.parse('2099-01-01') + bad = make_course('csc110-2024-09', start_at: preset_start) + good = make_course('csc111-2026-01', 'Foundations II') + + expect { task.invoke }.to output(/\[invalid\b/).to_stdout + bad.reload + expect(bad.start_at).to be_within(1.second).of(preset_start) + expect(bad.end_at).to be_nil + expect_dates(good, '2026-01-01', '2026-04-30') + end + end + + describe 'unparseable and dry-run' do + it 'skips a course whose name and display_name lack any year/session match' do + c = make_course('csc1500', 'Some title with no date') + task.invoke + c.reload + expect(c.start_at).to be_nil + expect(c.end_at).to be_nil + end + + it 'does not write to the DB when DRY_RUN=1' do + c = make_course('csc110-2024-09', 'Foundations') + stub_const('ENV', ENV.to_h.merge('DRY_RUN' => '1')) + task.invoke + c.reload + expect(c.start_at).to be_nil + expect(c.end_at).to be_nil + end + end + + describe 'timezone' do + it 'sets timestamps in Toronto local time' do + c = make_course('csc110-2024-09') + task.invoke + c.reload + expect(c.start_at.in_time_zone('America/Toronto').strftime('%Y-%m-%d %H:%M')).to eq('2024-09-01 00:00') + expect(c.end_at.in_time_zone('America/Toronto').strftime('%Y-%m-%d %H:%M')).to eq('2024-12-31 23:59') + end + end +end From 6943752a4ae1feff9a62b8440aa82d1d32eb9cbf Mon Sep 17 00:00:00 2001 From: donny-wong <141858744+donny-wong@users.noreply.github.com> Date: Fri, 15 May 2026 11:00:30 -0400 Subject: [PATCH 6/8] Fixed remark request display of original result's total mark (#7945) --- Changelog.md | 1 + app/controllers/results_controller.rb | 2 +- spec/controllers/results_controller_spec.rb | 30 +++++++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index 9d8eddfc1a..d6cc550506 100644 --- a/Changelog.md +++ b/Changelog.md @@ -26,6 +26,7 @@ - Fixed reserved interpolation key `%{format}` in `download_errors.unrecognized_format` locale string, renamed to `%{file_format}` (#7894) - Fix version mismatch between container client and database server (#7916) - Fixed filter Canvas Test Student from roster sync (#7926) +- Fix: include original total mark in JSON response for remark requests (#7945) ### 🔧 Internal changes - Added seed task to assign TAs to A1 groupings and criteria (#7867) diff --git a/app/controllers/results_controller.rb b/app/controllers/results_controller.rb index bdf8a629a5..82011db875 100644 --- a/app/controllers/results_controller.rb +++ b/app/controllers/results_controller.rb @@ -239,7 +239,7 @@ def show data[:assignment_max_mark] = assignment.max_mark end data[:total] = marks_map.pluck('mark') - data[:old_total] = old_marks.values_at(:mark).compact.sum + data[:old_total] = original_result&.get_total_mark || 0 # Tags all_tags = assignment.tags.pluck_to_hash(:id, :name) diff --git a/spec/controllers/results_controller_spec.rb b/spec/controllers/results_controller_spec.rb index 3a52981d4d..a859348c9f 100644 --- a/spec/controllers/results_controller_spec.rb +++ b/spec/controllers/results_controller_spec.rb @@ -1401,6 +1401,32 @@ def self.test_unauthorized(route_name) end end + shared_examples 'show result with old_total' do + before do + create(:ta_membership, role: ta, grouping: grouping) + end + + context 'when format is json' do + it 'returns the total mark of the original result in old_total when it exists' do + original_result = create(:complete_result, submission: submission) + allow_any_instance_of(Submission).to receive(:remark_submitted?).and_return(true) + allow_any_instance_of(Submission).to receive(:get_original_result).and_return(original_result) + allow(original_result).to receive(:get_total_mark).and_return(85.5) + + get :show, params: { course_id: course.id, id: incomplete_result.id }, format: :json + + expect(response).to have_http_status(:ok) + expect(response.parsed_body['old_total']).to eq(85.5) + end + + it 'returns 0 for old_total when no original result exists' do + allow_any_instance_of(Submission).to receive(:remark_submitted?).and_return(false) + get :show, params: { course_id: course.id, id: incomplete_result.id }, format: :json + expect(response.parsed_body['old_total']).to eq(0) + end + end + end + context 'A student' do before { sign_in student } @@ -1636,6 +1662,8 @@ def self.test_unauthorized(route_name) context 'An instructor' do before { sign_in instructor } + it_behaves_like 'show result with old_total' + context 'accessing set_released_to_students' do before do get :set_released_to_students, params: { course_id: course.id, @@ -2144,6 +2172,8 @@ def self.test_unauthorized(route_name) context 'A TA' do before { sign_in ta } + it_behaves_like 'show result with old_total' + [:set_released_to_students].each { |route_name| test_unauthorized(route_name) } context 'when groups information is anonymized' do From 78119c6461522c71eaae55a0b9f1cc9d6a477f5c Mon Sep 17 00:00:00 2001 From: David Liu Date: Fri, 15 May 2026 14:46:59 -0400 Subject: [PATCH 7/8] Fixed flaky git-hooks tests (#7950) --- Changelog.md | 1 + spec/support/git_hook_helper.rb | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Changelog.md b/Changelog.md index d6cc550506..bb9a74b25f 100644 --- a/Changelog.md +++ b/Changelog.md @@ -51,6 +51,7 @@ - Updated GitHub Actions dependencies and added Dependabot config for quarterly GitHub Actions updates (#7920) - Updated `pdfjs-dist` to v5.6.205 (#7942) - Switched SCSS files to use `@use` instead of `@import` to reduce bundle size (#7943) +- Fixed flaky git-hooks tests (#7950) ## [v2.9.6] diff --git a/spec/support/git_hook_helper.rb b/spec/support/git_hook_helper.rb index 9b2f114b06..46f540129f 100644 --- a/spec/support/git_hook_helper.rb +++ b/spec/support/git_hook_helper.rb @@ -41,8 +41,8 @@ end after do - FileUtils.rm_r(repo_path) - FileUtils.rm_r(repo_bare_path) + FileUtils.rm_rf(repo_path, secure: true) + FileUtils.rm_rf(repo_bare_path, secure: true) end def commit_changes(changes: '.') From e4ab049b4ec338b41cbc6ce34f50b84d2731857d Mon Sep 17 00:00:00 2001 From: Daniel Rafailov Date: Sun, 17 May 2026 15:04:26 -0400 Subject: [PATCH 8/8] Moved my changes in the Changelog file from the New features and improvements section to the Internal changes section --- Changelog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index bb9a74b25f..8fb92bc209 100644 --- a/Changelog.md +++ b/Changelog.md @@ -7,7 +7,6 @@ ### 🚨 Breaking changes ### ✨ New features and improvements -- Added tests for `MarksGradersController` to achieve full test coverage for `randomly_assign` (#7947) - Improve admin user list loading time by replacing ActiveRecord instantiation with direct column extraction (#7897) - Provide suggestions for partial student matching scans (#7760) - Allow inactive students to join groups (#7757) @@ -29,6 +28,7 @@ - Fix: include original total mark in JSON response for remark requests (#7945) ### 🔧 Internal changes +- Added tests for `MarksGradersController` to achieve full test coverage for `randomly_assign` (#7947) - Added seed task to assign TAs to A1 groupings and criteria (#7867) - Updated autotest seed files to ensure settings follow tester JSON schema (#7775) - Refactored grade entry form helper logic into `GradeEntryFormsController` and removed the newly-unused helper file. (#7789)