diff --git a/app/controllers/course/external_assessments_controller.rb b/app/controllers/course/external_assessments_controller.rb new file mode 100644 index 0000000000..cf2c02fdad --- /dev/null +++ b/app/controllers/course/external_assessments_controller.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true +class Course::ExternalAssessmentsController < Course::ComponentController + before_action :load_external_assessment, only: [:grades] + + def grades + authorize! :manage_gradebook_weights, current_course + # The gradebook keys students by user_id (see index/update_grade jbuilders), so the + # `studentId` param is a user_id, not a course_user PK. + course_user = current_course.course_users.find_by!(user_id: grade_params[:studentId]) + @grade = @external_assessment.external_assessment_grades. + find_or_initialize_by(course_user: course_user) + @grade.grade = normalized_grade(grade_params[:grade]) + @grade.save! + render 'update_grade' + rescue ActiveRecord::RecordNotUnique + retry + rescue ActiveRecord::RecordNotFound + head :not_found + rescue ActiveRecord::RecordInvalid => e + render json: { errors: { base: e.message } }, status: :unprocessable_entity + end + + private + + def component + current_component_host[:course_gradebook_component] + end + + def load_external_assessment + @external_assessment = Course::ExternalAssessment.for_course(current_course).find(params[:id]) + rescue ActiveRecord::RecordNotFound + head :not_found + end + + def grade_params + params.permit(:studentId, :grade) + end + + # Blank cell clears the grade to null (ungraded), never zero (decision #7). + def normalized_grade(value) + value.blank? ? nil : value + end +end diff --git a/app/controllers/course/gradebook_controller.rb b/app/controllers/course/gradebook_controller.rb index 08c7a65402..0964fe6539 100644 --- a/app/controllers/course/gradebook_controller.rb +++ b/app/controllers/course/gradebook_controller.rb @@ -15,6 +15,7 @@ def index load_weighted_view(assessment_ids) if @weighted_view_enabled load_grades(assessment_ids) @student_level_contributions = compute_student_level_contributions + load_externals end end end @@ -22,13 +23,7 @@ def index def update_weights authorize! :manage_gradebook_weights, current_course updates = (update_weights_params[:weights] || []).map { |entry| parse_weight_entry(entry) } - level_config = nil - # One transaction so a rejected level formula rolls back the tab-weight writes too, - # rather than leaving a partial save. - ActiveRecord::Base.transaction do - Course::Gradebook::TabContribution.bulk_update(course: current_course, updates: updates) - level_config = persist_level_contribution - end + level_config = persist_weight_updates(updates) response_body = { weights: serialize_weight_updates(updates) } response_body[:levelContribution] = serialize_level_contribution(level_config) if level_config render json: response_body @@ -38,6 +33,20 @@ def update_weights private + # Persists tab weights, external-assessment weights (negative tab ids), and the optional + # level contribution atomically, so a mixed save from the single weights request can't desync. + # Returns the persisted LevelConfig, or nil when no levelContribution was sent. + def persist_weight_updates(updates) + external_updates, tab_updates = updates.partition { |entry| entry[:tab_id] < 0 } + level_config = nil + ActiveRecord::Base.transaction do + Course::Gradebook::TabContribution.bulk_update(course: current_course, updates: tab_updates) + Course::Gradebook::ExternalContribution.bulk_update(course: current_course, updates: external_updates) + level_config = persist_level_contribution + end + level_config + end + def authorize_read_gradebook! authorize! :read_gradebook, current_course end @@ -150,6 +159,15 @@ def fetch_categories_and_tabs [tabs.map(&:category).uniq(&:id), tabs] end + def load_externals + @external_assessments = Course::ExternalAssessment.for_course(current_course). + includes(:gradebook_contribution, external_assessment_grades: :course_user).to_a + @external_grades = @external_assessments.flat_map(&:external_assessment_grades) + @external_contributions = @external_assessments. + index_by(&:id). + transform_values(&:gradebook_contribution) + end + def fetch_students current_course.course_users.students.without_phantom_users. calculated(:experience_points).includes(user: :emails).to_a. diff --git a/app/models/course.rb b/app/models/course.rb index e5b6c0cde6..18a719b889 100644 --- a/app/models/course.rb +++ b/app/models/course.rb @@ -57,6 +57,10 @@ class Course < ApplicationRecord # rubocop:disable Metrics/ClassLength dependent: :destroy, inverse_of: :course has_one :gradebook_level_config, class_name: 'Course::Gradebook::LevelConfig', dependent: :destroy, inverse_of: :course + has_many :gradebook_external_contributions, class_name: 'Course::Gradebook::ExternalContribution', + dependent: :destroy, inverse_of: :course + has_many :external_assessments, class_name: 'Course::ExternalAssessment', + inverse_of: :course, dependent: :destroy has_many :assessment_skills, class_name: 'Course::Assessment::Skill', dependent: :destroy has_many :assessment_skill_branches, class_name: 'Course::Assessment::SkillBranch', diff --git a/app/models/course/external_assessment.rb b/app/models/course/external_assessment.rb new file mode 100644 index 0000000000..329247c014 --- /dev/null +++ b/app/models/course/external_assessment.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true +# A gradebook component graded outside Coursemology (e.g. a midterm or final). +# It is a first-class gradebook contributor, NOT a Course::Assessment: it never +# touches attempts, EXP, statistics, todos, or the lesson plan. Its weight lives on +# its course_gradebook_contributions row; its display grouping is synthesised by the +# gradebook serializer (no real tab/category exists). +class Course::ExternalAssessment < ApplicationRecord + # Sentinel id for the serializer's synthetic "External Assessments" category. + # Native categories are positive; externals and their synthetic grouping are negative. + SYNTHETIC_CATEGORY_ID = -1 + SYNTHETIC_CATEGORY_TITLE = 'External Assessments' + + validates :title, length: { maximum: 255 }, presence: true + validates :title, uniqueness: { scope: :course_id } + validates :maximum_grade, presence: true + validates :maximum_grade, numericality: { greater_than_or_equal_to: 0 }, allow_nil: true + validates :creator, presence: true + validates :updater, presence: true + + belongs_to :course, inverse_of: :external_assessments + has_one :gradebook_contribution, class_name: 'Course::Gradebook::ExternalContribution', + inverse_of: :external_assessment, dependent: :destroy + has_many :external_assessment_grades, class_name: 'Course::ExternalAssessmentGrade', + inverse_of: :external_assessment, dependent: :delete_all + + scope :for_course, ->(course) { where(course_id: course.id) } + + # The negative serialized id used by the synthetic tab AND the leaf assessment. + def synthetic_tab_id + -id + end + + # Creates an external assessment and its gradebook contribution in one transaction. + # Raises ActiveRecord::RecordInvalid on a duplicate title within the course. + # rubocop:disable Metrics/ParameterLists -- factory mirrors the model's columns; named kwargs are clearer than a struct + def self.create_for_course!(course:, title:, maximum_grade:, weight: 0, + floor_at_zero: true, cap_at_maximum: true) + transaction do + external = course.external_assessments.create!( + title: title, maximum_grade: maximum_grade, + floor_at_zero: floor_at_zero, cap_at_maximum: cap_at_maximum + ) + Course::Gradebook::ExternalContribution.create!(course: course, external_assessment: external, + weight: weight) + external + end + end + # rubocop:enable Metrics/ParameterLists +end diff --git a/app/models/course/external_assessment_grade.rb b/app/models/course/external_assessment_grade.rb new file mode 100644 index 0000000000..70518c79c9 --- /dev/null +++ b/app/models/course/external_assessment_grade.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true +# One external grade for a (external assessment, course_user). The binding key is +# course_user_id (the authoritative link to the person); imported_identifier is a +# non-authoritative snapshot of the email/Student ID used at import (audit + upsert +# mismatch detection), null for grades typed/edited inline. +class Course::ExternalAssessmentGrade < ApplicationRecord + validates :course_user, presence: true + validates :grade, numericality: true, allow_nil: true + validates :course_user_id, uniqueness: { scope: :external_assessment_id } + validates :creator, presence: true + validates :updater, presence: true + + belongs_to :external_assessment, class_name: 'Course::ExternalAssessment', + inverse_of: :external_assessment_grades + belongs_to :course_user, inverse_of: :external_assessment_grades +end diff --git a/app/models/course/gradebook/external_contribution.rb b/app/models/course/gradebook/external_contribution.rb new file mode 100644 index 0000000000..a274c965f1 --- /dev/null +++ b/app/models/course/gradebook/external_contribution.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true +class Course::Gradebook::ExternalContribution < ApplicationRecord + belongs_to :course, inverse_of: :gradebook_external_contributions + belongs_to :external_assessment, class_name: 'Course::ExternalAssessment', + inverse_of: :gradebook_contribution + + validates :creator, presence: true + validates :updater, presence: true + validates :weight, numericality: { greater_than_or_equal_to: 0, less_than_or_equal_to: 100 } + validates :external_assessment_id, uniqueness: true + validate :course_matches_external_assessment + + # Upserts one external-assessment contribution per entry. Entries arrive with the gradebook's + # negative-tab_id wire encoding (tab_id == -external_assessment_id), shared with the tab payload. + # Raises ActiveRecord::RecordNotFound for an unknown/foreign external, ActiveRecord::RecordInvalid + # on an out-of-range weight; the transaction rolls back. + # + # @param course [Course] + # @param updates [Array] each { tab_id: (negative external id), weight: } + def self.bulk_update(course:, updates:) + externals_by_id = course.external_assessments. + where(id: updates.map { |e| -e[:tab_id] }).index_by(&:id) + updates.each { |e| raise ActiveRecord::RecordNotFound unless externals_by_id.key?(-e[:tab_id]) } + + transaction do + updates.each { |entry| upsert(course, externals_by_id[-entry[:tab_id]], entry[:weight]) } + end + end + + # Upserts a single contribution for the given external assessment. + def self.upsert(course, external, weight) + contribution = find_or_initialize_by(external_assessment_id: external.id) + contribution.course = course + contribution.weight = weight + contribution.save! + end + private_class_method :upsert + + private + + def course_matches_external_assessment + return if external_assessment.nil? || course.nil? + + errors.add(:course, :invalid) if external_assessment.course_id != course_id + end +end diff --git a/app/models/course_user.rb b/app/models/course_user.rb index 944fe25e09..6682fb1fac 100644 --- a/app/models/course_user.rb +++ b/app/models/course_user.rb @@ -50,6 +50,8 @@ class CourseUser < ApplicationRecord inverse_of: :course_user, dependent: :destroy has_many :groups, through: :group_users, class_name: 'Course::Group', source: :group has_many :personal_times, class_name: 'Course::PersonalTime', inverse_of: :course_user, dependent: :destroy + has_many :external_assessment_grades, class_name: 'Course::ExternalAssessmentGrade', + inverse_of: :course_user, dependent: :destroy belongs_to :reference_timeline, class_name: 'Course::ReferenceTimeline', inverse_of: :course_users, optional: true default_scope { where(deleted_at: nil) } diff --git a/app/views/course/external_assessments/update_grade.json.jbuilder b/app/views/course/external_assessments/update_grade.json.jbuilder new file mode 100644 index 0000000000..f997450dfc --- /dev/null +++ b/app/views/course/external_assessments/update_grade.json.jbuilder @@ -0,0 +1,4 @@ +# frozen_string_literal: true +json.studentId @grade.course_user.user_id +json.assessmentId(-@grade.external_assessment_id) +json.grade @grade.grade&.to_f diff --git a/app/views/course/gradebook/index.json.jbuilder b/app/views/course/gradebook/index.json.jbuilder index 623a70d813..383454faed 100644 --- a/app/views/course/gradebook/index.json.jbuilder +++ b/app/views/course/gradebook/index.json.jbuilder @@ -2,32 +2,72 @@ json.weightedViewEnabled @weighted_view_enabled json.canManageWeights can?(:manage_gradebook_weights, current_course) -json.categories @categories do |cat| - json.id cat.id - json.title cat.title +json.categories do + json.array!(@categories) do |cat| + json.id cat.id + json.title cat.title + end + if @external_assessments.any? + json.child! do + json.id Course::ExternalAssessment::SYNTHETIC_CATEGORY_ID + json.title Course::ExternalAssessment::SYNTHETIC_CATEGORY_TITLE + end + end end -json.tabs @tabs do |tab| - json.id tab.id - json.title tab.title - json.categoryId tab.category_id - if @weighted_view_enabled - contribution = @tab_contributions[tab.id] - json.gradebookWeight (contribution&.weight || 0).to_f - json.weightMode(contribution&.weight_mode || 'equal') - json.keepHighest(contribution&.keep_highest || 0) +json.tabs do + json.array!(@tabs) do |tab| + json.id tab.id + json.title tab.title + json.categoryId tab.category_id + if @weighted_view_enabled + contribution = @tab_contributions[tab.id] + json.gradebookWeight (contribution&.weight || 0).to_f + json.weightMode(contribution&.weight_mode || 'equal') + json.keepHighest(contribution&.keep_highest || 0) + end + end + @external_assessments.each do |external| + json.child! do + json.id external.synthetic_tab_id + json.title external.title + json.categoryId Course::ExternalAssessment::SYNTHETIC_CATEGORY_ID + if @weighted_view_enabled + contribution = @external_contributions[external.id] + json.gradebookWeight (contribution&.weight || 0).to_f + json.weightMode 'equal' + end + end end end -json.assessments @published_assessments do |assessment| - json.id assessment.id - json.title assessment.title - json.tabId assessment.tab_id - json.maxGrade @assessment_max_grades[assessment.id] || 0 - if @weighted_view_enabled - contribution = @assessment_contributions[assessment.id] - json.gradebookWeight contribution&.weight&.to_f - json.gradebookExcluded(contribution&.excluded || false) +json.assessments do + json.array!(@published_assessments) do |assessment| + json.id assessment.id + json.title assessment.title + json.tabId assessment.tab_id + json.maxGrade @assessment_max_grades[assessment.id] || 0 + if @weighted_view_enabled + contribution = @assessment_contributions[assessment.id] + json.gradebookWeight contribution&.weight&.to_f + json.gradebookExcluded(contribution&.excluded || false) + end + end + @external_assessments.each do |external| + json.child! do + json.id(-external.id) + json.title external.title + json.tabId external.synthetic_tab_id + json.maxGrade external.maximum_grade.to_f + json.external true + json.floorAtZero external.floor_at_zero + json.capAtMaximum external.cap_at_maximum + if @weighted_view_enabled + contribution = @external_contributions[external.id] + json.gradebookWeight contribution&.weight&.to_f + json.gradebookExcluded false + end + end end end @@ -41,11 +81,20 @@ json.students @students do |course_user| json.levelContribution @student_level_contributions[course_user.user_id] end -json.submissions @submissions do |sub| - json.studentId sub.student_id - json.assessmentId sub.assessment_id - json.submissionId sub.submission_id - json.grade sub.grade&.to_f +json.submissions do + json.array!(@submissions) do |sub| + json.submissionId sub.submission_id + json.studentId sub.student_id + json.assessmentId sub.assessment_id + json.grade sub.grade&.to_f + end + @external_grades.each do |grade| + json.child! do + json.studentId grade.course_user.user_id + json.assessmentId(-grade.external_assessment_id) + json.grade grade.grade&.to_f + end + end end json.gamificationEnabled current_course.gamified? @@ -67,3 +116,5 @@ json.levelContribution do json.clamp true end end + +json.userId current_user&.id diff --git a/client/app/api/course/Gradebook.ts b/client/app/api/course/Gradebook.ts index 7603f1f2a1..4fe79086b9 100644 --- a/client/app/api/course/Gradebook.ts +++ b/client/app/api/course/Gradebook.ts @@ -1,4 +1,8 @@ -import { GradebookData, UpdateWeightsPayload } from 'types/course/gradebook'; +import { + ExternalGradePayload, + GradebookData, + UpdateWeightsPayload, +} from 'types/course/gradebook'; import { APIResponse } from 'api/types'; @@ -18,4 +22,14 @@ export default class GradebookAPI extends BaseCourseAPI { ): APIResponse { return this.client.patch(`${this.#urlPrefix}/weights`, payload); } + + setExternalGrade( + id: number, + payload: { studentId: number; grade: number | null }, + ): APIResponse { + return this.client.put( + `${this.#urlPrefix}/external_assessments/${id}/grades`, + payload, + ); + } } diff --git a/client/app/bundles/course/gradebook/__tests__/buildAssessmentColumnIds.test.ts b/client/app/bundles/course/gradebook/__tests__/buildAssessmentColumnIds.test.ts new file mode 100644 index 0000000000..d231ce6559 --- /dev/null +++ b/client/app/bundles/course/gradebook/__tests__/buildAssessmentColumnIds.test.ts @@ -0,0 +1,31 @@ +import { + buildAssessmentColumnId, + parseAssessmentColumnId, +} from '../components/buildAssessmentColumnIds'; + +describe('assessment column ids', () => { + it('round-trips a positive (regular) assessment id', () => { + expect(parseAssessmentColumnId(buildAssessmentColumnId(100))).toBe(100); + }); + + it('round-trips a negative (external) assessment id', () => { + expect(parseAssessmentColumnId(buildAssessmentColumnId(-5))).toBe(-5); + }); + + it('returns null for a non-assessment column id', () => { + expect(parseAssessmentColumnId('name')).toBeNull(); + expect(parseAssessmentColumnId('totalXp')).toBeNull(); + }); + + it('round-trips a zero id (falsy but valid)', () => { + expect(parseAssessmentColumnId(buildAssessmentColumnId(0))).toBe(0); + }); + + it('returns null for a malformed or unanchored asn- column id', () => { + expect(parseAssessmentColumnId('asn-')).toBeNull(); + expect(parseAssessmentColumnId('asn-abc')).toBeNull(); + expect(parseAssessmentColumnId('asn-1.5')).toBeNull(); + expect(parseAssessmentColumnId('asn-12x')).toBeNull(); + expect(parseAssessmentColumnId('xasn-12')).toBeNull(); + }); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts b/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts index 4506642ae9..ef94ec6a4c 100644 --- a/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts +++ b/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts @@ -1368,3 +1368,101 @@ describe('customTabImbalanced', () => { expect(customTabImbalanced(tab, tabAssessments)).toBe(false); }); }); + +describe('external-assessment grade bounding (floorAtZero / capAtMaximum)', () => { + it('equal mode: capAtMaximum clamps an over-max grade before ratioing', () => { + // a1 graded 120/100 with capAtMaximum → effective 100 → ratio 1.0 (not 1.2) + // a2 graded 25/50 → 0.5 ; subtotal = (1.0 + 0.5)/2 = 0.75 + const result = computeTabSubtotal({ + studentId: 1, + tab: { id: 10, title: 'M', categoryId: 0 }, + assessments: [ + { id: 1, tabId: 10, maxGrade: 100, title: 'A', capAtMaximum: true }, + { id: 2, tabId: 10, maxGrade: 50, title: 'B' }, + ], + submissions: subs([ + { studentId: 1, assessmentId: 1, grade: 120 }, + { studentId: 1, assessmentId: 2, grade: 25 }, + ]), + }); + expect(result).toBeCloseTo(0.75); + }); + + it('equal mode: floorAtZero clamps a negative grade to 0 before ratioing', () => { + // a1 graded -40/100 with floorAtZero → effective 0 → ratio 0 (not -0.4) + // a2 graded 50/50 → 1.0 ; subtotal = (0 + 1.0)/2 = 0.5 + const result = computeTabSubtotal({ + studentId: 1, + tab: { id: 10, title: 'M', categoryId: 0 }, + assessments: [ + { id: 1, tabId: 10, maxGrade: 100, title: 'A', floorAtZero: true }, + { id: 2, tabId: 10, maxGrade: 50, title: 'B' }, + ], + submissions: subs([ + { studentId: 1, assessmentId: 1, grade: -40 }, + { studentId: 1, assessmentId: 2, grade: 50 }, + ]), + }); + expect(result).toBeCloseTo(0.5); + }); + + it('native assessment (no flags) passes an over-max grade through uncapped', () => { + // a1 graded 120/100, no capAtMaximum → ratio 1.2 (native behaviour unchanged) + const result = computeTabSubtotal({ + studentId: 1, + tab: { id: 10, title: 'M', categoryId: 0 }, + assessments: [{ id: 1, tabId: 10, maxGrade: 100, title: 'A' }], + submissions: subs([{ studentId: 1, assessmentId: 1, grade: 120 }]), + }); + expect(result).toBeCloseTo(1.2); + }); + + it('custom mode: bounds the grade before applying the assessment weight', () => { + // a1 120/100 capAtMaximum weight 70 → effective 100 → 1.0*70 = 70 + // a2 50/50 weight 30 → 1.0*30 = 30 ; subtotal = (70 + 30)/100 = 1.0 + const result = computeTabSubtotal({ + studentId: 1, + tab: { + id: 10, + title: 'M', + categoryId: 0, + gradebookWeight: 100, + weightMode: 'custom' as const, + }, + assessments: [ + { + id: 1, + tabId: 10, + maxGrade: 100, + title: 'A', + gradebookWeight: 70, + capAtMaximum: true, + }, + { id: 2, tabId: 10, maxGrade: 50, title: 'B', gradebookWeight: 30 }, + ], + submissions: subs([ + { studentId: 1, assessmentId: 1, grade: 120 }, + { studentId: 1, assessmentId: 2, grade: 50 }, + ]), + }); + expect(result).toBeCloseTo(1.0); + }); + + it('breakdown: ratio/points reflect the bounded grade while grade stays raw', () => { + // a1 120/100 capAtMaximum, equal mode, n=1, tab weight 60 + // effective 100 → ratio 1.0 → points = (1.0/1)*60 = 60 + // raw grade preserved for the "120/100" display + const [tab] = computeStudentBreakdown({ + studentId: 1, + tabs: [{ id: 10, title: 'M', categoryId: 0, gradebookWeight: 60 }], + assessments: [ + { id: 1, tabId: 10, maxGrade: 100, title: 'A', capAtMaximum: true }, + ], + submissions: subs([{ studentId: 1, assessmentId: 1, grade: 120 }]), + }); + const a = tab.assessments.find((x) => x.assessmentId === 1)!; + expect(a.grade).toBe(120); // raw, for display + expect(a.ratio).toBeCloseTo(1.0); // bounded fraction earned + expect(a.points).toBeCloseTo(60); + }); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/store.test.ts b/client/app/bundles/course/gradebook/__tests__/store.test.ts index 6fb04c9719..cde8e561a3 100644 --- a/client/app/bundles/course/gradebook/__tests__/store.test.ts +++ b/client/app/bundles/course/gradebook/__tests__/store.test.ts @@ -518,3 +518,63 @@ describe('UPDATE_TAB_WEIGHTS — keepHighest', () => { expect(next.tabs[1].keepHighest).toBe(2); }); }); + +describe('external assessment reducers', () => { + const state = { + categories: [{ id: 1, title: 'Cat A' }], + tabs: [{ id: 10, title: 'Tab 1', categoryId: 1 }], + assessments: [{ id: 100, title: 'Quiz 1', tabId: 10, maxGrade: 10 }], + students: [], + submissions: [{ studentId: 1, assessmentId: 100, grade: 8 }], + gamificationEnabled: false, + userId: 0, + weightedViewEnabled: false, + canManageWeights: true, + levelContribution: { + enabled: false, + formula: '', + weight: 0, + show: false, + clamp: true, + }, + courseMaxLevel: 0, + }; + + it('setExternalGrade upserts a submission row by (studentId, assessmentId)', () => { + const inserted = reducer( + state, + actions.setExternalGrade({ studentId: 1, assessmentId: -5, grade: 42 }), + ); + expect( + inserted.submissions.find( + (s) => s.studentId === 1 && s.assessmentId === -5, + )?.grade, + ).toBe(42); + + const updated = reducer( + inserted, + actions.setExternalGrade({ studentId: 1, assessmentId: -5, grade: 17 }), + ); + expect( + updated.submissions.filter( + (s) => s.studentId === 1 && s.assessmentId === -5, + ), + ).toHaveLength(1); + expect( + updated.submissions.find( + (s) => s.studentId === 1 && s.assessmentId === -5, + )?.grade, + ).toBe(17); + }); + + it('setExternalGrade can clear a grade to null', () => { + const next = reducer( + state, + actions.setExternalGrade({ studentId: 1, assessmentId: -5, grade: null }), + ); + expect( + next.submissions.find((s) => s.studentId === 1 && s.assessmentId === -5) + ?.grade, + ).toBeNull(); + }); +}); diff --git a/client/app/bundles/course/gradebook/components/GradebookTable.tsx b/client/app/bundles/course/gradebook/components/GradebookTable.tsx index eac4f66507..3cc71f803b 100644 --- a/client/app/bundles/course/gradebook/components/GradebookTable.tsx +++ b/client/app/bundles/course/gradebook/components/GradebookTable.tsx @@ -9,6 +9,7 @@ import { import { defineMessages } from 'react-intl'; import { Checkbox, + Chip, Paper, type SxProps, Table, @@ -18,10 +19,13 @@ import { TableHead, TableRow, TableSortLabel, + TextField, type Theme, Tooltip, } from '@mui/material'; +import { lighten } from '@mui/material/styles'; import { flexRender } from '@tanstack/react-table'; +import palette from 'theme/palette'; import Link from 'lib/components/core/Link'; import type { @@ -36,10 +40,13 @@ import { DEFAULT_TABLE_ROWS_PER_PAGE, } from 'lib/constants/sharedConstants'; import { getEditSubmissionURL } from 'lib/helpers/url-builders'; +import { useAppDispatch } from 'lib/hooks/store'; +import toast from 'lib/hooks/toast'; import useTranslation from 'lib/hooks/useTranslation'; import tableTranslations from 'lib/translations/table'; import { GAMIFICATION_COL_IDS } from '../constants'; +import { setExternalGrade } from '../operations'; import type { AssessmentData, CategoryData, @@ -65,6 +72,10 @@ const COL_WIDTHS = { const CHECKBOX_WIDTH = 56; +// Faint blue wash that marks external-assessment columns. Kept very light (4% of +// info.main) so the band reads as a subtle tint, not a coloured header. +export const EXTERNAL_ASSESSMENT_BACKGROUND = lighten(palette.info.main, 0.96); + const getColWidth = (id: string): number => COL_WIDTHS[id as keyof typeof COL_WIDTHS] ?? COL_WIDTHS.assessment; @@ -110,6 +121,22 @@ const translations = defineMessages({ defaultMessage: 'No grade or gamification columns selected - export will include student info only.', }, + externalBadge: { + id: 'course.gradebook.GradebookTable.externalBadge', + defaultMessage: 'External', + }, + externalGradeAria: { + id: 'course.gradebook.GradebookTable.externalGradeAria', + defaultMessage: '{title} grade for {name}', + }, + gradeSaveError: { + id: 'course.gradebook.GradebookTable.gradeSaveError', + defaultMessage: 'Could not save the grade. Please try again.', + }, + gradeSaved: { + id: 'course.gradebook.GradebookTable.gradeSaved', + defaultMessage: 'Grade saved. {title} · {name}: {oldGrade} → {newGrade}', + }, }); const HeaderLabel = forwardRef< @@ -177,6 +204,96 @@ const HeaderLabel = forwardRef< }); HeaderLabel.displayName = 'HeaderLabel'; +const ExternalGradeCell = ({ + assessmentId, + studentId, + studentName, + title, + value, +}: { + assessmentId: number; + studentId: number; + studentName: string; + title: string; + value: number | null | undefined; +}): JSX.Element => { + const { t } = useTranslation(); + const dispatch = useAppDispatch(); + const [editing, setEditing] = useState(false); + const [text, setText] = useState(''); + const [localValue, setLocalValue] = useState( + value, + ); + + const commit = async (): Promise => { + setEditing(false); + const trimmed = text.trim(); + const next = trimmed === '' ? null : Number(trimmed); + if (trimmed !== '' && Number.isNaN(next)) return; + if (next === (localValue ?? null)) return; + const prev = localValue; + setLocalValue(next); + try { + await dispatch(setExternalGrade(assessmentId, studentId, next)); + // Persistent confirmation: external grades flow to exported finals, and the + // optimistic update has already discarded the old value from the cell — this + // toast is the only in-session record of what changed, so it must echo the + // full mutation (who · which assessment · old → new) to catch a row/column + // misclick and stay until the user dismisses it (no auto-dismiss). + toast.success( + t(translations.gradeSaved, { + title, + name: studentName, + oldGrade: prev ?? '—', + newGrade: next ?? '—', + }), + { autoClose: false }, + ); + } catch { + setLocalValue(prev); + toast.error(t(translations.gradeSaveError)); + } + }; + + if (editing) { + return ( + setText(e.target.value)} + onKeyDown={(e) => { + if (e.key === 'Enter') commit(); + if (e.key === 'Escape') setEditing(false); + }} + size="small" + value={text} + variant="standard" + /> + ); + } + + return ( + { + setText(localValue == null ? '' : String(localValue)); + setEditing(true); + }} + role="button" + style={{ cursor: 'pointer', display: 'inline-block', minWidth: 24 }} + tabIndex={0} + > + {localValue == null ? '—' : localValue} + + ); +}; + interface GradebookRow { studentId: number; name: string; @@ -212,14 +329,14 @@ const GradebookTable = ({ const { t } = useTranslation(); const submissionsByStudent = useMemo(() => { - const map = new Map>(); + const map = new Map(); submissions.forEach((s) => { - let byAssessment = map.get(s.studentId); - if (!byAssessment) { - byAssessment = new Map(); - map.set(s.studentId, byAssessment); + const existing = map.get(s.studentId); + if (existing) { + existing.push(s); + } else { + map.set(s.studentId, [s]); } - byAssessment.set(s.assessmentId, s); }); return map; }, [submissions]); @@ -227,11 +344,11 @@ const GradebookTable = ({ const rows = useMemo( () => students.map((student) => { - const subs = submissionsByStudent.get(student.id); + const subs = submissionsByStudent.get(student.id) ?? []; const grades: Partial> = {}; const submissionIds: Partial> = {}; assessments.forEach((a) => { - const sub = subs?.get(a.id); + const sub = subs.find((s) => s.assessmentId === a.id); if (sub != null) { grades[a.id] = sub.grade; submissionIds[a.id] = sub.submissionId; @@ -329,6 +446,17 @@ const GradebookTable = ({ }, }, cell: (row) => { + if (asn.external) { + return ( + + ); + } const grade = row.grades[asn.id]; if (grade === undefined) return '—'; if (grade === null) return ''; @@ -361,6 +489,16 @@ const GradebookTable = ({ [assessments], ); + const externalAssessmentColumnIds = useMemo( + () => + new Set( + assessments + .filter((assessment) => assessment.external) + .map((assessment) => buildAssessmentColumnId(assessment.id)), + ), + [assessments], + ); + const columnPicker = useMemo( () => ({ render: (context: ColumnPickerRenderContext) => ( @@ -597,6 +735,7 @@ const GradebookTable = ({ const isLeft = isLeftAligned(id); const fits = headerFits[id] ?? false; const sort = sortByColId.get(id); + const isExternalCol = externalAssessmentColumnIds.has(id); const labelNode = ( @@ -607,6 +746,17 @@ const GradebookTable = ({ ); + const sortedLabel = sort ? ( + + {labelNode} + + ) : ( + labelNode + ); return ( - {sort ? ( - - {labelNode} - + {sortedLabel} + + ) : ( - labelNode + sortedLabel )} ); @@ -675,11 +831,11 @@ const GradebookTable = ({ {visibleCols.map((c) => { const id = c.id ?? (c.of as string); const asnId = parseAssessmentColumnId(id); - let cellContent: string = ''; - if (id === 'name') cellContent = t(translations.maxMarks); + let cellNode: React.ReactNode = ''; + if (id === 'name') cellNode = t(translations.maxMarks); else if (asnId !== null) { const maxGrade = assessmentMaxGrades.get(asnId); - cellContent = maxGrade != null ? `/${maxGrade}` : ''; + cellNode = maxGrade != null ? `/${maxGrade}` : ''; } return ( - {cellContent} + {cellNode} ); })} @@ -773,17 +929,22 @@ const GradebookTable = ({ borderRight: `1px solid ${theme.palette.grey[200]}`, }, }); + let cellSx: SxProps | undefined; + if (cell.column.id === 'name') cellSx = nameCellSx; + else if ( + externalAssessmentColumnIds.has(cell.column.id) + ) + // bgcolor is the faint external-column tint. + cellSx = { + bgcolor: EXTERNAL_ASSESSMENT_BACKGROUND, + }; return ( {flexRender( cell.column.columnDef.cell, diff --git a/client/app/bundles/course/gradebook/components/WeightedGradebookTable.tsx b/client/app/bundles/course/gradebook/components/WeightedGradebookTable.tsx index d9fe764ae0..ffdc9e6a24 100644 --- a/client/app/bundles/course/gradebook/components/WeightedGradebookTable.tsx +++ b/client/app/bundles/course/gradebook/components/WeightedGradebookTable.tsx @@ -433,7 +433,7 @@ const WeightedGradebookTable = ({ const breakdownDisplayValue = (a: AssessmentContribution): number | null => { if (displayMode === 'percent') { - return a.grade === null ? null : gradeRatio(a.grade, a.maxGrade) * 100; + return a.grade === null ? null : a.ratio * 100; } return a.points; }; diff --git a/client/app/bundles/course/gradebook/components/buildAssessmentColumnIds.ts b/client/app/bundles/course/gradebook/components/buildAssessmentColumnIds.ts index d12a4bd26a..13f84cf48a 100644 --- a/client/app/bundles/course/gradebook/components/buildAssessmentColumnIds.ts +++ b/client/app/bundles/course/gradebook/components/buildAssessmentColumnIds.ts @@ -2,6 +2,6 @@ export const buildAssessmentColumnId = (asnId: number): string => `asn-${asnId}`; export const parseAssessmentColumnId = (colId: string): number | null => { - const match = colId.match(/^asn-(\d+)$/); + const match = colId.match(/^asn-(-?\d+)$/); return match ? Number(match[1]) : null; }; diff --git a/client/app/bundles/course/gradebook/computeWeighted.ts b/client/app/bundles/course/gradebook/computeWeighted.ts index 0382eda80e..3c651bb8d2 100644 --- a/client/app/bundles/course/gradebook/computeWeighted.ts +++ b/client/app/bundles/course/gradebook/computeWeighted.ts @@ -29,8 +29,11 @@ export interface WeightedRow { export interface AssessmentContribution { assessmentId: number; title: string; - grade: number | null; + grade: number | null; // raw grade, for the "grade/maxGrade" display maxGrade: number; + // Bounded fraction earned (effectiveGrade/maxGrade), 0 when ungraded. Use this — + // not grade/maxGrade — for the percent display so it matches the weighted points. + ratio: number; points: number; // contribution to this tab's weighted-points cell // Share of the overall grade this assessment carries, in percentage points. // Equal mode: the tab's weight split evenly across its assessments. @@ -56,6 +59,19 @@ const gradeKey = (studentId: number, assessmentId: number): string => export const gradeRatio = (grade: number, maxGrade: number): number => maxGrade === 0 ? 0 : grade / maxGrade; +// Per-assessment grade bounding (external assessments only). Applied at READ time +// so the toggles stay reversible and the stored grade is never mutated. Native +// assessments leave both flags undefined → passthrough (unchanged behaviour). +export const effectiveGrade = ( + grade: number, + a: Pick, +): number => { + let g = grade; + if (a.floorAtZero && g < 0) g = 0; + if (a.capAtMaximum && g > a.maxGrade) g = a.maxGrade; + return g; +}; + // Index submissions by (student, assessment) once: O(submissions). const buildGradeLookup = (submissions: GradeEntry[]): GradeLookup => { const lookup: GradeLookup = new Map(); @@ -94,7 +110,7 @@ const equalSubtotal = ( const keepN = tab.keepHighest ?? 0; const ratios = included.map((a) => { const grade = gradeLookup.get(gradeKey(studentId, a.id)); - return grade != null ? gradeRatio(grade, a.maxGrade) : 0; + return grade != null ? gradeRatio(effectiveGrade(grade, a), a.maxGrade) : 0; }); ratios.sort((x, y) => x - y); // ascending const keep = keepN > 0 ? Math.min(keepN, included.length) : included.length; @@ -120,7 +136,8 @@ const customSubtotal = ( const grade = gradeLookup.get(gradeKey(studentId, a.id)); const assessmentWeight = a.gradebookWeight ?? 0; if (grade != null) - numerator += gradeRatio(grade, a.maxGrade) * assessmentWeight; + numerator += + gradeRatio(effectiveGrade(grade, a), a.maxGrade) * assessmentWeight; hasContributing = true; }); return hasContributing ? numerator / tabWeight : null; @@ -318,7 +335,8 @@ export const computeStudentBreakdown = ({ const excluded = !!a.gradebookExcluded; const dropped = droppedIds.has(a.id); const grade = gradeLookup.get(gradeKey(studentId, a.id)) ?? null; - const ratio = grade != null ? gradeRatio(grade, a.maxGrade) : 0; + const ratio = + grade != null ? gradeRatio(effectiveGrade(grade, a), a.maxGrade) : 0; let points: number; let effectiveWeight: number; if (excluded || dropped) { @@ -336,6 +354,7 @@ export const computeStudentBreakdown = ({ title: a.title, grade, maxGrade: a.maxGrade, + ratio, points, effectiveWeight, excluded, @@ -355,6 +374,7 @@ export const computeStudentBreakdown = ({ title: 'Level', grade: level ?? 0, maxGrade: courseMaxLevel ?? 0, + ratio: gradeRatio(level ?? 0, courseMaxLevel ?? 0), points: lvl, effectiveWeight: levelContribution.weight, excluded: false, diff --git a/client/app/bundles/course/gradebook/operations.ts b/client/app/bundles/course/gradebook/operations.ts index 344f24fb7c..55ad9e6827 100644 --- a/client/app/bundles/course/gradebook/operations.ts +++ b/client/app/bundles/course/gradebook/operations.ts @@ -34,4 +34,31 @@ export const updateGradebookWeights = dispatch(actions.updateTabWeights(responseData)); }; +// Optimistic: apply the new grade immediately, then reconcile with the server. +// On failure, restore the previous value and rethrow so the caller can toast. +export const setExternalGrade = + (assessmentId: number, studentId: number, grade: number | null): Operation => + async (dispatch, getState) => { + const prev = + getState().gradebook.submissions.find( + (s) => s.studentId === studentId && s.assessmentId === assessmentId, + )?.grade ?? null; + dispatch(actions.setExternalGrade({ studentId, assessmentId, grade })); + try { + const response = await CourseAPI.gradebook.setExternalGrade( + -assessmentId, + { + studentId, + grade, + }, + ); + dispatch(actions.setExternalGrade(response.data)); + } catch (error) { + dispatch( + actions.setExternalGrade({ studentId, assessmentId, grade: prev }), + ); + throw error; + } + }; + export default fetchGradebook; diff --git a/client/app/bundles/course/gradebook/selectors.ts b/client/app/bundles/course/gradebook/selectors.ts index 6256f7af9f..e714e12b97 100644 --- a/client/app/bundles/course/gradebook/selectors.ts +++ b/client/app/bundles/course/gradebook/selectors.ts @@ -32,3 +32,7 @@ export const getLevelContribution = ( getLocalState(state).levelContribution; export const getCourseMaxLevel = (state: AppState): number => getLocalState(state).courseMaxLevel; +export const getExternalAssessments = ( + state: AppState, +): GradebookState['assessments'] => + getLocalState(state).assessments.filter((a) => a.external); diff --git a/client/app/bundles/course/gradebook/store.ts b/client/app/bundles/course/gradebook/store.ts index 72abd1d4b9..dbd74a8342 100644 --- a/client/app/bundles/course/gradebook/store.ts +++ b/client/app/bundles/course/gradebook/store.ts @@ -1,5 +1,6 @@ import { produce } from 'immer'; import type { + ExternalGradePayload, GradebookData, LevelContributionData, UpdateWeightsPayload, @@ -16,6 +17,7 @@ import type { const SAVE_GRADEBOOK = 'course/gradebook/SAVE_GRADEBOOK'; const UPDATE_TAB_WEIGHTS = 'course/gradebook/UPDATE_TAB_WEIGHTS'; +const SET_EXTERNAL_GRADE = 'course/gradebook/SET_EXTERNAL_GRADE'; interface GradebookState { categories: CategoryData[]; @@ -40,6 +42,11 @@ interface UpdateTabWeightsAction { payload: UpdateWeightsPayload; } +interface SetExternalGradeAction { + type: typeof SET_EXTERNAL_GRADE; + payload: ExternalGradePayload; +} + const initialState: GradebookState = { categories: [], tabs: [], @@ -62,7 +69,10 @@ const initialState: GradebookState = { const reducer = produce( ( draft: GradebookState, - action: SaveGradebookAction | UpdateTabWeightsAction, + action: + | SaveGradebookAction + | UpdateTabWeightsAction + | SetExternalGradeAction, ) => { switch (action.type) { case SAVE_GRADEBOOK: { @@ -136,6 +146,15 @@ const reducer = produce( } break; } + case SET_EXTERNAL_GRADE: { + const { studentId, assessmentId, grade } = action.payload; + const existing = draft.submissions.find( + (s) => s.studentId === studentId && s.assessmentId === assessmentId, + ); + if (existing) existing.grade = grade; + else draft.submissions.push({ studentId, assessmentId, grade }); + break; + } default: break; } @@ -154,6 +173,9 @@ export const actions = { type: UPDATE_TAB_WEIGHTS, payload, }), + setExternalGrade: ( + payload: ExternalGradePayload, + ): SetExternalGradeAction => ({ type: SET_EXTERNAL_GRADE, payload }), }; export default reducer; diff --git a/client/app/bundles/course/statistics/pages/StatisticsIndex/index.tsx b/client/app/bundles/course/statistics/pages/StatisticsIndex/index.tsx index d1dd39720a..c1e230f896 100644 --- a/client/app/bundles/course/statistics/pages/StatisticsIndex/index.tsx +++ b/client/app/bundles/course/statistics/pages/StatisticsIndex/index.tsx @@ -115,7 +115,10 @@ const StatisticsIndex: FC = () => { return ( - + { diff --git a/client/app/lib/components/table/TanStackTableBuilder/columnsBuilder.ts b/client/app/lib/components/table/TanStackTableBuilder/columnsBuilder.ts index 8ebb312766..2b1a825b0e 100644 --- a/client/app/lib/components/table/TanStackTableBuilder/columnsBuilder.ts +++ b/client/app/lib/components/table/TanStackTableBuilder/columnsBuilder.ts @@ -61,6 +61,9 @@ const buildTanStackColumns = ( column.sortProps!.sort!(rowA.original, rowB.original) : 'alphanumeric', sortUndefined: column.sortProps?.undefinedPriority ?? false, + ...(column.sortProps?.descFirst !== undefined && { + sortDescFirst: column.sortProps.descFirst, + }), filterFn: column.filterProps?.shouldInclude && Object.assign( diff --git a/client/app/lib/components/table/builder/ColumnTemplate.ts b/client/app/lib/components/table/builder/ColumnTemplate.ts index cbfe6132ac..265e16f7f8 100644 --- a/client/app/lib/components/table/builder/ColumnTemplate.ts +++ b/client/app/lib/components/table/builder/ColumnTemplate.ts @@ -17,6 +17,7 @@ interface SearchingProps { interface SortingProps { sort?: (datumA: D, datumB: D) => number; undefinedPriority?: false | 'first' | 'last'; + descFirst?: boolean; } interface ColumnTemplate { diff --git a/client/app/lib/components/table/utils.ts b/client/app/lib/components/table/utils.ts index 6b84e7a373..916dfdba69 100644 --- a/client/app/lib/components/table/utils.ts +++ b/client/app/lib/components/table/utils.ts @@ -2,10 +2,13 @@ import { downloadFile } from 'utilities/downloadFile'; const DEFAULT_CSV_FILENAME = 'data' as const; +// Prepend UTF-8 BOM so Excel on macOS/Windows detects UTF-8 encoding instead +// of falling back to a legacy code page (Mac Roman / Windows-1252), which +// misreads multibyte characters like em dash (U+2014) as mojibake (e.g. "‚Äî"). export const downloadCsv = (csvData: string, filename?: string): void => { downloadFile( 'text/csv;charset=utf-8', - csvData, + `\uFEFF${csvData}`, `${filename ?? DEFAULT_CSV_FILENAME}.csv`, ); }; diff --git a/client/app/types/course/gradebook.ts b/client/app/types/course/gradebook.ts index 722f56ccbb..37eca4530a 100644 --- a/client/app/types/course/gradebook.ts +++ b/client/app/types/course/gradebook.ts @@ -19,6 +19,9 @@ export interface AssessmentData { maxGrade: number; gradebookWeight?: number | null; gradebookExcluded?: boolean; + external?: boolean; + floorAtZero?: boolean; + capAtMaximum?: boolean; } export interface StudentData { @@ -34,7 +37,7 @@ export interface StudentData { export interface SubmissionData { studentId: number; assessmentId: number; - submissionId: number; + submissionId?: number; grade: number | null; } @@ -87,3 +90,9 @@ export type FormulaNode = export interface LevelContributionSaveData extends LevelContributionData { formulaAst: FormulaNode | null; } + +export interface ExternalGradePayload { + studentId: number; + assessmentId: number; + grade: number | null; +} diff --git a/client/locales/en.json b/client/locales/en.json index d27bf74585..22d42bc456 100644 --- a/client/locales/en.json +++ b/client/locales/en.json @@ -9664,5 +9664,17 @@ }, "course.gradebook.WeightedViewHint.settingsLink": { "defaultMessage": "Gradebook settings" + }, + "course.gradebook.GradebookTable.externalBadge": { + "defaultMessage": "External" + }, + "course.gradebook.GradebookTable.externalGradeAria": { + "defaultMessage": "{title} grade for {name}" + }, + "course.gradebook.GradebookTable.gradeSaveError": { + "defaultMessage": "Could not save the grade. Please try again." + }, + "course.gradebook.GradebookTable.gradeSaved": { + "defaultMessage": "Grade saved. {title} · {name}: {oldGrade} → {newGrade}" } } diff --git a/client/locales/ko.json b/client/locales/ko.json index 641d51ce0f..07fff21080 100644 --- a/client/locales/ko.json +++ b/client/locales/ko.json @@ -9655,5 +9655,17 @@ }, "course.gradebook.WeightedViewHint.settingsLink": { "defaultMessage": "성적부 설정" + }, + "course.gradebook.GradebookTable.externalBadge": { + "defaultMessage": "외부" + }, + "course.gradebook.GradebookTable.externalGradeAria": { + "defaultMessage": "{name}의 {title} 성적" + }, + "course.gradebook.GradebookTable.gradeSaveError": { + "defaultMessage": "성적을 저장할 수 없습니다. 다시 시도해 주세요." + }, + "course.gradebook.GradebookTable.gradeSaved": { + "defaultMessage": "성적이 저장되었습니다. {title} · {name}: {oldGrade} → {newGrade}" } } diff --git a/client/locales/zh.json b/client/locales/zh.json index 5ff7532afa..f46c1576a1 100644 --- a/client/locales/zh.json +++ b/client/locales/zh.json @@ -9649,5 +9649,17 @@ }, "course.gradebook.WeightedViewHint.settingsLink": { "defaultMessage": "成绩册设置" + }, + "course.gradebook.GradebookTable.externalBadge": { + "defaultMessage": "外部" + }, + "course.gradebook.GradebookTable.externalGradeAria": { + "defaultMessage": "{name} 的 {title} 成绩" + }, + "course.gradebook.GradebookTable.gradeSaveError": { + "defaultMessage": "无法保存成绩。请重试。" + }, + "course.gradebook.GradebookTable.gradeSaved": { + "defaultMessage": "成绩已保存。{title} · {name}：{oldGrade} → {newGrade}" } } diff --git a/config/locales/en/activerecord/errors.yml b/config/locales/en/activerecord/errors.yml index 713858a6dc..c425d123fc 100644 --- a/config/locales/en/activerecord/errors.yml +++ b/config/locales/en/activerecord/errors.yml @@ -6,6 +6,10 @@ en: attributes: reference_timelines: must_have_at_most_one_default: 'must have at most one default' + course/gradebook/contribution: + attributes: + base: + exactly_one_contributor: "must reference exactly one contributor (a tab or an external assessment)" course/announcement: attributes: end_at: diff --git a/config/locales/ko/activerecord/errors.yml b/config/locales/ko/activerecord/errors.yml index 6b6c01e11c..76f97ba371 100644 --- a/config/locales/ko/activerecord/errors.yml +++ b/config/locales/ko/activerecord/errors.yml @@ -6,6 +6,10 @@ ko: attributes: reference_timelines: must_have_at_most_one_default: '최대 하나의 기본값만 있어야 합니다' + course/gradebook/contribution: + attributes: + base: + exactly_one_contributor: '정확히 하나의 기여 대상(탭 또는 외부 평가)을 참조해야 합니다' course/announcement: attributes: end_at: diff --git a/config/locales/zh/activerecord/errors.yml b/config/locales/zh/activerecord/errors.yml index 6a654814f3..c67c1acf63 100644 --- a/config/locales/zh/activerecord/errors.yml +++ b/config/locales/zh/activerecord/errors.yml @@ -6,6 +6,10 @@ zh: attributes: reference_timelines: must_have_at_most_one_default: '至少要包含一个默认值' + course/gradebook/contribution: + attributes: + base: + exactly_one_contributor: '必须且只能引用一个贡献项（一个标签页或一个外部评估）' course/announcement: attributes: end_at: diff --git a/config/routes.rb b/config/routes.rb index e0c882af7f..e04fd4d813 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -501,6 +501,11 @@ resource :gradebook, only: [] do get '/' => 'gradebook#index' patch '/weights' => 'gradebook#update_weights' + resources :external_assessments, only: [] do + member do + put 'grades' => 'external_assessments#grades' + end + end end scope module: :discussion do diff --git a/db/migrate/20260624000000_create_course_external_assessment_tables.rb b/db/migrate/20260624000000_create_course_external_assessment_tables.rb new file mode 100644 index 0000000000..949ba7ca51 --- /dev/null +++ b/db/migrate/20260624000000_create_course_external_assessment_tables.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true +# Squashes the original external-assessment migrations (20260615..20260623) into one: +# - create course_external_assessments / course_external_assessment_grades +# - create course_gradebook_external_contributions +# - floor_at_zero / cap_at_maximum bounds +# - grade columns at decimal(5,2) (Canvas/SoftMark record two decimal places) +class CreateCourseExternalAssessmentTables < ActiveRecord::Migration[7.2] + def change + create_table :course_external_assessments do |t| + t.references :course, null: false, + foreign_key: { to_table: :courses, + name: 'fk_course_external_assessments_course_id' }, + index: { name: 'fk__course_external_assessments_course_id' } + t.string :title, null: false + t.decimal :maximum_grade, precision: 5, scale: 2, null: false + t.boolean :floor_at_zero, null: false, default: true + t.boolean :cap_at_maximum, null: false, default: true + t.references :creator, null: false, + foreign_key: { to_table: :users, + name: 'fk_course_external_assessments_creator_id' }, + index: { name: 'fk__course_external_assessments_creator_id' } + t.references :updater, null: false, + foreign_key: { to_table: :users, + name: 'fk_course_external_assessments_updater_id' }, + index: { name: 'fk__course_external_assessments_updater_id' } + t.timestamps null: false + end + add_index :course_external_assessments, [:course_id, :title], + unique: true, name: 'index_course_external_assessments_on_course_id_and_title' + + create_table :course_external_assessment_grades do |t| + t.references :external_assessment, null: false, + foreign_key: { to_table: :course_external_assessments, + name: 'fk_course_external_assessment_grades_' \ + 'external_assessment_id' }, + index: { name: 'fk__course_external_assessment_grades_external_assessment_id' } + t.references :course_user, null: false, + foreign_key: { to_table: :course_users, + name: 'fk_course_external_assessment_grades_course_user_id' }, + index: { name: 'fk__course_external_assessment_grades_course_user_id' } + t.decimal :grade, precision: 5, scale: 2, null: true + t.string :imported_identifier, null: true + t.references :creator, null: false, + foreign_key: { to_table: :users, name: 'fk_course_external_assessment_grades_creator_id' }, + index: { name: 'fk__course_external_assessment_grades_creator_id' } + t.references :updater, null: false, + foreign_key: { to_table: :users, name: 'fk_course_external_assessment_grades_updater_id' }, + index: { name: 'fk__course_external_assessment_grades_updater_id' } + t.timestamps null: false + end + add_index :course_external_assessment_grades, [:external_assessment_id, :course_user_id], + unique: true, name: 'index_course_external_assessment_grades_on_ea_id_and_cu_id' + + # cgec = course_gradebook_external_contributions (abbreviated index/FK names mirror the + # cgac block; the full name would exceed Postgres' 63-char identifier limit). + create_table :course_gradebook_external_contributions do |t| + t.references :course, null: false, foreign_key: { to_table: :courses }, + index: { name: 'fk__cgec_course_id' } + t.references :external_assessment, null: false, + foreign_key: { to_table: :course_external_assessments, + on_delete: :cascade }, + index: { unique: true, name: 'index_cgec_on_external_assessment_id' } + t.decimal :weight, precision: 5, scale: 2, null: false, default: 0 + + t.references :creator, null: false, foreign_key: { to_table: :users }, + index: { name: 'fk__cgec_creator_id' } + t.references :updater, null: false, foreign_key: { to_table: :users }, + index: { name: 'fk__cgec_updater_id' } + t.timestamps null: false + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 582c239887..bb3a423813 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.2].define(version: 2026_06_18_000000) do +ActiveRecord::Schema[7.2].define(version: 2026_06_24_000000) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" enable_extension "uuid-ossp" @@ -764,6 +764,38 @@ t.index ["updater_id"], name: "fk__course_experience_points_records_updater_id" end + create_table "course_external_assessment_grades", force: :cascade do |t| + t.bigint "external_assessment_id", null: false + t.bigint "course_user_id", null: false + t.decimal "grade", precision: 5, scale: 2 + t.string "imported_identifier" + t.bigint "creator_id", null: false + t.bigint "updater_id", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["course_user_id"], name: "fk__course_external_assessment_grades_course_user_id" + t.index ["creator_id"], name: "fk__course_external_assessment_grades_creator_id" + t.index ["external_assessment_id", "course_user_id"], name: "index_course_external_assessment_grades_on_ea_id_and_cu_id", unique: true + t.index ["external_assessment_id"], name: "fk__course_external_assessment_grades_external_assessment_id" + t.index ["updater_id"], name: "fk__course_external_assessment_grades_updater_id" + end + + create_table "course_external_assessments", force: :cascade do |t| + t.bigint "course_id", null: false + t.string "title", null: false + t.decimal "maximum_grade", precision: 5, scale: 2, null: false + t.bigint "creator_id", null: false + t.bigint "updater_id", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.boolean "floor_at_zero", default: true, null: false + t.boolean "cap_at_maximum", default: true, null: false + t.index ["course_id", "title"], name: "index_course_external_assessments_on_course_id_and_title", unique: true + t.index ["course_id"], name: "fk__course_external_assessments_course_id" + t.index ["creator_id"], name: "fk__course_external_assessments_creator_id" + t.index ["updater_id"], name: "fk__course_external_assessments_updater_id" + end + create_table "course_forum_discussion_references", force: :cascade do |t| t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false @@ -871,6 +903,20 @@ t.index ["updater_id"], name: "fk__cgac_updater_id" end + create_table "course_gradebook_external_contributions", force: :cascade do |t| + t.bigint "course_id", null: false + t.bigint "external_assessment_id", null: false + t.decimal "weight", precision: 5, scale: 2, default: "0.0", null: false + t.bigint "creator_id", null: false + t.bigint "updater_id", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["course_id"], name: "fk__cgec_course_id" + t.index ["creator_id"], name: "fk__cgec_creator_id" + t.index ["external_assessment_id"], name: "index_cgec_on_external_assessment_id", unique: true + t.index ["updater_id"], name: "fk__cgec_updater_id" + end + create_table "course_gradebook_level_configs", force: :cascade do |t| t.bigint "course_id", null: false t.boolean "enabled", default: false, null: false @@ -1950,6 +1996,13 @@ add_foreign_key "course_experience_points_records", "users", column: "awarder_id", name: "fk_course_experience_points_records_awarder_id" add_foreign_key "course_experience_points_records", "users", column: "creator_id", name: "fk_course_experience_points_records_creator_id" add_foreign_key "course_experience_points_records", "users", column: "updater_id", name: "fk_course_experience_points_records_updater_id" + add_foreign_key "course_external_assessment_grades", "course_external_assessments", column: "external_assessment_id", name: "fk_course_external_assessment_grades_external_assessment_id" + add_foreign_key "course_external_assessment_grades", "course_users", name: "fk_course_external_assessment_grades_course_user_id" + add_foreign_key "course_external_assessment_grades", "users", column: "creator_id", name: "fk_course_external_assessment_grades_creator_id" + add_foreign_key "course_external_assessment_grades", "users", column: "updater_id", name: "fk_course_external_assessment_grades_updater_id" + add_foreign_key "course_external_assessments", "courses", name: "fk_course_external_assessments_course_id" + add_foreign_key "course_external_assessments", "users", column: "creator_id", name: "fk_course_external_assessments_creator_id" + add_foreign_key "course_external_assessments", "users", column: "updater_id", name: "fk_course_external_assessments_updater_id" add_foreign_key "course_forum_discussion_references", "course_forum_discussions", column: "discussion_id", name: "fk_course_forum_discussion_references_discussion_id" add_foreign_key "course_forum_discussion_references", "course_forum_imports", column: "forum_import_id", name: "fk_course_forum_discussion_references_forum_import_id" add_foreign_key "course_forum_discussion_references", "users", column: "creator_id", name: "fk_course_forum_discussion_references_creator_id" @@ -1972,6 +2025,10 @@ add_foreign_key "course_gradebook_assessment_contributions", "course_assessments", column: "assessment_id", on_delete: :cascade add_foreign_key "course_gradebook_assessment_contributions", "users", column: "creator_id" add_foreign_key "course_gradebook_assessment_contributions", "users", column: "updater_id" + add_foreign_key "course_gradebook_external_contributions", "course_external_assessments", column: "external_assessment_id", on_delete: :cascade + add_foreign_key "course_gradebook_external_contributions", "courses" + add_foreign_key "course_gradebook_external_contributions", "users", column: "creator_id" + add_foreign_key "course_gradebook_external_contributions", "users", column: "updater_id" add_foreign_key "course_gradebook_level_configs", "courses", on_delete: :cascade add_foreign_key "course_gradebook_level_configs", "users", column: "creator_id" add_foreign_key "course_gradebook_level_configs", "users", column: "updater_id" diff --git a/spec/controllers/course/external_assessments_controller_spec.rb b/spec/controllers/course/external_assessments_controller_spec.rb new file mode 100644 index 0000000000..c235b0e628 --- /dev/null +++ b/spec/controllers/course/external_assessments_controller_spec.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::ExternalAssessmentsController, type: :controller do + let(:instance) { Instance.default } + + with_tenant(:instance) do + let(:course) { create(:course) } + let(:manager) { create(:course_manager, course: course) } + let(:ta) { create(:course_teaching_assistant, course: course) } + + describe '#grades' do + render_views + let!(:external) { create(:course_external_assessment, course: course) } + let(:gb_student) { create(:course_student, course: course) } + + context 'as a manager' do + before { controller_sign_in(controller, manager.user) } + + # The gradebook frontend keys students by user_id (json.studentId == course_user.user_id), + # so #grades must resolve the course_user from the `studentId` param, not a course_user PK. + it 'inserts a grade for a student who has none' do + expect do + put :grades, params: { course_id: course.id, id: external.id, format: :json, + studentId: gb_student.user_id, grade: 88 } + end.to change { Course::ExternalAssessmentGrade.count }.by(1) + expect(response).to be_successful + data = JSON.parse(response.body) + expect(data['studentId']).to eq(gb_student.user_id) + expect(data['assessmentId']).to eq(-external.id) + expect(data['grade']).to eq(88.0) + expect(Course::ExternalAssessmentGrade.last.course_user).to eq(gb_student) + end + + it 'updates an existing grade in place (no duplicate row)' do + grade = create(:course_external_assessment_grade, + external_assessment: external, course_user: gb_student, grade: 10) + expect do + put :grades, params: { course_id: course.id, id: external.id, format: :json, + studentId: gb_student.user_id, grade: 20 } + end.not_to(change { Course::ExternalAssessmentGrade.count }) + expect(grade.reload.grade).to eq(20) + end + + it 'stores a grade with two decimal places without rounding' do + put :grades, params: { course_id: course.id, id: external.id, format: :json, + studentId: gb_student.user_id, grade: '87.25' } + expect(response).to be_successful + expect(Course::ExternalAssessmentGrade.last.grade).to eq(BigDecimal('87.25')) + end + + it 'clears a grade to null (ungraded) when grade is blank' do + grade = create(:course_external_assessment_grade, + external_assessment: external, course_user: gb_student, grade: 10) + put :grades, params: { course_id: course.id, id: external.id, format: :json, + studentId: gb_student.user_id, grade: '' } + expect(grade.reload.grade).to be_nil + end + + it 'returns 404 when the student does not belong to the course' do + other_student = create(:course_student) + put :grades, params: { course_id: course.id, id: external.id, format: :json, + studentId: other_student.user_id, grade: 50 } + expect(response).to have_http_status(:not_found) + end + end + + context 'as a teaching assistant' do + before { controller_sign_in(controller, ta.user) } + + it 'is denied' do + expect do + put :grades, params: { course_id: course.id, id: external.id, format: :json, + studentId: gb_student.user_id, grade: 5 } + end.to raise_error(CanCan::AccessDenied) + end + end + + context 'as a student' do + let(:viewer) { create(:course_student, course: course) } + before { controller_sign_in(controller, viewer.user) } + + it 'is denied' do + expect do + put :grades, params: { course_id: course.id, id: external.id, format: :json, + studentId: gb_student.user_id, grade: 5 } + end.to raise_error(CanCan::AccessDenied) + end + end + end + end +end diff --git a/spec/controllers/course/gradebook_controller_spec.rb b/spec/controllers/course/gradebook_controller_spec.rb index 499438a5c3..9c8defbe73 100644 --- a/spec/controllers/course/gradebook_controller_spec.rb +++ b/spec/controllers/course/gradebook_controller_spec.rb @@ -326,6 +326,114 @@ expect(sub['grade']).to be_nil end end + + context 'when the course has an external assessment' do + render_views + let(:manager) { create(:course_manager, course: course) } + let(:gb_student) { create(:course_student, course: course) } + let!(:external) do + create(:course_external_assessment, course: course, title: 'Midterm', maximum_grade: 50) + end + let!(:external_grade) do + create(:course_external_assessment_grade, + external_assessment: external, course_user: gb_student, grade: 41) + end + before { controller_sign_in(controller, manager.user) } + + it 'merges the external into assessments with a negative id and external flag' do + subject + data = JSON.parse(response.body) + ext_row = data['assessments'].find { |a| a['id'] == -external.id } + expect(ext_row).to be_present + expect(ext_row['title']).to eq('Midterm') + expect(ext_row['external']).to be(true) + expect(ext_row['maxGrade']).to eq(50.0) + expect(ext_row['tabId']).to eq(external.synthetic_tab_id) + end + + it 'merges the external grade into submissions with a negative assessmentId' do + subject + data = JSON.parse(response.body) + sub = data['submissions'].find { |s| s['assessmentId'] == -external.id } + expect(sub).to be_present + expect(sub['studentId']).to eq(gb_student.user_id) + expect(sub['grade']).to eq(41.0) + end + + it 'emits a synthetic External Assessments category' do + subject + data = JSON.parse(response.body) + cat = data['categories'].find { |c| c['id'] == Course::ExternalAssessment::SYNTHETIC_CATEGORY_ID } + expect(cat).to be_present + expect(cat['title']).to eq('External Assessments') + end + + it 'emits a synthetic tab with negative id under the synthetic category' do + subject + data = JSON.parse(response.body) + tab = data['tabs'].find { |t| t['id'] == external.synthetic_tab_id } + expect(tab).to be_present + expect(tab['categoryId']).to eq(Course::ExternalAssessment::SYNTHETIC_CATEGORY_ID) + end + + it 'creates no real tab or category for the external' do + tab_count_before = course.assessment_tabs.count + cat_count_before = course.assessment_categories.count + subject + expect(course.assessment_tabs.count).to eq(tab_count_before) + expect(course.assessment_categories.count).to eq(cat_count_before) + expect(course.assessment_categories.where(title: 'External Assessments')).to be_empty + end + end + end + + describe 'GET #index with externals' do + render_views + let!(:course) { create(:course) } + let!(:external) do + Course::ExternalAssessment.create_for_course!(course: course, title: 'Midterm', + maximum_grade: 50.0, weight: 40) + end + let(:manager) { create(:course_manager, course: course) } + + before do + ctx = Struct.new(:current_course, :key).new(course, Course::GradebookComponent.key) + Course::Settings::GradebookComponent.new(ctx).weighted_view_enabled = true + course.save! + controller_sign_in(controller, manager.user) + end + + subject(:body) do + get(:index, params: { course_id: course }, format: :json) + JSON.parse(response.body) + end + + it 'emits a synthetic External Assessments category' do + cat = body['categories'].find { |c| c['id'] == Course::ExternalAssessment::SYNTHETIC_CATEGORY_ID } + expect(cat['title']).to eq('External Assessments') + end + + it 'emits one synthetic tab per external carrying its weight' do + tab = body['tabs'].find { |t| t['id'] == -external.id } + expect(tab['categoryId']).to eq(Course::ExternalAssessment::SYNTHETIC_CATEGORY_ID) + expect(tab['gradebookWeight']).to eq(40.0) + expect(tab['weightMode']).to eq('equal') + end + + it 'emits the external as a negative-id leaf under its synthetic tab' do + leaf = body['assessments'].find { |a| a['id'] == -external.id } + expect(leaf['external']).to be(true) + expect(leaf['tabId']).to eq(-external.id) + end + + it 'creates no real tab or category for the external' do + tab_count_before = course.assessment_tabs.count + cat_count_before = course.assessment_categories.count + body + expect(course.assessment_tabs.count).to eq(tab_count_before) + expect(course.assessment_categories.count).to eq(cat_count_before) + expect(course.assessment_categories.where(title: 'External Assessments')).to be_empty + end end describe 'PATCH update_weights' do @@ -490,6 +598,29 @@ def weight_for(tab) }, format: :json expect(response).to have_http_status(:unprocessable_content) end + + it 'persists an external weight to ExternalContribution via its negative tab id' do + external = create(:course_external_assessment, course: course, title: 'Midterm', maximum_grade: 100) + patch :update_weights, + params: { course_id: course.id, weights: [tabId: external.synthetic_tab_id, weight: 35] }, + format: :json + expect(response).to have_http_status(:ok) + expect(Course::Gradebook::ExternalContribution.find_by(external_assessment_id: external.id).weight). + to eq(35) + end + + it 'persists native and external weights together atomically' do + external = create(:course_external_assessment, course: course, title: 'Final', maximum_grade: 100) + patch :update_weights, + params: { course_id: course.id, + weights: [{ tabId: tab1.id, weight: 70 }, + { tabId: external.synthetic_tab_id, weight: 30 }] }, + format: :json + expect(response).to have_http_status(:ok) + expect(Course::Gradebook::TabContribution.find_by(tab_id: tab1.id).weight).to eq(70) + expect(Course::Gradebook::ExternalContribution.find_by(external_assessment_id: external.id).weight). + to eq(30) + end end context 'as TA' do @@ -553,13 +684,13 @@ def weight_for(tab) it 'persists custom mode + assessment weights and echoes them back' do patch :update_weights, as: :json, params: { course_id: course.id, - weights: [{ + weights: [ tabId: tab.id, weight: '50', weightMode: 'custom', assessmentWeights: [ { assessmentId: a1.id, weight: '30' }, { assessmentId: a2.id, weight: '20' } ] - }] + ] } expect(response).to have_http_status(:ok) body = JSON.parse(response.body) @@ -575,10 +706,10 @@ def weight_for(tab) it 'returns 422 when custom weights do not sum to the tab total' do patch :update_weights, as: :json, params: { course_id: course.id, - weights: [{ + weights: [ tabId: tab.id, weight: '50', weightMode: 'custom', - assessmentWeights: [{ assessmentId: a1.id, weight: '10' }] - }] + assessmentWeights: [assessmentId: a1.id, weight: '10'] + ] } expect(response).to have_http_status(:unprocessable_content) end @@ -586,10 +717,10 @@ def weight_for(tab) it 'persists and echoes per-assessment exclusion in equal mode' do patch :update_weights, as: :json, params: { course_id: course.id, - weights: [{ + weights: [ tabId: tab.id, weight: '50', weightMode: 'equal', excludedAssessmentIds: [a1.id] - }] + ] } expect(response).to have_http_status(:ok) expect(a1.reload.gradebook_assessment_contribution.excluded).to eq(true) diff --git a/spec/factories/course_external_assessments.rb b/spec/factories/course_external_assessments.rb new file mode 100644 index 0000000000..5aa07599ba --- /dev/null +++ b/spec/factories/course_external_assessments.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true +FactoryBot.define do + factory :course_external_assessment, class: Course::ExternalAssessment do + course + sequence(:title) { |n| "External #{n}" } + maximum_grade { 100.0 } + end + + factory :course_external_assessment_grade, class: Course::ExternalAssessmentGrade do + external_assessment { association(:course_external_assessment) } + course_user { association(:course_user) } + grade { 50.0 } + end +end diff --git a/spec/factories/course_gradebook_external_contributions.rb b/spec/factories/course_gradebook_external_contributions.rb new file mode 100644 index 0000000000..c6a3c6362f --- /dev/null +++ b/spec/factories/course_gradebook_external_contributions.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true +FactoryBot.define do + factory :course_gradebook_external_contribution, class: Course::Gradebook::ExternalContribution.name do + association :external_assessment, factory: :course_external_assessment + course { external_assessment.course } + weight { 0 } + end +end diff --git a/spec/models/course/external_assessment_grade_spec.rb b/spec/models/course/external_assessment_grade_spec.rb new file mode 100644 index 0000000000..6f0acb69c7 --- /dev/null +++ b/spec/models/course/external_assessment_grade_spec.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::ExternalAssessmentGrade, type: :model do + let(:instance) { Instance.default } + + with_tenant(:instance) do + describe 'validations' do + subject { build(:course_external_assessment_grade) } + + it { is_expected.to be_valid } + + it 'allows a null grade (ungraded)' do + subject.grade = nil + expect(subject).to be_valid + end + + it 'allows a grade greater than the maximum (no ceiling, bonus-consistent)' do + subject.external_assessment.maximum_grade = 10 + subject.grade = 15 + expect(subject).to be_valid + end + + it 'allows a negative grade (penalties; floor applied via floor_at_zero, not validation)' do + subject.grade = -5 + expect(subject).to be_valid + end + + it 'enforces one grade per (external_assessment, course_user)' do + existing = create(:course_external_assessment_grade) + duplicate = build(:course_external_assessment_grade, + external_assessment: existing.external_assessment, + course_user: existing.course_user) + expect(duplicate).not_to be_valid + end + + it 'allows the same course_user under a different external_assessment' do + existing = create(:course_external_assessment_grade) + other = build(:course_external_assessment_grade, course_user: existing.course_user) + expect(other).to be_valid + end + + it 'allows the same external_assessment for a different course_user' do + existing = create(:course_external_assessment_grade) + other = build(:course_external_assessment_grade, + external_assessment: existing.external_assessment) + expect(other).to be_valid + end + + it 'requires a course_user' do + subject.course_user = nil + expect(subject).not_to be_valid + end + + it 'rejects a non-numeric grade string' do + subject.grade = 'abc' + expect(subject).not_to be_valid + end + + it 'requires an external_assessment' do + subject.external_assessment = nil + expect(subject).not_to be_valid + end + end + + describe 'determinacy — grade binds to course_user, not the identifier string' do + it 'does not move an existing grade when the student external_id changes after import' do + grade = create(:course_external_assessment_grade, imported_identifier: 'A0001X', grade: 5) + course_user = grade.course_user + + course_user.update!(external_id: 'A9999Z') + + expect(grade.reload.course_user_id).to eq(course_user.id) + expect(grade.grade).to eq(5) + end + end + end +end diff --git a/spec/models/course/external_assessment_spec.rb b/spec/models/course/external_assessment_spec.rb new file mode 100644 index 0000000000..21451a2cc3 --- /dev/null +++ b/spec/models/course/external_assessment_spec.rb @@ -0,0 +1,128 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::ExternalAssessment, type: :model do + let(:instance) { Instance.default } + + with_tenant(:instance) do + let(:course) { create(:course) } + + describe 'associations' do + it { is_expected.to belong_to(:course) } + it { is_expected.to have_one(:gradebook_contribution).dependent(:destroy) } + it { is_expected.to have_many(:external_assessment_grades).dependent(:delete_all) } + end + + describe 'dependent destroy' do + it 'destroys the gradebook contribution and grades when destroyed' do + external = described_class.create_for_course!(course: course, title: 'Final', maximum_grade: 80.0) + create(:course_external_assessment_grade, external_assessment: external) + expect do + external.destroy + end.to change(Course::Gradebook::ExternalContribution, :count).by(-1). + and change(Course::ExternalAssessmentGrade, :count).by(-1) + end + end + + describe 'validations' do + subject { build(:course_external_assessment, course: course) } + it { is_expected.to validate_presence_of(:title) } + it { is_expected.to validate_length_of(:title).is_at_most(255) } + it { is_expected.to validate_presence_of(:maximum_grade) } + + it 'enforces course-scoped unique titles' do + create(:course_external_assessment, course: course, title: 'Midterm') + dup = build(:course_external_assessment, course: course, title: 'Midterm') + expect(dup).not_to be_valid + expect(dup.errors[:title]).to include(I18n.t('errors.messages.taken')) + end + + it 'allows the same title in different courses' do + create(:course_external_assessment, course: course, title: 'Midterm') + other = build(:course_external_assessment, course: create(:course), title: 'Midterm') + expect(other).to be_valid + end + + it 'rejects a negative maximum_grade' do + subject.maximum_grade = -1 + expect(subject).not_to be_valid + expect(subject.errors[:maximum_grade]).to be_present + end + + it 'accepts a zero maximum_grade' do + subject.maximum_grade = 0 + expect(subject).to be_valid + end + end + + describe '.create_for_course!' do + it 'creates the external and its contribution row' do + external = nil + expect do + external = described_class.create_for_course!(course: course, title: 'Final', + maximum_grade: 80.0, weight: 30) + end.to change(Course::Gradebook::ExternalContribution, :count).by(1) + expect(external.course).to eq(course) + expect(external.gradebook_contribution.weight).to eq(30) + end + + it 'does not create any assessment tab or category' do + course # ensure course (and its default tab/category) is created before measuring + expect do + described_class.create_for_course!(course: course, title: 'Final', maximum_grade: 80.0) + end.to not_change(Course::Assessment::Tab, :count).and not_change(Course::Assessment::Category, :count) + end + + it 'raises on duplicate title within the course' do + described_class.create_for_course!(course: course, title: 'Final', maximum_grade: 80.0) + expect do + described_class.create_for_course!(course: course, title: 'Final', maximum_grade: 80.0) + end.to raise_error(ActiveRecord::RecordInvalid) + end + + it 'rolls back the external when contribution creation fails' do + allow(Course::Gradebook::ExternalContribution).to receive(:create!). + and_raise(ActiveRecord::RecordInvalid.new(Course::Gradebook::ExternalContribution.new)) + expect do + expect do + described_class.create_for_course!(course: course, title: 'Final', maximum_grade: 80.0) + end.to raise_error(ActiveRecord::RecordInvalid) + end.to not_change(described_class, :count) + end + end + + describe '#synthetic_tab_id' do + it 'returns the negative of the record id' do + external = create(:course_external_assessment, course: course) + expect(external.synthetic_tab_id).to eq(-external.id) + end + end + + describe '.for_course' do + it 'returns only externals in the course' do + mine = create(:course_external_assessment, course: course) + create(:course_external_assessment, course: create(:course)) + expect(described_class.for_course(course)).to contain_exactly(mine) + end + end + + describe 'grade-bounding defaults' do + it 'defaults floor_at_zero and cap_at_maximum to true' do + external = Course::ExternalAssessment.create_for_course!( + course: course, title: 'Midterm', maximum_grade: 50 + ) + expect(external.floor_at_zero).to be(true) + expect(external.cap_at_maximum).to be(true) + end + + it 'honours explicit bound flags' do + external = Course::ExternalAssessment.create_for_course!( + course: course, title: 'Bonus', maximum_grade: 10, + floor_at_zero: false, cap_at_maximum: false + ) + expect(external.floor_at_zero).to be(false) + expect(external.cap_at_maximum).to be(false) + end + end + end +end diff --git a/spec/models/course/gradebook/external_contribution_spec.rb b/spec/models/course/gradebook/external_contribution_spec.rb new file mode 100644 index 0000000000..b381ec3d0c --- /dev/null +++ b/spec/models/course/gradebook/external_contribution_spec.rb @@ -0,0 +1,100 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::Gradebook::ExternalContribution do + let!(:instance) { Instance.default } + with_tenant(:instance) do + let(:course) { create(:course) } + let(:external) { create(:course_external_assessment, course: course) } + + describe 'validations' do + it 'is valid with an external assessment and matching course' do + expect(build(:course_gradebook_external_contribution, + external_assessment: external, course: course)).to be_valid + end + + it 'requires an external assessment' do + contribution = build(:course_gradebook_external_contribution, + external_assessment: external, course: course) + contribution.external_assessment = nil + expect(contribution).not_to be_valid + end + + it 'enforces one row per external assessment' do + create(:course_gradebook_external_contribution, external_assessment: external, course: course) + duplicate = build(:course_gradebook_external_contribution, external_assessment: external, course: course) + expect(duplicate).not_to be_valid + end + + it 'rejects a course that does not match the external assessment course' do + contribution = build(:course_gradebook_external_contribution, external_assessment: external) + contribution.course = create(:course) + expect(contribution).not_to be_valid + end + + it 'rejects a negative weight' do + expect(build(:course_gradebook_external_contribution, + external_assessment: external, course: course, weight: -1)).not_to be_valid + end + + it 'rejects a weight above 100' do + expect(build(:course_gradebook_external_contribution, + external_assessment: external, course: course, weight: 101)).not_to be_valid + end + + it 'accepts a weight of exactly 100' do + expect(build(:course_gradebook_external_contribution, + external_assessment: external, course: course, weight: 100)).to be_valid + end + end + + describe 'dependent destroy' do + it 'is destroyed when its external assessment is destroyed' do + create(:course_gradebook_external_contribution, external_assessment: external, course: course) + expect { external.destroy! }.to change { described_class.count }.by(-1) + end + end + + describe '.bulk_update' do + let(:ext1) { create(:course_external_assessment, course: course) } + let(:ext2) { create(:course_external_assessment, course: course) } + + def weight_for(external) + described_class.find_by(external_assessment_id: external.id)&.weight + end + + it 'upserts a contribution per external, decoding the negative tab_id' do + described_class.bulk_update( + course: course, + updates: [{ tab_id: -ext1.id, weight: 60 }, { tab_id: -ext2.id, weight: 40 }] + ) + expect(weight_for(ext1)).to eq(60) + expect(weight_for(ext2)).to eq(40) + expect(described_class.find_by(external_assessment_id: ext1.id).course_id).to eq(course.id) + end + + it 'is a no-op for an empty updates array' do + expect { described_class.bulk_update(course: course, updates: []) }. + not_to change(described_class, :count) + end + + it 'is transactional — an invalid weight rolls everything back' do + create(:course_gradebook_external_contribution, external_assessment: ext1, course: course, weight: 10) + expect do + described_class.bulk_update( + course: course, + updates: [{ tab_id: -ext1.id, weight: 50 }, { tab_id: -ext2.id, weight: 999 }] + ) + end.to raise_error(ActiveRecord::RecordInvalid) + expect(weight_for(ext1)).to eq(10) + end + + it 'rejects an external assessment from another course' do + other = create(:course_external_assessment, course: create(:course)) + expect do + described_class.bulk_update(course: course, updates: [tab_id: -other.id, weight: 50]) + end.to raise_error(ActiveRecord::RecordNotFound) + end + end + end +end