From 51f96d58cd093e08eb5bebc7b88f5211f0b5d5f5 Mon Sep 17 00:00:00 2001 From: lws49 Date: Tue, 16 Jun 2026 15:33:07 +0800 Subject: [PATCH] feat(gradebook): add Level contribution to weighted gradebook - map student Level to grade-points via a safe parsed arithmetic formula - fold the Level term into weighted Total and per-student breakdown - add Configure Contributions controls: formula, level, show, clamp - persist a singleton LevelConfig per course (new table + migration) - surface a Level column in the weighted table, toggleable via column picker - warn when a contribution falls outside the range of level contributions - add toggle to clamp contributions within the range of level contributions --- .../course/gradebook_controller.rb | 77 ++- app/models/course.rb | 2 + app/models/course/gradebook/level_config.rb | 127 +++++ .../course/gradebook/index.json.jbuilder | 19 + .../__tests__/ConfigureWeightsPrompt.test.tsx | 346 ++++++++++++- .../__tests__/GradebookIndex.test.tsx | 25 + .../__tests__/GradebookTable.test.tsx | 7 + .../__tests__/WeightedGradebookTable.test.tsx | 478 ++++++++++++++++++ .../__tests__/computeWeighted.test.ts | 240 ++++++++- .../gradebook/__tests__/levelFormula.test.ts | 360 +++++++++++++ .../gradebook/__tests__/operations.test.ts | 55 ++ .../course/gradebook/__tests__/store.test.ts | 300 +++++++++++ .../components/ConfigureWeightsPrompt.tsx | 298 ++++++++++- .../components/WeightedGradebookTable.tsx | 415 ++++++++++++++- .../course/gradebook/computeWeighted.ts | 119 ++++- .../bundles/course/gradebook/levelFormula.ts | 380 ++++++++++++++ .../bundles/course/gradebook/operations.ts | 25 +- .../gradebook/pages/GradebookIndex/index.tsx | 7 + .../app/bundles/course/gradebook/selectors.ts | 6 + client/app/bundles/course/gradebook/store.ts | 35 ++ client/app/types/course/gradebook.ts | 29 ++ client/locales/en.json | 84 ++- client/locales/ko.json | 87 ++++ client/locales/zh.json | 87 ++++ ...16000000_create_gradebook_level_configs.rb | 30 ++ db/schema.rb | 22 +- .../course/gradebook_controller_spec.rb | 189 +++++++ .../course_gradebook_level_configs.rb | 11 + .../course/gradebook/level_config_spec.rb | 336 ++++++++++++ 29 files changed, 4123 insertions(+), 73 deletions(-) create mode 100644 app/models/course/gradebook/level_config.rb create mode 100644 client/app/bundles/course/gradebook/__tests__/levelFormula.test.ts create mode 100644 client/app/bundles/course/gradebook/__tests__/operations.test.ts create mode 100644 client/app/bundles/course/gradebook/levelFormula.ts create mode 100644 db/migrate/20260616000000_create_gradebook_level_configs.rb create mode 100644 spec/factories/course_gradebook_level_configs.rb create mode 100644 spec/models/course/gradebook/level_config_spec.rb diff --git a/app/controllers/course/gradebook_controller.rb b/app/controllers/course/gradebook_controller.rb index 375972e7eb..3a12f399c8 100644 --- a/app/controllers/course/gradebook_controller.rb +++ b/app/controllers/course/gradebook_controller.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true -class Course::GradebookController < Course::ComponentController +class Course::GradebookController < Course::ComponentController # rubocop:disable Metrics/ClassLength before_action :authorize_read_gradebook! before_action :preload_levels, only: [:index] @@ -10,13 +10,11 @@ def index @published_assessments = fetch_published_assessments @categories, @tabs = fetch_categories_and_tabs @students = fetch_students + @course_max_level = [current_course.levels.count - 1, 0].max assessment_ids = @published_assessments.pluck(:id) - load_weighted_view_contributions(assessment_ids) if @weighted_view_enabled - @assessment_max_grades = Course::Assessment.max_grades(assessment_ids) - @submissions = Course::Assessment::Submission.grade_summary( - student_ids: @students.map(&:user_id), - assessment_ids: assessment_ids - ) + load_weighted_view(assessment_ids) if @weighted_view_enabled + load_grades(assessment_ids) + @student_level_contributions = compute_student_level_contributions end end end @@ -25,7 +23,10 @@ def update_weights authorize! :manage_gradebook_weights, current_course updates = (update_weights_params[:weights] || []).map { |entry| parse_weight_entry(entry) } Course::Gradebook::TabContribution.bulk_update(course: current_course, updates: updates) - render json: { weights: serialize_weight_updates(updates) } + level_config = persist_level_contribution + response_body = { weights: serialize_weight_updates(updates) } + response_body[:levelContribution] = serialize_level_contribution(level_config) if level_config + render json: response_body rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotFound => e render json: { errors: { base: e.message } }, status: :unprocessable_entity end @@ -36,6 +37,11 @@ def authorize_read_gradebook! authorize! :read_gradebook, current_course end + def load_weighted_view(assessment_ids) + @level_config = current_course.gradebook_level_config + load_weighted_view_contributions(assessment_ids) + end + def load_weighted_view_contributions(assessment_ids) @tab_contributions = Course::Gradebook::TabContribution. where(tab_id: @tabs.map(&:id)).index_by(&:tab_id) @@ -43,6 +49,14 @@ def load_weighted_view_contributions(assessment_ids) where(assessment_id: assessment_ids).index_by(&:assessment_id) end + def load_grades(assessment_ids) + @assessment_max_grades = Course::Assessment.max_grades(assessment_ids) + @submissions = Course::Assessment::Submission.grade_summary( + student_ids: @students.map(&:user_id), + assessment_ids: assessment_ids + ) + end + # Weights are stored as DECIMAL(5,2); round at the boundary so the echoed response # matches the persisted value and the custom-weight sum check stays exact at 2dp. def parse_weight_entry(entry) @@ -64,6 +78,42 @@ def update_weights_params ) end + def persist_level_contribution + attrs = level_contribution_attrs + return nil if attrs.nil? + + Course::Gradebook::LevelConfig.upsert_for(course: current_course, attrs: attrs) + end + + def level_contribution_attrs + lc = params[:levelContribution] + return nil if lc.blank? + + permitted = lc.permit(:enabled, :formula, :weight, :show, :clamp, formulaAst: {}) + formula_ast = permitted[:formulaAst] + attrs = { + enabled: permitted[:enabled], + formula: permitted[:formula], + formula_ast: formula_ast.present? ? formula_ast.to_h : nil, + weight: permitted[:weight], + show: permitted[:show] + } + # Only forward :clamp when the client actually sent it, so LevelConfig.upsert_for's + # default-true fallback applies on omission rather than persisting NULL. + attrs[:clamp] = permitted[:clamp] if permitted.key?(:clamp) + attrs + end + + def serialize_level_contribution(config) + { + enabled: config.enabled, + formula: config.formula, + weight: config.weight.to_f, + show: config.show, + clamp: config.clamp + } + end + def serialize_weight_updates(updates) updates.map do |u| entry = { tabId: u[:tab_id], weight: u[:weight], weightMode: u[:weight_mode].to_s, @@ -88,7 +138,8 @@ def fetch_categories_and_tabs def fetch_students current_course.course_users.students.without_phantom_users. - calculated(:experience_points).includes(user: :emails).to_a + calculated(:experience_points).includes(user: :emails).to_a. + sort_by { |cu| cu.user.name } end def fetch_published_assessments @@ -105,4 +156,12 @@ def fetch_published_assessments def preload_levels current_course.levels.to_a end + + def compute_student_level_contributions + return {} unless @level_config&.enabled + + @students.to_h do |cu| + [cu.user_id, @level_config.evaluate_for(level: cu.level_number)] + end + end end diff --git a/app/models/course.rb b/app/models/course.rb index ed3220b27b..e5b6c0cde6 100644 --- a/app/models/course.rb +++ b/app/models/course.rb @@ -55,6 +55,8 @@ class Course < ApplicationRecord # rubocop:disable Metrics/ClassLength has_many :assessments, through: :assessment_categories has_many :gradebook_contributions, class_name: 'Course::Gradebook::TabContribution', dependent: :destroy, inverse_of: :course + has_one :gradebook_level_config, class_name: 'Course::Gradebook::LevelConfig', + dependent: :destroy, inverse_of: :course 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/gradebook/level_config.rb b/app/models/course/gradebook/level_config.rb new file mode 100644 index 0000000000..97c35c58fa --- /dev/null +++ b/app/models/course/gradebook/level_config.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true +class Course::Gradebook::LevelConfig < ApplicationRecord # rubocop:disable Metrics/ClassLength + belongs_to :course, inverse_of: :gradebook_level_config + + validates :weight, numericality: { greater_than_or_equal_to: 0, less_than_or_equal_to: 100 } + validates :formula, presence: true, if: :enabled + validates :formula_ast, presence: true, if: :enabled + validates :course_id, uniqueness: true + validate :formula_ast_structure + + # Upserts the singleton Level config for a course from a normalized attrs hash + # (symbol keys, snake_case). Raises ActiveRecord::RecordInvalid on validation failure. + def self.upsert_for(course:, attrs:) + config = find_or_initialize_by(course_id: course.id) + config.assign_attributes( + enabled: ActiveRecord::Type::Boolean.new.cast(attrs[:enabled]), + formula: attrs[:formula].to_s, + formula_ast: attrs[:formula_ast], + weight: attrs[:weight].to_f, + show: ActiveRecord::Type::Boolean.new.cast(attrs[:show]) == true, + clamp: attrs.key?(:clamp) ? ActiveRecord::Type::Boolean.new.cast(attrs[:clamp]) : true + ) + config.save! + config + end + + def evaluate_for(level:) + return nil unless enabled && formula_ast.present? + + result = evaluate_node(formula_ast, level.to_f) + return nil unless result.is_a?(Numeric) && result.finite? + + value = result.to_f + clamp ? value.clamp(0.0, weight.to_f) : value + rescue StandardError + nil + end + + BINOPS = %w[+ - * /].freeze + CALL1_FNS = %w[floor ceil round].freeze + CALL2_FNS = %w[min max].freeze + + def self.valid_ast?(node, depth = 0) + return false if depth > 20 + return false unless node.is_a?(Hash) && node['type'].is_a?(String) + + valid_node_type?(node, depth) + end + + def self.valid_node_type?(node, depth) + case node['type'] + when 'num' then node['value'].is_a?(Numeric) + when 'var' then node['name'] == 'level' + when 'neg' then valid_operand?(node, 'operand', depth) + when 'binop' then valid_binop?(node, depth) + when 'call1' then valid_call1?(node, depth) + when 'call2' then valid_call2?(node, depth) + else false + end + end + + def self.valid_operand?(node, key, depth) + node.key?(key) && valid_ast?(node[key], depth + 1) + end + + def self.valid_binop?(node, depth) + BINOPS.include?(node['op']) && + valid_operand?(node, 'left', depth) && + valid_operand?(node, 'right', depth) + end + + def self.valid_call1?(node, depth) + CALL1_FNS.include?(node['fn']) && valid_operand?(node, 'arg', depth) + end + + def self.valid_call2?(node, depth) + CALL2_FNS.include?(node['fn']) && + valid_operand?(node, 'a', depth) && + valid_operand?(node, 'b', depth) + end + private_class_method :valid_node_type?, :valid_operand?, :valid_binop?, :valid_call1?, :valid_call2? + + private + + def formula_ast_structure + return if formula_ast.blank? + + errors.add(:formula_ast, 'has an invalid structure') unless self.class.valid_ast?(formula_ast) + end + + def evaluate_node(node, level) + case node['type'] + when 'num' then node['value'].to_f + when 'var' then level + when 'neg' then -evaluate_node(node['operand'], level) + when 'binop' then evaluate_binop(node, level) + when 'call1' then evaluate_call1(node, level) + when 'call2' then evaluate_call2(node, level) + end + end + + def evaluate_binop(node, level) + l = evaluate_node(node['left'], level) + r = evaluate_node(node['right'], level) + case node['op'] + when '+' then l + r + when '-' then l - r + when '*' then l * r + when '/' then (r == 0) ? Float::NAN : l / r + end + end + + def evaluate_call1(node, level) + a = evaluate_node(node['arg'], level) + case node['fn'] + when 'floor' then a.floor.to_f + when 'ceil' then a.ceil.to_f + when 'round' then a.round.to_f + end + end + + def evaluate_call2(node, level) + a = evaluate_node(node['a'], level) + b = evaluate_node(node['b'], level) + (node['fn'] == 'min') ? [a, b].min : [a, b].max + end +end diff --git a/app/views/course/gradebook/index.json.jbuilder b/app/views/course/gradebook/index.json.jbuilder index 3241fa2b68..a493cfe953 100644 --- a/app/views/course/gradebook/index.json.jbuilder +++ b/app/views/course/gradebook/index.json.jbuilder @@ -37,6 +37,7 @@ json.students @students do |course_user| json.externalId course_user.external_id json.level course_user.level_number json.totalXp course_user.experience_points + json.levelContribution @student_level_contributions[course_user.user_id] end json.submissions @submissions do |sub| @@ -47,3 +48,21 @@ json.submissions @submissions do |sub| end json.gamificationEnabled current_course.gamified? + +json.courseMaxLevel @course_max_level + +json.levelContribution do + if @weighted_view_enabled && @level_config + json.enabled @level_config.enabled + json.formula @level_config.formula + json.weight @level_config.weight.to_f + json.show @level_config.show + json.clamp @level_config.clamp + else + json.enabled false + json.formula '' + json.weight 0 + json.show false + json.clamp true + end +end diff --git a/client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx b/client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx index 2f1f87a0f6..78949dd4d2 100644 --- a/client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx +++ b/client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx @@ -21,6 +21,7 @@ const A1 = 'Assignment 1'; const A2 = 'Assignment 2'; const ASSIGN_A1 = 'Assignments: Assignment 1'; const INCLUDE_A1 = 'Include Assignment 1 in grade'; +const LEVEL_FORMULA = 'min(level, 10) * 0.8'; const categories = [{ id: 1, title: 'Missions' }]; const tabs = [ @@ -33,13 +34,43 @@ const assessments = [ { id: 102, title: A2, tabId: 10, maxGrade: 50 }, ]; +const defaultLevelContribution = { + enabled: false, + formula: '', + weight: 0, + show: false, + clamp: true, +}; + +const enabledLevel = (over = {}): Record => ({ + enabled: true, + formula: LEVEL_FORMULA, + weight: 8, + show: true, + clamp: true, + ...over, +}); + +const students = [ + { id: 1, name: 'A', email: 'a@x', externalId: null, level: 5, totalXp: 0 }, + { id: 2, name: 'B', email: 'b@x', externalId: null, level: 12, totalXp: 0 }, +]; + +const levelZeroStudent = [ + { id: 1, name: 'A', email: 'a@x', externalId: null, level: 0, totalXp: 0 }, +]; + const setup = (overrides = {}): ReturnType => render( , @@ -191,24 +222,27 @@ describe('', () => { ); fireEvent.click(screen.getByRole('button', { name: /save/i })); await waitFor(() => { - expect(operations.updateGradebookWeights).toHaveBeenCalledWith([ - { - tabId: 10, - weight: 50, - weightMode: 'custom', - excludedAssessmentIds: [], - assessmentWeights: [ - { assessmentId: 101, weight: 25 }, - { assessmentId: 102, weight: 25 }, - ], - }, - { - tabId: 11, - weight: 50, - weightMode: 'equal', - excludedAssessmentIds: [], - }, - ]); + expect(operations.updateGradebookWeights).toHaveBeenCalledWith( + [ + { + tabId: 10, + weight: 50, + weightMode: 'custom', + excludedAssessmentIds: [], + assessmentWeights: [ + { assessmentId: 101, weight: 25 }, + { assessmentId: 102, weight: 25 }, + ], + }, + { + tabId: 11, + weight: 50, + weightMode: 'equal', + excludedAssessmentIds: [], + }, + ], + expect.objectContaining({ enabled: false }), + ); }); }); @@ -272,6 +306,41 @@ describe('', () => { within(modeGroup('Optional')).getByRole('radio', { name: /custom/i }), ).toBeDisabled(); }); + + it('defaults the clamp toggle to checked', () => { + setup({ gamificationEnabled: true, levelContribution: enabledLevel() }); + expect( + screen.getByRole('checkbox', { name: /Keep results between/i }), + ).toBeChecked(); + }); + + it('sends clamp in the save payload', async () => { + setup({ gamificationEnabled: true, levelContribution: enabledLevel() }); + fireEvent.click( + screen.getByRole('checkbox', { name: /Keep results between/i }), + ); + fireEvent.click(screen.getByRole('button', { name: /save/i })); + await waitFor(() => + expect(operations.updateGradebookWeights).toHaveBeenCalled(), + ); + const arg = (operations.updateGradebookWeights as jest.Mock).mock + .calls[0][1]; + expect(arg.clamp).toBe(false); + }); + + it('suppresses the range alert when clamp is off', () => { + setup({ + gamificationEnabled: true, + students, // levels 5, 12 + levelContribution: enabledLevel({ + formula: 'level * 5', + weight: 10, + clamp: false, + }), + // raw 25 and 60 are above 10, but clamp is off -> no alert + }); + expect(screen.queryByText(/above 10/i)).not.toBeInTheDocument(); + }); }); describe('per-assessment exclusion', () => { @@ -544,3 +613,244 @@ describe('per-assessment exclusion', () => { }); }); }); + +describe('level contribution section', () => { + beforeEach(() => jest.clearAllMocks()); + + it('is not rendered when gamificationEnabled is false', () => { + setup({ gamificationEnabled: false }); + expect(screen.queryByText(/level contribution/i)).not.toBeInTheDocument(); + }); + + it('renders the level section when gamificationEnabled is true', () => { + setup({ gamificationEnabled: true }); + expect(screen.getByText(/level contribution/i)).toBeInTheDocument(); + }); + + it('seeds enabled, formula, weight, and show from the levelContribution prop', () => { + setup({ gamificationEnabled: true, levelContribution: enabledLevel() }); + expect(screen.getByLabelText(/formula/i)).toHaveValue(LEVEL_FORMULA); + // Exact name: the clamp label ("...max level contributions") also matches + // /level contribution/i, so an exact string isolates the enable toggle. + expect( + screen.getByRole('checkbox', { name: 'Level contribution' }), + ).toBeChecked(); + }); + + it('no longer offers a "custom max level" control', () => { + setup({ gamificationEnabled: true, levelContribution: enabledLevel() }); + expect( + screen.queryByRole('checkbox', { name: /custom max level/i }), + ).not.toBeInTheDocument(); + // The removed control was a numeric field; scope to spinbutton so the clamp + // checkbox ("...max level contributions") isn't mistaken for it. + expect( + screen.queryByRole('spinbutton', { name: /max level/i }), + ).not.toBeInTheDocument(); + }); + + it('shows the highest student level and the course maximum level', () => { + setup({ + gamificationEnabled: true, + courseMaxLevel: 14, + students, // levels 5 and 12 + levelContribution: enabledLevel(), + }); + expect(screen.getByText(/Highest student level: 12/)).toBeInTheDocument(); + expect(screen.getByText(/Course maximum level: 14/)).toBeInTheDocument(); + }); + + it('renders the level weight as a bare number with no caption or tooltip', () => { + setup({ gamificationEnabled: true, levelContribution: enabledLevel() }); + expect( + screen.getByRole('spinbutton', { name: 'Level contribution' }), + ).toBeInTheDocument(); + expect(screen.queryByText(/Suggested maximum/i)).not.toBeInTheDocument(); + expect(screen.queryByText(/never caps or blocks/i)).not.toBeInTheDocument(); + }); + + it('describes the level term without the word "bonus"', () => { + setup({ gamificationEnabled: true, levelContribution: enabledLevel() }); + expect( + screen.getByText(/Adds grade-points from each student/i), + ).toBeInTheDocument(); + expect(screen.queryByText(/bonus/i)).not.toBeInTheDocument(); + }); + + it('always shows the formula syntax reference, even when the formula is valid', () => { + setup({ + gamificationEnabled: true, + students, // valid default formula min(level, 10) for these students + levelContribution: enabledLevel(), + }); + expect( + screen.getByText(/floor, ceil, round, min and max/i), + ).toBeInTheDocument(); + expect(screen.getByText(/arithmetic operators/i)).toBeInTheDocument(); + }); + + it('shows no warning when every contribution is in range', () => { + setup({ + gamificationEnabled: true, + students, // levels 5, 12 → min(level,10)*0.8 = 4, 8 — both within 0..8 + levelContribution: enabledLevel(), + }); + expect(screen.queryByText(/below 0/i)).not.toBeInTheDocument(); + expect(screen.queryByText(/above/i)).not.toBeInTheDocument(); + }); + + it('names the offending student(s) only on the lower bound when none exceed the max', () => { + setup({ + gamificationEnabled: true, + students, // levels 5, 12 + levelContribution: enabledLevel({ formula: 'level - 8', weight: 10 }), + // contributions -3 (A) and 4 (B): only A is below 0 + }); + expect(screen.getByText('A (-3.00) is below 0.')).toBeInTheDocument(); + expect( + screen.getByText(/These contributions will be set to 0\./), + ).toBeInTheDocument(); + expect(screen.queryByText(/above/i)).not.toBeInTheDocument(); + }); + + it('names the worst offenders (value, highest first) only on the upper bound', () => { + setup({ + gamificationEnabled: true, + students, // levels 5, 12 + levelContribution: enabledLevel({ formula: 'level * 5', weight: 10 }), + // contributions 25 (A) and 60 (B), both above 10 + }); + expect( + screen.getByText('B (60.00) and A (25.00) are above 10.'), + ).toBeInTheDocument(); + expect( + screen.getByText(/These contributions will be set to 10\./), + ).toBeInTheDocument(); + expect(screen.queryByText(/below 0/i)).not.toBeInTheDocument(); + }); + + it('caps the list at two names and appends "and N more"', () => { + const many = [ + { + id: 1, + name: 'S1', + email: 'a@x', + externalId: null, + level: 3, + totalXp: 0, + }, + { + id: 2, + name: 'S2', + email: 'b@x', + externalId: null, + level: 4, + totalXp: 0, + }, + { + id: 3, + name: 'S3', + email: 'c@x', + externalId: null, + level: 5, + totalXp: 0, + }, + { + id: 4, + name: 'S4', + email: 'd@x', + externalId: null, + level: 6, + totalXp: 0, + }, + ]; + setup({ + gamificationEnabled: true, + students: many, + levelContribution: enabledLevel({ formula: 'level * 5', weight: 10 }), + // contributions 15,20,25,30 — all above 10; top two then "and 2 more" + }); + expect( + screen.getByText('S4 (30.00), S3 (25.00) and 2 more are above 10.'), + ).toBeInTheDocument(); + }); + + it('names offenders on both bounds with a combined fix instruction', () => { + setup({ + gamificationEnabled: true, + students, // levels 5, 12 + levelContribution: enabledLevel({ + formula: 'level * 5 - 30', + weight: 10, + }), + // contributions -5 (A) and 30 (B): one below 0, one above 10 + }); + expect( + screen.getByText('B (30.00) is above 10. A (-5.00) is below 0.'), + ).toBeInTheDocument(); + expect( + screen.getByText( + /Contributions below 0 will be set to 0, and contributions above 10 will be set to 10\./, + ), + ).toBeInTheDocument(); + }); + + it('seeds the level weight to the course max level when unconfigured (works on first open)', () => { + setup({ + gamificationEnabled: true, + courseMaxLevel: 30, + students, // levels 5, 12 + // fresh course: enabled, but no formula / weight yet + levelContribution: { enabled: true, formula: '', weight: 0, show: false }, + }); + // weight defaults to 30 and the formula seeds to min(level, 30) → in range, + // so there is no out-of-range warning. + expect( + screen.getByRole('spinbutton', { name: 'Level contribution' }), + ).toHaveValue(30); + expect(screen.queryByText(/below 0/i)).not.toBeInTheDocument(); + }); + + it('shows a parse error and disables Save when the formula is invalid', () => { + setup({ + gamificationEnabled: true, + levelContribution: enabledLevel({ formula: 'level +' }), + }); + expect(screen.getByRole('button', { name: /save/i })).toBeDisabled(); + }); + + it('includes levelContribution in the save payload without a maxLevel field', async () => { + setup({ gamificationEnabled: true, levelContribution: enabledLevel() }); + fireEvent.click(screen.getByRole('button', { name: /save/i })); + await waitFor(() => + expect(operations.updateGradebookWeights).toHaveBeenCalled(), + ); + const [, lvl] = (operations.updateGradebookWeights as jest.Mock).mock + .calls[0]; + expect(lvl).toMatchObject({ + enabled: true, + formula: LEVEL_FORMULA, + weight: 8, + show: true, + }); + expect(lvl).not.toHaveProperty('maxLevel'); + }); + + it('adds the level weight to the Total when enabled', () => { + // tabs sum to 100; level adds 8 → Total should be 108 + setup({ gamificationEnabled: true, levelContribution: enabledLevel() }); + expect(screen.getByText(/Total:\s*108%/)).toBeInTheDocument(); + }); + + it('warns and names students when the formula divides by zero for them', async () => { + setup({ + gamificationEnabled: true, + students: levelZeroStudent, + levelContribution: enabledLevel({ formula: '100 / level' }), + }); + // ...render with the level contribution enabled and a student at level 0... + // ...set the formula field to '100 / level'... + expect(await screen.findByText(/divides by zero/i)).toBeInTheDocument(); + expect(screen.getByText(/set to 0/i)).toBeInTheDocument(); + }); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/GradebookIndex.test.tsx b/client/app/bundles/course/gradebook/__tests__/GradebookIndex.test.tsx index 63853662cb..1c07b29cbc 100644 --- a/client/app/bundles/course/gradebook/__tests__/GradebookIndex.test.tsx +++ b/client/app/bundles/course/gradebook/__tests__/GradebookIndex.test.tsx @@ -34,6 +34,14 @@ const emptyState = { gamificationEnabled: false, weightedViewEnabled: false, canManageWeights: false, + courseMaxLevel: 0, + levelContribution: { + enabled: false, + formula: '', + weight: 0, + show: false, + clamp: true, + }, }, }; @@ -47,6 +55,14 @@ const noStudentsState = { gamificationEnabled: false, weightedViewEnabled: false, canManageWeights: false, + courseMaxLevel: 0, + levelContribution: { + enabled: false, + formula: '', + weight: 0, + show: false, + clamp: true, + }, }, }; @@ -63,6 +79,7 @@ const populatedState = { externalId: null, level: 3, totalXp: 150, + levelContribution: null, }, ], submissions: [ @@ -71,6 +88,14 @@ const populatedState = { gamificationEnabled: false, weightedViewEnabled: false, canManageWeights: false, + courseMaxLevel: 0, + levelContribution: { + enabled: false, + formula: '', + weight: 0, + show: false, + clamp: true, + }, }, }; diff --git a/client/app/bundles/course/gradebook/__tests__/GradebookTable.test.tsx b/client/app/bundles/course/gradebook/__tests__/GradebookTable.test.tsx index 449fcc5a61..b70451c63b 100644 --- a/client/app/bundles/course/gradebook/__tests__/GradebookTable.test.tsx +++ b/client/app/bundles/course/gradebook/__tests__/GradebookTable.test.tsx @@ -24,6 +24,7 @@ const students: StudentData[] = [ externalId: null, level: 3, totalXp: 150, + levelContribution: null, }, { id: 2, @@ -32,6 +33,7 @@ const students: StudentData[] = [ externalId: null, level: 5, totalXp: 300, + levelContribution: null, }, ]; const submissions: SubmissionData[] = [ @@ -46,6 +48,7 @@ const makeStudents = (n: number): StudentData[] => externalId: null, level: 1, totalXp: 0, + levelContribution: null, })); // Asserts the given texts appear in this top-to-bottom DOM order. @@ -420,6 +423,7 @@ describe('GradebookTable', () => { externalId: 'EXT-001', level: 3, totalXp: 150, + levelContribution: null, }, { id: 2, @@ -428,6 +432,7 @@ describe('GradebookTable', () => { externalId: null, level: 5, totalXp: 300, + levelContribution: null, }, ]; @@ -468,6 +473,7 @@ describe('GradebookTable', () => { externalId: '', level: 3, totalXp: 150, + levelContribution: null, }, ]; renderWith(studentsWithBlankExtId); @@ -658,6 +664,7 @@ describe('GradebookTable', () => { externalId: null, level: 1, totalXp: 0, + levelContribution: null, }); const renderGrades = ( diff --git a/client/app/bundles/course/gradebook/__tests__/WeightedGradebookTable.test.tsx b/client/app/bundles/course/gradebook/__tests__/WeightedGradebookTable.test.tsx index f0464daae6..fcfdb31736 100644 --- a/client/app/bundles/course/gradebook/__tests__/WeightedGradebookTable.test.tsx +++ b/client/app/bundles/course/gradebook/__tests__/WeightedGradebookTable.test.tsx @@ -1,6 +1,7 @@ import userEvent from '@testing-library/user-event'; import { store as appStore } from 'store'; import { render, screen, waitFor, within } from 'test-utils'; +import TestApp from 'utilities/TestApp'; import WeightedGradebookTable from '../components/WeightedGradebookTable'; import type { @@ -18,6 +19,7 @@ jest.mock('lib/components/wrappers/I18nProvider'); const USER_ID = 42; const WEIGHTED_STORAGE_KEY = `${USER_ID}:gradebook_weighted_columns_1`; const EXTERNAL_ID = 'External ID'; +const LEVEL_CONTRIBUTION = 'Level Contribution'; // Mirrors the component's data-testid for a breakdown row: // `breakdown-row-${studentId}-${tabId}-${assessmentId}`. @@ -65,6 +67,7 @@ const makeStudent = (id: number, name: string): StudentData => ({ externalId: null, level: 1, totalXp: 0, + levelContribution: null, }); const makeSub = ( @@ -73,6 +76,14 @@ const makeSub = ( grade: number | null, ): SubmissionData => ({ submissionId: 0, studentId, assessmentId, grade }); +const defaultLevelContribution = { + enabled: false, + formula: '', + weight: 0, + show: false, + clamp: true, +}; + interface RenderWeightedOptions { categories?: CategoryData[]; tabs?: TabData[]; @@ -82,6 +93,9 @@ interface RenderWeightedOptions { canManageWeights?: boolean; courseTitle?: string; courseId?: number; + gamificationEnabled?: boolean; + courseMaxLevel?: number; + levelContribution?: typeof defaultLevelContribution; } const renderWeighted = ( @@ -100,7 +114,10 @@ const renderWeighted = ( canManageWeights={opts.canManageWeights ?? true} categories={cats} courseId={opts.courseId ?? 1} + courseMaxLevel={opts.courseMaxLevel ?? 0} courseTitle={opts.courseTitle ?? 'Test Course'} + gamificationEnabled={opts.gamificationEnabled ?? false} + levelContribution={opts.levelContribution ?? defaultLevelContribution} students={students} submissions={submissions} tabs={tabs} @@ -834,6 +851,90 @@ describe('WeightedGradebookTable', () => { expect(cells[3]).toBeEmptyDOMElement(); }); + it('aligns breakdown tab values with the parent columns when both level columns are shown', async () => { + const user = userEvent.setup(); + renderWeighted({ + ...expandable, + students: [ + { ...makeStudent(1, 'Alice'), level: 8, levelContribution: 15 }, + ], + gamificationEnabled: true, + courseMaxLevel: 30, + levelContribution: { + enabled: true, + show: true, + weight: 30, + formula: 'level', + clamp: true, + }, + }); + await user.click(screen.getByRole('button', { name: /expand Alice/i })); + + // With Raw Level + Level Contribution present and no identity columns: + // checkbox | Name | Level | Level Contribution | Missions | Quizzes | Total + const detail = await screen.findByTestId(breakdownRowId(1, 10, 1)); + const cells = within(detail).getAllByRole('cell'); + expect(cells).toHaveLength(7); + // Mission 1's contribution (24) lands under Missions (index 4) — not + // shifted left into the Level (2) or Level Contribution (3) columns. + expect(cells[2]).toBeEmptyDOMElement(); // Raw Level + expect(cells[3]).toBeEmptyDOMElement(); // Level Contribution + expect(cells[4]).toHaveTextContent('24'); // Missions + }); + + it('gives every column an explicit-width colgroup , including Total', () => { + renderWeighted({ + ...expandable, + students: [ + { ...makeStudent(1, 'Alice'), level: 8, levelContribution: 15 }, + ], + gamificationEnabled: true, + courseMaxLevel: 30, + levelContribution: { + enabled: true, + show: true, + weight: 30, + formula: 'level', + clamp: true, + }, + }); + + const cols = [...document.querySelectorAll('colgroup col')]; + const studentRow = screen.getByText('Alice').closest('tr')!; + const cells = within(studentRow).getAllByRole('cell'); + expect(cells).toHaveLength(7); + expect(cols).toHaveLength(cells.length); + cols.forEach((col) => expect(col.style.width).not.toBe('')); + }); + + it('shows the level contribution in its own column on the level breakdown row', async () => { + const user = userEvent.setup(); + renderWeighted({ + ...expandable, + students: [ + { ...makeStudent(1, 'Alice'), level: 8, levelContribution: 15 }, + ], + gamificationEnabled: true, + courseMaxLevel: 30, + levelContribution: { + enabled: true, + show: true, + weight: 30, + formula: 'level', + clamp: true, + }, + }); + await user.click(screen.getByRole('button', { name: /expand Alice/i })); + + // Synthetic Level breakdown row (tabId -1, assessmentId -1): its + // contribution (15) sits under Level Contribution, tab columns stay empty. + const levelRow = await screen.findByTestId(breakdownRowId(1, -1, -1)); + const cells = within(levelRow).getAllByRole('cell'); + expect(cells).toHaveLength(7); + expect(cells[3]).toHaveTextContent('15'); // Level Contribution + expect(cells[4]).toBeEmptyDOMElement(); // Missions empty on the level row + }); + it('renders each assessment as a grade/maxGrade percentage in percent mode', async () => { const user = userEvent.setup(); renderWeighted(expandable); @@ -1331,3 +1432,380 @@ describe('WeightedGradebookTable', () => { }); }); }); + +describe('level contribution columns', () => { + beforeEach(() => localStorage.clear()); + + // `enabled` drives the always-on "Level Contribution" column; `show` + // additionally drives the raw "Level" column. Formula uses only the `level` + // grammar variable so these tests assert layout without coupling to the + // contribution-value computation (owned elsewhere). + const levelOn = { + enabled: true, + formula: 'level', + weight: 10, + show: true, + clamp: true, + }; + + // The first row carries every column's top header cell left-to-right + // (Name, raw Level, Level Contribution, category groups, Total) — the cleanest + // place to assert presence and ordering. + const headerRow1 = (): HTMLElement => + document.querySelector('thead tr') as HTMLElement; + + it('shows neither level column when gamification is off', () => { + renderWeighted({ gamificationEnabled: false, levelContribution: levelOn }); + expect(screen.queryByText(LEVEL_CONTRIBUTION)).not.toBeInTheDocument(); + expect(screen.queryByText('Level')).not.toBeInTheDocument(); + }); + + it('shows neither level column when the contribution is disabled', () => { + renderWeighted({ + gamificationEnabled: true, + levelContribution: { ...levelOn, enabled: false }, + }); + expect(screen.queryByText(LEVEL_CONTRIBUTION)).not.toBeInTheDocument(); + expect(screen.queryByText('Level')).not.toBeInTheDocument(); + }); + + it('shows Level Contribution but hides raw Level when show is off', () => { + renderWeighted({ + gamificationEnabled: true, + levelContribution: { ...levelOn, show: false }, + }); + expect( + within(headerRow1()).getByText(LEVEL_CONTRIBUTION), + ).toBeInTheDocument(); + expect(within(headerRow1()).queryByText('Level')).not.toBeInTheDocument(); + }); + + it('shows both Level Contribution and raw Level when enabled and show are on', () => { + renderWeighted({ gamificationEnabled: true, levelContribution: levelOn }); + expect( + within(headerRow1()).getByText(LEVEL_CONTRIBUTION), + ).toBeInTheDocument(); + expect(within(headerRow1()).getByText('Level')).toBeInTheDocument(); + }); + + it('orders raw Level last among student info and Level Contribution first among contributions', () => { + renderWeighted({ + gamificationEnabled: true, + levelContribution: levelOn, + students: [{ ...makeStudent(1, 'Alice'), externalId: 'EXT-1' }], + }); + const headers = within(headerRow1()) + .getAllByRole('columnheader') + .map((c) => c.textContent); + const ext = headers.indexOf('External ID'); + const raw = headers.indexOf('Level'); + const contrib = headers.indexOf(LEVEL_CONTRIBUTION); + const cat = headers.indexOf('Cat A'); + // External ID (student info) < raw Level (last student info) < Level + // Contribution (first contribution) < Cat A (tab group). + expect(ext).toBeLessThan(raw); + expect(raw).toBeLessThan(contrib); + expect(contrib).toBeLessThan(cat); + }); + + it("renders each student's actual level in the raw Level column", () => { + renderWeighted({ + gamificationEnabled: true, + // level * 2 keeps the contribution value (14) distinct from the raw level + // (7) so the assertion targets the raw Level cell unambiguously. + levelContribution: { ...levelOn, formula: 'level * 2' }, + students: [{ ...makeStudent(1, 'Alice'), level: 7 }], + }); + const aliceRow = screen.getByText('Alice').closest('tr')!; + expect(within(aliceRow).getByText('7')).toBeInTheDocument(); + }); + + it('lists the Level row first in the expanded breakdown', async () => { + const user = userEvent.setup(); + renderWeighted({ + gamificationEnabled: true, + levelContribution: { ...levelOn, formula: 'level' }, + tabs: [makeTab(10, 'Tab 1', 1, 100)], + assessments: [makeAssessment(100, 'Quiz 1', 10, 10)], + // BE pre-computes the per-student contribution; Alice (level 1) under + // formula 'level' → 1. The component renders this value, not the formula. + students: [{ ...makeStudent(1, 'Alice'), levelContribution: 1 }], + submissions: [makeSub(1, 100, 8)], + }); + await user.click(screen.getByRole('button', { name: /expand Alice/i })); + const bdRows = await screen.findAllByTestId(/^breakdown-row-/); + // Synthetic level tab uses LEVEL_TAB_ID (-1); its row must come first, before + // the real tab (id 10) rows — mirroring the column order. + expect(bdRows[0].getAttribute('data-testid')).toMatch( + /^breakdown-row-1--1-/, + ); + expect( + bdRows.some((r) => r.getAttribute('data-testid')?.includes('-10-')), + ).toBe(true); + }); + + it('shows the raw level (no max-level denominator) in the Level breakdown row', async () => { + const user = userEvent.setup(); + renderWeighted({ + gamificationEnabled: true, + // courseMaxLevel plays no part in the contribution, so showing "14/20" + // would falsely imply a level/maxLevel derivation. Expect a plain "Level 14". + levelContribution: { ...levelOn, formula: 'level', weight: 20 }, + courseMaxLevel: 20, + tabs: [makeTab(10, 'Tab 1', 1, 100)], + assessments: [makeAssessment(100, 'Quiz 1', 10, 10)], + // BE-supplied contribution; Alice (level 14) under formula 'level' → 14. + students: [ + { ...makeStudent(1, 'Alice'), level: 14, levelContribution: 14 }, + ], + submissions: [makeSub(1, 100, 8)], + }); + await user.click(screen.getByRole('button', { name: /expand Alice/i })); + const levelRow = (await screen.findAllByTestId(/^breakdown-row-1--1-/))[0]; + expect(within(levelRow).getByText(/Level 14/)).toBeInTheDocument(); + // The misleading "14/20" fraction must NOT appear. + expect( + within(levelRow).queryByText(/14\s*\/\s*\d+/), + ).not.toBeInTheDocument(); + }); + + // Over-budget warning: the level weight is a suggested maximum, so a formula can + // push a student's contribution past it or below 0. The Level Contribution subheader + // shows a bound-aware message (above-only, below-only, or both). + it('shows above-only tooltip when a student exceeds the level weight', () => { + renderWeighted({ + gamificationEnabled: true, + // level * 5 → Alice (level 3) contributes 15, over the 10-pt budget. + levelContribution: { ...levelOn, formula: 'level * 5', weight: 10 }, + students: [{ ...makeStudent(1, 'Alice'), level: 3 }], + }); + expect( + screen.getByLabelText(/above the maximum level contributions/i), + ).toBeInTheDocument(); + }); + + it('shows below-only tooltip when a student contribution falls below 0', () => { + renderWeighted({ + gamificationEnabled: true, + // level - 10 → Alice (level 1) contributes -9, below 0. + levelContribution: { ...levelOn, formula: 'level - 10', weight: 10 }, + students: [{ ...makeStudent(1, 'Alice'), level: 1 }], + }); + expect(screen.getByLabelText(/below 0/i)).toBeInTheDocument(); + }); + + it('shows both-bounds tooltip when students breach both bounds', () => { + renderWeighted({ + gamificationEnabled: true, + // level - 3 with weight 5: Alice (level 1) → -2 (below 0), Bob (level 10) → 7 (above 5). + levelContribution: { ...levelOn, formula: 'level - 3', weight: 5 }, + students: [ + { ...makeStudent(1, 'Alice'), level: 1 }, + { ...makeStudent(2, 'Bob'), level: 10 }, + ], + }); + expect( + screen.getByLabelText(/outside the valid range/i), + ).toBeInTheDocument(); + }); + + it('does not flag the subheader when every student is within budget', () => { + renderWeighted({ + gamificationEnabled: true, + // level → Alice (level 3) contributes 3, within the 10-pt budget. + levelContribution: { ...levelOn, formula: 'level', weight: 10 }, + students: [{ ...makeStudent(1, 'Alice'), level: 3 }], + }); + expect( + screen.queryByLabelText( + /above the level weight|below 0|outside the valid range/i, + ), + ).not.toBeInTheDocument(); + }); + + // Regression: neither level column is user-pickable, so a stale persisted-hidden + // entry from a prior session must NOT keep them hidden (no picker to recover). + it('keeps Level Contribution visible despite a stale persisted-hidden entry', () => { + localStorage.setItem( + WEIGHTED_STORAGE_KEY, + JSON.stringify({ levelContribution: false }), + ); + renderWeighted({ gamificationEnabled: true, levelContribution: levelOn }); + expect( + within(headerRow1()).getByText(LEVEL_CONTRIBUTION), + ).toBeInTheDocument(); + }); + + it('keeps raw Level visible despite a stale persisted-hidden entry', () => { + localStorage.setItem( + WEIGHTED_STORAGE_KEY, + JSON.stringify({ level: false }), + ); + renderWeighted({ gamificationEnabled: true, levelContribution: levelOn }); + expect(within(headerRow1()).getByText('Level')).toBeInTheDocument(); + }); + + // Regression: toggling settings while the table is already mounted (e.g. the + // teacher saves the dialog) must reveal the affected column live. + it('reveals raw Level when show is toggled on after mount', () => { + const props = { + assessments: [makeAssessment(100, 'Quiz 1', 10, 150)], + canManageWeights: true, + categories: [makeCategory(1, 'Cat A')], + courseId: 1, + courseMaxLevel: 10, + courseTitle: 'Test Course', + gamificationEnabled: true, + students: [makeStudent(1, 'Alice')], + submissions: [], + tabs: [makeTab(10, 'Tab 1', 1, 100)], + }; + const { rerender } = render( + , + { state: userState }, + ); + expect(within(headerRow1()).queryByText('Level')).not.toBeInTheDocument(); + + // rerender bypasses test-utils' TestApp wrapper, so re-wrap to keep providers. + rerender( + + + , + ); + expect(within(headerRow1()).getByText('Level')).toBeInTheDocument(); + }); + + it('reveals Level Contribution when the contribution is enabled after mount', () => { + const props = { + assessments: [makeAssessment(100, 'Quiz 1', 10, 150)], + canManageWeights: true, + categories: [makeCategory(1, 'Cat A')], + courseId: 1, + courseMaxLevel: 10, + courseTitle: 'Test Course', + gamificationEnabled: true, + students: [makeStudent(1, 'Alice')], + submissions: [], + tabs: [makeTab(10, 'Tab 1', 1, 100)], + }; + const { rerender } = render( + , + { state: userState }, + ); + expect( + within(headerRow1()).queryByText(LEVEL_CONTRIBUTION), + ).not.toBeInTheDocument(); + + rerender( + + + , + ); + expect( + within(headerRow1()).getByText(LEVEL_CONTRIBUTION), + ).toBeInTheDocument(); + }); + + it('shows a receipt icon and clamped value when a contribution is capped above', async () => { + renderWeighted({ + gamificationEnabled: true, + courseMaxLevel: 30, + students: [ + { ...makeStudent(1, 'Alice'), level: 8, levelContribution: 10 }, + ], + levelContribution: { + enabled: true, + show: false, + weight: 10, + formula: 'level * 5', // raw 40 -> clamped 10 + clamp: true, + }, + }); + const row = screen.getByText('Alice').closest('tr')!; + expect(within(row).getByTestId('WarningAmberIcon')).toBeInTheDocument(); + }); + + it('shows no receipt icon when clamp is off', () => { + renderWeighted({ + gamificationEnabled: true, + courseMaxLevel: 30, + students: [ + { ...makeStudent(1, 'Alice'), level: 8, levelContribution: 40 }, + ], + levelContribution: { + enabled: true, + show: false, + weight: 10, + formula: 'level * 5', // raw 40, shown raw because clamp off + clamp: false, + }, + }); + const row = screen.getByText('Alice').closest('tr')!; + expect( + within(row).queryByTestId('WarningAmberIcon'), + ).not.toBeInTheDocument(); + }); + + // Divide-by-zero: the formula is mathematically undefined for this student. + // Store/backend coerce it to null; the cell must surface 0 + a warning to + // match the clamp cells and the dialog's "set to 0" copy. + const divByZeroProps = { + gamificationEnabled: true, + courseMaxLevel: 30, + students: [ + { ...makeStudent(1, 'Alice'), level: 8, levelContribution: null }, + ], + levelContribution: { + enabled: true, + show: false, + weight: 30, + formula: '30 / (level - 8)', // at level 8 -> divide by zero -> NaN + clamp: true, + }, + }; + + it('renders 0 with a warning icon when the formula divides by zero', () => { + renderWeighted(divByZeroProps); + // checkbox | Name | Level Contribution | Tab 1 | Total + const cells = within(screen.getByText('Alice').closest('tr')!).getAllByRole( + 'cell', + ); + expect( + within(cells[2]).getByTestId('WarningAmberIcon'), + ).toBeInTheDocument(); + expect(cells[2]).toHaveTextContent('0'); + }); + + it('explains the divide-by-zero fallback in the cell tooltip on hover', async () => { + const user = userEvent.setup(); + renderWeighted(divByZeroProps); + const row = screen.getByText('Alice').closest('tr')!; + await user.hover(within(row).getByTestId('WarningAmberIcon')); + expect(await screen.findByRole('tooltip')).toHaveTextContent( + /divides by zero/i, + ); + }); + + it('still shows 0 with a warning on divide-by-zero when clamp is off', () => { + renderWeighted({ + ...divByZeroProps, + levelContribution: { ...divByZeroProps.levelContribution, clamp: false }, + }); + const cells = within(screen.getByText('Alice').closest('tr')!).getAllByRole( + 'cell', + ); + expect( + within(cells[2]).getByTestId('WarningAmberIcon'), + ).toBeInTheDocument(); + expect(cells[2]).toHaveTextContent('0'); + }); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts b/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts index e6fe8abe6a..091424f2af 100644 --- a/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts +++ b/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts @@ -1,13 +1,19 @@ // client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts +import type { LevelContributionData } from 'types/course/gradebook'; + import { computeStudentBreakdown, computeStudentTotal, computeTabSubtotal, computeWeightedRows, + LEVEL_TAB_ID, + levelOffenders, + levelOutOfRange, resolveTabWeights, sumWeights, usingDefaultWeights, } from '../computeWeighted'; +import { parseFormula } from '../levelFormula'; const assessments = [ { id: 1, tabId: 10, maxGrade: 100, title: 'A' }, @@ -20,7 +26,7 @@ const subs = ( ): { studentId: number; assessmentId: number; grade: number | null }[] => entries; -describe('computeTabSubtotal — equal mode (default)', () => { +describe('computeTabSubtotal - equal mode (default)', () => { it('returns null when tab has no assessments', () => { expect( computeTabSubtotal({ @@ -76,7 +82,7 @@ describe('computeTabSubtotal — equal mode (default)', () => { }); }); -describe('computeTabSubtotal — custom mode', () => { +describe('computeTabSubtotal - custom mode', () => { const customTab = { id: 10, title: 'M', @@ -359,7 +365,7 @@ describe('sumWeights', () => { }); }); -describe('resolveTabWeights — equal-split default when unconfigured', () => { +describe('resolveTabWeights - equal-split default when unconfigured', () => { const twoTabs = [ { id: 10, title: 'M', categoryId: 0, gradebookWeight: 0 }, { id: 20, title: 'T', categoryId: 0, gradebookWeight: 0 }, @@ -475,6 +481,7 @@ describe('computeWeightedRows', () => { externalId: null, level: 1, totalXp: 0, + levelContribution: null, }, { id: 2, @@ -483,6 +490,7 @@ describe('computeWeightedRows', () => { externalId: null, level: 1, totalXp: 0, + levelContribution: null, }, ]; const rowSubmissions = subs([ @@ -584,7 +592,7 @@ describe('computeWeightedRows', () => { }); }); -describe('computeWeightedRows — identity passthrough', () => { +describe('computeWeightedRows - identity passthrough', () => { it('carries name, email and externalId from each student onto the row', () => { const students = [ { @@ -594,6 +602,7 @@ describe('computeWeightedRows — identity passthrough', () => { externalId: 'EXT-1', level: 5, totalXp: 1234, + levelContribution: null, }, ]; const tabs = [ @@ -703,7 +712,7 @@ describe('computeStudentBreakdown', () => { }); }); -describe('exclusion — equal mode', () => { +describe('exclusion - equal mode', () => { it('averages over included assessments only (excluded dropped from numerator and count)', () => { // a1 80/100=0.8 included, a2 excluded -> subtotal = 0.8 / 1 = 0.8 const withExcluded = [ @@ -736,7 +745,7 @@ describe('exclusion — equal mode', () => { }); }); -describe('exclusion — custom mode', () => { +describe('exclusion - custom mode', () => { it('drops excluded assessments from the numerator', () => { // tab weight 30; a1 weight 30 graded 90/100=0.9 -> 0.9*30=27; a2 excluded. // subtotal = 27 / 30 = 0.9 @@ -771,7 +780,7 @@ describe('exclusion — custom mode', () => { }); }); -describe('breakdown — exclusion', () => { +describe('breakdown - exclusion', () => { const bdAssessments = [ { id: 1, tabId: 10, maxGrade: 100, title: 'A' }, { id: 2, tabId: 10, maxGrade: 50, title: 'B', gradebookExcluded: true }, @@ -868,3 +877,220 @@ describe('zero-maxGrade assessments (0/0 must not produce NaN)', () => { expect(a.points).toBe(0); }); }); + +const lc = ( + over: Partial = {}, +): LevelContributionData => ({ + enabled: true, + formula: 'level / 30 * 8', // cap baked in literally - no maxLevel variable + weight: 8, + show: false, + clamp: true, + ...over, +}); + +describe('computeWeightedRows - level contribution', () => { + const baseStudent = { + id: 1, + name: 'A', + email: 'a@x', + externalId: null, + level: 15, + totalXp: 0, + }; + + it('reads levelContribution from the student object', () => { + const rows = computeWeightedRows({ + students: [{ ...baseStudent, levelContribution: 4 }], + tabs: [], + assessments: [], + submissions: [], + }); + expect(rows[0].levelContribution).toBeCloseTo(4); + expect(rows[0].total).toBeCloseTo(4); + }); + + it('contributes null when the student levelContribution is null', () => { + const rows = computeWeightedRows({ + students: [{ ...baseStudent, levelContribution: null }], + tabs: [], + assessments: [], + submissions: [], + }); + expect(rows[0].levelContribution).toBeNull(); + expect(rows[0].total).toBeNull(); + }); + + it('treats levelContribution as null when showLevelContribution is false', () => { + const rows = computeWeightedRows({ + students: [{ ...baseStudent, levelContribution: 4 }], + tabs: [], + assessments: [], + submissions: [], + showLevelContribution: false, + }); + expect(rows[0].levelContribution).toBeNull(); + expect(rows[0].total).toBeNull(); + }); + + it('carries level from student regardless of showLevelContribution', () => { + const rows = computeWeightedRows({ + students: [{ ...baseStudent, levelContribution: null }], + tabs: [], + assessments: [], + submissions: [], + }); + expect(rows[0].level).toBe(15); + }); +}); + +describe('levelOutOfRange', () => { + it('flags a student whose contribution exceeds the weight', () => { + const parsed = parseFormula('level'); // raw level as points + expect(parsed.ok).toBe(true); + if (parsed.ok) { + expect( + levelOutOfRange( + [{ level: 50 }], + lc({ formula: 'level', weight: 8 }), + parsed, + ), + ).toBe(true); + expect( + levelOutOfRange( + [{ level: 5 }], + lc({ formula: 'level', weight: 8 }), + parsed, + ), + ).toBe(false); + } + }); +}); + +describe('levelOffenders', () => { + const A = { id: 1, name: 'A', level: 5 }; + const B = { id: 2, name: 'B', level: 12 }; + const C = { id: 3, name: 'C', level: 3 }; + + it('returns no offenders when the formula does not parse', () => { + const parsed = parseFormula('level /'); + expect(levelOffenders([A], parsed, 10)).toEqual({ + below: [], + above: [], + unscoreable: [], + }); + }); + + it('returns no offenders when every contribution is within [0, max]', () => { + const parsed = parseFormula('level * 0.1'); // 0.5, 1.2 - within [0, 10] + if (!parsed.ok) throw new Error('expected ok'); + expect(levelOffenders([A, B], parsed, 10)).toEqual({ + below: [], + above: [], + unscoreable: [], + }); + }); + + it('lists students above the max, most extreme first', () => { + const parsed = parseFormula('level * 5'); // A 25, B 60, C 15 + if (!parsed.ok) throw new Error('expected ok'); + const { above, below } = levelOffenders([A, B, C], parsed, 10); + expect(below).toEqual([]); + expect(above.map((o) => o.name)).toEqual(['B', 'A', 'C']); + expect(above[0]).toMatchObject({ id: 2, name: 'B', value: 60 }); + }); + + it('lists students below 0, most negative first', () => { + const parsed = parseFormula('level - 8'); // A -3, B 4, C -5 + if (!parsed.ok) throw new Error('expected ok'); + const { above, below } = levelOffenders([A, B, C], parsed, 10); + expect(above).toEqual([]); + expect(below.map((o) => o.name)).toEqual(['C', 'A']); + expect(below[0]).toMatchObject({ id: 3, name: 'C', value: -5 }); + }); + + it('splits offenders across both bounds', () => { + const parsed = parseFormula('level * 5 - 30'); // A -5, B 30 + if (!parsed.ok) throw new Error('expected ok'); + const { above, below } = levelOffenders([A, B], parsed, 10); + expect(below.map((o) => o.value)).toEqual([-5]); + expect(above.map((o) => o.value)).toEqual([30]); + }); +}); + +describe('computeStudentBreakdown - level', () => { + it('appends a synthetic Level entry when enabled with pre-computed points', () => { + const breakdown = computeStudentBreakdown({ + studentId: 1, + tabs: [], + assessments: [], + submissions: [], + level: 15, + levelContribution: lc(), + levelContributionPoints: 4, // pre-computed: 15/30*8=4 + }); + const levelTab = breakdown.find((tb) => tb.tabId === LEVEL_TAB_ID); + expect(levelTab).toBeDefined(); + expect(levelTab!.assessments[0].title).toBe('Level'); + expect(levelTab!.assessments[0].points).toBeCloseTo(4); + expect(levelTab!.assessments[0].effectiveWeight).toBe(8); + }); + + it('omits the Level entry when disabled', () => { + const breakdown = computeStudentBreakdown({ + studentId: 1, + tabs: [], + assessments: [], + submissions: [], + level: 15, + levelContribution: lc({ enabled: false }), + levelContributionPoints: null, + }); + expect(breakdown.find((tb) => tb.tabId === LEVEL_TAB_ID)).toBeUndefined(); + }); + + it('omits the Level entry when levelContributionPoints is null (even if enabled)', () => { + const breakdown = computeStudentBreakdown({ + studentId: 1, + tabs: [], + assessments: [], + submissions: [], + level: 15, + levelContribution: lc(), + levelContributionPoints: null, + }); + expect(breakdown.find((tb) => tb.tabId === LEVEL_TAB_ID)).toBeUndefined(); + }); +}); + +describe('levelOffenders unscoreable bucket', () => { + const students = [ + { id: 1, name: 'Ann', level: 0 }, + { id: 2, name: 'Bob', level: 5 }, + { id: 3, name: 'Cy', level: 10 }, + ]; + + it('collects students whose contribution is NaN (divide-by-zero)', () => { + const parsed = parseFormula('100 / level'); // level 0 -> NaN + const r = levelOffenders(students, parsed, 50); + expect(r.unscoreable.map((o) => o.id)).toEqual([1]); + expect(r.unscoreable[0].level).toBe(0); + expect(r.below).toHaveLength(0); + expect(r.above).toHaveLength(0); + }); + + it('is empty when no student divides by zero', () => { + const parsed = parseFormula('level'); + expect(levelOffenders(students, parsed, 50).unscoreable).toHaveLength(0); + }); + + it('sorts unscoreable by level ascending', () => { + const parsed = parseFormula('100 / (level - 5)'); // NaN at level 5 only + const many = [ + { id: 1, name: 'Zoe', level: 5 }, + { id: 2, name: 'Amy', level: 5 }, + ]; + const r = levelOffenders(many, parsed, 50); + expect(r.unscoreable.map((o) => o.name)).toEqual(['Amy', 'Zoe']); + }); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/levelFormula.test.ts b/client/app/bundles/course/gradebook/__tests__/levelFormula.test.ts new file mode 100644 index 0000000000..4ea820b2a6 --- /dev/null +++ b/client/app/bundles/course/gradebook/__tests__/levelFormula.test.ts @@ -0,0 +1,360 @@ +import type { FormulaNode } from 'types/course/gradebook'; + +import { + evaluateNode, + parseFormula, + seedLevelFormula, + serializeFormula, +} from '../levelFormula'; + +const evalOk = (src: string, level: number): number => { + const parsed = parseFormula(src); + if (!parsed.ok) throw new Error(`expected ok, got: ${parsed.error}`); + return parsed.evaluate(level); +}; + +describe('parseFormula', () => { + it('evaluates arithmetic with correct precedence', () => { + expect(evalOk('1 + 2 * 3', 0)).toBe(7); + expect(evalOk('(1 + 2) * 3', 0)).toBe(9); + expect(evalOk('10 / 2 - 1', 0)).toBe(4); + }); + + it('binds level as a raw variable (no clamp)', () => { + expect(evalOk('level * 0.05', 15)).toBeCloseTo(0.75); + expect(evalOk('level', 40)).toBe(40); + }); + + it('caps with min (the common cap idiom)', () => { + expect(evalOk('min(level, 25) * 0.05', 25)).toBeCloseTo(1.25); + expect(evalOk('min(level, 25) * 0.05', 40)).toBeCloseTo(1.25); // capped + expect(evalOk('min(level, 25) * 0.05', 10)).toBeCloseTo(0.5); + }); + + it('supports min and max (two-arg)', () => { + expect(evalOk('max(0, level - 5) * 0.1', 5)).toBe(0); + expect(evalOk('max(0, level - 5) * 0.1', 30)).toBeCloseTo(2.5); + }); + + it('supports floor, ceil, round (one-arg)', () => { + expect(evalOk('floor(level / 5)', 12)).toBe(2); + expect(evalOk('ceil(level / 10)', 11)).toBe(2); + expect(evalOk('round(level / 3)', 5)).toBe(2); // 1.66 -> 2 + }); + + it('supports unary minus', () => { + expect(evalOk('-level + 10', 3)).toBe(7); + }); + + it('rejects maxLevel — it is no longer a variable', () => { + expect(parseFormula('maxLevel').ok).toBe(false); + expect(parseFormula('level / maxLevel').ok).toBe(false); + }); + + it('rejects unknown identifiers (no eval surface)', () => { + expect(parseFormula('system').ok).toBe(false); + expect(parseFormula('window').ok).toBe(false); + expect(parseFormula('level.constructor').ok).toBe(false); + }); + + it('rejects unknown functions and wrong arity', () => { + expect(parseFormula('sqrt(level)').ok).toBe(false); + expect(parseFormula('eval(level)').ok).toBe(false); + expect(parseFormula('min(level)').ok).toBe(false); // min needs 2 args + expect(parseFormula('floor(level, 2)').ok).toBe(false); // floor needs 1 arg + }); + + it('rejects malformed input with a message', () => { + const r = parseFormula('level / '); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.error).toMatch(/unfinished/i); + }); +}); + +// Plain-language errors for non-technical staff. The thrown message is shown +// verbatim under the formula field, so it must read like prose, never compiler +// jargon (no "identifier", "token", "rparen"). +const errorOf = (src: string): string => { + const r = parseFormula(src); + if (r.ok) throw new Error(`expected a parse error for "${src}"`); + return r.error; +}; + +describe('parseFormula — plain-language errors', () => { + it('never leaks compiler jargon', () => { + const jargon = /identifier|token|rparen|lparen|trailing input/i; + [ + 'lveel', + 'min(level 20)', + 'level %2', + 'level / ', + 'min(level,)', + 'level 2', + 'flor(level)', + ].forEach((src) => { + expect(errorOf(src)).not.toMatch(jargon); + }); + }); + + it('suggests the nearest variable for a misspelt word', () => { + expect(errorOf('lveel')).toContain('"lveel" is not recognised'); + expect(errorOf('lveel')).toContain('did you mean "level"'); + expect(errorOf('levl')).toContain('did you mean "level"'); + }); + + it('suggests the nearest function for a misspelt call', () => { + expect(errorOf('flor(level)')).toContain('did you mean "floor"'); + expect(errorOf('mn(level, 2)')).toContain('did you mean "min"'); + }); + + it('lists the valid words when nothing is close', () => { + const msg = errorOf('xyzzy'); + expect(msg).toContain('"xyzzy" is not recognised'); + expect(msg).not.toMatch(/did you mean/i); + expect(msg).toMatch(/level, floor, ceil, round, min, max/); + }); + + it('lists only functions when an unknown call has no close match', () => { + const msg = errorOf('frobnicate(level)'); + expect(msg).not.toMatch(/did you mean/i); + expect(msg).toMatch(/floor, ceil, round, min, max/); + expect(msg).not.toMatch(/level,/); + }); + + it('explains a missing comma with an example', () => { + expect(errorOf('min(level 20)')).toMatch(/comma/i); + expect(errorOf('min(level 20)')).toMatch(/min\(level, 25\)/); + }); + + it('explains a missing closing bracket', () => { + expect(errorOf('min(level, 20')).toMatch(/closing bracket/i); + }); + + it('explains a stray symbol', () => { + expect(errorOf('level % 2')).toContain('"%" cannot be used'); + }); + + it('explains an unfinished formula', () => { + expect(errorOf('level *')).toMatch(/unfinished/i); + }); + + it('flags two values with a missing operator', () => { + expect(errorOf('level 2')).toMatch(/operator is missing/i); + expect(errorOf('level 2')).toMatch(/level × 2/); + }); + + it('explains an invalid number', () => { + expect(errorOf('1.2.3')).toContain('"1.2.3" is not a valid number'); + }); +}); + +describe('seedLevelFormula', () => { + it('seeds default formula to `level`', () => { + expect(seedLevelFormula).toBe('level'); + }); +}); + +describe('evaluateNode — direct traversal', () => { + it('num: returns the literal value', () => { + const node: FormulaNode = { type: 'num', value: 3.5 }; + expect(evaluateNode(node, { level: 0 })).toBe(3.5); + }); + + it('var: returns the level from scope', () => { + const node: FormulaNode = { type: 'var', name: 'level' }; + expect(evaluateNode(node, { level: 15 })).toBe(15); + }); + + it('neg: negates operand', () => { + const node: FormulaNode = { + type: 'neg', + operand: { type: 'num', value: 4 }, + }; + expect(evaluateNode(node, { level: 0 })).toBe(-4); + }); + + it('binop +: adds left and right', () => { + const node: FormulaNode = { + type: 'binop', + op: '+', + left: { type: 'num', value: 3 }, + right: { type: 'var', name: 'level' }, + }; + expect(evaluateNode(node, { level: 7 })).toBe(10); + }); + + it('binop /: returns NaN on div-by-zero', () => { + const node: FormulaNode = { + type: 'binop', + op: '/', + left: { type: 'var', name: 'level' }, + right: { type: 'num', value: 0 }, + }; + expect(evaluateNode(node, { level: 5 })).toBeNaN(); + }); + + it('call1 floor: applies Math.floor', () => { + const node: FormulaNode = { + type: 'call1', + fn: 'floor', + arg: { + type: 'binop', + op: '/', + left: { type: 'var', name: 'level' }, + right: { type: 'num', value: 5 }, + }, + }; + expect(evaluateNode(node, { level: 12 })).toBe(2); // floor(12/5) = floor(2.4) = 2 + }); + + it('call2 min: returns smaller of two args', () => { + const node: FormulaNode = { + type: 'call2', + fn: 'min', + a: { type: 'var', name: 'level' }, + b: { type: 'num', value: 20 }, + }; + expect(evaluateNode(node, { level: 15 })).toBe(15); + expect(evaluateNode(node, { level: 25 })).toBe(20); + }); + + it('call2 max: returns larger of two args', () => { + const node: FormulaNode = { + type: 'call2', + fn: 'max', + a: { type: 'num', value: 0 }, + b: { + type: 'binop', + op: '-', + left: { type: 'var', name: 'level' }, + right: { type: 'num', value: 5 }, + }, + }; + expect(evaluateNode(node, { level: 3 })).toBe(0); // max(0, -2) + expect(evaluateNode(node, { level: 8 })).toBe(3); // max(0, 3) + }); +}); + +describe('serializeFormula — round-trips', () => { + it('serializes a number literal', () => { + const result = serializeFormula('3.5'); + expect(result.ok).toBe(true); + if (result.ok) expect(result.ast).toEqual({ type: 'num', value: 3.5 }); + }); + + it('serializes the level variable', () => { + const result = serializeFormula('level'); + expect(result.ok).toBe(true); + if (result.ok) expect(result.ast).toEqual({ type: 'var', name: 'level' }); + }); + + it('serializes unary minus', () => { + const result = serializeFormula('-level'); + expect(result.ok).toBe(true); + if (result.ok) + expect(result.ast).toEqual({ + type: 'neg', + operand: { type: 'var', name: 'level' }, + }); + }); + + it('serializes a binop tree for level * 0.5', () => { + const result = serializeFormula('level * 0.5'); + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.ast).toEqual({ + type: 'binop', + op: '*', + left: { type: 'var', name: 'level' }, + right: { type: 'num', value: 0.5 }, + }); + } + }); + + it('serializes min(level, 20)', () => { + const result = serializeFormula('min(level, 20)'); + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.ast).toEqual({ + type: 'call2', + fn: 'min', + a: { type: 'var', name: 'level' }, + b: { type: 'num', value: 20 }, + }); + } + }); + + it('serializes floor(level / 5)', () => { + const result = serializeFormula('floor(level / 5)'); + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.ast).toEqual({ + type: 'call1', + fn: 'floor', + arg: { + type: 'binop', + op: '/', + left: { type: 'var', name: 'level' }, + right: { type: 'num', value: 5 }, + }, + }); + } + }); + + it('returns ok:false on invalid input', () => { + const result = serializeFormula('level /'); + expect(result.ok).toBe(false); + }); + + it('round-trip: serialized AST evaluates identically to parseFormula', () => { + const formula = 'min(level, 25) * 0.05'; + const parsed = parseFormula(formula); + const serialized = serializeFormula(formula); + expect(parsed.ok).toBe(true); + expect(serialized.ok).toBe(true); + if (parsed.ok && serialized.ok) { + [10, 25, 40].forEach((level) => { + expect(evaluateNode(serialized.ast, { level })).toBeCloseTo( + parsed.evaluate(level), + ); + }); + } + }); +}); + +describe('constant divide-by-zero (always divides by zero)', () => { + it('rejects a literal divide by zero', () => { + const r = parseFormula('level / 0'); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.error).toMatch(/always divides by zero/i); + }); + + it('rejects a constant expression that evaluates to zero', () => { + const r = parseFormula('100 / (5 - 5)'); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.error).toMatch(/always divides by zero/i); + }); + + it('rejects a nested constant zero divisor', () => { + const r = parseFormula('level / (1 / 0)'); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.error).toMatch(/always divides by zero/i); + }); + + it('accepts a constant non-zero divisor', () => { + expect(parseFormula('floor(level / 5)').ok).toBe(true); + }); + + it('accepts a level-dependent divisor', () => { + expect(parseFormula('100 / level').ok).toBe(true); + expect(parseFormula('100 / (level - 2)').ok).toBe(true); + }); + + it('returns NaN when the divisor evaluates to zero for a student', () => { + expect(evalOk('level / (level - 5)', 5)).toBeNaN(); + }); + + it('still divides normally when the divisor is non-zero', () => { + expect(evalOk('level / (level - 5)', 7)).toBeCloseTo(3.5); + }); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/operations.test.ts b/client/app/bundles/course/gradebook/__tests__/operations.test.ts new file mode 100644 index 0000000000..e1a3fca281 --- /dev/null +++ b/client/app/bundles/course/gradebook/__tests__/operations.test.ts @@ -0,0 +1,55 @@ +import CourseAPI from 'api/course'; + +import { updateGradebookWeights } from '../operations'; + +jest.mock('api/course', () => ({ + gradebook: { + updateWeights: jest.fn(), + }, +})); + +const mockedUpdateWeights = CourseAPI.gradebook.updateWeights as jest.Mock; + +describe('updateGradebookWeights', () => { + beforeEach(() => { + mockedUpdateWeights.mockReset(); + }); + + // The backend response does NOT echo formulaAst. The operation must merge the + // client-side formulaAst back into the dispatched payload so the reducer can + // optimistically recompute each student's levelContribution. Dispatching the + // raw response strands formulaAst undefined → reducer nulls every student. + it('merges formulaAst back into the dispatched action', async () => { + const formulaAst = { + type: 'call2' as const, + fn: 'min' as const, + a: { type: 'var' as const, name: 'level' as const }, + b: { type: 'num' as const, value: 30 }, + }; + mockedUpdateWeights.mockResolvedValue({ + data: { + weights: [], + levelContribution: { + enabled: true, + formula: 'min(level, 30)', + weight: 10, + show: false, + }, + }, + }); + + const dispatch = jest.fn(); + await updateGradebookWeights([], { + enabled: true, + formula: 'min(level, 30)', + formulaAst, + weight: 10, + show: false, + clamp: true, + })(dispatch, jest.fn(), {}); + + expect(dispatch).toHaveBeenCalledTimes(1); + const dispatched = dispatch.mock.calls[0][0]; + expect(dispatched.payload.levelContribution.formulaAst).toEqual(formulaAst); + }); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/store.test.ts b/client/app/bundles/course/gradebook/__tests__/store.test.ts index 97143ade93..394d406a0b 100644 --- a/client/app/bundles/course/gradebook/__tests__/store.test.ts +++ b/client/app/bundles/course/gradebook/__tests__/store.test.ts @@ -15,6 +15,14 @@ const baseState = { gamificationEnabled: false, weightedViewEnabled: false, canManageWeights: false, + courseMaxLevel: 0, + levelContribution: { + enabled: false, + formula: '', + weight: 0, + show: false, + clamp: true, + }, }; describe('UPDATE_TAB_WEIGHTS reducer', () => { @@ -126,6 +134,14 @@ describe('UPDATE_TAB_WEIGHTS reducer', () => { gamificationEnabled: false, weightedViewEnabled: true, canManageWeights: true, + courseMaxLevel: 0, + levelContribution: { + enabled: false, + formula: '', + weight: 0, + show: false, + clamp: true, + }, }), ); @@ -151,3 +167,287 @@ describe('UPDATE_TAB_WEIGHTS reducer', () => { ); }); }); + +describe('level contribution', () => { + it('stores levelContribution and courseMaxLevel on SAVE_GRADEBOOK', () => { + const data = { + categories: [], + tabs: [], + assessments: [], + students: [], + submissions: [], + gamificationEnabled: true, + weightedViewEnabled: true, + canManageWeights: true, + courseMaxLevel: 20, + levelContribution: { + enabled: true, + formula: 'level / 20 * 8', + weight: 8, + show: true, + clamp: true, + }, + }; + const next = reducer(undefined, actions.saveGradebook(data)); + expect(next.courseMaxLevel).toBe(20); + expect(next.levelContribution.enabled).toBe(true); + expect(next.levelContribution.weight).toBe(8); + }); + + it('applies an echoed levelContribution on UPDATE_TAB_WEIGHTS', () => { + const next = reducer( + undefined, + actions.updateTabWeights({ + weights: [], + levelContribution: { + enabled: true, + formula: 'level', + formulaAst: null, + weight: 5, + show: false, + clamp: true, + }, + }), + ); + expect(next.levelContribution.enabled).toBe(true); + expect(next.levelContribution.weight).toBe(5); + }); + + it('recomputes student levelContribution from formulaAst when enabled', () => { + const stateWithStudents = reducer( + undefined, + actions.saveGradebook({ + categories: [], + tabs: [], + assessments: [], + students: [ + { + id: 1, + name: 'A', + email: 'a@x', + externalId: null, + level: 10, + totalXp: 0, + levelContribution: null, + }, + { + id: 2, + name: 'B', + email: 'b@x', + externalId: null, + level: 20, + totalXp: 0, + levelContribution: null, + }, + ], + submissions: [], + gamificationEnabled: true, + weightedViewEnabled: true, + canManageWeights: true, + courseMaxLevel: 30, + levelContribution: { + enabled: false, + formula: '', + weight: 0, + show: false, + clamp: true, + }, + }), + ); + // formula: 'level * 0.5' → AST: binop(*) [var(level), num(0.5)] + const formulaAst = { + type: 'binop' as const, + op: '*' as const, + left: { type: 'var' as const, name: 'level' as const }, + right: { type: 'num' as const, value: 0.5 }, + }; + const next = reducer( + stateWithStudents, + actions.updateTabWeights({ + weights: [], + levelContribution: { + enabled: true, + formula: 'level * 0.5', + formulaAst, + weight: 15, + show: false, + clamp: true, + }, + }), + ); + expect(next.students[0].levelContribution).toBeCloseTo(5); // 10 * 0.5 + expect(next.students[1].levelContribution).toBeCloseTo(10); // 20 * 0.5 + }); + + it('clamps recomputed levelContribution to [0, weight] when clamp is on', () => { + const stateWithStudents = reducer( + undefined, + actions.saveGradebook({ + categories: [], + tabs: [], + assessments: [], + students: [ + { + id: 1, + name: 'A', + email: 'a@x', + externalId: null, + level: 10, + totalXp: 0, + levelContribution: null, + }, + { + id: 2, + name: 'B', + email: 'b@x', + externalId: null, + level: 20, + totalXp: 0, + levelContribution: null, + }, + ], + submissions: [], + gamificationEnabled: true, + weightedViewEnabled: true, + canManageWeights: true, + courseMaxLevel: 30, + levelContribution: { + enabled: false, + formula: '', + weight: 0, + show: false, + clamp: true, + }, + }), + ); + // formula: 'level + 5' → AST: binop(+) [var(level), num(5)] + const formulaAst = { + type: 'binop' as const, + op: '+' as const, + left: { type: 'var' as const, name: 'level' as const }, + right: { type: 'num' as const, value: 5 }, + }; + const next = reducer( + stateWithStudents, + actions.updateTabWeights({ + weights: [], + levelContribution: { + enabled: true, + formula: 'level + 5', + formulaAst, + weight: 12, + show: false, + clamp: true, + }, + }), + ); + // raw 10 + 5 = 15 → clamped to weight 12; raw 20 + 5 = 25 → clamped to 12 + expect(next.students[0].levelContribution).toBeCloseTo(12); + expect(next.students[1].levelContribution).toBeCloseTo(12); + }); + + it('leaves recomputed levelContribution unclamped when clamp is off', () => { + const stateWithStudents = reducer( + undefined, + actions.saveGradebook({ + categories: [], + tabs: [], + assessments: [], + students: [ + { + id: 1, + name: 'A', + email: 'a@x', + externalId: null, + level: 10, + totalXp: 0, + levelContribution: null, + }, + ], + submissions: [], + gamificationEnabled: true, + weightedViewEnabled: true, + canManageWeights: true, + courseMaxLevel: 30, + levelContribution: { + enabled: false, + formula: '', + weight: 0, + show: false, + clamp: true, + }, + }), + ); + const formulaAst = { + type: 'binop' as const, + op: '+' as const, + left: { type: 'var' as const, name: 'level' as const }, + right: { type: 'num' as const, value: 5 }, + }; + const next = reducer( + stateWithStudents, + actions.updateTabWeights({ + weights: [], + levelContribution: { + enabled: true, + formula: 'level + 5', + formulaAst, + weight: 12, + show: false, + clamp: false, + }, + }), + ); + // clamp off → raw 10 + 5 = 15 kept as-is + expect(next.students[0].levelContribution).toBeCloseTo(15); + }); + + it('sets all students levelContribution to null when level contribution disabled', () => { + const stateWithStudents = reducer( + undefined, + actions.saveGradebook({ + categories: [], + tabs: [], + assessments: [], + students: [ + { + id: 1, + name: 'A', + email: 'a@x', + externalId: null, + level: 10, + totalXp: 0, + levelContribution: 5, + }, + ], + submissions: [], + gamificationEnabled: true, + weightedViewEnabled: true, + canManageWeights: true, + courseMaxLevel: 30, + levelContribution: { + enabled: true, + formula: 'level * 0.5', + weight: 15, + show: false, + clamp: true, + }, + }), + ); + const next = reducer( + stateWithStudents, + actions.updateTabWeights({ + weights: [], + levelContribution: { + enabled: false, + formula: '', + formulaAst: null, + weight: 0, + show: false, + clamp: true, + }, + }), + ); + expect(next.students[0].levelContribution).toBeNull(); + }); +}); diff --git a/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx b/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx index 8937cd5388..46cfdad6d9 100644 --- a/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx +++ b/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx @@ -5,6 +5,7 @@ import { Alert, Checkbox, Collapse, + FormControlLabel, IconButton, Stack, TextField, @@ -13,6 +14,9 @@ import { import type { AssessmentData, CategoryData, + LevelContributionData, + LevelContributionSaveData, + StudentData, TabData, } from 'types/course/gradebook'; @@ -23,7 +27,18 @@ import toast from 'lib/hooks/toast'; import useTranslation from 'lib/hooks/useTranslation'; import formTranslations from 'lib/translations/form'; -import { resolveTabWeights, usingDefaultWeights } from '../computeWeighted'; +import type { LevelOffender } from '../computeWeighted'; +import { + levelOffenders, + resolveTabWeights, + usingDefaultWeights, +} from '../computeWeighted'; +import { + ParsedFormula, + parseFormula, + seedLevelFormula, + serializeFormula, +} from '../levelFormula'; import { updateGradebookWeights } from '../operations'; const translations = defineMessages({ @@ -128,12 +143,81 @@ const translations = defineMessages({ defaultMessage: 'No weights set yet - these are suggested defaults with every tab counting equally. Save to confirm, or adjust below.', }, + levelTitle: { + id: 'course.gradebook.ConfigureWeightsPrompt.levelTitle', + defaultMessage: 'Level contribution', + }, + levelSubtitle: { + id: 'course.gradebook.ConfigureWeightsPrompt.levelSubtitle', + defaultMessage: + "Adds grade-points from each student's level, on top of tab contributions.", + }, + levelHighestStudent: { + id: 'course.gradebook.ConfigureWeightsPrompt.levelHighestStudent', + defaultMessage: 'Highest student level: {level}', + }, + levelCourseMax: { + id: 'course.gradebook.ConfigureWeightsPrompt.levelCourseMax', + defaultMessage: 'Course maximum level: {courseMaxLevel}', + }, + levelClampLabel: { + id: 'course.gradebook.ConfigureWeightsPrompt.levelClampLabel', + defaultMessage: 'Keep results between 0 and max level contributions', + }, + levelFormulaLabel: { + id: 'course.gradebook.ConfigureWeightsPrompt.levelFormulaLabel', + defaultMessage: 'Formula', + }, + levelFormulaHelper: { + id: 'course.gradebook.ConfigureWeightsPrompt.levelFormulaHelper', + defaultMessage: + 'Formulas may contain numbers, arithmetic operators (+ − * /), parentheses, "level" as a variable, and the functions floor, ceil, round, min and max.', + }, + levelShowLabel: { + id: 'course.gradebook.ConfigureWeightsPrompt.levelShowLabel', + defaultMessage: 'Show level column in table', + }, + levelOffendersAbove: { + id: 'course.gradebook.ConfigureWeightsPrompt.levelOffendersAbove', + defaultMessage: + '{count, plural, =1 {{name1} is above {max}.} =2 {{name1} and {name2} are above {max}.} other {{name1}, {name2} and {extra} more are above {max}.}}', + }, + levelOffendersBelow: { + id: 'course.gradebook.ConfigureWeightsPrompt.levelOffendersBelow', + defaultMessage: + '{count, plural, =1 {{name1} is below 0.} =2 {{name1} and {name2} are below 0.} other {{name1}, {name2} and {extra} more are below 0.}}', + }, + levelFixAtMost: { + id: 'course.gradebook.ConfigureWeightsPrompt.levelFixAtMost', + defaultMessage: 'These contributions will be set to {max}.', + }, + levelFixAtLeast: { + id: 'course.gradebook.ConfigureWeightsPrompt.levelFixAtLeast', + defaultMessage: 'These contributions will be set to 0.', + }, + levelFixBetween: { + id: 'course.gradebook.ConfigureWeightsPrompt.levelFixBetween', + defaultMessage: + 'Contributions below 0 will be set to 0, and contributions above {max} will be set to {max}.', + }, + levelUnscoreable: { + id: 'course.gradebook.ConfigureWeightsPrompt.levelUnscoreable', + defaultMessage: + '{count, plural, =1 {The formula divides by zero for {name1}, so their level contribution is set to 0.} =2 {The formula divides by zero for {name1} and {name2}, so their level contributions are set to 0.} other {The formula divides by zero for {name1}, {name2} and {extra} more, so their level contributions are set to 0.}}', + }, }); type WeightMode = 'equal' | 'custom'; const r2 = (n: number): number => Math.round(n * 100) / 100; const cents = (n: number): number => Math.round(n * 100); +// "Bob (60.00)" — student name with their contribution at 2dp, for the warning. +const fmtOffender = (o: LevelOffender): string => + `${o.name} (${r2(o.value).toFixed(2)})`; +// "Bob (level 0)" — student name with their level, for the divide-by-zero warning +// (the contribution value is undefined, so we show level instead). +const fmtUnscoreable = (o: LevelOffender): string => + `${o.name} (level ${o.level})`; // Distribute a tab total across assessment ids at 2dp; the last id absorbs the rounding // remainder so the seeded values sum back exactly to total. const distributeEqual = ( @@ -156,6 +240,10 @@ interface Props { categories: CategoryData[]; tabs: TabData[]; assessments: AssessmentData[]; + gamificationEnabled: boolean; + courseMaxLevel: number; + levelContribution: LevelContributionData; + students: StudentData[]; } const ConfigureWeightsPrompt: FC = ({ @@ -164,6 +252,10 @@ const ConfigureWeightsPrompt: FC = ({ categories, tabs, assessments, + gamificationEnabled, + courseMaxLevel, + levelContribution, + students, }) => { const { t } = useTranslation(); const dispatch = useAppDispatch(); @@ -204,6 +296,17 @@ const ConfigureWeightsPrompt: FC = ({ const [expanded, setExpanded] = useState>({}); const [submitting, setSubmitting] = useState(false); + const [levelEnabled, setLevelEnabled] = useState(levelContribution.enabled); + const [levelFormula, setLevelFormula] = useState( + levelContribution.formula || seedLevelFormula, + ); + // Seed the suggested maximum to the course's top level. + const [levelWeight, setLevelWeight] = useState( + levelContribution.weight || courseMaxLevel, + ); + const [levelShow, setLevelShow] = useState(levelContribution.show); + const [levelClamp, setLevelClamp] = useState(levelContribution.clamp); + useEffect(() => { if (open) { setWeights(seedWeights()); @@ -211,6 +314,11 @@ const ConfigureWeightsPrompt: FC = ({ setAssessmentWeights(seedAssessmentWeights()); setExcluded(seedExclusions()); setExpanded({}); + setLevelEnabled(levelContribution.enabled); + setLevelFormula(levelContribution.formula || seedLevelFormula); + setLevelWeight(levelContribution.weight || courseMaxLevel); + setLevelShow(levelContribution.show); + setLevelClamp(levelContribution.clamp); } }, [open]); @@ -239,7 +347,31 @@ const ConfigureWeightsPrompt: FC = ({ const effectiveWeight = (tabId: number): number => isAllExcluded(tabId) ? 0 : weights[tabId] ?? 0; - const sum = tabs.reduce((acc, tb) => acc + effectiveWeight(tb.id), 0); + const parsedLevel: ParsedFormula | null = + levelEnabled && levelFormula ? parseFormula(levelFormula) : null; + const levelParseError = + parsedLevel && !parsedLevel.ok ? parsedLevel.error : null; + // Students whose contribution falls outside [0, levelWeight], split by bound — + // the out-of-range warning names the worst offenders on each side. + const offenders = levelOffenders(students, parsedLevel, levelWeight); + // The cohort's real top level — distinct from courseMaxLevel (the course ceiling). + // Helps staff pick a sensible cap; null when there are no students to read. + const highestStudentLevel = students.length + ? Math.max(...students.map((s) => s.level)) + : null; + // Which bound(s) the cohort breaches — drives a message scoped to the actual + // problem (only-below, only-above, or both). + const hasBelow = offenders.below.length > 0; + const hasAbove = offenders.above.length > 0; + const hasUnscoreable = offenders.unscoreable.length > 0; + // Fix instruction matching the breached bound(s). + let levelFixMessage = translations.levelFixAtLeast; + if (hasBelow && hasAbove) levelFixMessage = translations.levelFixBetween; + else if (hasAbove) levelFixMessage = translations.levelFixAtMost; + + const sum = + tabs.reduce((acc, tb) => acc + effectiveWeight(tb.id), 0) + + (levelEnabled ? levelWeight : 0); const hasInvalid = Object.values(weights).some((w) => validate(w) !== null) || Object.values(assessmentWeights).some((w) => validate(w) !== null); @@ -278,9 +410,19 @@ const ConfigureWeightsPrompt: FC = ({ setExpanded((prev) => ({ ...prev, [tabId]: !prev[tabId] })); const handleSave = async (): Promise => { - if (hasInvalid || hasUnbalanced) return; + if (hasInvalid || hasUnbalanced || !!levelParseError) return; setSubmitting(true); try { + const serialized = levelEnabled ? serializeFormula(levelFormula) : null; + const formulaAst = serialized?.ok ? serialized.ast : null; + const lcPayload: LevelContributionSaveData = { + enabled: levelEnabled, + formula: levelFormula, + formulaAst, + weight: levelWeight, + show: levelShow, + clamp: levelClamp, + }; await dispatch( updateGradebookWeights( tabs.map((tb) => { @@ -304,6 +446,7 @@ const ConfigureWeightsPrompt: FC = ({ } return entry; }), + lcPayload, ), ); onClose(); @@ -319,7 +462,9 @@ const ConfigureWeightsPrompt: FC = ({ onClickPrimary={handleSave} onClose={onClose} open={open} - primaryDisabled={submitting || hasInvalid || hasUnbalanced} + primaryDisabled={ + submitting || hasInvalid || hasUnbalanced || !!levelParseError + } primaryLabel={t(formTranslations.save)} title={t(translations.promptTitle)} > @@ -585,6 +730,151 @@ const ConfigureWeightsPrompt: FC = ({ ))} + {gamificationEnabled && ( +
+
+ setLevelEnabled(e.target.checked)} + size="small" + /> + } + label={ + + {t(translations.levelTitle)} + + } + sx={{ flex: 1, mr: 0 }} + /> + setLevelWeight((prev) => r2(prev))} + onChange={(e) => + setLevelWeight( + e.target.value === '' ? 0 : Number(e.target.value), + ) + } + size="small" + sx={{ width: 96 }} + type="number" + value={levelEnabled ? levelWeight : 0} + /> +
+ {levelEnabled && ( + + + {t(translations.levelSubtitle)} + + + {t(translations.levelFormulaHelper)} + + setLevelFormula(e.target.value)} + size="small" + value={levelFormula} + /> + {highestStudentLevel != null && ( + + {t(translations.levelHighestStudent, { + level: highestStudentLevel, + })} + + )} + + {t(translations.levelCourseMax, { courseMaxLevel })} + + setLevelShow(e.target.checked)} + size="small" + sx={{ py: 0.25 }} + /> + } + label={ + + {t(translations.levelShowLabel)} + + } + /> + setLevelClamp(e.target.checked)} + size="small" + sx={{ py: 0.25 }} + /> + } + label={ + + {t(translations.levelClampLabel)} + + } + /> + {((levelClamp && (hasBelow || hasAbove)) || hasUnscoreable) && ( + + + {[ + levelClamp && + hasAbove && + t(translations.levelOffendersAbove, { + count: offenders.above.length, + name1: fmtOffender(offenders.above[0]), + name2: offenders.above[1] + ? fmtOffender(offenders.above[1]) + : '', + extra: Math.max(0, offenders.above.length - 2), + max: levelWeight, + }), + levelClamp && + hasBelow && + t(translations.levelOffendersBelow, { + count: offenders.below.length, + name1: fmtOffender(offenders.below[0]), + name2: offenders.below[1] + ? fmtOffender(offenders.below[1]) + : '', + extra: Math.max(0, offenders.below.length - 2), + }), + hasUnscoreable && + t(translations.levelUnscoreable, { + count: offenders.unscoreable.length, + name1: fmtUnscoreable(offenders.unscoreable[0]), + name2: offenders.unscoreable[1] + ? fmtUnscoreable(offenders.unscoreable[1]) + : '', + extra: Math.max(0, offenders.unscoreable.length - 2), + }), + ] + .filter(Boolean) + .join(' ')} + + {levelClamp && (hasBelow || hasAbove) && ( + + {t(levelFixMessage, { max: levelWeight })} + + )} + + )} + + )} +
+ )} {t(translations.total, { sum })} diff --git a/client/app/bundles/course/gradebook/components/WeightedGradebookTable.tsx b/client/app/bundles/course/gradebook/components/WeightedGradebookTable.tsx index b02d21c017..9edccf4c95 100644 --- a/client/app/bundles/course/gradebook/components/WeightedGradebookTable.tsx +++ b/client/app/bundles/course/gradebook/components/WeightedGradebookTable.tsx @@ -1,10 +1,11 @@ import { Fragment, useLayoutEffect, useMemo, useRef, useState } from 'react'; -import { defineMessages } from 'react-intl'; +import { defineMessages, MessageDescriptor } from 'react-intl'; import { Download, InfoOutlined, KeyboardArrowDown, KeyboardArrowRight, + WarningAmber, } from '@mui/icons-material'; import { Alert, @@ -24,6 +25,7 @@ import { import type { AssessmentData, CategoryData, + LevelContributionData, StudentData, SubmissionData, TabData, @@ -45,9 +47,12 @@ import { computeStudentBreakdown, computeWeightedRows, gradeRatio, + LEVEL_TAB_ID, + levelOffenders, resolveTabWeights, usingDefaultWeights, } from '../computeWeighted'; +import { parseFormula } from '../levelFormula'; import ConfigureWeightsPrompt from './ConfigureWeightsPrompt'; import ProjectedTotalHint, { @@ -165,6 +170,46 @@ const translations = defineMessages({ id: 'course.gradebook.WeightedGradebookTable.weightedTotal', defaultMessage: 'Weighted Total', }, + levelHeader: { + id: 'course.gradebook.WeightedGradebookTable.levelHeader', + defaultMessage: 'Level', + }, + levelContributionHeader: { + id: 'course.gradebook.WeightedGradebookTable.levelContributionHeader', + defaultMessage: 'Level Contribution', + }, + levelBreakdownDetail: { + id: 'course.gradebook.WeightedGradebookTable.levelBreakdownDetail', + defaultMessage: 'Level {level}', + }, + levelOverBudgetAboveOnly: { + id: 'course.gradebook.WeightedGradebookTable.levelOverBudgetAboveOnly', + defaultMessage: + 'Some level contributions are above the maximum level contributions and have been capped.', + }, + levelOverBudgetBelowOnly: { + id: 'course.gradebook.WeightedGradebookTable.levelOverBudgetBelowOnly', + defaultMessage: + 'Some level contributions are below 0 and have been raised to 0.', + }, + levelOverBudgetBoth: { + id: 'course.gradebook.WeightedGradebookTable.levelOverBudgetBoth', + defaultMessage: + 'Some level contributions are outside the valid range (below 0 or above the maximum level contributions) and are floored and capped accordingly.', + }, + levelCellCappedAbove: { + id: 'course.gradebook.WeightedGradebookTable.levelCellCappedAbove', + defaultMessage: 'Set to {weight} because the formula produced {raw}.', + }, + levelCellCappedBelow: { + id: 'course.gradebook.WeightedGradebookTable.levelCellCappedBelow', + defaultMessage: 'Set to 0 because the formula produced {raw}.', + }, + levelCellDivByZero: { + id: 'course.gradebook.WeightedGradebookTable.levelCellDivByZero', + defaultMessage: + 'Set to 0 because the formula divides by zero for this level.', + }, }); type DisplayMode = 'points' | 'percent'; @@ -178,6 +223,9 @@ interface Props { canManageWeights: boolean; courseTitle: string; courseId: number; + gamificationEnabled: boolean; + courseMaxLevel: number; + levelContribution: LevelContributionData; } const precisionNeeded = (v: number): 0 | 1 | 2 => { @@ -209,6 +257,9 @@ const NAME_WIDTH_EXPANDED = 260; const EMAIL_WIDTH = 250; const EXTERNAL_ID_WIDTH = 150; const TAB_WIDTH = 120; +const LEVEL_WIDTH = 90; +const LEVEL_CONTRIBUTIONS_WIDTH = 110; +const TOTAL_WIDTH = 120; const WeightedGradebookTable = ({ categories, @@ -219,6 +270,9 @@ const WeightedGradebookTable = ({ canManageWeights, courseTitle, courseId, + gamificationEnabled, + courseMaxLevel, + levelContribution, }: Props): JSX.Element => { const { t } = useTranslation(); const [configureOpen, setConfigureOpen] = useState(false); @@ -253,11 +307,86 @@ const WeightedGradebookTable = ({ ); }, [assessments]); - const totalWeight = resolvedTabs.reduce( + // Level Contribution column: shown whenever the contribution is active (drives + // the total + per-row points too). + const showLevelContribution = + gamificationEnabled && levelContribution.enabled; + // Raw Level column: the actual student level, shown alongside the contribution + // so staff can sanity-check it. Gated additionally on `show`. + // + // Both columns are server-controlled (not in the column picker), so their + // *presence* — not just their default visibility — is gated here, and they are + // locked visible (below). Plumbing these through `defaultVisible` failed: + // defaultVisible is a one-time seed that loses to stale persisted localStorage, + // stranding a column hidden with no picker entry to recover it. + const showRawLevel = showLevelContribution && levelContribution.show; + + // The level weight is a suggested maximum (never caps the total), so a formula + // can push a student's contribution past it. Flag that on the column subheader. + // Reuses the dialog's check so both views warn on identical conditions. + const levelBudgetOffenders = useMemo( + () => + showLevelContribution + ? levelOffenders( + students, + parseFormula(levelContribution.formula), + levelContribution.weight, + ) + : null, + [showLevelContribution, students, levelContribution], + ); + + // Students for whom the formula is undefined (divide-by-zero -> NaN). The + // store/backend store these as null; we surface them as 0 + a warning. + const unscoreableIds = useMemo( + () => new Set(levelBudgetOffenders?.unscoreable.map((o) => o.id) ?? []), + [levelBudgetOffenders], + ); + const levelOverBudget = + levelBudgetOffenders !== null && + (levelBudgetOffenders.below.length > 0 || + levelBudgetOffenders.above.length > 0); + const levelClampByStudent = useMemo(() => { + const map = new Map< + number, + { bound: 'below' | 'above' | 'unscoreable'; raw: number } + >(); + if (showLevelContribution && levelBudgetOffenders) { + if (levelContribution.clamp) { + levelBudgetOffenders.below.forEach((o) => + map.set(o.id, { bound: 'below', raw: o.value }), + ); + levelBudgetOffenders.above.forEach((o) => + map.set(o.id, { bound: 'above', raw: o.value }), + ); + } + levelBudgetOffenders.unscoreable.forEach((o) => + map.set(o.id, { bound: 'unscoreable', raw: NaN }), + ); + } + return map; + }, [showLevelContribution, levelContribution.clamp, levelBudgetOffenders]); + const getLevelOverBudgetTranslation = (): MessageDescriptor => { + const below = levelBudgetOffenders?.below.length ?? 0; + const above = levelBudgetOffenders?.above.length ?? 0; + if (below > 0 && above > 0) return translations.levelOverBudgetBoth; + if (above > 0) return translations.levelOverBudgetAboveOnly; + return translations.levelOverBudgetBelowOnly; + }; + const getLevelCellTranslation = ( + bound: 'below' | 'above' | 'unscoreable', + ): MessageDescriptor => { + if (bound === 'unscoreable') return translations.levelCellDivByZero; + if (bound === 'below') return translations.levelCellCappedBelow; + return translations.levelCellCappedAbove; + }; + const tabTotalWeight = resolvedTabs.reduce( (acc, tab) => acc + (allExcludedTabIds.has(tab.id) ? 0 : tab.gradebookWeight ?? 0), 0, ); + const totalWeight = + tabTotalWeight + (showLevelContribution ? levelContribution.weight : 0); const allWeightsZero = totalWeight === 0; const totalDisplayValue = (total: number | null): number | null => { @@ -296,20 +425,54 @@ const WeightedGradebookTable = ({ const nameWidth = expandedIds.size > 0 ? NAME_WIDTH_EXPANDED : NAME_WIDTH_COLLAPSED; + const studentLevelById = useMemo( + () => new Map(students.map((s) => [s.id, s.level])), + [students], + ); + + const studentLevelContributionById = useMemo( + () => new Map(students.map((s) => [s.id, s.levelContribution ?? null])), + [students], + ); + const breakdownsByStudent = useMemo( () => new Map( - [...expandedIds].map((studentId) => [ - studentId, - computeStudentBreakdown({ + [...expandedIds].map((studentId) => { + const breakdown = computeStudentBreakdown({ studentId, tabs: resolvedTabs, assessments, submissions, - }), - ]), + level: studentLevelById.get(studentId) ?? 0, + levelContribution: showLevelContribution + ? levelContribution + : undefined, + levelContributionPoints: showLevelContribution + ? studentLevelContributionById.get(studentId) ?? null + : null, + courseMaxLevel, + }); + // Level row first, mirroring the Level Contribution column being the + // first contribution column (left of the tabs). + const ordered = [ + ...breakdown.filter((tb) => tb.tabId === LEVEL_TAB_ID), + ...breakdown.filter((tb) => tb.tabId !== LEVEL_TAB_ID), + ]; + return [studentId, ordered] as [studentId: number, typeof ordered]; + }), ), - [expandedIds, resolvedTabs, assessments, submissions], + [ + expandedIds, + resolvedTabs, + assessments, + submissions, + studentLevelById, + studentLevelContributionById, + showLevelContribution, + levelContribution, + courseMaxLevel, + ], ); const row1Ref = useRef(null); @@ -369,16 +532,26 @@ const WeightedGradebookTable = ({ return () => observer.disconnect(); }, [visibleCategories, resolvedTabs]); - const rows = useMemo( - () => - computeWeightedRows({ - students, - tabs: resolvedTabs, - assessments, - submissions, - }), - [students, resolvedTabs, assessments, submissions], - ); + const rows = useMemo(() => { + const base = computeWeightedRows({ + students, + tabs: resolvedTabs, + assessments, + submissions, + showLevelContribution, + }); + if (unscoreableIds.size === 0) return base; + return base.map((r) => + unscoreableIds.has(r.studentId) ? { ...r, levelContribution: 0 } : r, + ); + }, [ + students, + resolvedTabs, + assessments, + submissions, + showLevelContribution, + unscoreableIds, + ]); const columnPrecisions = useMemo(() => { const tabPrecs = resolvedTabs.map((tab, idx) => @@ -391,6 +564,7 @@ const WeightedGradebookTable = ({ return { tabs: tabPrecs, total: columnPrecision(rows.map((r) => totalDisplayValue(r.total))), + level: columnPrecision(rows.map((r) => r.levelContribution ?? null)), }; }, [rows, resolvedTabs, displayMode, totalWeight]); @@ -429,6 +603,31 @@ const WeightedGradebookTable = ({ }, ]; + // Raw Level — the right-most student-info column, just left of the + // contribution columns. The actual student level, shown for verification. + if (showRawLevel) { + cols.push({ + id: 'level', + title: t(translations.levelHeader), + accessorFn: (row) => `${row.level}`, + cell: (row) => `${row.level}`, + csvDownloadable: true, + }); + } + + // Level Contribution — the left-most contribution column, immediately right + // of the student-info columns and before the per-tab columns. + if (showLevelContribution) { + cols.push({ + id: 'levelContribution', + title: t(translations.levelContributionHeader), + accessorFn: (row) => fmtCsv(row.levelContribution ?? null), + cell: (row) => + fmtDisplay(row.levelContribution ?? null, columnPrecisions.level), + csvDownloadable: true, + }); + } + resolvedTabs.forEach((tab, idx) => { const weight = tab.gradebookWeight ?? 0; const prec = columnPrecisions.tabs[idx]; @@ -460,19 +659,31 @@ const WeightedGradebookTable = ({ columnPrecisions, displayMode, totalWeight, + showLevelContribution, + showRawLevel, + levelContribution.weight, ]); + // Lock the level columns visible whenever present so a stale persisted-hidden + // entry can't keep them hidden (the picker doesn't expose them to recover). + const lockedColumns = useMemo(() => { + const locked = ['name']; + if (showLevelContribution) locked.push('levelContribution'); + if (showRawLevel) locked.push('level'); + return locked; + }, [showLevelContribution, showRawLevel]); + const columnPicker = useMemo( () => ({ render: (context: ColumnPickerRenderContext) => ( ), - locked: ['name'], + locked: lockedColumns, triggerLabel: t(translations.selectColumns), dialogTitle: t(translations.dialogTitle), storageKey: `gradebook_weighted_columns_${courseId}`, }), - [courseId, t], + [courseId, t, lockedColumns], ); const { @@ -513,6 +724,12 @@ const WeightedGradebookTable = ({ let lastIdentityField: 'name' | 'email' | 'externalId' = 'name'; if (showExternalId) lastIdentityField = 'externalId'; else if (showEmail) lastIdentityField = 'email'; + // These columns are locked visible when present, so visibility resolves to true; + // gating the manual header on it keeps the header in lockstep with the body + // (no colSpan drift) even on the first frame before the lock effect runs. + const showLevelContributionCol = + showLevelContribution && (visibility.levelContribution ?? true) === true; + const showRawLevelCol = showRawLevel && (visibility.level ?? true) === true; const allRowsSelected = body.allFilteredSelected ?? false; const someRowsSelected = body.someFilteredSelected ?? false; @@ -523,6 +740,11 @@ const WeightedGradebookTable = ({ ? t(translations.percentTotalExact) : t(translations.outOfWeight, { weight: totalWeight }); + const levelBudgetLabel = + displayMode === 'percent' + ? t(translations.percentOfGrade, { weight: levelContribution.weight }) + : t(translations.outOfWeight, { weight: levelContribution.weight }); + return (
{!allWeightsZero && !showingDefaults && } @@ -649,10 +871,14 @@ const WeightedGradebookTable = ({ {showEmail && } {showExternalId && } + {showRawLevelCol && } + {showLevelContributionCol && ( + + )} {resolvedTabs.map((tab) => ( ))} - + @@ -710,6 +936,36 @@ const WeightedGradebookTable = ({ {t(tableTranslations.externalId)} )} + {/* Raw Level — last student-info column (spans all 3 rows, no + weight subheader). */} + {showRawLevelCol && ( + + {t(translations.levelHeader)} + + )} + {/* Level Contribution — first contribution column, before the + category-grouped tabs. */} + {showLevelContributionCol && ( + + {t(translations.levelContributionHeader)} + + )} {visibleCategories.map((cat) => ( + {showLevelContributionCol && ( + + {levelContribution.clamp && + levelOverBudget && + levelBudgetOffenders ? ( + + + {levelBudgetLabel} + + + + ) : ( + levelBudgetLabel + )} + + )} {resolvedTabs.map((tab, i) => ( )} + {showRawLevelCol && ( + + {row.original.level} + + )} + {showLevelContributionCol && + ((): React.JSX.Element => { + const valueText = fmtDisplay( + row.original.levelContribution ?? null, + columnPrecisions.level, + ); + const info = levelClampByStudent.get( + row.original.studentId, + ); + return ( + + {info ? ( + + + + + {valueText} + + ) : ( + valueText + )} + + ); + })()} {row.original.subtotals.map((subtotal, i) => { const weight = resolvedTabs[i].gradebookWeight ?? 0; return ( @@ -918,7 +1249,7 @@ const WeightedGradebookTable = ({ {isExpanded && (breakdownsByStudent.get(studentId) ?? []).flatMap( - (tb, tabIdx) => + (tb) => tb.assessments.map((a) => { const isExcluded = a.excluded; const weightText = t( @@ -928,10 +1259,20 @@ const WeightedGradebookTable = ({ Math.round(a.effectiveWeight * 100) / 100, }, ); - const gradeText = + const assessmentGradeText = a.grade === null ? `-/${a.maxGrade}` : `${a.grade}/${a.maxGrade}`; + // The level row has no max — courseMaxLevel plays + // no part in the contribution, so showing + // "level/courseMaxLevel" would falsely imply a + // proportional derivation. Show the raw level only. + const gradeText = + tb.tabId === LEVEL_TAB_ID + ? t(translations.levelBreakdownDetail, { + level: a.grade ?? 0, + }) + : assessmentGradeText; return ( )} + {/* Mirror the parent row's two level columns so + the tab cells below stay column-aligned. Raw + Level has no per-assessment breakdown (empty); + Level Contribution carries its value on the + synthetic level row (tabId === LEVEL_TAB_ID), + matching how each tab cell carries its own. */} + {showRawLevelCol && } + {showLevelContributionCol && ( + + {tb.tabId === LEVEL_TAB_ID + ? fmtDisplay( + a.points, + columnPrecisions.level, + ) + : ''} + + )} {resolvedTabs.map((tab, i) => { const tabCellValue = isExcluded ? '—' @@ -1021,7 +1379,12 @@ const WeightedGradebookTable = ({ align="right" {...groupEndIf(tabIsCategoryEnd[i])} > - {i === tabIdx ? tabCellValue : ''} + {/* Place the value by tab id, not array + position, so the breakdown row order is + free to differ from the column order. */} + {tab.id === tb.tabId + ? tabCellValue + : ''} ); })} @@ -1044,8 +1407,12 @@ const WeightedGradebookTable = ({ setConfigureOpen(false)} open={configureOpen} + students={students} tabs={tabs} /> )} diff --git a/client/app/bundles/course/gradebook/computeWeighted.ts b/client/app/bundles/course/gradebook/computeWeighted.ts index 4eea81960b..40905fe848 100644 --- a/client/app/bundles/course/gradebook/computeWeighted.ts +++ b/client/app/bundles/course/gradebook/computeWeighted.ts @@ -1,19 +1,28 @@ // client/app/bundles/course/gradebook/computeWeighted.ts import { AssessmentData, + LevelContributionData, StudentData, SubmissionData, TabData, } from 'types/course/gradebook'; +import { ParsedFormula } from './levelFormula'; + type GradeEntry = Pick; +// Synthetic ids for the Level term — disjoint from real (positive) tab/assessment ids. +export const LEVEL_TAB_ID = -1; +export const LEVEL_ASSESSMENT_ID = -1; + export interface WeightedRow { studentId: number; name: string; email: string; externalId: string | null; subtotals: (number | null)[]; + level: number; + levelContribution: number | null; total: number | null; } @@ -139,6 +148,71 @@ const totalFromSubtotals = ( return contributingCount > 0 ? total : null; }; +// Combine the tab total with the Level term: null only when neither contributes. +const combineTotal = ( + tabTotal: number | null, + lvl: number | null, +): number | null => + tabTotal == null && lvl == null ? null : (tabTotal ?? 0) + (lvl ?? 0); + +// True when any student's contribution falls outside [0, weight] (drives the dialog warning). +export const levelOutOfRange = ( + students: { level: number }[], + cfg: LevelContributionData, + parsed: ParsedFormula, +): boolean => { + if (!parsed.ok) return false; + return students.some((s) => { + const p = parsed.evaluate(s.level); + return p < 0 || p > cfg.weight; + }); +}; + +export interface LevelOffender { + id: number; + name: string; + level: number; + value: number; +} + +export interface LevelOffenders { + // Below 0, most negative first; above max, highest first — so the dialog can + // name the worst offenders on each side. + below: LevelOffender[]; + above: LevelOffender[]; + // Students whose contribution is undefined (divide-by-zero) — value is NaN, + // so the dialog names them by level, not value. + unscoreable: LevelOffender[]; +} + +// Students whose Level contribution falls outside [0, max], split by which bound +// they breach. Empty on both sides when the formula is invalid. Feeds the dialog's +// out-of-range warning, which names the most extreme offenders. +export const levelOffenders = ( + students: { id: number; name: string; level: number }[], + parsed: ParsedFormula | null, + max: number, +): LevelOffenders => { + if (!parsed?.ok) return { below: [], above: [], unscoreable: [] }; + const evaluated = students.map((s) => ({ + id: s.id, + name: s.name, + level: s.level, + value: parsed.evaluate(s.level), + })); + return { + below: evaluated + .filter((e) => e.value < 0) + .sort((a, b) => a.value - b.value), + above: evaluated + .filter((e) => e.value > max) + .sort((a, b) => b.value - a.value), + unscoreable: evaluated + .filter((e) => Number.isNaN(e.value)) + .sort((a, b) => a.level - b.level || a.name.localeCompare(b.name)), + }; +}; + interface SubtotalArgs { studentId: number; tab: TabData; @@ -164,6 +238,11 @@ interface TotalArgs { tabs: TabData[]; assessments: AssessmentData[]; submissions: GradeEntry[]; + level?: number; + levelContribution?: LevelContributionData; + levelContributionPoints?: number | null; + // The course's current top level, used only as the Level breakdown row's denominator. + courseMaxLevel?: number; } export const computeStudentTotal = ({ @@ -171,6 +250,8 @@ export const computeStudentTotal = ({ tabs, assessments, submissions, + level, + levelContributionPoints, }: TotalArgs): number | null => { const gradeLookup = buildGradeLookup(submissions); const assessmentsByTab = buildAssessmentsByTab(assessments); @@ -182,7 +263,8 @@ export const computeStudentTotal = ({ gradeLookup, ), ); - return totalFromSubtotals(subtotals, tabs); + const tabTotal = totalFromSubtotals(subtotals, tabs); + return combineTotal(tabTotal, levelContributionPoints ?? null); }; export const computeStudentBreakdown = ({ @@ -190,10 +272,14 @@ export const computeStudentBreakdown = ({ tabs, assessments, submissions, + level, + levelContribution, + levelContributionPoints, + courseMaxLevel, }: TotalArgs): TabBreakdown[] => { const gradeLookup = buildGradeLookup(submissions); const assessmentsByTab = buildAssessmentsByTab(assessments); - return tabs.map((tab) => { + const result: TabBreakdown[] = tabs.map((tab) => { const list = assessmentsByTab.get(tab.id) ?? []; const weight = tab.gradebookWeight ?? 0; const includedCount = list.filter((a) => !a.gradebookExcluded).length; @@ -226,6 +312,25 @@ export const computeStudentBreakdown = ({ }); return { tabId: tab.id, assessments: contributions }; }); + + const lvl = levelContributionPoints ?? null; + if (levelContribution?.enabled && lvl != null) { + result.push({ + tabId: LEVEL_TAB_ID, + assessments: [ + { + assessmentId: LEVEL_ASSESSMENT_ID, + title: 'Level', + grade: level ?? 0, + maxGrade: courseMaxLevel ?? 0, + points: lvl, + effectiveWeight: levelContribution.weight, + excluded: false, + }, + ], + }); + } + return result; }; interface WeightedRowsArgs { @@ -233,6 +338,7 @@ interface WeightedRowsArgs { tabs: TabData[]; assessments: AssessmentData[]; submissions: GradeEntry[]; + showLevelContribution?: boolean; } // Batch entry point used by the table: builds the indexes ONCE and reuses them @@ -242,6 +348,7 @@ export const computeWeightedRows = ({ tabs, assessments, submissions, + showLevelContribution = true, }: WeightedRowsArgs): WeightedRow[] => { const gradeLookup = buildGradeLookup(submissions); const assessmentsByTab = buildAssessmentsByTab(assessments); @@ -254,13 +361,19 @@ export const computeWeightedRows = ({ gradeLookup, ), ); + const tabTotal = totalFromSubtotals(subtotals, tabs); + const lvl = showLevelContribution + ? student.levelContribution ?? null + : null; return { studentId: student.id, name: student.name, email: student.email, externalId: student.externalId, subtotals, - total: totalFromSubtotals(subtotals, tabs), + level: student.level, + levelContribution: lvl, + total: combineTotal(tabTotal, lvl), }; }); }; diff --git a/client/app/bundles/course/gradebook/levelFormula.ts b/client/app/bundles/course/gradebook/levelFormula.ts new file mode 100644 index 0000000000..bbeb8793b8 --- /dev/null +++ b/client/app/bundles/course/gradebook/levelFormula.ts @@ -0,0 +1,380 @@ +// A deliberately tiny arithmetic grammar mapping a student's Level to grade-points. +// PARSE, DON'T EVAL: the source is read into an AST of known node types and evaluated +// with plain arithmetic. No code path runs the string as code, so a hostile string can +// only ever produce a parse error. +// +// Grammar (standard precedence): +// expr := term (('+' | '-') term)* +// term := factor (('*' | '/') factor)* +// factor := '-' factor | primary +// primary := number | var | func '(' args ')' | '(' expr ')' +// var := 'level' +// func1 := 'floor' | 'ceil' | 'round' (1 arg) +// func2 := 'min' | 'max' (2 args) +// +// Examples (staff type these into the formula field): +// min(level, 20) * 0.5 → 0.5 grade-points per level, capped at level 20 (max 10) +// floor(level / 5) → 1 grade-point per 5 levels +// level → 1 grade-point per level, uncapped +// Anything outside this grammar (assignments, unknown identifiers, unknown +// functions, stray characters, …) is rejected at parse time — parseFormula +// returns { ok: false, error } rather than throwing or running the input. + +import type { FormulaNode } from 'types/course/gradebook'; + +interface Scope { + level: number; +} + +const FUNCTIONS_1: Record number> = { + floor: Math.floor, + ceil: Math.ceil, + round: Math.round, +}; +const FUNCTIONS_2: Record number> = { + min: Math.min, + max: Math.max, +}; +const VARIABLES = new Set(['level']); + +const FUNCTION_NAMES = [ + ...Object.keys(FUNCTIONS_1), + ...Object.keys(FUNCTIONS_2), +]; +const VALID_NAMES = [...VARIABLES, ...FUNCTION_NAMES]; + +const editDistance = (a: string, b: string): number => { + const rows = a.length; + const cols = b.length; + const dp: number[][] = Array.from({ length: rows + 1 }, () => + new Array(cols + 1).fill(0), + ); + for (let i = 0; i <= rows; i += 1) dp[i][0] = i; + for (let j = 0; j <= cols; j += 1) dp[0][j] = j; + for (let i = 1; i <= rows; i += 1) { + for (let j = 1; j <= cols; j += 1) { + const cost = a[i - 1] === b[j - 1] ? 0 : 1; + dp[i][j] = Math.min( + dp[i - 1][j] + 1, + dp[i][j - 1] + 1, + dp[i - 1][j - 1] + cost, + ); + } + } + return dp[rows][cols]; +}; + +const nearestName = ( + name: string, + candidates: readonly string[], +): string | null => { + const lower = name.toLowerCase(); + let best: string | null = null; + let bestDistance = Infinity; + candidates.forEach((candidate) => { + const distance = editDistance(lower, candidate); + if (distance < bestDistance) { + bestDistance = distance; + best = candidate; + } + }); + return bestDistance <= 2 ? best : null; +}; + +const unrecognisedError = ( + name: string, + candidates: readonly string[], +): string => { + const suggestion = nearestName(name, candidates); + return suggestion + ? `"${name}" is not recognised - did you mean "${suggestion}"?` + : `"${name}" is not recognised. You can use: ${candidates.join(', ')}.`; +}; + +export type ParsedFormula = + | { ok: true; evaluate: (level: number) => number } + | { ok: false; error: string }; + +type Token = + | { type: 'num'; value: number } + | { type: 'ident'; value: string } + | { type: 'op'; value: '+' | '-' | '*' | '/' } + | { type: 'lparen' } + | { type: 'rparen' } + | { type: 'comma' }; + +const isDigit = (c: string): boolean => c >= '0' && c <= '9'; +const isAlpha = (c: string): boolean => + (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z'); + +const tokenize = (src: string): Token[] => { + const tokens: Token[] = []; + let i = 0; + while (i < src.length) { + const c = src[i]; + if (c === ' ' || c === '\t' || c === '\n' || c === '\r') { + i += 1; + } else if (isDigit(c) || c === '.') { + let j = i + 1; + while (j < src.length && (isDigit(src[j]) || src[j] === '.')) j += 1; + const text = src.slice(i, j); + const value = Number(text); + if (!Number.isFinite(value)) + throw new Error(`"${text}" is not a valid number.`); + tokens.push({ type: 'num', value }); + i = j; + } else if (isAlpha(c)) { + let j = i + 1; + while (j < src.length && isAlpha(src[j])) j += 1; + tokens.push({ type: 'ident', value: src.slice(i, j) }); + i = j; + } else if (c === '+' || c === '-' || c === '−' || c === '*' || c === '/') { + const op = c === '−' ? '-' : c; + tokens.push({ type: 'op', value: op as '+' | '-' | '*' | '/' }); + i += 1; + } else if (c === '(') { + tokens.push({ type: 'lparen' }); + i += 1; + } else if (c === ')') { + tokens.push({ type: 'rparen' }); + i += 1; + } else if (c === ',') { + tokens.push({ type: 'comma' }); + i += 1; + } else { + throw new Error(`"${c}" cannot be used in a formula.`); + } + } + return tokens; +}; + +// Recursive-descent parser building a serializable FormulaNode AST. +const buildAst = (tokens: Token[]): FormulaNode => { + let pos = 0; + const peek = (): Token | undefined => tokens[pos]; + const eat = (): Token => { + const t = tokens[pos]; + if (!t) throw new Error('The formula looks unfinished.'); + pos += 1; + return t; + }; + const expect = (type: 'comma' | 'rparen'): void => { + const t = peek(); + if (!t || t.type !== type) { + throw new Error( + type === 'comma' + ? 'This needs a comma - for example min(level, 25).' + : 'This needs a closing bracket ")".', + ); + } + pos += 1; + }; + + let parseExpr: () => FormulaNode; + + const parsePrimary = (): FormulaNode => { + const t = eat(); + if (t.type === 'num') return { type: 'num', value: t.value }; + if (t.type === 'lparen') { + const inner = parseExpr(); + expect('rparen'); + return inner; + } + if (t.type === 'ident') { + const name = t.value; + if (peek()?.type === 'lparen') { + eat(); // consume '(' + if (FUNCTIONS_1[name]) { + const arg = parseExpr(); + expect('rparen'); + return { type: 'call1', fn: name as 'floor' | 'ceil' | 'round', arg }; + } + if (FUNCTIONS_2[name]) { + const a = parseExpr(); + expect('comma'); + const b = parseExpr(); + expect('rparen'); + return { type: 'call2', fn: name as 'min' | 'max', a, b }; + } + throw new Error(unrecognisedError(name, FUNCTION_NAMES)); + } + if (!VARIABLES.has(name)) + throw new Error(unrecognisedError(name, VALID_NAMES)); + return { type: 'var', name: 'level' }; + } + throw new Error('The formula is not complete here.'); + }; + + const parseFactor = (): FormulaNode => { + const t = peek(); + if (t && t.type === 'op' && t.value === '-') { + eat(); + const operand = parseFactor(); + return { type: 'neg', operand }; + } + return parsePrimary(); + }; + + const parseTerm = (): FormulaNode => { + let left = parseFactor(); + let t = peek(); + while (t && t.type === 'op' && (t.value === '*' || t.value === '/')) { + eat(); + const right = parseFactor(); + const op = t.value as '*' | '/'; + left = { type: 'binop', op, left, right }; + t = peek(); + } + return left; + }; + + parseExpr = (): FormulaNode => { + let left = parseTerm(); + let t = peek(); + while (t && t.type === 'op' && (t.value === '+' || t.value === '-')) { + eat(); + const right = parseTerm(); + const op = t.value as '+' | '-'; + left = { type: 'binop', op, left, right }; + t = peek(); + } + return left; + }; + + const ast = parseExpr(); + if (pos !== tokens.length) + throw new Error( + 'Looks like an operator is missing - for example level × 2.', + ); + return ast; +}; + +const evaluateBinop = ( + op: '+' | '-' | '*' | '/', + left: number, + right: number, +): number => { + switch (op) { + case '+': + return left + right; + case '-': + return left - right; + case '*': + return left * right; + case '/': + return right === 0 ? NaN : left / right; + default: + return 0; + } +}; + +// Traverse a FormulaNode AST, substituting scope.level for 'var' nodes. +// Used by both parseFormula (FE evaluation) and serializeFormula callers. +export const evaluateNode = (node: FormulaNode, scope: Scope): number => { + switch (node.type) { + case 'num': + return node.value; + case 'var': + return scope.level; + case 'neg': + return -evaluateNode(node.operand, scope); + case 'binop': { + const left = evaluateNode(node.left, scope); + const right = evaluateNode(node.right, scope); + return evaluateBinop(node.op, left, right); + } + case 'call1': { + const arg = evaluateNode(node.arg, scope); + return FUNCTIONS_1[node.fn](arg); + } + case 'call2': { + const left = evaluateNode(node.a, scope); + const right = evaluateNode(node.b, scope); + return FUNCTIONS_2[node.fn](left, right); + } + default: + return 0; + } +}; + +const dependsOnLevel = (node: FormulaNode): boolean => { + switch (node.type) { + case 'var': + return true; + case 'num': + return false; + case 'neg': + return dependsOnLevel(node.operand); + case 'binop': + return dependsOnLevel(node.left) || dependsOnLevel(node.right); + case 'call1': + return dependsOnLevel(node.arg); + case 'call2': + return dependsOnLevel(node.a) || dependsOnLevel(node.b); + default: + return false; + } +}; + +const hasConstantZeroDivisor = (node: FormulaNode): boolean => { + switch (node.type) { + case 'num': + case 'var': + return false; + case 'neg': + return hasConstantZeroDivisor(node.operand); + case 'binop': + if ( + node.op === '/' && + !dependsOnLevel(node.right) && + evaluateNode(node.right, { level: 0 }) === 0 + ) + return true; + return ( + hasConstantZeroDivisor(node.left) || hasConstantZeroDivisor(node.right) + ); + case 'call1': + return hasConstantZeroDivisor(node.arg); + case 'call2': + return hasConstantZeroDivisor(node.a) || hasConstantZeroDivisor(node.b); + default: + return false; + } +}; + +// Parse src into a serializable FormulaNode AST. Returns ok:false on any parse error. +// Used by the save flow to produce the AST that is sent to and stored by the backend. +export const serializeFormula = ( + src: string, +): { ok: true; ast: FormulaNode } | { ok: false; error: string } => { + try { + const tokens = tokenize(src); + if (tokens.length === 0) throw new Error('Enter a formula.'); + const ast = buildAst(tokens); + if (hasConstantZeroDivisor(ast)) + throw new Error('This formula always divides by zero.'); + return { ok: true, ast }; + } catch (e) { + return { + ok: false, + error: e instanceof Error ? e.message : 'This formula is not valid.', + }; + } +}; + +// Parse src and return an evaluate function. Public API is unchanged — callers +// get { ok: true, evaluate } or { ok: false, error }. Backed by evaluateNode. +export const parseFormula = (src: string): ParsedFormula => { + const result = serializeFormula(src); + if (!result.ok) return result; + const { ast } = result; + return { + ok: true, + evaluate: (level): number => { + const val = evaluateNode(ast, { level }); + return Number.isFinite(val) ? val : NaN; + }, + }; +}; + +export const seedLevelFormula = 'level'; diff --git a/client/app/bundles/course/gradebook/operations.ts b/client/app/bundles/course/gradebook/operations.ts index ae2f962fbb..344f24fb7c 100644 --- a/client/app/bundles/course/gradebook/operations.ts +++ b/client/app/bundles/course/gradebook/operations.ts @@ -1,5 +1,8 @@ import type { Operation } from 'store'; -import type { UpdateWeightsPayload } from 'types/course/gradebook'; +import type { + LevelContributionSaveData, + UpdateWeightsPayload, +} from 'types/course/gradebook'; import CourseAPI from 'api/course'; @@ -11,10 +14,24 @@ const fetchGradebook = (): Operation => async (dispatch) => { }; export const updateGradebookWeights = - (weights: UpdateWeightsPayload['weights']): Operation => + ( + weights: UpdateWeightsPayload['weights'], + levelContribution?: LevelContributionSaveData, + ): Operation => async (dispatch) => { - const response = await CourseAPI.gradebook.updateWeights({ weights }); - dispatch(actions.updateTabWeights(response.data)); + const response = await CourseAPI.gradebook.updateWeights( + levelContribution ? { weights, levelContribution } : { weights }, + ); + // BE response does not echo formulaAst; merge it back so the store reducer + // can optimistically recompute per-student levelContribution without a refetch. + const responseData = { ...response.data }; + if (levelContribution && responseData.levelContribution) { + responseData.levelContribution = { + ...responseData.levelContribution, + formulaAst: levelContribution.formulaAst, + }; + } + dispatch(actions.updateTabWeights(responseData)); }; export default fetchGradebook; diff --git a/client/app/bundles/course/gradebook/pages/GradebookIndex/index.tsx b/client/app/bundles/course/gradebook/pages/GradebookIndex/index.tsx index 095d3758fc..03f36e4174 100644 --- a/client/app/bundles/course/gradebook/pages/GradebookIndex/index.tsx +++ b/client/app/bundles/course/gradebook/pages/GradebookIndex/index.tsx @@ -20,7 +20,9 @@ import { getAssessments, getCanManageWeights, getCategories, + getCourseMaxLevel, getGamificationEnabled, + getLevelContribution, getStudents, getSubmissions, getTabs, @@ -75,6 +77,8 @@ const GradebookIndex: FC = () => { const gamificationEnabled = useAppSelector(getGamificationEnabled); const weightedViewEnabled = useAppSelector(getWeightedViewEnabled); const canManageWeights = useAppSelector(getCanManageWeights); + const courseMaxLevel = useAppSelector(getCourseMaxLevel); + const levelContribution = useAppSelector(getLevelContribution); useEffect(() => { dispatch(fetchGradebook()) @@ -104,7 +108,10 @@ const GradebookIndex: FC = () => { canManageWeights={canManageWeights} categories={categories} courseId={courseId} + courseMaxLevel={courseMaxLevel} courseTitle={courseTitle} + gamificationEnabled={gamificationEnabled} + levelContribution={levelContribution} students={students} submissions={submissions} tabs={tabs} diff --git a/client/app/bundles/course/gradebook/selectors.ts b/client/app/bundles/course/gradebook/selectors.ts index 0fb7d1398d..6256f7af9f 100644 --- a/client/app/bundles/course/gradebook/selectors.ts +++ b/client/app/bundles/course/gradebook/selectors.ts @@ -26,3 +26,9 @@ export const getWeightedViewEnabled = (state: AppState): boolean => getLocalState(state).weightedViewEnabled; export const getCanManageWeights = (state: AppState): boolean => getLocalState(state).canManageWeights; +export const getLevelContribution = ( + state: AppState, +): GradebookState['levelContribution'] => + getLocalState(state).levelContribution; +export const getCourseMaxLevel = (state: AppState): number => + getLocalState(state).courseMaxLevel; diff --git a/client/app/bundles/course/gradebook/store.ts b/client/app/bundles/course/gradebook/store.ts index 4777e16b3b..e5a30f044c 100644 --- a/client/app/bundles/course/gradebook/store.ts +++ b/client/app/bundles/course/gradebook/store.ts @@ -1,9 +1,11 @@ import { produce } from 'immer'; import type { GradebookData, + LevelContributionData, UpdateWeightsPayload, } from 'types/course/gradebook'; +import { evaluateNode } from './levelFormula'; import type { AssessmentData, CategoryData, @@ -24,6 +26,8 @@ interface GradebookState { gamificationEnabled: boolean; weightedViewEnabled: boolean; canManageWeights: boolean; + levelContribution: LevelContributionData; + courseMaxLevel: number; } interface SaveGradebookAction { @@ -45,6 +49,14 @@ const initialState: GradebookState = { gamificationEnabled: false, weightedViewEnabled: false, canManageWeights: false, + levelContribution: { + enabled: false, + formula: '', + weight: 0, + show: false, + clamp: true, + }, + courseMaxLevel: 0, }; const reducer = produce( @@ -62,6 +74,9 @@ const reducer = produce( draft.gamificationEnabled = action.payload.gamificationEnabled; draft.weightedViewEnabled = action.payload.weightedViewEnabled; draft.canManageWeights = action.payload.canManageWeights; + draft.levelContribution = + action.payload.levelContribution ?? initialState.levelContribution; + draft.courseMaxLevel = action.payload.courseMaxLevel ?? 0; break; } case UPDATE_TAB_WEIGHTS: { @@ -97,6 +112,26 @@ const reducer = produce( } }, ); + if (action.payload.levelContribution) { + const { formulaAst, ...lcData } = action.payload.levelContribution; + draft.levelContribution = lcData; + if (lcData.enabled && formulaAst) { + draft.students.forEach((student) => { + const val = evaluateNode(formulaAst, { level: student.level }); + if (!Number.isFinite(val)) { + student.levelContribution = null; + } else { + student.levelContribution = lcData.clamp + ? Math.min(Math.max(val, 0), lcData.weight) + : val; + } + }); + } else { + draft.students.forEach((student) => { + student.levelContribution = null; + }); + } + } break; } default: diff --git a/client/app/types/course/gradebook.ts b/client/app/types/course/gradebook.ts index c9009afbfe..49a9c62818 100644 --- a/client/app/types/course/gradebook.ts +++ b/client/app/types/course/gradebook.ts @@ -27,6 +27,7 @@ export interface StudentData { externalId: string | null; level: number; totalXp: number; + levelContribution: number | null; } export interface SubmissionData { @@ -36,6 +37,14 @@ export interface SubmissionData { grade: number | null; } +export interface LevelContributionData { + enabled: boolean; + formula: string; + weight: number; + show: boolean; + clamp: boolean; +} + export interface GradebookData { categories: CategoryData[]; tabs: TabData[]; @@ -45,6 +54,8 @@ export interface GradebookData { gamificationEnabled: boolean; weightedViewEnabled: boolean; canManageWeights: boolean; + courseMaxLevel: number; + levelContribution: LevelContributionData; } export interface UpdateWeightsPayload { @@ -55,4 +66,22 @@ export interface UpdateWeightsPayload { excludedAssessmentIds?: number[]; assessmentWeights?: { assessmentId: number; weight: number }[]; }[]; + levelContribution?: LevelContributionSaveData; +} + +export type FormulaNode = + | { type: 'num'; value: number } + | { type: 'var'; name: 'level' } + | { type: 'neg'; operand: FormulaNode } + | { + type: 'binop'; + op: '+' | '-' | '*' | '/'; + left: FormulaNode; + right: FormulaNode; + } + | { type: 'call1'; fn: 'floor' | 'ceil' | 'round'; arg: FormulaNode } + | { type: 'call2'; fn: 'min' | 'max'; a: FormulaNode; b: FormulaNode }; + +export interface LevelContributionSaveData extends LevelContributionData { + formulaAst: FormulaNode | null; } diff --git a/client/locales/en.json b/client/locales/en.json index fd663f4020..642b295640 100644 --- a/client/locales/en.json +++ b/client/locales/en.json @@ -8147,9 +8147,6 @@ "lib.components.table.MuiColumnPickerPrompt.defaultTitle": { "defaultMessage": "Select columns" }, - "lib.components.table.MuiColumnPickerPrompt.export": { - "defaultMessage": "Apply and Export" - }, "lib.components.table.MuiTableToolbar.directExport": { "defaultMessage": "Export" }, @@ -9293,6 +9290,78 @@ "course.gradebook.ConfigureWeightsPrompt.defaultsHint": { "defaultMessage": "No weights set yet - these are suggested defaults with every tab counting equally. Save to confirm, or adjust below." }, + "course.gradebook.ConfigureWeightsPrompt.levelTitle": { + "defaultMessage": "Level contribution" + }, + "course.gradebook.ConfigureWeightsPrompt.levelSubtitle": { + "defaultMessage": "Adds grade-points from each student's level, on top of tab contributions." + }, + "course.gradebook.ConfigureWeightsPrompt.levelHighestStudent": { + "defaultMessage": "Highest student level: {level}" + }, + "course.gradebook.ConfigureWeightsPrompt.levelCourseMax": { + "defaultMessage": "Course maximum level: {courseMaxLevel}" + }, + "course.gradebook.ConfigureWeightsPrompt.levelFormulaLabel": { + "defaultMessage": "Formula" + }, + "course.gradebook.ConfigureWeightsPrompt.levelFormulaHelper": { + "defaultMessage": "Formulas may contain numbers, arithmetic operators (+ − * /), parentheses, \"level\" as a variable, and the functions floor, ceil, round, min and max." + }, + "course.gradebook.ConfigureWeightsPrompt.levelPreview": { + "defaultMessage": "At max level ({maxLevel}): {pts} pts" + }, + "course.gradebook.ConfigureWeightsPrompt.levelCustomMaxLabel": { + "defaultMessage": "Use custom max level" + }, + "course.gradebook.ConfigureWeightsPrompt.levelMaxLevelLabel": { + "defaultMessage": "Max level" + }, + "course.gradebook.ConfigureWeightsPrompt.levelMaxLevelInvalid": { + "defaultMessage": "Max level must be at least 1" + }, + "course.gradebook.ConfigureWeightsPrompt.levelShowLabel": { + "defaultMessage": "Show level column in table" + }, + "course.gradebook.ConfigureWeightsPrompt.levelOffendersAbove": { + "defaultMessage": "{count, plural, =1 {{name1} is above {max}.} =2 {{name1} and {name2} are above {max}.} other {{name1}, {name2} and {extra} more are above {max}.}}" + }, + "course.gradebook.ConfigureWeightsPrompt.levelOffendersBelow": { + "defaultMessage": "{count, plural, =1 {{name1} is below 0.} =2 {{name1} and {name2} are below 0.} other {{name1}, {name2} and {extra} more are below 0.}}" + }, + "course.gradebook.ConfigureWeightsPrompt.levelFixAtMost": { + "defaultMessage": "These contributions will be set to {max}." + }, + "course.gradebook.ConfigureWeightsPrompt.levelFixAtLeast": { + "defaultMessage": "These contributions will be set to 0." + }, + "course.gradebook.ConfigureWeightsPrompt.levelFixBetween": { + "defaultMessage": "Contributions below 0 will be set to 0, and contributions above {max} will be set to {max}." + }, + "course.gradebook.ConfigureWeightsPrompt.levelClampLabel": { + "defaultMessage": "Keep results between 0 and max level contributions" + }, + "course.gradebook.ConfigureWeightsPrompt.levelUnscoreable": { + "defaultMessage": "{count, plural, =1 {The formula divides by zero for {name1}, so their level contribution is set to 0.} =2 {The formula divides by zero for {name1} and {name2}, so their level contributions are set to 0.} other {The formula divides by zero for {name1}, {name2} and {extra} more, so their level contributions are set to 0.}}" + }, + "course.gradebook.WeightedGradebookTable.levelOverBudgetAboveOnly": { + "defaultMessage": "Some level contributions are above the maximum level contributions and have been capped." + }, + "course.gradebook.WeightedGradebookTable.levelOverBudgetBelowOnly": { + "defaultMessage": "Some level contributions are below 0 and have been raised to 0." + }, + "course.gradebook.WeightedGradebookTable.levelOverBudgetBoth": { + "defaultMessage": "Some level contributions are outside the valid range (below 0 or above the maximum level contributions) and are floored and capped accordingly." + }, + "course.gradebook.WeightedGradebookTable.levelCellCappedAbove": { + "defaultMessage": "Set to {weight} because the formula produced {raw}." + }, + "course.gradebook.WeightedGradebookTable.levelCellCappedBelow": { + "defaultMessage": "Set to 0 because the formula produced {raw}." + }, + "course.gradebook.WeightedGradebookTable.levelCellDivByZero": { + "defaultMessage": "Set to 0 because the formula divides by zero for this level." + }, "course.gradebook.ConfigureWeightsPrompt.descriptionDrop": { "defaultMessage": "In Equal mode, optionally drop each student's N lowest-scoring assessments before averaging." }, @@ -9419,6 +9488,15 @@ "course.gradebook.WeightedGradebookTable.weightsDoNotSum": { "defaultMessage": "Weights do not sum to 100. Total may be inaccurate." }, + "course.gradebook.WeightedGradebookTable.levelHeader": { + "defaultMessage": "Level" + }, + "course.gradebook.WeightedGradebookTable.levelContributionHeader": { + "defaultMessage": "Level Contribution" + }, + "course.gradebook.WeightedGradebookTable.levelBreakdownDetail": { + "defaultMessage": "Level {level}" + }, "course.gradebook.TotalHint.policy": { "defaultMessage": "Totals count ungraded assessments as 0." }, diff --git a/client/locales/ko.json b/client/locales/ko.json index d26fc2c8ca..8943b8d058 100644 --- a/client/locales/ko.json +++ b/client/locales/ko.json @@ -5396,6 +5396,9 @@ "course.gradebook.GradebookIndex.noStudents": { "defaultMessage": "학생 없음" }, + "course.gradebook.GradebookIndex.noStudentsHint": { + "defaultMessage": "학생이 강좌에 참여하면 여기에 성적이 표시됩니다." + }, "course.gradebook.GradebookTable.maxMarks": { "defaultMessage": "최고 점수" }, @@ -5405,6 +5408,9 @@ "course.gradebook.GradebookTable.noDataColumnsHintWithGamification": { "defaultMessage": "성적 또는 게임화 열을 선택하지 않았습니다. 내보내기에는 학생 정보만 포함됩니다." }, + "course.gradebook.GradeLinkHint.hint": { + "defaultMessage": "각 성적은 학생 제출물의 점수 합계입니다. 성적을 클릭하면 해당 제출물을 열고 점수를 조정할 수 있습니다." + }, "course.group.GroupCreationForm.description": { "defaultMessage": "설명 (선택사항)" }, @@ -9278,6 +9284,60 @@ "course.gradebook.ConfigureWeightsPrompt.defaultsHint": { "defaultMessage": "아직 설정된 가중치가 없습니다. 모든 탭이 동일한 비중으로 반영되도록 제안된 기본값입니다. 저장하여 확정하거나 아래에서 조정하세요." }, + "course.gradebook.ConfigureWeightsPrompt.levelTitle": { + "defaultMessage": "레벨 기여도" + }, + "course.gradebook.ConfigureWeightsPrompt.levelSubtitle": { + "defaultMessage": "탭 기여도에 더해 각 학생의 레벨에서 성적 점수를 추가합니다." + }, + "course.gradebook.ConfigureWeightsPrompt.levelHighestStudent": { + "defaultMessage": "최고 학생 레벨: {level}" + }, + "course.gradebook.ConfigureWeightsPrompt.levelCourseMax": { + "defaultMessage": "강좌 최대 레벨: {courseMaxLevel}" + }, + "course.gradebook.ConfigureWeightsPrompt.levelFormulaLabel": { + "defaultMessage": "수식" + }, + "course.gradebook.ConfigureWeightsPrompt.levelFormulaHelper": { + "defaultMessage": "수식에는 숫자, 산술 연산자(+ − * /), 괄호, 변수 \"level\", 그리고 floor, ceil, round, min, max 함수를 사용할 수 있습니다." + }, + "course.gradebook.ConfigureWeightsPrompt.levelPreview": { + "defaultMessage": "최대 레벨({maxLevel})에서: {pts}점" + }, + "course.gradebook.ConfigureWeightsPrompt.levelCustomMaxLabel": { + "defaultMessage": "사용자 지정 최대 레벨 사용" + }, + "course.gradebook.ConfigureWeightsPrompt.levelMaxLevelLabel": { + "defaultMessage": "최대 레벨" + }, + "course.gradebook.ConfigureWeightsPrompt.levelMaxLevelInvalid": { + "defaultMessage": "최대 레벨은 1 이상이어야 합니다" + }, + "course.gradebook.ConfigureWeightsPrompt.levelShowLabel": { + "defaultMessage": "표에 레벨 열 표시" + }, + "course.gradebook.ConfigureWeightsPrompt.levelOffendersAbove": { + "defaultMessage": "{count, plural, =1 {{name1}이(가) {max}보다 높습니다.} =2 {{name1} 및 {name2}이(가) {max}보다 높습니다.} other {{name1}, {name2} 및 그 외 {extra}명이 {max}보다 높습니다.}}" + }, + "course.gradebook.ConfigureWeightsPrompt.levelOffendersBelow": { + "defaultMessage": "{count, plural, =1 {{name1}이(가) 0보다 낮습니다.} =2 {{name1} 및 {name2}이(가) 0보다 낮습니다.} other {{name1}, {name2} 및 그 외 {extra}명이 0보다 낮습니다.}}" + }, + "course.gradebook.ConfigureWeightsPrompt.levelFixAtMost": { + "defaultMessage": "이 기여도는 {max}(으)로 설정됩니다." + }, + "course.gradebook.ConfigureWeightsPrompt.levelFixAtLeast": { + "defaultMessage": "이 기여도는 0으로 설정됩니다." + }, + "course.gradebook.ConfigureWeightsPrompt.levelFixBetween": { + "defaultMessage": "0보다 낮은 기여도는 0으로 설정되고, {max}보다 높은 값은 {max}(으)로 설정됩니다." + }, + "course.gradebook.ConfigureWeightsPrompt.levelClampLabel": { + "defaultMessage": "기여도를 0에서 최대 레벨 기여도 사이로 유지" + }, + "course.gradebook.ConfigureWeightsPrompt.levelUnscoreable": { + "defaultMessage": "{count, plural, =1 {{name1}의 경우 수식이 0으로 나누기를 하므로, 해당 레벨 기여도가 0으로 설정됩니다.} =2 {{name1} 및 {name2}의 경우 수식이 0으로 나누기를 하므로, 해당 레벨 기여도가 0으로 설정됩니다.} other {{name1}, {name2} 및 그 외 {extra}명의 경우 수식이 0으로 나누기를 하므로, 해당 레벨 기여도가 0으로 설정됩니다.}}" + }, "course.gradebook.ConfigureWeightsPrompt.descriptionDrop": { "defaultMessage": "동일 모드에서는 평균을 내기 전에 각 학생의 점수가 가장 낮은 평가 N개를 선택적으로 제외할 수 있습니다." }, @@ -9404,6 +9464,33 @@ "course.gradebook.WeightedGradebookTable.weightsDoNotSum": { "defaultMessage": "가중치 합계가 100이 아닙니다. 총점이 정확하지 않을 수 있습니다." }, + "course.gradebook.WeightedGradebookTable.levelOverBudgetAboveOnly": { + "defaultMessage": "일부 레벨 기여도가 레벨 가중치를 초과하여 제한되었습니다." + }, + "course.gradebook.WeightedGradebookTable.levelOverBudgetBelowOnly": { + "defaultMessage": "일부 레벨 기여도가 0 미만이어서 0으로 올려졌습니다." + }, + "course.gradebook.WeightedGradebookTable.levelOverBudgetBoth": { + "defaultMessage": "일부 레벨 기여도가 유효 범위를 벗어났습니다 — 0 미만이거나 레벨 가중치를 초과합니다." + }, + "course.gradebook.WeightedGradebookTable.levelCellCappedAbove": { + "defaultMessage": "수식 결과가 {raw}이므로 {weight}(으)로 설정되었습니다." + }, + "course.gradebook.WeightedGradebookTable.levelCellCappedBelow": { + "defaultMessage": "수식 결과가 {raw}이므로 0으로 설정되었습니다." + }, + "course.gradebook.WeightedGradebookTable.levelCellDivByZero": { + "defaultMessage": "이 레벨에서 수식이 0으로 나누기를 하므로 0으로 설정되었습니다." + }, + "course.gradebook.WeightedGradebookTable.levelHeader": { + "defaultMessage": "레벨" + }, + "course.gradebook.WeightedGradebookTable.levelContributionHeader": { + "defaultMessage": "레벨 기여도" + }, + "course.gradebook.WeightedGradebookTable.levelBreakdownDetail": { + "defaultMessage": "레벨 {level}" + }, "course.gradebook.TotalHint.policy": { "defaultMessage": "총점은 채점되지 않은 평가를 0점으로 계산합니다." }, diff --git a/client/locales/zh.json b/client/locales/zh.json index 970c2dca7a..44c9472902 100644 --- a/client/locales/zh.json +++ b/client/locales/zh.json @@ -5390,6 +5390,9 @@ "course.gradebook.GradebookIndex.noStudents": { "defaultMessage": "无学生" }, + "course.gradebook.GradebookIndex.noStudentsHint": { + "defaultMessage": "学生加入课程后，成绩将显示在这里。" + }, "course.gradebook.GradebookTable.maxMarks": { "defaultMessage": "最高分" }, @@ -5399,6 +5402,9 @@ "course.gradebook.GradebookTable.noDataColumnsHintWithGamification": { "defaultMessage": "未选择任何成绩或游戏化列——导出内容将仅包含学生信息。" }, + "course.gradebook.GradeLinkHint.hint": { + "defaultMessage": "每个成绩都是学生提交内容中各项分数的总和。点击任一成绩即可打开该提交内容并调整分数。" + }, "course.group.GroupCreationForm.description": { "defaultMessage": "说明（可选）" }, @@ -9272,6 +9278,63 @@ "course.gradebook.ConfigureWeightsPrompt.defaultsHint": { "defaultMessage": "尚未设置权重。以下为建议默认值，所有标签页均按相同比重计算。保存以确认，或在下方进行调整。" }, + "course.gradebook.ConfigureWeightsPrompt.levelTitle": { + "defaultMessage": "等级贡献值" + }, + "course.gradebook.ConfigureWeightsPrompt.levelSubtitle": { + "defaultMessage": "在标签页贡献值之外，根据每位学生的等级添加成绩点数。" + }, + "course.gradebook.ConfigureWeightsPrompt.levelHighestStudent": { + "defaultMessage": "最高学生等级：{level}" + }, + "course.gradebook.ConfigureWeightsPrompt.levelCourseMax": { + "defaultMessage": "课程最高等级：{courseMaxLevel}" + }, + "course.gradebook.ConfigureWeightsPrompt.levelFormulaLabel": { + "defaultMessage": "公式" + }, + "course.gradebook.ConfigureWeightsPrompt.levelFormulaHelper": { + "defaultMessage": "公式可以包含数字、算术运算符（+ − * /）、括号、作为变量的 \"level\"，以及函数 floor、ceil、round、min 和 max。" + }, + "course.gradebook.ConfigureWeightsPrompt.levelPreview": { + "defaultMessage": "最高等级（{maxLevel}）时：{pts} 分" + }, + "course.gradebook.ConfigureWeightsPrompt.levelCustomMaxLabel": { + "defaultMessage": "使用自定义最高等级" + }, + "course.gradebook.ConfigureWeightsPrompt.levelMaxLevelLabel": { + "defaultMessage": "最高等级" + }, + "course.gradebook.ConfigureWeightsPrompt.levelMaxLevelInvalid": { + "defaultMessage": "最高等级必须至少为 1" + }, + "course.gradebook.ConfigureWeightsPrompt.levelShowLabel": { + "defaultMessage": "在表格中显示等级列" + }, + "course.gradebook.ConfigureWeightsPrompt.levelOffendersAbove": { + "defaultMessage": "{count, plural, =1 {{name1} 高于 {max}。} =2 {{name1} 和 {name2} 高于 {max}。} other {{name1}、{name2} 以及另外 {extra} 人高于 {max}。}}" + }, + "course.gradebook.ConfigureWeightsPrompt.levelOffendersBelow": { + "defaultMessage": "{count, plural, =1 {{name1} 低于 0。} =2 {{name1} 和 {name2} 低于 0。} other {{name1}、{name2} 以及另外 {extra} 人低于 0。}}" + }, + "course.gradebook.ConfigureWeightsPrompt.levelFixAtMost": { + "defaultMessage": "这些贡献值将被设置为 {max}。" + }, + "course.gradebook.ConfigureWeightsPrompt.levelFixAtLeast": { + "defaultMessage": "这些贡献值将被设置为 0。" + }, + "course.gradebook.ConfigureWeightsPrompt.levelFixBetween": { + "defaultMessage": "低于 0 的贡献值将被设置为 0，高于 {max} 的值将被设置为 {max}。" + }, + "course.gradebook.ConfigureWeightsPrompt.levelClampLabel": { + "defaultMessage": "将贡献值保持在 0 到最高等级贡献值之间" + }, + "course.gradebook.ConfigureWeightsPrompt.levelUnscoreable": { + "defaultMessage": "{count, plural, =1 {公式对 {name1} 进行了除以零运算，因此其等级贡献值被设为 0。} =2 {公式对 {name1} 和 {name2} 进行了除以零运算，因此他们的等级贡献值被设为 0。} other {公式对 {name1}、{name2} 以及另外 {extra} 人进行了除以零运算，因此他们的等级贡献值被设为 0。}}" + }, + "course.gradebook.WeightedGradebookTable.levelCellDivByZero": { + "defaultMessage": "由于该等级的公式进行了除以零运算，因此设置为 0。" + }, "course.gradebook.ConfigureWeightsPrompt.descriptionDrop": { "defaultMessage": "在等权模式下，可选择在求平均分前剔除每位学生得分最低的 N 个评估。" }, @@ -9398,6 +9461,30 @@ "course.gradebook.WeightedGradebookTable.weightsDoNotSum": { "defaultMessage": "权重合计不为 100。总成绩可能不准确。" }, + "course.gradebook.WeightedGradebookTable.levelOverBudgetAboveOnly": { + "defaultMessage": "部分等级贡献值超出等级权重上限，已被限制。" + }, + "course.gradebook.WeightedGradebookTable.levelOverBudgetBelowOnly": { + "defaultMessage": "部分等级贡献值低于 0，已被提升至 0。" + }, + "course.gradebook.WeightedGradebookTable.levelOverBudgetBoth": { + "defaultMessage": "部分等级贡献值超出有效范围 — 低于 0 或超出等级权重上限。" + }, + "course.gradebook.WeightedGradebookTable.levelCellCappedAbove": { + "defaultMessage": "由于公式产生了 {raw}，因此设置为 {weight}。" + }, + "course.gradebook.WeightedGradebookTable.levelCellCappedBelow": { + "defaultMessage": "由于公式产生了 {raw}，因此设置为 0。" + }, + "course.gradebook.WeightedGradebookTable.levelHeader": { + "defaultMessage": "等级" + }, + "course.gradebook.WeightedGradebookTable.levelContributionHeader": { + "defaultMessage": "等级贡献值" + }, + "course.gradebook.WeightedGradebookTable.levelBreakdownDetail": { + "defaultMessage": "等级 {level}" + }, "course.gradebook.TotalHint.policy": { "defaultMessage": "总成绩将未评分的评估按 0 分计算。" }, diff --git a/db/migrate/20260616000000_create_gradebook_level_configs.rb b/db/migrate/20260616000000_create_gradebook_level_configs.rb new file mode 100644 index 0000000000..671c2fb904 --- /dev/null +++ b/db/migrate/20260616000000_create_gradebook_level_configs.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true +class CreateGradebookLevelConfigs < ActiveRecord::Migration[7.2] + def change + create_table :course_gradebook_level_configs do |t| + t.references :course, null: false, + foreign_key: { to_table: :courses, on_delete: :cascade }, + index: { + unique: true, + name: 'index_course_gradebook_level_configs_on_course_id' + } + + t.boolean :enabled, null: false, default: false + t.string :formula, null: false, default: '' + t.jsonb :formula_ast + t.decimal :weight, precision: 5, scale: 2, null: false, default: 0 + t.boolean :show, null: false, default: false + t.boolean :clamp, null: false, default: true + + t.references :creator, null: false, + foreign_key: { to_table: :users }, + index: { name: 'fk__course_gradebook_level_configs_creator_id' } + + t.references :updater, null: false, + foreign_key: { to_table: :users }, + index: { name: 'fk__course_gradebook_level_configs_updater_id' } + + t.timestamps null: false + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 2a1fc7a65f..9bb292b31b 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_11_000000) do +ActiveRecord::Schema[7.2].define(version: 2026_06_16_000000) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" enable_extension "uuid-ossp" @@ -869,6 +869,23 @@ t.index ["updater_id"], name: "fk__cgac_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 + t.string "formula", default: "", null: false + t.jsonb "formula_ast" + t.decimal "weight", precision: 5, scale: 2, default: "0.0", null: false + t.boolean "show", default: false, null: false + t.boolean "clamp", default: true, 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: "index_course_gradebook_level_configs_on_course_id", unique: true + t.index ["creator_id"], name: "fk__course_gradebook_level_configs_creator_id" + t.index ["updater_id"], name: "fk__course_gradebook_level_configs_updater_id" + end + create_table "course_gradebook_tab_contributions", force: :cascade do |t| t.bigint "course_id", null: false t.bigint "tab_id", null: false @@ -1946,6 +1963,9 @@ 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_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" add_foreign_key "course_gradebook_tab_contributions", "course_assessment_tabs", column: "tab_id", on_delete: :cascade add_foreign_key "course_gradebook_tab_contributions", "courses" add_foreign_key "course_gradebook_tab_contributions", "users", column: "creator_id" diff --git a/spec/controllers/course/gradebook_controller_spec.rb b/spec/controllers/course/gradebook_controller_spec.rb index 2c1a573664..7c7542b659 100644 --- a/spec/controllers/course/gradebook_controller_spec.rb +++ b/spec/controllers/course/gradebook_controller_spec.rb @@ -54,6 +54,26 @@ expect(data).to have_key(key), "expected response to have key '#{key}'" end end + + it 'serializes courseMaxLevel and a default levelContribution' do + subject + data = JSON.parse(response.body) + expect(data).to have_key('courseMaxLevel') + expect(data['courseMaxLevel']).to be_a(Integer) + expect(data['levelContribution']).to include( + 'enabled' => false, 'formula' => '', 'weight' => 0, 'show' => false + ) + expect(data['levelContribution']).to include('clamp' => true) + end + + it 'includes levelContribution (null) on each student when level contribution disabled' do + subject + data = JSON.parse(response.body) + data['students'].each do |s| + expect(s).to have_key('levelContribution') + expect(s['levelContribution']).to be_nil + end + end end context 'when an observer visits the page' do @@ -129,6 +149,113 @@ end end + context 'with weighted view enabled and a level config with formula_ast' do + let(:manager) { create(:course_manager, course: course) } + let!(:student_cu) { create(:course_student, course: course) } + + before do + # Enable weighted view via the persisted course component setting, so the + # controller's @settings.weighted_view_enabled reads true on the real request. + gradebook_settings = Course::Settings::GradebookComponent.new( + OpenStruct.new(current_course: course, key: Course::GradebookComponent.key) + ) + gradebook_settings.weighted_view_enabled = true + course.save! + controller_sign_in(controller, manager.user) + # Create a level config with a known AST: formula = 'level * 0.5' + ast = { + 'type' => 'binop', 'op' => '*', + 'left' => { 'type' => 'var', 'name' => 'level' }, + 'right' => { 'type' => 'num', 'value' => 0.5 } + } + Course::Gradebook::LevelConfig.upsert_for( + course: course, + attrs: { enabled: true, formula: 'level * 0.5', formula_ast: ast, weight: 10, show: false } + ) + end + + it 'includes levelContribution on each student' do + get :index, params: { course_id: course.id }, format: :json + data = JSON.parse(response.body) + student_data = data['students'].find { |s| s['id'] == student_cu.user_id } + expect(student_data).not_to be_nil + expect(student_data).to have_key('levelContribution') + # level is 0 initially; 0 * 0.5 = 0.0 + expect(student_data['levelContribution'].to_f).to be_within(0.001).of(0.0) + end + end + + context 'with weighted view enabled, a level config, and a student above level 0' do + let(:manager) { create(:course_manager, course: course) } + let!(:level1) { create(:course_level, course: course, experience_points_threshold: 100) } + let!(:student_cu) { create(:course_student, course: course) } + let!(:exp_record) do + create(:course_experience_points_record, course_user: student_cu, points_awarded: 500) + end + + before do + gradebook_settings = Course::Settings::GradebookComponent.new( + OpenStruct.new(current_course: course, key: Course::GradebookComponent.key) + ) + gradebook_settings.weighted_view_enabled = true + course.save! + controller_sign_in(controller, manager.user) + # formula = 'level' → contribution equals the student's level_number. + Course::Gradebook::LevelConfig.upsert_for( + course: course, + attrs: { enabled: true, formula: 'level', + formula_ast: { 'type' => 'var', 'name' => 'level' }, + weight: 10, show: false } + ) + end + + it 'computes a non-null levelContribution from the student level_number' do + get :index, params: { course_id: course.id }, format: :json + data = JSON.parse(response.body) + student_data = data['students'].find { |s| s['id'] == student_cu.user_id } + expect(student_data).not_to be_nil + # 500 EXP places the student in level 1 (threshold 100); formula 'level' → 1.0. + expect(student_data['level']).to eq(1) + expect(student_data['levelContribution']).not_to be_nil + expect(student_data['levelContribution'].to_f).to be_within(0.001).of(1.0) + end + end + + context 'with a clamping level config and an out-of-range student' do + let(:manager) { create(:course_manager, course: course) } + let!(:level1) { create(:course_level, course: course, experience_points_threshold: 100) } + let!(:student_cu) { create(:course_student, course: course) } + let!(:exp_record) do + create(:course_experience_points_record, course_user: student_cu, points_awarded: 500) + end + + before do + gradebook_settings = Course::Settings::GradebookComponent.new( + OpenStruct.new(current_course: course, key: Course::GradebookComponent.key) + ) + gradebook_settings.weighted_view_enabled = true + course.save! + controller_sign_in(controller, manager.user) + # formula 'level * 5', weight 3, clamp on: level 1 -> raw 5 -> clamped 3. + Course::Gradebook::LevelConfig.upsert_for( + course: course, + attrs: { enabled: true, formula: 'level * 5', + formula_ast: { 'type' => 'binop', 'op' => '*', + 'left' => { 'type' => 'var', 'name' => 'level' }, + 'right' => { 'type' => 'num', 'value' => 5 } }, + weight: 3, show: false, clamp: true } + ) + end + + it 'serializes the clamped per-student contribution' do + get :index, params: { course_id: course.id }, format: :json + data = JSON.parse(response.body) + student_data = data['students'].find { |s| s['id'] == student_cu.user_id } + expect(student_data['levelContribution'].to_f).to be_within(0.001).of(3.0) + expect(data['levelContribution']).to include('clamp' => true) + end + end + context 'when a student has an external ID' do let!(:student) { create(:course_student, course: course, external_id: 'EXT-123') } before { controller_sign_in(controller, manager.user) } @@ -282,6 +409,68 @@ def weight_for(tab) expect(JSON.parse(response.body)['weights'].first['weight']).to eq(33.33) expect(weight_for(tab1)).to eq(33.33) end + + it 'persists an enabled levelContribution and echoes it back' do + patch :update_weights, params: { + course_id: course.id, + weights: [], + levelContribution: { + enabled: true, formula: 'min(level, 30) * 0.05', weight: 8, show: true + } + }, format: :json + expect(response).to have_http_status(:ok) + config = course.reload.gradebook_level_config + expect(config.enabled).to eq(true) + expect(config.formula).to eq('min(level, 30) * 0.05') + expect(config.weight).to eq(8) + echoed = JSON.parse(response.body)['levelContribution'] + expect(echoed).to include('enabled' => true, 'weight' => 8.0) + expect(echoed).not_to have_key('maxLevel') + end + + it 'persists the clamp flag sent in the levelContribution payload' do + patch :update_weights, params: { + course_id: course.id, + weights: [], + levelContribution: { + enabled: true, formula: 'level', weight: 8, show: false, clamp: false + } + }, format: :json + expect(response).to have_http_status(:ok) + expect(course.reload.gradebook_level_config.clamp).to eq(false) + expect(JSON.parse(response.body)['levelContribution']).to include('clamp' => false) + end + + it 'rejects an enabled levelContribution with a blank formula (422)' do + patch :update_weights, params: { + course_id: course.id, weights: [], + levelContribution: { enabled: true, formula: '', weight: 8, show: false } + }, format: :json + expect(response).to have_http_status(:unprocessable_content) + expect(course.reload.gradebook_level_config).to be_nil + end + + it 'leaves the config untouched when no levelContribution is sent' do + patch :update_weights, params: { course_id: course.id, **valid_payload }, format: :json + expect(response).to have_http_status(:ok) + expect(JSON.parse(response.body)).not_to have_key('levelContribution') + expect(course.reload.gradebook_level_config).to be_nil + end + + it 'rejects a tampered formulaAst with 422' do + patch :update_weights, params: { + course_id: course.id, + weights: [], + levelContribution: { + enabled: true, + formula: 'level', + formulaAst: { type: 'evil', payload: 'x' }, + weight: 5, + show: false + } + }, format: :json + expect(response).to have_http_status(:unprocessable_content) + end end context 'as TA' do diff --git a/spec/factories/course_gradebook_level_configs.rb b/spec/factories/course_gradebook_level_configs.rb new file mode 100644 index 0000000000..e6637cad7e --- /dev/null +++ b/spec/factories/course_gradebook_level_configs.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true +FactoryBot.define do + factory :course_gradebook_level_config, class: Course::Gradebook::LevelConfig.name do + course + enabled { false } + formula { '' } + weight { 0 } + show { false } + clamp { true } + end +end diff --git a/spec/models/course/gradebook/level_config_spec.rb b/spec/models/course/gradebook/level_config_spec.rb new file mode 100644 index 0000000000..98bd798611 --- /dev/null +++ b/spec/models/course/gradebook/level_config_spec.rb @@ -0,0 +1,336 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::Gradebook::LevelConfig do + let!(:instance) { Instance.default } + with_tenant(:instance) do + let(:course) { create(:course) } + + it { is_expected.to belong_to(:course) } + + describe 'validations' do + it 'is valid with defaults (disabled, blank formula)' do + expect(build(:course_gradebook_level_config, course: course)).to be_valid + end + + it 'requires a formula when enabled' do + config = build(:course_gradebook_level_config, course: course, enabled: true, formula: '') + expect(config).not_to be_valid + end + + it 'allows a blank formula when disabled' do + config = build(:course_gradebook_level_config, course: course, enabled: false, formula: '') + expect(config).to be_valid + end + + it 'rejects a negative weight' do + config = build(:course_gradebook_level_config, course: course, weight: -1) + expect(config).not_to be_valid + end + + it 'enforces one config per course' do + create(:course_gradebook_level_config, course: course) + expect(build(:course_gradebook_level_config, course: course)).not_to be_valid + end + end + + it 'is destroyed with its course' do + create(:course_gradebook_level_config, course: course) + expect { course.destroy! }.to change { described_class.count }.by(-1) + end + + describe '.upsert_for' do + it 'creates a config from a camelCase-free attrs hash' do + config = described_class.upsert_for( + course: course, + attrs: { enabled: true, formula: 'min(level, 30) * 0.05', weight: 8, show: true } + ) + expect(config.enabled).to eq(true) + expect(config.formula).to eq('min(level, 30) * 0.05') + expect(config.weight).to eq(8) + expect(config.show).to eq(true) + end + + it 'updates the existing singleton on a second call' do + described_class.upsert_for(course: course, attrs: { enabled: true, formula: 'level', weight: 2 }) + described_class.upsert_for(course: course, attrs: { enabled: false, formula: '', weight: 0 }) + expect(described_class.where(course_id: course.id).count).to eq(1) + expect(course.reload.gradebook_level_config.enabled).to eq(false) + end + + it 'raises RecordInvalid when enabled with a blank formula' do + expect do + described_class.upsert_for(course: course, attrs: { enabled: true, formula: '', weight: 1 }) + end.to raise_error(ActiveRecord::RecordInvalid) + end + end + + describe '.valid_ast?' do + it 'accepts a num node' do + expect(described_class.valid_ast?({ 'type' => 'num', 'value' => 3.5 })).to be true + end + + it 'accepts a var node for level' do + expect(described_class.valid_ast?({ 'type' => 'var', 'name' => 'level' })).to be true + end + + it 'rejects var with wrong name' do + expect(described_class.valid_ast?({ 'type' => 'var', 'name' => 'xp' })).to be false + end + + it 'accepts a neg node with valid operand' do + expect(described_class.valid_ast?({ 'type' => 'neg', + 'operand' => { 'type' => 'var', 'name' => 'level' } })).to be true + end + + it 'accepts a binop node with valid op and children' do + node = { + 'type' => 'binop', 'op' => '*', + 'left' => { 'type' => 'var', 'name' => 'level' }, + 'right' => { 'type' => 'num', 'value' => 0.5 } + } + expect(described_class.valid_ast?(node)).to be true + end + + it 'rejects binop with invalid op' do + node = { + 'type' => 'binop', 'op' => '%', + 'left' => { 'type' => 'num', 'value' => 1 }, + 'right' => { 'type' => 'num', 'value' => 1 } + } + expect(described_class.valid_ast?(node)).to be false + end + + it 'accepts a call1 node' do + node = { 'type' => 'call1', 'fn' => 'floor', 'arg' => { 'type' => 'var', 'name' => 'level' } } + expect(described_class.valid_ast?(node)).to be true + end + + it 'rejects call1 with unknown fn' do + node = { 'type' => 'call1', 'fn' => 'sqrt', 'arg' => { 'type' => 'var', 'name' => 'level' } } + expect(described_class.valid_ast?(node)).to be false + end + + it 'accepts a call2 node' do + node = { + 'type' => 'call2', 'fn' => 'min', + 'a' => { 'type' => 'var', 'name' => 'level' }, + 'b' => { 'type' => 'num', 'value' => 20 } + } + expect(described_class.valid_ast?(node)).to be true + end + + it 'rejects call2 with unknown fn' do + node = { + 'type' => 'call2', 'fn' => 'pow', + 'a' => { 'type' => 'num', 'value' => 2 }, + 'b' => { 'type' => 'num', 'value' => 3 } + } + expect(described_class.valid_ast?(node)).to be false + end + + it 'rejects an unknown type' do + expect(described_class.valid_ast?({ 'type' => 'evil', 'payload' => 'x' })).to be false + end + + it 'rejects a non-hash' do + expect(described_class.valid_ast?('not a hash')).to be false + expect(described_class.valid_ast?(nil)).to be false + end + + it 'rejects a tree exceeding depth 20' do + # Build a deeply-nested neg chain: neg(neg(neg(...(var)...))) with 21 levels + deep = { 'type' => 'var', 'name' => 'level' } + 21.times { deep = { 'type' => 'neg', 'operand' => deep } } + expect(described_class.valid_ast?(deep)).to be false + end + + it 'rejects a node with a missing required key' do + expect(described_class.valid_ast?({ 'type' => 'binop', 'op' => '+', + 'left' => { 'type' => 'num', 'value' => 1 } })).to be false + end + end + + describe '#evaluate_for' do + # These examples verify raw AST evaluation, independent of clamping, so they + # disable clamp (the factory weight is 0, which would otherwise clamp every + # result to 0). Clamping itself is covered in '#evaluate_for clamping' below. + let(:config) do + build(:course_gradebook_level_config, course: course, enabled: true, clamp: false) + end + + it 'evaluates a num node (constant formula)' do + config.formula_ast = { 'type' => 'num', 'value' => 5.0 } + expect(config.evaluate_for(level: 10)).to eq(5.0) + end + + it 'evaluates a var node (returns the level)' do + config.formula_ast = { 'type' => 'var', 'name' => 'level' } + expect(config.evaluate_for(level: 15)).to eq(15.0) + end + + it 'evaluates a binop multiply: level * 0.5' do + config.formula_ast = { + 'type' => 'binop', 'op' => '*', + 'left' => { 'type' => 'var', 'name' => 'level' }, + 'right' => { 'type' => 'num', 'value' => 0.5 } + } + expect(config.evaluate_for(level: 20)).to be_within(0.001).of(10.0) + end + + it 'evaluates a call2 min: min(level, 20) * 0.5' do + config.formula_ast = { + 'type' => 'binop', 'op' => '*', + 'left' => { + 'type' => 'call2', 'fn' => 'min', + 'a' => { 'type' => 'var', 'name' => 'level' }, + 'b' => { 'type' => 'num', 'value' => 20 } + }, + 'right' => { 'type' => 'num', 'value' => 0.5 } + } + expect(config.evaluate_for(level: 15)).to be_within(0.001).of(7.5) + expect(config.evaluate_for(level: 25)).to be_within(0.001).of(10.0) + end + + it 'evaluates floor: floor(level / 5)' do + config.formula_ast = { + 'type' => 'call1', 'fn' => 'floor', + 'arg' => { + 'type' => 'binop', 'op' => '/', + 'left' => { 'type' => 'var', 'name' => 'level' }, + 'right' => { 'type' => 'num', 'value' => 5 } + } + } + expect(config.evaluate_for(level: 12)).to eq(2.0) # floor(12/5) = 2 + end + + it 'returns nil when the divisor is zero' do + config.formula_ast = { + 'type' => 'binop', 'op' => '/', + 'left' => { 'type' => 'var', 'name' => 'level' }, + 'right' => { 'type' => 'num', 'value' => 0 } + } + expect(config.evaluate_for(level: 5)).to be_nil + end + + it 'returns nil only for the student whose level zeroes the divisor' do + # 100 / level: undefined at level 0, defined elsewhere + config.formula_ast = { + 'type' => 'binop', 'op' => '/', + 'left' => { 'type' => 'num', 'value' => 100 }, + 'right' => { 'type' => 'var', 'name' => 'level' } + } + expect(config.evaluate_for(level: 0)).to be_nil + expect(config.evaluate_for(level: 5)).to eq(20.0) + end + + it 'returns nil when disabled' do + config.enabled = false + config.formula_ast = { 'type' => 'var', 'name' => 'level' } + expect(config.evaluate_for(level: 10)).to be_nil + end + + it 'returns nil when formula_ast is nil' do + config.formula_ast = nil + expect(config.evaluate_for(level: 10)).to be_nil + end + end + + describe '.upsert_for — formula_ast' do + let(:valid_ast) do + { + 'type' => 'binop', 'op' => '*', + 'left' => { 'type' => 'var', 'name' => 'level' }, + 'right' => { 'type' => 'num', 'value' => 0.5 } + } + end + + it 'stores a valid formula_ast' do + config = described_class.upsert_for( + course: course, + attrs: { enabled: true, formula: 'level * 0.5', formula_ast: valid_ast, weight: 10 } + ) + expect(config.formula_ast).to eq(valid_ast) + end + + it 'stores nil formula_ast when not provided' do + config = described_class.upsert_for( + course: course, + attrs: { enabled: true, formula: 'level', weight: 5 } + ) + expect(config.formula_ast).to be_nil + end + + it 'raises RecordInvalid for a malformed formula_ast' do + expect do + described_class.upsert_for( + course: course, + attrs: { enabled: true, formula: 'level', formula_ast: { 'type' => 'evil' }, weight: 5 } + ) + end.to raise_error(ActiveRecord::RecordInvalid) + end + end + describe '#evaluate_for clamping' do + # formula 'level * 5' + let(:times5) do + { 'type' => 'binop', 'op' => '*', + 'left' => { 'type' => 'var', 'name' => 'level' }, + 'right' => { 'type' => 'num', 'value' => 5 } } + end + # formula 'level - 5' + let(:minus5) do + { 'type' => 'binop', 'op' => '-', + 'left' => { 'type' => 'var', 'name' => 'level' }, + 'right' => { 'type' => 'num', 'value' => 5 } } + end + + it 'caps a value above the weight to the weight when clamp is on' do + config = build(:course_gradebook_level_config, course: course, enabled: true, + formula: 'level * 5', formula_ast: times5, + weight: 3, clamp: true) + expect(config.evaluate_for(level: 2)).to eq(3.0) # raw 10 -> 3 + end + + it 'floors a negative value to 0 when clamp is on' do + config = build(:course_gradebook_level_config, course: course, enabled: true, + formula: 'level - 5', formula_ast: minus5, + weight: 10, clamp: true) + expect(config.evaluate_for(level: 1)).to eq(0.0) # raw -4 -> 0 + end + + it 'leaves an in-range value unchanged when clamp is on' do + config = build(:course_gradebook_level_config, course: course, enabled: true, + formula: 'level * 5', formula_ast: times5, + weight: 100, clamp: true) + expect(config.evaluate_for(level: 1)).to eq(5.0) + end + + it 'returns the raw value when clamp is off' do + config = build(:course_gradebook_level_config, course: course, enabled: true, + formula: 'level * 5', formula_ast: times5, + weight: 3, clamp: false) + expect(config.evaluate_for(level: 2)).to eq(10.0) + end + end + + describe '.upsert_for clamp' do + it 'persists clamp false' do + described_class.upsert_for( + course: course, + attrs: { enabled: true, formula: 'level', + formula_ast: { 'type' => 'var', 'name' => 'level' }, + weight: 10, show: false, clamp: false } + ) + expect(course.reload.gradebook_level_config.clamp).to be(false) + end + + it 'defaults clamp to true when the key is omitted' do + described_class.upsert_for( + course: course, + attrs: { enabled: false, formula: '', formula_ast: nil, weight: 0, show: false } + ) + expect(course.reload.gradebook_level_config.clamp).to be(true) + end + end + end +end