From da2242e84e02345da97917c237ca5a1afb5f6bd8 Mon Sep 17 00:00:00 2001 From: lws49 Date: Thu, 11 Jun 2026 17:35:03 +0800 Subject: [PATCH 1/4] feat(gradebook): add gradebook with column picker, CSV export, grade links & sorting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce the course gradebook: a frozen-column table of students × assessments with a column picker, CSV export, and per-grade links into submissions. Gradebook table - Page with TanStack-backed table: pinned checkbox + Name columns, sticky header and Max Marks rows, frozen-column border seams that survive sticky scroll compositing. - Column picker (assessments grouped by tab/category) and CSV export of the selected columns; empty-state hint when no data columns are chosen. - External ID column, shown when any student has one. - Grade cells link to the student's submission; a dismissible GradeLinkHint banner explains the affordance (persisted per-user via useDismissibleOnce). - "Search students" global search. - Default sort with name ascending, null/undefined at bottom of sort regardless of order Shared table builder - getColumnCanGlobalFilter is gated on column visibility ("search what you see"): hiding a column via the picker removes it from search, and the nullable-first-row type sniff is bypassed. Affects all TanStack tables. Backend - Gradebook controller, ability and course component; index JSON serializes students, assessments, submissions (with submissionId) and gamification. - Submission grade query also selects the submission id for grade links. - Remove the redundant ScoreAssessmentSummary download from Statistics. --- .../components/course/gradebook_component.rb | 23 + .../course/gradebook_controller.rb | 49 ++ .../course/statistics/aggregate_controller.rb | 7 - .../assessments_score_summary_download_job.rb | 17 - .../course/gradebook_ability_component.rb | 9 + app/models/course/assessment.rb | 16 + app/models/course/assessment/submission.rb | 21 + ...essments_score_summary_download_service.rb | 82 -- .../course/courses/sidebar.json.jbuilder | 1 + .../course/gradebook/index.json.jbuilder | 36 + .../aggregate/all_assessments.json.jbuilder | 3 + client/app/api/course/Gradebook.ts | 15 + .../api/course/Statistics/CourseStatistics.ts | 8 - client/app/api/course/index.js | 2 + .../bundles/course/container/CourseLoader.ts | 8 + .../container/__tests__/CourseLoader.test.ts | 46 + .../__tests__/GradeLinkHint.test.tsx | 53 ++ .../__tests__/GradebookColumnTree.test.tsx | 305 +++++++ .../__tests__/GradebookIndex.test.tsx | 149 ++++ .../__tests__/GradebookTable.test.tsx | 789 +++++++++++++++++ .../gradebook/components/GradeLinkHint.tsx | 47 + .../components/GradebookColumnTree.tsx | 219 +++++ .../gradebook/components/GradebookTable.tsx | 809 ++++++++++++++++++ .../components/buildAssessmentColumnIds.ts | 7 + .../app/bundles/course/gradebook/constants.ts | 5 + .../app/bundles/course/gradebook/handles.ts | 21 + .../bundles/course/gradebook/operations.ts | 12 + .../gradebook/pages/GradebookIndex/index.tsx | 106 +++ .../app/bundles/course/gradebook/selectors.ts | 24 + client/app/bundles/course/gradebook/store.ts | 63 ++ client/app/bundles/course/gradebook/types.ts | 8 + .../bundles/course/statistics/operations.ts | 24 - .../AssessmentsScoreSummaryDownload.tsx | 115 --- .../assessments/AssessmentsStatistics.tsx | 1 + .../AssessmentsStatisticsTable.tsx | 53 +- .../AssessmentsStatisticsTable.test.tsx | 58 ++ .../pages/StatisticsIndex/index.tsx | 5 +- client/app/bundles/course/statistics/types.ts | 1 + client/app/bundles/course/translations.ts | 4 + .../tables/InvitationResultExistingTable.tsx | 1 - .../tables/InvitationResultPrimaryTable.tsx | 1 - client/app/bundles/users/store.ts | 13 + client/app/bundles/users/types.ts | 7 + .../core/buttons/DownloadButton.tsx | 2 +- .../lib/components/core/dialogs/Prompt.tsx | 3 + .../MuiTableAdapter/MuiColumnPickerPrompt.tsx | 12 +- .../TanStackTableBuilder/columnsBuilder.ts | 3 + .../useTanStackTableBuilder.tsx | 104 ++- .../table/__tests__/csvGenerator.test.ts | 22 +- .../useTanStackTableBuilder.test.tsx | 239 +++++- .../components/table/__tests__/utils.test.ts | 33 + .../lib/components/table/adapters/Toolbar.ts | 2 +- .../table/builder/ColumnTemplate.ts | 1 + .../table/builder/featureTemplates.ts | 3 + client/app/lib/components/table/utils.ts | 5 +- client/app/lib/constants/icons.ts | 3 + client/app/lib/helpers/url-builders.js | 3 + .../__tests__/useDismissibleOnce.test.tsx | 62 ++ client/app/lib/hooks/useDismissibleOnce.ts | 48 ++ client/app/lib/translations/table.ts | 4 + client/app/routers/course/gradebook.tsx | 23 + client/app/routers/course/index.tsx | 2 + client/app/store.ts | 2 + client/app/types/course/courses.ts | 1 + client/app/types/course/gradebook.ts | 42 + client/locales/en.json | 77 +- client/locales/ko.json | 72 +- client/locales/zh.json | 76 +- config/locales/en/course/gradebook.yml | 5 + config/locales/en/csv.yml | 8 - config/locales/ko/course/gradebook.yml | 5 + config/locales/ko/csv.yml | 8 - config/locales/zh/course/gradebook.yml | 5 + config/locales/zh/csv.yml | 8 - config/routes.rb | 5 +- .../coursemology/seed_600_gradebook.rake | 239 ++++++ lib/tasks/coursemology/seed_gradebook.rake | 246 ++++++ .../course/gradebook_controller_spec.rb | 204 +++++ .../statistics/aggregate_controller_spec.rb | 23 + .../course/assessment/submission_spec.rb | 66 ++ spec/models/course/assessment_spec.rb | 29 + ...ent_score_summary_download_service_spec.rb | 162 ---- 82 files changed, 4585 insertions(+), 515 deletions(-) create mode 100644 app/controllers/components/course/gradebook_component.rb create mode 100644 app/controllers/course/gradebook_controller.rb delete mode 100644 app/jobs/course/statistics/assessments_score_summary_download_job.rb create mode 100644 app/models/components/course/gradebook_ability_component.rb delete mode 100644 app/services/course/statistics/assessments_score_summary_download_service.rb create mode 100644 app/views/course/gradebook/index.json.jbuilder create mode 100644 client/app/api/course/Gradebook.ts create mode 100644 client/app/bundles/course/container/__tests__/CourseLoader.test.ts create mode 100644 client/app/bundles/course/gradebook/__tests__/GradeLinkHint.test.tsx create mode 100644 client/app/bundles/course/gradebook/__tests__/GradebookColumnTree.test.tsx create mode 100644 client/app/bundles/course/gradebook/__tests__/GradebookIndex.test.tsx create mode 100644 client/app/bundles/course/gradebook/__tests__/GradebookTable.test.tsx create mode 100644 client/app/bundles/course/gradebook/components/GradeLinkHint.tsx create mode 100644 client/app/bundles/course/gradebook/components/GradebookColumnTree.tsx create mode 100644 client/app/bundles/course/gradebook/components/GradebookTable.tsx create mode 100644 client/app/bundles/course/gradebook/components/buildAssessmentColumnIds.ts create mode 100644 client/app/bundles/course/gradebook/constants.ts create mode 100644 client/app/bundles/course/gradebook/handles.ts create mode 100644 client/app/bundles/course/gradebook/operations.ts create mode 100644 client/app/bundles/course/gradebook/pages/GradebookIndex/index.tsx create mode 100644 client/app/bundles/course/gradebook/selectors.ts create mode 100644 client/app/bundles/course/gradebook/store.ts create mode 100644 client/app/bundles/course/gradebook/types.ts delete mode 100644 client/app/bundles/course/statistics/pages/StatisticsIndex/assessments/AssessmentsScoreSummaryDownload.tsx create mode 100644 client/app/bundles/course/statistics/pages/StatisticsIndex/assessments/__tests__/AssessmentsStatisticsTable.test.tsx create mode 100644 client/app/lib/components/table/__tests__/utils.test.ts create mode 100644 client/app/lib/hooks/__tests__/useDismissibleOnce.test.tsx create mode 100644 client/app/lib/hooks/useDismissibleOnce.ts create mode 100644 client/app/routers/course/gradebook.tsx create mode 100644 client/app/types/course/gradebook.ts create mode 100644 config/locales/en/course/gradebook.yml create mode 100644 config/locales/ko/course/gradebook.yml create mode 100644 config/locales/zh/course/gradebook.yml create mode 100644 lib/tasks/coursemology/seed_600_gradebook.rake create mode 100644 lib/tasks/coursemology/seed_gradebook.rake create mode 100644 spec/controllers/course/gradebook_controller_spec.rb delete mode 100644 spec/services/course/statistics/assessment_score_summary_download_service_spec.rb diff --git a/app/controllers/components/course/gradebook_component.rb b/app/controllers/components/course/gradebook_component.rb new file mode 100644 index 00000000000..fe2ccfe09e2 --- /dev/null +++ b/app/controllers/components/course/gradebook_component.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true +class Course::GradebookComponent < SimpleDelegator + include Course::ControllerComponentHost::Component + + def self.display_name + 'Gradebook' + end + + def sidebar_items + return [] unless can?(:read_gradebook, current_course) + + [ + { + key: self.class.key, + icon: :gradebook, + title: I18n.t('course.gradebook.component.sidebar_title'), + type: :normal, + weight: 9, + path: course_gradebook_path(current_course) + } + ] + end +end diff --git a/app/controllers/course/gradebook_controller.rb b/app/controllers/course/gradebook_controller.rb new file mode 100644 index 00000000000..71cfa4fc145 --- /dev/null +++ b/app/controllers/course/gradebook_controller.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true +class Course::GradebookController < Course::ComponentController + before_action :authorize_read_gradebook! + + def index + respond_to do |format| + format.json do + @published_assessments = fetch_published_assessments + @categories, @tabs = fetch_categories_and_tabs + @students = fetch_students + assessment_ids = @published_assessments.pluck(:id) + @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 + end + end + + private + + def authorize_read_gradebook! + authorize! :read_gradebook, current_course + end + + def component + current_component_host[:course_gradebook_component] + end + + def fetch_categories_and_tabs + tabs = @published_assessments.map(&:tab).uniq(&:id) + [tabs.map(&:category).uniq(&:id), tabs] + end + + def fetch_students + current_course.levels.to_a + current_course.course_users.students.without_phantom_users. + calculated(:experience_points).includes(:user).to_a + end + + def fetch_published_assessments + current_course.assessments. + published. + includes(tab: :category). + joins(tab: :category). + reorder('course_assessment_categories.weight, course_assessment_tabs.weight, course_assessments.id') + end +end diff --git a/app/controllers/course/statistics/aggregate_controller.rb b/app/controllers/course/statistics/aggregate_controller.rb index 2e44dc06f81..306984f30e6 100644 --- a/app/controllers/course/statistics/aggregate_controller.rb +++ b/app/controllers/course/statistics/aggregate_controller.rb @@ -47,13 +47,6 @@ def activity_get_help @course_user_hash = current_course.course_users.index_by(&:user_id) end - def download_score_summary - job = Course::Statistics::AssessmentsScoreSummaryDownloadJob. - perform_later(current_course, params[:assessment_ids]).job - - render partial: 'jobs/submitted', locals: { job: job } - end - private def sanitize_date_range(start_at_param, end_at_param) diff --git a/app/jobs/course/statistics/assessments_score_summary_download_job.rb b/app/jobs/course/statistics/assessments_score_summary_download_job.rb deleted file mode 100644 index 86ce040b994..00000000000 --- a/app/jobs/course/statistics/assessments_score_summary_download_job.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true -class Course::Statistics::AssessmentsScoreSummaryDownloadJob < ApplicationJob - include TrackableJob - queue_as :lowest - retry_on StandardError, attempts: 0 - - protected - - def perform_tracked(course, assessment_ids) - file_name = "#{Pathname.normalize_filename(course.title)}_score_summary_#{Time.now.strftime '%Y%m%d_%H%M'}.csv" - service = Course::Statistics::AssessmentsScoreSummaryDownloadService.new(course, assessment_ids, file_name) - csv_file = service.generate - redirect_to SendFile.send_file(csv_file, file_name) - ensure - service&.cleanup - end -end diff --git a/app/models/components/course/gradebook_ability_component.rb b/app/models/components/course/gradebook_ability_component.rb new file mode 100644 index 00000000000..d5b9862f299 --- /dev/null +++ b/app/models/components/course/gradebook_ability_component.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true +module Course::GradebookAbilityComponent + include AbilityHost::Component + + def define_permissions + can :read_gradebook, Course, id: course.id if course_user&.staff? + super + end +end diff --git a/app/models/course/assessment.rb b/app/models/course/assessment.rb index 3128ed42528..aec93f9de08 100644 --- a/app/models/course/assessment.rb +++ b/app/models/course/assessment.rb @@ -160,6 +160,22 @@ def self.use_relative_model_naming? true end + # Returns a hash of assessment_id => max_grade (sum of question maximum_grades). + def self.max_grades(assessment_ids) + return {} if assessment_ids.empty? + + rows = find_by_sql( + sanitize_sql_array([<<-SQL.squish, assessment_ids]) + SELECT cqa.assessment_id, COALESCE(SUM(caq.maximum_grade), 0) AS max_grade + FROM course_question_assessments cqa + JOIN course_assessment_questions caq ON caq.id = cqa.question_id + WHERE cqa.assessment_id IN (?) + GROUP BY cqa.assessment_id + SQL + ) + rows.to_h { |row| [row.assessment_id, row.max_grade.to_f] } + end + def to_partial_path 'course/assessment/assessments/assessment' end diff --git a/app/models/course/assessment/submission.rb b/app/models/course/assessment/submission.rb index c4919d6ec14..cb5df4c80f6 100644 --- a/app/models/course/assessment/submission.rb +++ b/app/models/course/assessment/submission.rb @@ -323,6 +323,27 @@ def self.on_dependent_status_change(answer) answer.submission.last_graded_time = Time.now end + # Returns an array of submission rows for the given students and assessments. + # Each row has: student_id (creator_id), assessment_id, grade (float). + # Only graded/published submissions are included. + def self.grade_summary(student_ids:, assessment_ids:) + return [] if student_ids.empty? || assessment_ids.empty? + + find_by_sql( + sanitize_sql_array([<<-SQL.squish, student_ids, assessment_ids]) + SELECT cas.creator_id AS student_id, cas.assessment_id, + cas.id AS submission_id, SUM(caa.grade) AS grade + FROM course_assessment_submissions cas + JOIN course_assessment_answers caa ON caa.submission_id = cas.id + WHERE cas.creator_id IN (?) + AND cas.assessment_id IN (?) + AND cas.workflow_state IN ('graded', 'published') + AND caa.current_answer = TRUE + GROUP BY cas.creator_id, cas.assessment_id, cas.id + SQL + ) + end + private # Queues the submission for auto grading, after the submission has changed to the submitted state. diff --git a/app/services/course/statistics/assessments_score_summary_download_service.rb b/app/services/course/statistics/assessments_score_summary_download_service.rb deleted file mode 100644 index 88e98756c6a..00000000000 --- a/app/services/course/statistics/assessments_score_summary_download_service.rb +++ /dev/null @@ -1,82 +0,0 @@ -# frozen_string_literal: true -require 'csv' -class Course::Statistics::AssessmentsScoreSummaryDownloadService - include TmpCleanupHelper - include ApplicationFormattersHelper - - def initialize(course, assessment_ids, file_name) - @course = course - @assessment_ids = assessment_ids - @file_name = file_name - @base_dir = Dir.mktmpdir('assessment-score-summary-') - end - - def generate - ActsAsTenant.without_tenant do - generate_csv_report - end - end - - def generate_csv_report - assessment_score_summary_file_path = File.join(@base_dir, @file_name) - - load_total_grades - CSV.open(assessment_score_summary_file_path, 'w') do |csv| - download_score_summary(csv) - end - - assessment_score_summary_file_path - end - - private - - def cleanup_entries - [@base_dir] - end - - def load_total_grades - @course_assessment_hash = Course::Assessment.where(id: @assessment_ids, course_id: @course.id).to_h do |assessment| - [assessment.id, assessment] - end - - @assessments = assessments - @submissions = Course::Assessment::Submission.where(assessment_id: @assessments.map(&:id)). - calculated(:grade). - preload(creator: :course_users) - - @submission_grade_hash = submission_grade_hash - @all_students = @course.course_users.students.order_alphabetically.preload(user: :emails) - @include_external_id = @course.course_users.students.where.not(external_id: [nil, '']).exists? - end - - def submission_grade_hash - @submissions.to_h do |submission| - course_user = submission.creator.course_users.find { |u| u.course_id == @course.id } - [[course_user.id, submission.assessment_id], submission.grade] - end - end - - def assessments - @assessment_ids.filter { |assessment_id| !@course_assessment_hash[assessment_id.to_i].nil? }.map do |assessment_id| - @course_assessment_hash[assessment_id.to_i] - end - end - - def download_score_summary(csv) - # header - csv << [ - I18n.t('csv.score_summary.headers.name'), - I18n.t('csv.score_summary.headers.email'), - I18n.t('csv.score_summary.headers.type'), - *(@include_external_id ? [I18n.t('csv.score_summary.headers.external_id')] : []), - *@assessments.map(&:title) - ] - - # content - @all_students.each do |student| - csv << [student.name, student.user.email, student.phantom? ? 'phantom' : 'normal', - *(@include_external_id ? [student.external_id.presence || ''] : []), - *@assessments.flat_map { |a| @submission_grade_hash[[student.id, a.id]] || '' }] - end - end -end diff --git a/app/views/course/courses/sidebar.json.jbuilder b/app/views/course/courses/sidebar.json.jbuilder index d7de1bebca1..1a31ba3f24f 100644 --- a/app/views/course/courses/sidebar.json.jbuilder +++ b/app/views/course/courses/sidebar.json.jbuilder @@ -4,6 +4,7 @@ json.courseUrl course_path(current_course) json.courseLogoUrl url_to_course_logo(current_course) json.courseUserUrl url_to_user_or_course_user(current_course, current_course_user) json.userName current_user&.name +json.userId current_user&.id if current_course_user.present? && can?(:read, current_course) json.courseUserName current_course_user.name diff --git a/app/views/course/gradebook/index.json.jbuilder b/app/views/course/gradebook/index.json.jbuilder new file mode 100644 index 00000000000..d2180773a40 --- /dev/null +++ b/app/views/course/gradebook/index.json.jbuilder @@ -0,0 +1,36 @@ +# frozen_string_literal: true +json.categories @categories do |cat| + json.id cat.id + json.title cat.title +end + +json.tabs @tabs do |tab| + json.id tab.id + json.title tab.title + json.categoryId tab.category_id +end + +json.assessments @published_assessments do |assessment| + json.id assessment.id + json.title assessment.title + json.tabId assessment.tab_id + json.maxGrade @assessment_max_grades[assessment.id] || 0 +end + +json.students @students do |course_user| + json.id course_user.user_id + json.name course_user.name + json.email course_user.user.email + json.externalId course_user.external_id + json.level course_user.level_number + json.totalXp course_user.experience_points +end + +json.submissions @submissions do |sub| + json.studentId sub.student_id + json.assessmentId sub.assessment_id + json.submissionId sub.submission_id + json.grade sub.grade&.to_f +end + +json.gamificationEnabled current_course.gamified? diff --git a/app/views/course/statistics/aggregate/all_assessments.json.jbuilder b/app/views/course/statistics/aggregate/all_assessments.json.jbuilder index 458efa1067c..7bc08582f3d 100644 --- a/app/views/course/statistics/aggregate/all_assessments.json.jbuilder +++ b/app/views/course/statistics/aggregate/all_assessments.json.jbuilder @@ -35,3 +35,6 @@ json.assessments @assessments do |assessment| json.numAttempted @num_attempted_students_hash[assessment.id] || 0 json.numLate @num_late_students_hash[assessment.id] || 0 end + +json.gradebookEnabled current_course.component_enabled?(Course::GradebookComponent) && + can?(:read_gradebook, current_course) diff --git a/client/app/api/course/Gradebook.ts b/client/app/api/course/Gradebook.ts new file mode 100644 index 00000000000..e00c94a64c3 --- /dev/null +++ b/client/app/api/course/Gradebook.ts @@ -0,0 +1,15 @@ +import { GradebookData } from 'types/course/gradebook'; + +import { APIResponse } from 'api/types'; + +import BaseCourseAPI from './Base'; + +export default class GradebookAPI extends BaseCourseAPI { + get #urlPrefix(): string { + return `/courses/${this.courseId}/gradebook`; + } + + index(): APIResponse { + return this.client.get(this.#urlPrefix); + } +} diff --git a/client/app/api/course/Statistics/CourseStatistics.ts b/client/app/api/course/Statistics/CourseStatistics.ts index 6e2d3e370f4..b478bec85df 100644 --- a/client/app/api/course/Statistics/CourseStatistics.ts +++ b/client/app/api/course/Statistics/CourseStatistics.ts @@ -1,5 +1,3 @@ -import { JobSubmitted } from 'types/jobs'; - import { APIResponse } from 'api/types'; import { AssessmentsStatistics, @@ -53,10 +51,4 @@ export default class CourseStatisticsAPI extends BaseCourseAPI { params, }); } - - downloadScoreSummary(assessmentIds: number[]): APIResponse { - return this.client.get(`${this.#urlPrefix}/assessments/download`, { - params: { assessment_ids: assessmentIds }, - }); - } } diff --git a/client/app/api/course/index.js b/client/app/api/course/index.js index 8f5df6176fe..355a5878c53 100644 --- a/client/app/api/course/index.js +++ b/client/app/api/course/index.js @@ -12,6 +12,7 @@ import DuplicationAPI from './Duplication'; import EnrolRequestsAPI from './EnrolRequests'; import ExperiencePointsRecordAPI from './ExperiencePointsRecord'; import ForumAPI from './Forum'; +import GradebookAPI from './Gradebook'; import GroupsAPI from './Groups'; import LeaderboardAPI from './Leaderboard'; import LearningMapAPI from './LearningMap'; @@ -48,6 +49,7 @@ const CourseAPI = { experiencePointsRecord: new ExperiencePointsRecordAPI(), folders: new FoldersAPI(), forum: ForumAPI, + gradebook: new GradebookAPI(), groups: new GroupsAPI(), leaderboard: new LeaderboardAPI(), learningMap: new LearningMapAPI(), diff --git a/client/app/bundles/course/container/CourseLoader.ts b/client/app/bundles/course/container/CourseLoader.ts index b55a5866b8f..8abe2df1ec1 100644 --- a/client/app/bundles/course/container/CourseLoader.ts +++ b/client/app/bundles/course/container/CourseLoader.ts @@ -3,9 +3,11 @@ import { useLoaderData, useOutletContext, } from 'react-router-dom'; +import { dispatch as imperativeDispatch } from 'store'; import { CourseLayoutData, SidebarItemData } from 'types/course/courses'; import CourseAPI from 'api/course'; +import { actions as userActions } from 'bundles/users/store'; import { syncSignals } from 'lib/hooks/unread'; const extractUnreadCountsInto = ( @@ -34,6 +36,12 @@ export const loader: LoaderFunction = async ({ params }) => { const response = await CourseAPI.courses.fetchLayout(id); + // Hydrate the authenticated user's id into the global store. It is otherwise + // only populated on the user profile page, so any course page relying on it + // (e.g. per-user localStorage namespacing for table column prefs) would read 0. + const { userId } = response.data; + if (userId != null) imperativeDispatch(userActions.setCurrentUserId(userId)); + syncSignals(extractUnreadCountsFromLayoutData(response.data)); return response.data; diff --git a/client/app/bundles/course/container/__tests__/CourseLoader.test.ts b/client/app/bundles/course/container/__tests__/CourseLoader.test.ts new file mode 100644 index 00000000000..3db67b15dce --- /dev/null +++ b/client/app/bundles/course/container/__tests__/CourseLoader.test.ts @@ -0,0 +1,46 @@ +import { store } from 'store'; + +import CourseAPI from 'api/course'; + +import { loader } from '../CourseLoader'; + +jest.mock('api/course', () => ({ + __esModule: true, + default: { courses: { fetchLayout: jest.fn() } }, +})); + +const mockFetchLayout = CourseAPI.courses.fetchLayout as jest.Mock; + +const runLoader = (courseId: string): Promise => + // The router passes { request, params }; only params.courseId is read here, so + // we route through `unknown` to call it with just the params it reads. + ( + loader as unknown as (args: { + params: { courseId: string }; + }) => Promise + )({ + params: { courseId }, + }); + +describe('CourseLoader loader', () => { + it('hydrates the authenticated user id into the global store', async () => { + mockFetchLayout.mockResolvedValueOnce({ + data: { courseTitle: 'C', userName: 'Alice', userId: 77 }, + }); + + await runLoader('1'); + + expect(store.getState().global.user.user.id).toBe(77); + }); + + it('leaves the global user id untouched when the payload has no user id', async () => { + store.dispatch({ type: 'system/SET_CURRENT_USER_ID', userId: 5 }); + mockFetchLayout.mockResolvedValueOnce({ + data: { courseTitle: 'C', userName: null, userId: null }, + }); + + await runLoader('1'); + + expect(store.getState().global.user.user.id).toBe(5); + }); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/GradeLinkHint.test.tsx b/client/app/bundles/course/gradebook/__tests__/GradeLinkHint.test.tsx new file mode 100644 index 00000000000..2ad267268aa --- /dev/null +++ b/client/app/bundles/course/gradebook/__tests__/GradeLinkHint.test.tsx @@ -0,0 +1,53 @@ +import { store as appStore } from 'store'; +import { fireEvent, render, screen, waitFor } from 'test-utils'; + +import GradeLinkHint, { + GRADE_LINK_HINT_KEY, +} from '../components/GradeLinkHint'; + +const USER_ID = 42; +const STORAGE_KEY = `${USER_ID}:${GRADE_LINK_HINT_KEY}`; + +// The dismissal flag is namespaced by the authenticated user id from the global +// user store, which the course layout loader hydrates on every course page. +const userState = { + global: { + ...appStore.getState().global, + user: { + ...appStore.getState().global.user, + user: { id: USER_ID, name: '', imageUrl: '' }, + }, + }, +}; + +const renderHint = (): void => { + render(, { state: userState }); +}; + +beforeEach(() => localStorage.clear()); + +describe('GradeLinkHint', () => { + it('explains that grades are clickable and lead to the submission', async () => { + renderHint(); + expect(await screen.findByText(/click any grade/i)).toBeInTheDocument(); + }); + + it('hides and persists dismissal when the close button is clicked', async () => { + renderHint(); + fireEvent.click(await screen.findByRole('button', { name: /close/i })); + + expect(screen.queryByText(/click any grade/i)).not.toBeInTheDocument(); + expect(localStorage.getItem(STORAGE_KEY)).toBe('true'); + }); + + it('does not render when already dismissed', async () => { + localStorage.setItem(STORAGE_KEY, 'true'); + renderHint(); + // Wait for the async locale load to settle (spinner gone) before asserting + // absence, so we are not just observing the pre-load loading state. + await waitFor(() => + expect(screen.queryByRole('progressbar')).not.toBeInTheDocument(), + ); + expect(screen.queryByText(/click any grade/i)).not.toBeInTheDocument(); + }); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/GradebookColumnTree.test.tsx b/client/app/bundles/course/gradebook/__tests__/GradebookColumnTree.test.tsx new file mode 100644 index 00000000000..280f3c71b7d --- /dev/null +++ b/client/app/bundles/course/gradebook/__tests__/GradebookColumnTree.test.tsx @@ -0,0 +1,305 @@ +import { IntlProvider } from 'react-intl'; +import { fireEvent, render, screen } from '@testing-library/react'; + +import { buildAssessmentColumnId } from '../components/buildAssessmentColumnIds'; +import GradebookColumnTree from '../components/GradebookColumnTree'; +import type { AssessmentData, CategoryData, TabData } from '../types'; + +const categories: CategoryData[] = [{ id: 1, title: 'Cat A' }]; +const tabs: TabData[] = [{ id: 10, title: 'Tab 1', categoryId: 1 }]; +const assessments: AssessmentData[] = [ + { id: 100, title: 'Quiz 1', tabId: 10, maxGrade: 10 }, + { id: 101, title: 'Quiz 2', tabId: 10, maxGrade: 10 }, +]; + +const asnId100 = buildAssessmentColumnId(100); +const asnId101 = buildAssessmentColumnId(101); +const allIds = ['name', 'email', 'externalId', 'level', asnId100, asnId101]; + +const wrap = (node: JSX.Element): JSX.Element => ( + + {node} + +); + +describe('GradebookColumnTree', () => { + it('renders Student info and Grades branch labels', () => { + const visibility = Object.fromEntries(allIds.map((id) => [id, true])); + render( + wrap( + visibility[id] ?? true} + setManyVisible={jest.fn()} + setVisible={jest.fn()} + tabs={tabs} + />, + ), + ); + expect(screen.getByText('Student info')).toBeInTheDocument(); + expect(screen.getByText('Grades')).toBeInTheDocument(); + }); + + it('renders Gamification branch when gamificationEnabled', () => { + const visibility = Object.fromEntries(allIds.map((id) => [id, true])); + render( + wrap( + visibility[id] ?? true} + setManyVisible={jest.fn()} + setVisible={jest.fn()} + tabs={tabs} + />, + ), + ); + expect(screen.getByText('Gamification')).toBeInTheDocument(); + expect( + screen.getByRole('checkbox', { name: /^level$/i }), + ).toBeInTheDocument(); + expect( + screen.getByRole('checkbox', { name: /^total xp$/i }), + ).toBeInTheDocument(); + }); + + it('hides Gamification branch when gamificationEnabled is false', () => { + const visibility = Object.fromEntries(allIds.map((id) => [id, true])); + render( + wrap( + visibility[id] ?? true} + setManyVisible={jest.fn()} + setVisible={jest.fn()} + tabs={tabs} + />, + ), + ); + expect(screen.queryByText('Gamification')).not.toBeInTheDocument(); + expect( + screen.queryByRole('checkbox', { name: /^level$/i }), + ).not.toBeInTheDocument(); + }); + + it('renders an External ID checkbox in the Student info group', () => { + const visibility = Object.fromEntries(allIds.map((id) => [id, true])); + render( + wrap( + visibility[id] ?? true} + setManyVisible={jest.fn()} + setVisible={jest.fn()} + tabs={tabs} + />, + ), + ); + expect( + screen.getByRole('checkbox', { name: /external id/i }), + ).toBeInTheDocument(); + }); + + it('clicking the External ID checkbox calls setVisible with its column id', () => { + const setVisible = jest.fn(); + const visibility = Object.fromEntries(allIds.map((id) => [id, true])); + render( + wrap( + visibility[id] ?? true} + setManyVisible={jest.fn()} + setVisible={setVisible} + tabs={tabs} + />, + ), + ); + fireEvent.click(screen.getByRole('checkbox', { name: /external id/i })); + expect(setVisible).toHaveBeenCalledWith('externalId', expect.any(Boolean)); + }); + + it('name checkbox is disabled and always checked', () => { + const visibility: Record = { + name: false, + email: true, + [asnId100]: true, + [asnId101]: true, + }; + render( + wrap( + visibility[id] ?? true} + setManyVisible={jest.fn()} + setVisible={jest.fn()} + tabs={tabs} + />, + ), + ); + const nameCheckbox = screen.getByRole('checkbox', { name: /^name/i }); + expect(nameCheckbox).toBeDisabled(); + expect(nameCheckbox).toBeChecked(); + }); + + it('non-name student info checkboxes are enabled and reflect visibility state', () => { + const visibility: Record = { + name: true, + email: false, + [asnId100]: true, + [asnId101]: true, + }; + render( + wrap( + visibility[id] ?? true} + setManyVisible={jest.fn()} + setVisible={jest.fn()} + tabs={tabs} + />, + ), + ); + const emailCheckbox = screen.getByRole('checkbox', { name: /^email$/i }); + expect(emailCheckbox).not.toBeDisabled(); + expect(emailCheckbox).not.toBeChecked(); + }); + + it('clicking a student info checkbox calls setVisible with its column id', () => { + const setVisible = jest.fn(); + const visibility = Object.fromEntries(allIds.map((id) => [id, true])); + render( + wrap( + visibility[id] ?? true} + setManyVisible={jest.fn()} + setVisible={setVisible} + tabs={tabs} + />, + ), + ); + fireEvent.click(screen.getByRole('checkbox', { name: /^email$/i })); + expect(setVisible).toHaveBeenCalledWith('email', expect.any(Boolean)); + }); + + it('renders Category, Tab, and assessment checkboxes', () => { + const visibility = Object.fromEntries(allIds.map((id) => [id, true])); + render( + wrap( + visibility[id] ?? true} + setManyVisible={jest.fn()} + setVisible={jest.fn()} + tabs={tabs} + />, + ), + ); + expect(screen.getByText('Cat A')).toBeInTheDocument(); + expect(screen.getByText('Tab 1')).toBeInTheDocument(); + expect( + screen.getByRole('checkbox', { name: /quiz 1/i }), + ).toBeInTheDocument(); + expect( + screen.getByRole('checkbox', { name: /quiz 2/i }), + ).toBeInTheDocument(); + }); + + it('clicking an assessment checkbox calls setVisible with the single column id', () => { + const setVisible = jest.fn(); + const visibility = Object.fromEntries(allIds.map((id) => [id, true])); + render( + wrap( + visibility[id] ?? true} + setManyVisible={jest.fn()} + setVisible={setVisible} + tabs={tabs} + />, + ), + ); + fireEvent.click(screen.getByRole('checkbox', { name: /quiz 1/i })); + expect(setVisible).toHaveBeenCalledWith(asnId100, expect.any(Boolean)); + }); + + it('renders "Always included" chip next to the Name row', () => { + const visibility = Object.fromEntries(allIds.map((id) => [id, true])); + render( + wrap( + visibility[id] ?? true} + setManyVisible={jest.fn()} + setVisible={jest.fn()} + tabs={tabs} + />, + ), + ); + expect(screen.getByText('Always included')).toBeInTheDocument(); + }); + + it('does not render "Always included" chip next to email row', () => { + const visibility = Object.fromEntries(allIds.map((id) => [id, true])); + render( + wrap( + visibility[id] ?? true} + setManyVisible={jest.fn()} + setVisible={jest.fn()} + tabs={tabs} + />, + ), + ); + expect(screen.getAllByText('Always included')).toHaveLength(1); + }); + + it('Student info branch is indeterminate when some but not all student cols are visible', () => { + const visibility: Record = { + name: true, + email: false, + [asnId100]: true, + [asnId101]: true, + }; + render( + wrap( + visibility[id] ?? true} + setManyVisible={jest.fn()} + setVisible={jest.fn()} + tabs={tabs} + />, + ), + ); + expect( + screen.getByRole('checkbox', { name: /student info/i }), + ).toHaveAttribute('data-indeterminate', 'true'); + }); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/GradebookIndex.test.tsx b/client/app/bundles/course/gradebook/__tests__/GradebookIndex.test.tsx new file mode 100644 index 00000000000..25b66fd3ed3 --- /dev/null +++ b/client/app/bundles/course/gradebook/__tests__/GradebookIndex.test.tsx @@ -0,0 +1,149 @@ +import { fireEvent, render, screen, waitFor } from 'test-utils'; + +import toast from 'lib/hooks/toast'; + +import fetchGradebook from '../operations'; +import GradebookIndex from '../pages/GradebookIndex'; + +jest.mock('../../container/CourseLoader', () => ({ + useCourseContext: (): { courseTitle: string; id: number } => ({ + courseTitle: 'Test Course', + id: 1, + }), +})); + +jest.mock('lib/hooks/toast', () => ({ + __esModule: true, + default: { error: jest.fn(), success: jest.fn() }, +})); + +jest.mock('../operations', () => ({ + __esModule: true, + default: jest.fn(() => (): Promise => Promise.resolve()), +})); + +const mockFetchGradebook = fetchGradebook as jest.Mock; + +const emptyState = { + gradebook: { + categories: [], + tabs: [], + assessments: [], + students: [], + submissions: [], + gamificationEnabled: false, + }, +}; + +const noStudentsState = { + gradebook: { + categories: [{ id: 1, title: 'Cat A' }], + tabs: [{ id: 10, title: 'Tab 1', categoryId: 1 }], + assessments: [{ id: 100, title: 'Quiz 1', tabId: 10, maxGrade: 10 }], + students: [], + submissions: [], + gamificationEnabled: false, + }, +}; + +const populatedState = { + gradebook: { + categories: [{ id: 1, title: 'Cat A' }], + tabs: [{ id: 10, title: 'Tab 1', categoryId: 1 }], + assessments: [{ id: 100, title: 'Quiz 1', tabId: 10, maxGrade: 10 }], + students: [ + { + id: 1, + name: 'Alice', + email: 'alice@example.com', + externalId: null, + level: 3, + totalXp: 150, + }, + ], + submissions: [ + { studentId: 1, assessmentId: 100, submissionId: 1000, grade: 8 }, + ], + gamificationEnabled: false, + }, +}; + +const populatedStateWithGamification = { + gradebook: { + ...populatedState.gradebook, + gamificationEnabled: true, + }, +}; + +beforeEach(() => { + jest.clearAllMocks(); + mockFetchGradebook.mockReturnValue((): Promise => Promise.resolve()); +}); + +describe('GradebookIndex', () => { + it('shows loading indicator initially', () => { + render(, { state: emptyState }); + expect(screen.getByRole('progressbar')).toBeInTheDocument(); + }); + + it('shows the gradebook table after data loads', async () => { + render(, { state: populatedState }); + expect( + await screen.findByRole('button', { name: /export/i }), + ).toBeInTheDocument(); + }); + + it('shows the page title', async () => { + render(, { state: populatedState }); + expect(await screen.findByText('Gradebook')).toBeInTheDocument(); + }); + + it('shows empty students message when there are no students', async () => { + render(, { state: noStudentsState }); + expect( + await screen.findByText('No students enrolled yet'), + ).toBeInTheDocument(); + }); + + it('shows empty students message when both assessments and students are absent', async () => { + render(, { state: emptyState }); + expect( + await screen.findByText('No students enrolled yet'), + ).toBeInTheDocument(); + }); + + it('shows error toast when fetch fails', async () => { + mockFetchGradebook.mockReturnValueOnce( + (): Promise => Promise.reject(new Error('Network error')), + ); + render(, { state: emptyState }); + await waitFor(() => expect(toast.error).toHaveBeenCalled()); + }); + + it('shows grade-only hint in column picker when gamification is disabled and no data cols selected', async () => { + render(, { state: populatedState }); + fireEvent.click( + await screen.findByRole('button', { name: /select columns/i }), + ); + expect( + await screen.findByText( + 'No grade columns selected - export will include student info only.', + ), + ).toBeInTheDocument(); + }); + + it('shows grade-and-gamification hint in column picker when gamification is enabled and no data cols selected', async () => { + render(, { state: populatedStateWithGamification }); + fireEvent.click( + await screen.findByRole('button', { name: /select columns/i }), + ); + fireEvent.click( + await screen.findByRole('checkbox', { name: /gamification/i }), + ); + expect( + await screen.findByText( + 'No grade or gamification columns selected - export will include student info only.', + ), + ).toBeInTheDocument(); + }); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/GradebookTable.test.tsx b/client/app/bundles/course/gradebook/__tests__/GradebookTable.test.tsx new file mode 100644 index 00000000000..904d4513cb6 --- /dev/null +++ b/client/app/bundles/course/gradebook/__tests__/GradebookTable.test.tsx @@ -0,0 +1,789 @@ +import userEvent from '@testing-library/user-event'; +import { store as appStore } from 'store'; +import { render, screen, waitFor, within } from 'test-utils'; + +import GradebookTable from '../components/GradebookTable'; +import type { + AssessmentData, + CategoryData, + StudentData, + SubmissionData, + TabData, +} from '../types'; + +const categories: CategoryData[] = [{ id: 1, title: 'Cat A' }]; +const tabs: TabData[] = [{ id: 10, title: 'Tab 1', categoryId: 1 }]; +const assessments: AssessmentData[] = [ + { id: 100, title: 'Quiz 1', tabId: 10, maxGrade: 10 }, +]; +const students: StudentData[] = [ + { + id: 1, + name: 'Alice', + email: 'alice@example.com', + externalId: null, + level: 3, + totalXp: 150, + }, + { + id: 2, + name: 'Bob', + email: 'bob@example.com', + externalId: null, + level: 5, + totalXp: 300, + }, +]; +const submissions: SubmissionData[] = [ + { submissionId: 0, studentId: 1, assessmentId: 100, grade: 8 }, +]; + +const makeStudents = (n: number): StudentData[] => + Array.from({ length: n }, (_, i) => ({ + id: i + 1, + name: `Student ${i + 1}`, + email: `student${i + 1}@example.com`, + externalId: null, + level: 1, + totalXp: 0, + })); + +// Asserts the given texts appear in this top-to-bottom DOM order. +const expectInOrder = (names: string[]): void => { + for (let i = 0; i < names.length - 1; i += 1) { + const earlier = screen.getByText(names[i]); + const later = screen.getByText(names[i + 1]); + expect( + // eslint-disable-next-line no-bitwise + earlier.compareDocumentPosition(later) & Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy(); + } +}; + +// User id used in all renders so localStorage is keyed as `${USER_ID}:gradebook_columns_1`. +// It is seeded into the global user store, which the course layout loader hydrates +// on every course page; the builder namespaces column persistence by that id. +const USER_ID = 42; +const STORAGE_KEY = `${USER_ID}:gradebook_columns_1`; +const SORT_STORAGE_KEY = `${STORAGE_KEY}_sort`; + +const userState = { + global: { + ...appStore.getState().global, + user: { + ...appStore.getState().global.user, + user: { id: USER_ID, name: '', imageUrl: '' }, + }, + }, +}; + +interface RenderOptions { + gamificationEnabled?: boolean; +} + +const renderTable = ({ + gamificationEnabled = true, +}: RenderOptions = {}): void => { + render( + , + { state: userState }, + ); +}; + +const renderTableWithAssessmentVisible = ( + options: RenderOptions = {}, +): void => { + localStorage.setItem( + STORAGE_KEY, + JSON.stringify({ + name: true, + email: true, + 'asn-100': true, + }), + ); + renderTable(options); +}; + +describe('GradebookTable', () => { + beforeEach(() => localStorage.clear()); + + it('renders both student names', async () => { + renderTableWithAssessmentVisible(); + expect(await screen.findByText('Alice')).toBeInTheDocument(); + expect(await screen.findByText('Bob')).toBeInTheDocument(); + }); + + it('renders two header rows (column titles and max marks)', async () => { + localStorage.setItem( + STORAGE_KEY, + JSON.stringify({ + name: true, + email: true, + + 'asn-100': true, + }), + ); + const { container } = render( + , + { state: userState }, + ); + await screen.findByText('Alice'); + expect(container.querySelectorAll('thead tr')).toHaveLength(2); + }); + + it('shows Select Columns button and Export button', async () => { + renderTableWithAssessmentVisible(); + await screen.findByText('Alice'); + expect( + screen.getByRole('button', { name: /select columns/i }), + ).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /export/i })).toBeInTheDocument(); + }); + + describe('export button label reflects selection', () => { + it('shows "Export all rows" when no rows are selected', async () => { + renderTableWithAssessmentVisible(); + await screen.findByText('Alice'); + expect( + screen.getByRole('button', { name: /export all rows/i }), + ).toBeInTheDocument(); + }); + + it('shows tooltip "all rows will be exported" when no rows are selected', async () => { + const user = userEvent.setup(); + renderTableWithAssessmentVisible(); + const exportBtn = await screen.findByRole('button', { + name: /export all rows/i, + }); + await user.hover(exportBtn); + expect( + await screen.findByText(/all rows will be exported/i), + ).toBeInTheDocument(); + }); + + it('hides the tooltip when a row is selected', async () => { + const user = userEvent.setup(); + renderTableWithAssessmentVisible(); + const checkboxes = await screen.findAllByRole('checkbox'); + await user.click(checkboxes[1]); + const exportBtn = await screen.findByRole('button', { + name: /export 1 row/i, + }); + await user.hover(exportBtn); + expect( + screen.queryByText(/all rows will be exported/i), + ).not.toBeInTheDocument(); + }); + + it('shows "Export 1 row" when one row is selected', async () => { + const user = userEvent.setup(); + renderTableWithAssessmentVisible(); + const checkboxes = await screen.findAllByRole('checkbox'); + await user.click(checkboxes[1]); + await waitFor(() => + expect( + screen.getByRole('button', { name: /export 1 row/i }), + ).toBeInTheDocument(), + ); + }); + + it('shows "Export all rows" when all rows are selected via the corner checkbox', async () => { + const user = userEvent.setup(); + renderTableWithAssessmentVisible(); + const checkboxes = await screen.findAllByRole('checkbox'); + await user.click(checkboxes[0]); + await waitFor(() => + expect( + screen.getByRole('button', { name: /export all rows/i }), + ).toBeInTheDocument(), + ); + expect( + screen.queryByRole('button', { name: /export \d+ row/i }), + ).not.toBeInTheDocument(); + }); + }); + + it('shows the Max Marks header row', async () => { + renderTableWithAssessmentVisible(); + expect(await screen.findByText('Max Marks')).toBeInTheDocument(); + }); + + it('renders row selection checkboxes', async () => { + renderTableWithAssessmentVisible(); + await screen.findByText('Alice'); + expect(screen.getAllByRole('checkbox').length).toBeGreaterThanOrEqual(2); + }); + + describe('row selection', () => { + it('keeps search input visible after selecting a row', async () => { + const user = userEvent.setup(); + renderTableWithAssessmentVisible(); + const checkboxes = await screen.findAllByRole('checkbox'); + await user.click(checkboxes[1]); + expect(screen.getByRole('textbox')).toBeInTheDocument(); + }); + + it('keeps Export button visible after selecting a row', async () => { + const user = userEvent.setup(); + renderTableWithAssessmentVisible(); + const checkboxes = await screen.findAllByRole('checkbox'); + await user.click(checkboxes[1]); + expect( + screen.getByRole('button', { name: /export/i }), + ).toBeInTheDocument(); + }); + }); + + it('does not show assessment columns in the table by default', async () => { + renderTable(); + await screen.findByText('Alice'); + expect(screen.queryByText('Quiz 1')).not.toBeInTheDocument(); + }); + + it('shows gamification columns by default when gamification is enabled', async () => { + renderTable({ gamificationEnabled: true }); + expect(await screen.findByText('Level')).toBeInTheDocument(); + expect(screen.getByText('Total XP')).toBeInTheDocument(); + }); + + describe('gamification columns', () => { + it('shows level and totalXp in the column picker when gamification is enabled', async () => { + const user = userEvent.setup(); + renderTable({ gamificationEnabled: true }); + const selectColumnsBtn = await screen.findByRole('button', { + name: /select columns/i, + }); + await user.click(selectColumnsBtn); + const dialog = await screen.findByRole('dialog'); + expect(within(dialog).getByText('Level')).toBeInTheDocument(); + expect(within(dialog).getByText('Total XP')).toBeInTheDocument(); + }); + }); + + describe('locked name column', () => { + it('name is always visible even when localStorage sets it to false', async () => { + localStorage.setItem( + STORAGE_KEY, + JSON.stringify({ + name: false, + email: true, + + 'asn-100': true, + }), + ); + renderTable(); + await waitFor(() => + expect(screen.getByText('Alice')).toBeInTheDocument(), + ); + }); + }); + + describe('gamification disabled', () => { + it('level and totalXp absent from table headers when gamification is disabled', async () => { + localStorage.setItem( + STORAGE_KEY, + JSON.stringify({ + name: true, + email: true, + + level: true, + totalXp: true, + 'asn-100': true, + }), + ); + renderTable({ gamificationEnabled: false }); + await screen.findByText('Alice'); + expect(screen.queryByText('Level')).not.toBeInTheDocument(); + expect(screen.queryByText('Total XP')).not.toBeInTheDocument(); + }); + }); + + it('shows the table when gamification columns are visible and assessments are deselected', async () => { + localStorage.setItem(STORAGE_KEY, JSON.stringify({ 'asn-100': false })); + renderTable({ gamificationEnabled: true }); + expect(await screen.findByText('Alice')).toBeInTheDocument(); + expect(screen.getByRole('table')).toBeInTheDocument(); + }); + + it('export button is always enabled regardless of which columns are selected', async () => { + localStorage.setItem(STORAGE_KEY, JSON.stringify({ 'asn-100': false })); + renderTable({ gamificationEnabled: false }); + await screen.findByText('Alice'); + expect(screen.getByRole('button', { name: /export/i })).not.toBeDisabled(); + }); + + it('shows the table (not an empty state) when all assessments are deselected', async () => { + localStorage.setItem(STORAGE_KEY, JSON.stringify({ 'asn-100': false })); + renderTable({ gamificationEnabled: false }); + expect(await screen.findByRole('table')).toBeInTheDocument(); + expect(await screen.findByText('Alice')).toBeInTheDocument(); + }); + + it('shows the table when all optional columns are deselected with gamification', async () => { + localStorage.setItem( + STORAGE_KEY, + JSON.stringify({ 'asn-100': false, level: false, totalXp: false }), + ); + renderTable({ gamificationEnabled: true }); + expect(await screen.findByRole('table')).toBeInTheDocument(); + expect(await screen.findByText('Alice')).toBeInTheDocument(); + }); + + it('shows pagination when all assessments are deselected', async () => { + localStorage.setItem(STORAGE_KEY, JSON.stringify({ 'asn-100': false })); + renderTable({ gamificationEnabled: false }); + await screen.findByText('Alice'); + expect(screen.getByText(/rows per page/i)).toBeInTheDocument(); + }); + + it('shows the table with assessment columns when restored from localStorage', async () => { + localStorage.setItem( + STORAGE_KEY, + JSON.stringify({ + name: true, + email: true, + + 'asn-100': true, + }), + ); + renderTable(); + expect(await screen.findByText('Quiz 1')).toBeInTheDocument(); + }); + + // An assessment selected (and persisted) earlier may be deleted before the next + // visit, leaving a stale column id in localStorage. The table must ignore it + // rather than crash, and still render the surviving assessment columns. + it('does not crash when localStorage references a deleted assessment', async () => { + localStorage.setItem( + STORAGE_KEY, + JSON.stringify({ + name: true, + email: true, + 'asn-100': true, + 'asn-999': true, + }), + ); + renderTable(); + expect(await screen.findByText('Alice')).toBeInTheDocument(); + // The surviving assessment still renders; the deleted one is silently dropped. + expect(screen.getByText('Quiz 1')).toBeInTheDocument(); + }); + + describe('search', () => { + it('filters by name', async () => { + const user = userEvent.setup(); + renderTableWithAssessmentVisible(); + const input = await screen.findByRole('textbox'); + await user.type(input, 'Alice'); + await waitFor(() => + expect(screen.queryByText('Bob')).not.toBeInTheDocument(), + ); + expect(screen.getByText('Alice')).toBeInTheDocument(); + }); + + it('filters by email', async () => { + const user = userEvent.setup(); + renderTableWithAssessmentVisible(); + const input = await screen.findByRole('textbox'); + await user.type(input, 'bob@example.com'); + await waitFor(() => + expect(screen.queryByText('Alice')).not.toBeInTheDocument(), + ); + expect(screen.getByText('Bob')).toBeInTheDocument(); + }); + }); + + describe('external ID column', () => { + const studentsWithExtId: StudentData[] = [ + { + id: 1, + name: 'Alice', + email: 'alice@example.com', + externalId: 'EXT-001', + level: 3, + totalXp: 150, + }, + { + id: 2, + name: 'Bob', + email: 'bob@example.com', + externalId: null, + level: 5, + totalXp: 300, + }, + ]; + + const renderWith = (studs: StudentData[]): void => { + render( + , + { state: userState }, + ); + }; + + it('shows the External ID column by default when a student has an external ID', async () => { + renderWith(studentsWithExtId); + expect(await screen.findByText('External ID')).toBeInTheDocument(); + expect(screen.getByText('EXT-001')).toBeInTheDocument(); + }); + + it('hides the External ID column by default when no student has an external ID', async () => { + renderWith(students); + await screen.findByText('Alice'); + expect(screen.queryByText('External ID')).not.toBeInTheDocument(); + }); + + it('treats a blank external ID as none and hides the column by default', async () => { + const studentsWithBlankExtId: StudentData[] = [ + { + id: 1, + name: 'Alice', + email: 'alice@example.com', + externalId: '', + level: 3, + totalXp: 150, + }, + ]; + renderWith(studentsWithBlankExtId); + await screen.findByText('Alice'); + expect(screen.queryByText('External ID')).not.toBeInTheDocument(); + }); + + it('offers the External ID checkbox in the picker even when no student has one', async () => { + const user = userEvent.setup(); + renderWith(students); + const btn = await screen.findByRole('button', { + name: /select columns/i, + }); + await user.click(btn); + const dialog = await screen.findByRole('dialog'); + expect( + within(dialog).getByRole('checkbox', { name: /external id/i }), + ).toBeInTheDocument(); + }); + + it('filters by external ID', async () => { + const user = userEvent.setup(); + renderWith(studentsWithExtId); + const input = await screen.findByRole('textbox'); + await user.type(input, 'EXT-001'); + await waitFor(() => + expect(screen.queryByText('Bob')).not.toBeInTheDocument(), + ); + expect(screen.getByText('Alice')).toBeInTheDocument(); + }); + }); + + describe('sorting', () => { + it('defaults to sorting by name ascending on first load', async () => { + // Raw prop order is Bob, then Alice. A working default name-asc sort must + // reorder the rendered rows to Alice, then Bob. + render( + , + { state: userState }, + ); + await screen.findByText('Alice'); + expectInOrder(['Alice', 'Bob']); + }); + + it('cycles back to ascending on a third click (sort is never cleared)', async () => { + const user = userEvent.setup(); + renderTableWithAssessmentVisible(); + await screen.findByText('Alice'); + await user.click(screen.getByRole('button', { name: /name/i })); // → desc + await waitFor(() => expectInOrder(['Bob', 'Alice'])); + await user.click(screen.getByRole('button', { name: /name/i })); // → asc again + await waitFor(() => expectInOrder(['Alice', 'Bob'])); + }); + + it('toggles to descending when the name header is clicked', async () => { + const user = userEvent.setup(); + renderTableWithAssessmentVisible(); + await screen.findByText('Alice'); + expectInOrder(['Alice', 'Bob']); // default ascending + await user.click(screen.getByRole('button', { name: /name/i })); + await waitFor(() => expectInOrder(['Bob', 'Alice'])); + }); + + it('preserves the active sort after searching', async () => { + const user = userEvent.setup(); + renderTableWithAssessmentVisible(); + await screen.findByText('Alice'); + // Switch to descending so the order is distinct from both raw and default. + await user.click(screen.getByRole('button', { name: /name/i })); + await waitFor(() => expectInOrder(['Bob', 'Alice'])); + // A search matching both students must not reset the descending sort. + await user.type(screen.getByRole('textbox'), 'example.com'); + await waitFor(() => + expect(screen.getByText('Alice')).toBeInTheDocument(), + ); + expectInOrder(['Bob', 'Alice']); + }); + + it('resets to name ascending when the sorted column is hidden', async () => { + const user = userEvent.setup(); + // Start with Quiz 1 visible and sort by it descending. + localStorage.setItem( + STORAGE_KEY, + JSON.stringify({ name: true, 'asn-100': true }), + ); + render( + , + { state: userState }, + ); + await screen.findByText('Alice'); + await user.click(screen.getByRole('button', { name: /quiz 1/i })); // sort by grade asc + await user.click(screen.getByRole('button', { name: /quiz 1/i })); // → desc + await waitFor(() => expectInOrder(['Alice', 'Bob'])); // Alice has grade 8, Bob has none + + // Hide Quiz 1 via the column picker. + await user.click(screen.getByRole('button', { name: /select columns/i })); + const dialog = await screen.findByRole('dialog'); + await user.click( + within(dialog).getByRole('checkbox', { name: /quiz 1/i }), + ); + await user.click(screen.getByRole('button', { name: /apply/i })); + + // Sort should reset to name ascending: Alice before Bob. + await waitFor(() => expectInOrder(['Alice', 'Bob'])); + }); + + it('saves the sort to localStorage when the user clicks a column header', async () => { + const user = userEvent.setup(); + renderTableWithAssessmentVisible(); + await screen.findByText('Alice'); + await user.click(screen.getByRole('button', { name: /name/i })); // → desc + await waitFor(() => expectInOrder(['Bob', 'Alice'])); + expect( + JSON.parse(localStorage.getItem(SORT_STORAGE_KEY) ?? 'null'), + ).toEqual([{ id: 'name', desc: true }]); + }); + + it('restores sort from localStorage on re-mount', async () => { + // Pre-seed descending name sort so the table should render Bob before Alice. + localStorage.setItem( + SORT_STORAGE_KEY, + JSON.stringify([{ id: 'name', desc: true }]), + ); + renderTableWithAssessmentVisible(); + await screen.findByText('Alice'); + expectInOrder(['Bob', 'Alice']); + }); + + it('persists the name-ascending reset to localStorage when a sorted column is hidden', async () => { + const user = userEvent.setup(); + localStorage.setItem( + STORAGE_KEY, + JSON.stringify({ name: true, 'asn-100': true }), + ); + render( + , + { state: userState }, + ); + await screen.findByText('Alice'); + await user.click(screen.getByRole('button', { name: /quiz 1/i })); // asc + await user.click(screen.getByRole('button', { name: /quiz 1/i })); // desc + + await user.click(screen.getByRole('button', { name: /select columns/i })); + const dialog = await screen.findByRole('dialog'); + await user.click( + within(dialog).getByRole('checkbox', { name: /quiz 1/i }), + ); + await user.click(screen.getByRole('button', { name: /apply/i })); + + await waitFor(() => expectInOrder(['Alice', 'Bob'])); + expect( + JSON.parse(localStorage.getItem(SORT_STORAGE_KEY) ?? 'null'), + ).toEqual([{ id: 'name', desc: false }]); + }); + + describe('assessment grade sorting', () => { + const studentFor = (id: number, name: string): StudentData => ({ + id, + name, + email: `${name.toLowerCase()}@example.com`, + externalId: null, + level: 1, + totalXp: 0, + }); + + const renderGrades = ( + studs: StudentData[], + subs: SubmissionData[], + ): void => { + localStorage.setItem( + STORAGE_KEY, + JSON.stringify({ name: true, 'asn-100': true }), + ); + render( + , + { state: userState }, + ); + }; + + // Alice graded 8, Bob graded 3, Carol no submission (undefined), + // Dave submitted but ungraded (null). + const mixedStudents: StudentData[] = [ + studentFor(1, 'Alice'), + studentFor(2, 'Bob'), + studentFor(3, 'Carol'), + studentFor(4, 'Dave'), + ]; + const mixedSubmissions: SubmissionData[] = [ + { submissionId: 1, studentId: 1, assessmentId: 100, grade: 8 }, + { submissionId: 2, studentId: 2, assessmentId: 100, grade: 3 }, + { submissionId: 4, studentId: 4, assessmentId: 100, grade: null }, + ]; + + it('sorts missing grades to the bottom in ascending order', async () => { + const user = userEvent.setup(); + renderGrades(mixedStudents, mixedSubmissions); + await screen.findByText('Alice'); + await user.click(screen.getByRole('button', { name: /quiz 1/i })); + // Ascending: Bob(3), Alice(8), then the two missing rows last. + await waitFor(() => expectInOrder(['Bob', 'Alice'])); + expectInOrder(['Alice', 'Carol']); + expectInOrder(['Alice', 'Dave']); + }); + + it('keeps missing grades at the bottom in descending order', async () => { + const user = userEvent.setup(); + renderGrades(mixedStudents, mixedSubmissions); + await screen.findByText('Alice'); + await user.click(screen.getByRole('button', { name: /quiz 1/i })); // asc + await user.click(screen.getByRole('button', { name: /quiz 1/i })); // desc + // Descending: Alice(8), Bob(3), then the two missing rows still last. + await waitFor(() => expectInOrder(['Alice', 'Bob'])); + expectInOrder(['Bob', 'Carol']); + expectInOrder(['Bob', 'Dave']); + }); + + it('sorts grades numerically (9 before 10), not lexically', async () => { + const user = userEvent.setup(); + renderGrades( + [studentFor(1, 'Alice'), studentFor(2, 'Bob')], + [ + { submissionId: 1, studentId: 1, assessmentId: 100, grade: 9 }, + { submissionId: 2, studentId: 2, assessmentId: 100, grade: 10 }, + ], + ); + await screen.findByText('Alice'); + await user.click(screen.getByRole('button', { name: /quiz 1/i })); + // Numeric ascending: 9 (Alice) before 10 (Bob). Lexical would reverse this. + await waitFor(() => expectInOrder(['Alice', 'Bob'])); + }); + }); + }); + + describe('cross-page selection', () => { + it('export label reflects selection count across pages', async () => { + const user = userEvent.setup(); + localStorage.setItem( + STORAGE_KEY, + JSON.stringify({ + name: true, + email: true, + + 'asn-100': true, + }), + ); + render( + , + { state: userState }, + ); + + const checkboxes = await screen.findAllByRole('checkbox'); + await user.click(checkboxes[1]); + await waitFor(() => + expect( + screen.getByRole('button', { name: /export 1 row/i }), + ).toBeInTheDocument(), + ); + + // Default page size is DEFAULT_TABLE_ROWS_PER_PAGE (100), so 101 students + // span two pages: Student 1 is on page 1, Student 101 alone on page 2. + await user.click( + screen.getByRole('button', { name: /go to next page/i }), + ); + await waitFor(() => + expect(screen.getByText('Student 101')).toBeInTheDocument(), + ); + expect(screen.queryByText('Student 1')).not.toBeInTheDocument(); + + expect( + screen.getByRole('button', { name: /export 1 row/i }), + ).toBeInTheDocument(); + }); + }); +}); diff --git a/client/app/bundles/course/gradebook/components/GradeLinkHint.tsx b/client/app/bundles/course/gradebook/components/GradeLinkHint.tsx new file mode 100644 index 00000000000..7b2e8f44a1f --- /dev/null +++ b/client/app/bundles/course/gradebook/components/GradeLinkHint.tsx @@ -0,0 +1,47 @@ +import { FC } from 'react'; +import { defineMessages } from 'react-intl'; +import { Alert, Typography } from '@mui/material'; + +import { getUserEntity } from 'bundles/users/selectors'; +import { useAppSelector } from 'lib/hooks/store'; +import useDismissibleOnce from 'lib/hooks/useDismissibleOnce'; +import useTranslation from 'lib/hooks/useTranslation'; + +export const GRADE_LINK_HINT_KEY = 'gradebook_grade_link_hint'; + +const translations = defineMessages({ + hint: { + id: 'course.gradebook.GradeLinkHint.hint', + defaultMessage: + "Each grade is the total of the marks in a student's submission. Click any grade to open that submission and adjust the marks.", + }, +}); + +/** + * One-time, dismissable nudge explaining that each grade in the "All assessments" + * table is a link into the student's submission. Grades have no gradebook-level edit + * (an assessment's total is always the sum of its question marks), so clicking into the + * submission is the only path to changing a grade — an affordance that surprises users + * because a number is not expected to be clickable. + * Dismissal is remembered per user via localStorage (see useDismissibleOnce). + */ +const GradeLinkHint: FC = () => { + const { t } = useTranslation(); + const userId = useAppSelector(getUserEntity).id; + const { dismissed, dismiss } = useDismissibleOnce( + GRADE_LINK_HINT_KEY, + userId, + ); + + if (dismissed) return null; + + return ( +
+ + {t(translations.hint)} + +
+ ); +}; + +export default GradeLinkHint; diff --git a/client/app/bundles/course/gradebook/components/GradebookColumnTree.tsx b/client/app/bundles/course/gradebook/components/GradebookColumnTree.tsx new file mode 100644 index 00000000000..e2b5d526485 --- /dev/null +++ b/client/app/bundles/course/gradebook/components/GradebookColumnTree.tsx @@ -0,0 +1,219 @@ +import { useMemo } from 'react'; +import { defineMessages } from 'react-intl'; +import { Chip } from '@mui/material'; + +import IndentedCheckbox from 'lib/components/core/IndentedCheckbox'; +import { + ColumnPickerRenderContext, + ColumnPickerTreeGroup, +} from 'lib/components/table'; +import useTranslation from 'lib/hooks/useTranslation'; +import tableTranslations from 'lib/translations/table'; + +import { + GAMIFICATION_COL_IDS, + type GamificationColId, + STUDENT_INFO_COL_IDS, + type StudentInfoColId, +} from '../constants'; +import type { AssessmentData, CategoryData, TabData } from '../types'; + +import { + buildAssessmentColumnId, + parseAssessmentColumnId, +} from './buildAssessmentColumnIds'; + +const translations = defineMessages({ + studentInfo: { + id: 'course.gradebook.GradebookColumnTree.studentInfo', + defaultMessage: 'Student info', + }, + gamification: { + id: 'course.gradebook.GradebookColumnTree.gamification', + defaultMessage: 'Gamification', + }, + grades: { + id: 'course.gradebook.GradebookColumnTree.grades', + defaultMessage: 'Grades', + }, + alwaysIncluded: { + id: 'course.gradebook.GradebookColumnTree.alwaysIncluded', + defaultMessage: 'Always included', + }, +}); + +interface GradebookColumnTreeProps extends ColumnPickerRenderContext { + categories: CategoryData[]; + tabs: TabData[]; + assessments: AssessmentData[]; + gamificationEnabled: boolean; +} + +const GAMIFICATION_ALL_IDS = [...GAMIFICATION_COL_IDS]; + +const GradebookColumnTree = ({ + isVisible, + setVisible, + setManyVisible, + categories, + tabs, + assessments, + gamificationEnabled, +}: GradebookColumnTreeProps): JSX.Element => { + const { t } = useTranslation(); + const context: ColumnPickerRenderContext = { + isVisible, + setVisible, + setManyVisible, + }; + + const asnIds = useMemo( + () => assessments.map((a) => buildAssessmentColumnId(a.id)), + [assessments], + ); + + const tabAsnIds = useMemo(() => { + const map = new Map(); + assessments.forEach((a) => { + const existing = map.get(a.tabId) ?? []; + map.set(a.tabId, [...existing, buildAssessmentColumnId(a.id)]); + }); + return map; + }, [assessments]); + + const catTabs = useMemo(() => { + const map = new Map(); + tabs.forEach((tab) => { + const existing = map.get(tab.categoryId) ?? []; + map.set(tab.categoryId, [...existing, tab]); + }); + return map; + }, [tabs]); + + const asnById = useMemo( + () => new Map(assessments.map((a) => [a.id, a])), + [assessments], + ); + + const catAsnIds = useMemo(() => { + const map = new Map(); + tabs.forEach((tab) => { + const tabIds = tabAsnIds.get(tab.id) ?? []; + const existing = map.get(tab.categoryId) ?? []; + map.set(tab.categoryId, [...existing, ...tabIds]); + }); + return map; + }, [tabs, tabAsnIds]); + + return ( +
+ + {STUDENT_INFO_COL_IDS.map((id: StudentInfoColId) => + id === 'name' ? ( + + {t(tableTranslations[id])} + + + } + /> + ) : ( + setVisible(id, e.target.checked)} + /> + ), + )} + + + {gamificationEnabled && ( + + {GAMIFICATION_COL_IDS.map((id: GamificationColId) => ( + setVisible(id, e.target.checked)} + /> + ))} + + )} + + + {categories.map((cat) => { + const catIds = catAsnIds.get(cat.id) ?? []; + const thisCatTabs = catTabs.get(cat.id) ?? []; + return ( + + {thisCatTabs.map((tab) => { + const tabIds = tabAsnIds.get(tab.id) ?? []; + return ( + + {tabIds.map((id) => { + const asnId = parseAssessmentColumnId(id); + const asn = + asnId !== null ? asnById.get(asnId) : undefined; + if (!asn) return null; + return ( + setVisible(id, e.target.checked)} + /> + ); + })} + + ); + })} + + ); + })} + +
+ ); +}; + +export default GradebookColumnTree; diff --git a/client/app/bundles/course/gradebook/components/GradebookTable.tsx b/client/app/bundles/course/gradebook/components/GradebookTable.tsx new file mode 100644 index 00000000000..5c6c7abe4d1 --- /dev/null +++ b/client/app/bundles/course/gradebook/components/GradebookTable.tsx @@ -0,0 +1,809 @@ +import { + forwardRef, + useCallback, + useLayoutEffect, + useMemo, + useRef, + useState, +} from 'react'; +import { defineMessages } from 'react-intl'; +import { + Checkbox, + Paper, + type SxProps, + Table, + TableBody, + TableCell, + TableContainer, + TableHead, + TableRow, + TableSortLabel, + type Theme, + Tooltip, +} from '@mui/material'; +import { flexRender } from '@tanstack/react-table'; + +import Link from 'lib/components/core/Link'; +import type { + ColumnPickerRenderContext, + ColumnTemplate, +} from 'lib/components/table/builder'; +import MuiTablePagination from 'lib/components/table/MuiTableAdapter/MuiTablePagination'; +import MuiTableToolbar from 'lib/components/table/MuiTableAdapter/MuiTableToolbar'; +import useTanStackTableBuilder from 'lib/components/table/TanStackTableBuilder'; +import { + DEFAULT_MINI_TABLE_ROWS_PER_PAGE, + DEFAULT_TABLE_ROWS_PER_PAGE, +} from 'lib/constants/sharedConstants'; +import { getEditSubmissionURL } from 'lib/helpers/url-builders'; +import useTranslation from 'lib/hooks/useTranslation'; +import tableTranslations from 'lib/translations/table'; + +import { GAMIFICATION_COL_IDS } from '../constants'; +import type { + AssessmentData, + CategoryData, + StudentData, + SubmissionData, + TabData, +} from '../types'; + +import { + buildAssessmentColumnId, + parseAssessmentColumnId, +} from './buildAssessmentColumnIds'; +import GradebookColumnTree from './GradebookColumnTree'; + +const COL_WIDTHS = { + name: 160, + email: 250, + externalId: 160, + level: 80, + totalXp: 120, + assessment: 150, +} as const; + +const CHECKBOX_WIDTH = 56; + +const getColWidth = (id: string): number => + COL_WIDTHS[id as keyof typeof COL_WIDTHS] ?? COL_WIDTHS.assessment; + +const isLeftAligned = (id: string): boolean => + id === 'name' || id === 'email' || id === 'externalId'; + +const translations = defineMessages({ + searchStudents: { + id: 'course.gradebook.GradebookIndex.searchStudents', + defaultMessage: 'Search students', + }, + exportButton: { + id: 'course.gradebook.GradebookIndex.exportButton', + defaultMessage: 'Export all rows', + }, + exportRows: { + id: 'course.gradebook.GradebookIndex.exportRows', + defaultMessage: 'Export {count, plural, one {# row} other {# rows}}', + }, + exportAllTooltip: { + id: 'course.gradebook.GradebookIndex.exportAllTooltip', + defaultMessage: 'No rows selected - all rows will be exported.', + }, + selectColumns: { + id: 'course.gradebook.GradebookIndex.selectColumns', + defaultMessage: 'Select Columns', + }, + dialogTitle: { + id: 'course.gradebook.GradebookIndex.dialogTitle', + defaultMessage: 'Select columns', + }, + maxMarks: { + id: 'course.gradebook.GradebookTable.maxMarks', + defaultMessage: 'Max Marks', + }, + noDataColumnsHint: { + id: 'course.gradebook.GradebookTable.noDataColumnsHint', + defaultMessage: + 'No grade columns selected - export will include student info only.', + }, + noDataColumnsHintWithGamification: { + id: 'course.gradebook.GradebookTable.noDataColumnsHintWithGamification', + defaultMessage: + 'No grade or gamification columns selected - export will include student info only.', + }, +}); + +const HeaderLabel = forwardRef< + HTMLSpanElement, + { text: string; onSingleLine: (fits: boolean) => void } +>(({ text, onSingleLine }, forwardedRef): JSX.Element => { + const innerRef = useRef(null); + const [display, setDisplay] = useState(text); + + useLayoutEffect(() => { + const el = innerRef.current; + if (!el) return; + + const lh = parseFloat(getComputedStyle(el).lineHeight) || 20; + const oneLineH = lh + 1; + const twoLineH = lh * 2 + 1; + + el.textContent = text; + + if (el.scrollHeight <= oneLineH) { + onSingleLine(true); + setDisplay(text); + return; + } + + onSingleLine(false); + + if (el.scrollHeight <= twoLineH) { + setDisplay(text); + return; + } + + let lo = 1; + let hi = text.length; + let best = `${text[0]}…`; + while (lo <= hi) { + const mid = Math.floor((lo + hi) / 2); + const candidate = `${text.slice(0, mid)}…`; + el.textContent = candidate; + if (el.scrollHeight <= twoLineH) { + best = candidate; + lo = mid + 1; + } else { + hi = mid - 1; + } + } + // Ensure DOM reflects `best` before React reconciles — the loop's last + // el.textContent assignment may be a too-long candidate, not `best`. + el.textContent = best; + setDisplay(best); + }, [text, onSingleLine]); + + return ( + { + innerRef.current = node; + if (typeof forwardedRef === 'function') forwardedRef(node); + else if (forwardedRef) forwardedRef.current = node; + }} + style={{ display: 'block' }} + > + {display} + + ); +}); +HeaderLabel.displayName = 'HeaderLabel'; + +interface GradebookRow { + studentId: number; + name: string; + email: string; + externalId: string | null; + level: number; + totalXp: number; + grades: Partial>; + submissionIds: Partial>; +} + +interface GradebookTableProps { + categories: CategoryData[]; + tabs: TabData[]; + assessments: AssessmentData[]; + students: StudentData[]; + submissions: SubmissionData[]; + courseTitle: string; + courseId: number; + gamificationEnabled: boolean; +} + +const GradebookTable = ({ + categories, + tabs, + assessments, + students, + submissions, + courseTitle, + courseId, + gamificationEnabled, +}: GradebookTableProps): JSX.Element => { + const { t } = useTranslation(); + + const submissionsByStudent = useMemo(() => { + const map = new Map(); + submissions.forEach((s) => { + const existing = map.get(s.studentId); + if (existing) { + existing.push(s); + } else { + map.set(s.studentId, [s]); + } + }); + return map; + }, [submissions]); + + const rows = useMemo( + () => + students.map((student) => { + const subs = submissionsByStudent.get(student.id) ?? []; + const grades: Partial> = {}; + const submissionIds: Partial> = {}; + assessments.forEach((a) => { + const sub = subs.find((s) => s.assessmentId === a.id); + if (sub != null) { + grades[a.id] = sub.grade; + submissionIds[a.id] = sub.submissionId; + } + }); + return { + studentId: student.id, + name: student.name, + email: student.email, + externalId: student.externalId, + level: student.level, + totalXp: student.totalXp, + grades, + submissionIds, + }; + }), + [students, assessments, submissionsByStudent], + ); + + const hasExternalIds = useMemo( + () => students.some((s) => s.externalId != null && s.externalId !== ''), + [students], + ); + + const columns = useMemo[]>(() => { + const cols: ColumnTemplate[] = [ + { + id: 'name', + title: t(tableTranslations.name), + of: 'name', + cell: (row) => row.name, + csvDownloadable: true, + searchable: true, + sortable: true, + searchProps: { getValue: (row) => row.name }, + }, + { + id: 'email', + title: t(tableTranslations.email), + of: 'email', + cell: (row) => row.email, + csvDownloadable: true, + searchable: true, + sortable: true, + }, + ]; + + // The External ID column is always offered in the picker, but only shown by + // default when the course actually uses external IDs (see column picker). + cols.push({ + id: 'externalId', + title: t(tableTranslations.externalId), + of: 'externalId', + cell: (row) => row.externalId ?? '', + csvDownloadable: true, + searchable: true, + sortable: true, + defaultVisible: hasExternalIds, + }); + + if (gamificationEnabled) { + cols.push({ + id: 'level', + title: t(tableTranslations.level), + of: 'level', + cell: (row) => row.level, + csvDownloadable: true, + sortable: true, + }); + cols.push({ + id: 'totalXp', + title: t(tableTranslations.totalXp), + of: 'totalXp', + cell: (row) => row.totalXp, + csvDownloadable: true, + sortable: true, + }); + } + + assessments.forEach((asn) => { + const colId = buildAssessmentColumnId(asn.id); + cols.push({ + id: colId, + title: asn.title, + // null (ungraded) → undefined so sortUndefined: 'last' fires for both missing and ungraded rows + accessorFn: (row) => row.grades[asn.id] ?? undefined, + sortable: true, + sortProps: { + undefinedPriority: 'last', + descFirst: false, + sort: (a, b) => { + const aGrade = a.grades[asn.id]; + const bGrade = b.grades[asn.id]; + if (aGrade == null || bGrade == null) return 0; + return aGrade - bGrade; + }, + }, + cell: (row) => { + const grade = row.grades[asn.id]; + if (grade === undefined) return '—'; + if (grade === null) return ''; + const submissionId = row.submissionIds[asn.id]; + if (submissionId != null) + return ( + + {grade} + + ); + return grade; + }, + csvDownloadable: true, + defaultVisible: false, + }); + }); + return cols; + }, [assessments, gamificationEnabled, hasExternalIds, t]); + + const assessmentMaxGrades = useMemo( + () => new Map(assessments.map((a) => [a.id, a.maxGrade])), + [assessments], + ); + + const dataColumnIds = useMemo( + () => [ + ...assessments.map((a) => buildAssessmentColumnId(a.id)), + ...GAMIFICATION_COL_IDS, + ], + [assessments], + ); + + const columnPicker = useMemo( + () => ({ + render: (context: ColumnPickerRenderContext) => ( + + ), + locked: ['name'], + triggerLabel: t(translations.selectColumns), + dialogTitle: t(translations.dialogTitle), + getExtraHeaderRows: (colIds): string[][] => { + const hasAssessments = colIds.some( + (id) => parseAssessmentColumnId(id) !== null, + ); + if (!hasAssessments) return []; + return [ + colIds.map((id) => { + if (id === 'name') return t(translations.maxMarks); + const asnId = parseAssessmentColumnId(id); + if (asnId !== null) + return String(assessmentMaxGrades.get(asnId) ?? ''); + return ''; + }), + ]; + }, + storageKey: `gradebook_columns_${courseId}`, + dataColumnIds, + noDataColumnsHint: gamificationEnabled + ? t(translations.noDataColumnsHintWithGamification) + : t(translations.noDataColumnsHint), + }), + [ + assessments, + categories, + gamificationEnabled, + tabs, + t, + assessmentMaxGrades, + courseId, + dataColumnIds, + ], + ); + + const { toolbar, body, pagination, header } = + useTanStackTableBuilder({ + data: rows, + columns, + getRowId: (row) => row.studentId.toString(), + getRowEqualityData: (row) => row, + indexing: { rowSelectable: true }, + pagination: { + initialPageSize: DEFAULT_TABLE_ROWS_PER_PAGE, + rowsPerPage: [ + DEFAULT_MINI_TABLE_ROWS_PER_PAGE, + 25, + 50, + DEFAULT_TABLE_ROWS_PER_PAGE, + ], + showAllRows: true, + }, + search: { searchPlaceholder: t(translations.searchStudents) }, + sort: { + initially: { by: 'name', order: 'asc' }, + enableRemoval: false, + resetOnHide: true, + }, + toolbar: { show: true, keepNative: true }, + csvDownload: { + filename: `${courseTitle}_gradebook`, + showDownloadButton: false, + }, + columnPicker, + }); + + const visibility = toolbar?.getColumnVisibility?.() ?? {}; + const isColVisible = (id: string): boolean => visibility[id] ?? true; + const visibleCols = columns.filter((c) => + isColVisible(c.id ?? (c.of as string)), + ); + + const sortByColId = new Map( + (header?.headers ?? []).map( + (h, i) => [h.id, header?.forEach(h, i).sorting] as const, + ), + ); + + const selectedCount = body.selectedCount ?? 0; + + const directExportLabel = useMemo((): string => { + const isPartialSelection = selectedCount > 0 && selectedCount < rows.length; + if (isPartialSelection) + return t(translations.exportRows, { count: selectedCount }); + return t(translations.exportButton); + }, [selectedCount, rows.length, t]); + + const toolbarWithLabel = toolbar?.columnPicker + ? { + ...toolbar, + columnPicker: { + ...toolbar.columnPicker, + directExportLabel, + directExportTooltip: + selectedCount === 0 ? t(translations.exportAllTooltip) : undefined, + }, + } + : toolbar; + + const totalWidth = useMemo( + () => + CHECKBOX_WIDTH + + visibleCols.reduce((sum, c) => { + const id = c.id ?? (c.of as string); + return sum + getColWidth(id); + }, 0), + [visibleCols], + ); + + const allRowsSelected = body.allFilteredSelected ?? false; + const someRowsSelected = body.someFilteredSelected ?? false; + const toggleAllRows = (): void => body.toggleAllFiltered?.(); + + const hasVisibleAssessments = useMemo( + () => + visibleCols.some( + (c) => parseAssessmentColumnId(c.id ?? (c.of as string)) !== null, + ), + [visibleCols], + ); + + const row1Ref = useRef(null); + const [row2Top, setRow2Top] = useState(0); + useLayoutEffect(() => { + setRow2Top(row1Ref.current?.offsetHeight ?? 0); + }, [visibleCols]); + + const headerFitsRef = useRef>({}); + const [headerFits, setHeaderFits] = useState>({}); + const onSingleLine = useCallback((id: string, fits: boolean): void => { + if (headerFitsRef.current[id] !== fits) { + headerFitsRef.current[id] = fits; + setHeaderFits((prev) => ({ ...prev, [id]: fits })); + } + }, []); + const singleLineCallbacks = useMemo( + () => + new Map( + visibleCols.map((c) => { + const id = c.id ?? (c.of as string); + return [id, (f: boolean): void => onSingleLine(id, f)]; + }), + ), + [visibleCols, onSingleLine], + ); + + return ( +
+ +
+ + {/* A bounded maxHeight is what makes `stickyHeader` actually stick: + `overflowX: 'auto'` already promotes this container to a scroll + container on both axes, so the sticky and the frozen + checkbox/Name columns pin relative to THIS element. Without a height + cap the container grows to fit every row and never scrolls + internally, leaving the header no scroll range. */} + + ({ + tableLayout: 'fixed', + borderCollapse: 'separate', + borderSpacing: 0, + + '& th, & td': { + boxSizing: 'border-box', + border: 0, + + // Draws the cell grid without relying on collapsed borders. + borderBottom: `0.5px solid ${theme.palette.grey[200]}`, + }, + })} + > + + + {visibleCols.map((c) => { + const id = c.id ?? (c.of as string); + return ; + })} + + + + ({ + top: 0, + zIndex: 3, + position: 'sticky', + left: 0, + bgcolor: 'background.default', + width: CHECKBOX_WIDTH, + minWidth: CHECKBOX_WIDTH, + maxWidth: CHECKBOX_WIDTH, + px: 0, + textAlign: 'center', + verticalAlign: 'middle', + // Solid 1px bottom seam under the frozen header columns. The + // table's 0.5px grid border (specificity 0,1,1) outranks a + // plain per-cell sx (0,1,0), so double the selector with `&&` + // (0,2,0) to win — and a full 1px survives sticky scroll + // compositing where 0.5px can drop and let the body show + // through the row1/row2 seam. + '&&': { + borderBottom: `1px solid ${theme.palette.grey[200]}`, + }, + })} + > + + + {visibleCols.map((c) => { + const id = c.id ?? (c.of as string); + const label = typeof c.title === 'string' ? c.title : id; + const isLeft = isLeftAligned(id); + const fits = headerFits[id] ?? false; + const sort = sortByColId.get(id); + const labelNode = ( + + + + + + ); + return ( + ({ + verticalAlign: isLeft || fits ? 'middle' : 'bottom', + ...(id === 'name' && { + position: 'sticky', + left: CHECKBOX_WIDTH, + zIndex: 3, + bgcolor: 'background.default', + // Right edge of the frozen region + matching 1px + // bottom seam. `&&` (0,2,0) is needed to beat the + // table's `& th` border rule (0,1,1). + '&&': { + borderRight: `1px solid ${theme.palette.grey[200]}`, + borderBottom: `1px solid ${theme.palette.grey[200]}`, + }, + }), + })} + > + {sort ? ( + + {labelNode} + + ) : ( + labelNode + )} + + ); + })} + + {hasVisibleAssessments && ( + ({ + '& .MuiTableCell-stickyHeader': { + top: row2Top, + }, + // Solid 1px bottom edge under the whole Max Marks row so the + // frozen columns read as a complete header block and the + // body never shows through on scroll. `& .MuiTableCell-root` + // (0,2,0) outranks the table's `& th` rule (0,1,1). + '& .MuiTableCell-root': { + borderTop: `1px solid ${theme.palette.grey[200]}`, + borderBottom: `1px solid ${theme.palette.grey[200]}`, + }, + })} + > + + {visibleCols.map((c) => { + const id = c.id ?? (c.of as string); + const asnId = parseAssessmentColumnId(id); + let cellContent: string = ''; + if (id === 'name') cellContent = t(translations.maxMarks); + else if (asnId !== null) { + const maxGrade = assessmentMaxGrades.get(asnId); + cellContent = maxGrade != null ? `/${maxGrade}` : ''; + } + return ( + ({ + bgcolor: 'grey.100', + ...(id === 'name' && { + position: 'sticky', + left: CHECKBOX_WIDTH, + zIndex: 3, + // Continue the frozen region's right edge. + '&&': { + borderTop: `1px solid ${theme.palette.grey[200]}`, + borderRight: `1px solid ${theme.palette.grey[200]}`, + }, + }), + })} + > + {cellContent} + + ); + })} + + )} + + + {body.rows.map((row, idx) => { + const rowProps = body.forEachRow(row, idx); + return ( + + ({ + position: 'sticky', + left: 0, + zIndex: 1, + bgcolor: 'background.paper', + width: CHECKBOX_WIDTH, + minWidth: CHECKBOX_WIDTH, + maxWidth: CHECKBOX_WIDTH, + px: 0, + textAlign: 'center', + // Sticky cells composite on their own layer, so this + // cell's `borderBottom` gets covered by the next row's + // opaque sticky background (Blink) — dropping the + // separator. Draw it as the lower row's `borderTop` + // instead; that border is owned by the row's own + // layer and always paints. Row 0's top edge is already + // the header cell's (higher z-index) bottom border. + borderBottom: 'none', + borderTop: + idx === 0 + ? undefined + : `0.5px solid ${theme.palette.grey[200]}`, + })} + > + + + {row + .getVisibleCells() + .filter((cell) => cell.column.id !== 'rowSelector') + .map((cell) => { + // Sticky cover for the frozen `name` column, mirroring + // the checkbox cell above. Declared as a directly-typed + // const so the callback is contextually typed (a ternary + // in the `sx` prop would strip that context). + const nameCellSx: SxProps = (theme) => ({ + position: 'sticky', + left: CHECKBOX_WIDTH, + zIndex: 1, + bgcolor: 'background.paper', + // Same sticky-layer cover as the checkbox column: draw + // the separator as the lower row's `borderTop`, not a + // covered `borderBottom`. + borderBottom: 'none', + borderTop: + idx === 0 + ? undefined + : `0.5px solid ${theme.palette.grey[200]}`, + // Continue the frozen region's right edge down the data + // rows. `&&` (0,2,0) beats the table's `& td` border + // rule (0,1,1). + '&&': { + borderRight: `1px solid ${theme.palette.grey[200]}`, + }, + }); + return ( + + {flexRender( + cell.column.columnDef.cell, + cell.getContext(), + )} + + ); + })} + + ); + })} + +
+ + {pagination && } + +
+
+ ); +}; + +export default GradebookTable; diff --git a/client/app/bundles/course/gradebook/components/buildAssessmentColumnIds.ts b/client/app/bundles/course/gradebook/components/buildAssessmentColumnIds.ts new file mode 100644 index 00000000000..d12a4bd26a7 --- /dev/null +++ b/client/app/bundles/course/gradebook/components/buildAssessmentColumnIds.ts @@ -0,0 +1,7 @@ +export const buildAssessmentColumnId = (asnId: number): string => + `asn-${asnId}`; + +export const parseAssessmentColumnId = (colId: string): number | null => { + const match = colId.match(/^asn-(\d+)$/); + return match ? Number(match[1]) : null; +}; diff --git a/client/app/bundles/course/gradebook/constants.ts b/client/app/bundles/course/gradebook/constants.ts new file mode 100644 index 00000000000..4c61d2723fb --- /dev/null +++ b/client/app/bundles/course/gradebook/constants.ts @@ -0,0 +1,5 @@ +export const STUDENT_INFO_COL_IDS = ['name', 'email', 'externalId'] as const; +export type StudentInfoColId = (typeof STUDENT_INFO_COL_IDS)[number]; + +export const GAMIFICATION_COL_IDS = ['level', 'totalXp'] as const; +export type GamificationColId = (typeof GAMIFICATION_COL_IDS)[number]; diff --git a/client/app/bundles/course/gradebook/handles.ts b/client/app/bundles/course/gradebook/handles.ts new file mode 100644 index 00000000000..0022bfbd02c --- /dev/null +++ b/client/app/bundles/course/gradebook/handles.ts @@ -0,0 +1,21 @@ +import { defineMessages } from 'react-intl'; + +import type { CrumbPath, DataHandle } from 'lib/hooks/router/dynamicNest'; + +const translations = defineMessages({ + header: { + id: 'course.gradebook.GradebookIndex.gradebook', + defaultMessage: 'Gradebook', + }, +}); + +export const gradebookHandle: DataHandle = (match) => { + const courseId = match.params.courseId; + + return { + getData: async (): Promise => ({ + activePath: `/courses/${courseId}/gradebook`, + content: { title: translations.header }, + }), + }; +}; diff --git a/client/app/bundles/course/gradebook/operations.ts b/client/app/bundles/course/gradebook/operations.ts new file mode 100644 index 00000000000..35790580ed0 --- /dev/null +++ b/client/app/bundles/course/gradebook/operations.ts @@ -0,0 +1,12 @@ +import type { Operation } from 'store'; + +import CourseAPI from 'api/course'; + +import { actions } from './store'; + +const fetchGradebook = (): Operation => async (dispatch) => { + const response = await CourseAPI.gradebook.index(); + dispatch(actions.saveGradebook(response.data)); +}; + +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 new file mode 100644 index 00000000000..6d90823b5e4 --- /dev/null +++ b/client/app/bundles/course/gradebook/pages/GradebookIndex/index.tsx @@ -0,0 +1,106 @@ +import { FC, useEffect, useState } from 'react'; +import { defineMessages } from 'react-intl'; +import { useParams } from 'react-router-dom'; +import { PeopleAlt } from '@mui/icons-material'; +import { Typography } from '@mui/material'; + +import Page from 'lib/components/core/layouts/Page'; +import LoadingIndicator from 'lib/components/core/LoadingIndicator'; +import { useAppDispatch, useAppSelector } from 'lib/hooks/store'; +import toast from 'lib/hooks/toast'; +import useTranslation from 'lib/hooks/useTranslation'; + +import { useCourseContext } from '../../../container/CourseLoader'; +import GradebookTable from '../../components/GradebookTable'; +import GradeLinkHint from '../../components/GradeLinkHint'; +import fetchGradebook from '../../operations'; +import { + getAssessments, + getCategories, + getGamificationEnabled, + getStudents, + getSubmissions, + getTabs, +} from '../../selectors'; + +const translations = defineMessages({ + gradebook: { + id: 'course.gradebook.GradebookIndex.gradebook', + defaultMessage: 'Gradebook', + }, + fetchFailure: { + id: 'course.gradebook.GradebookIndex.fetchFailure', + defaultMessage: 'Failed to retrieve Gradebook.', + }, + noStudents: { + id: 'course.gradebook.GradebookIndex.noStudents', + defaultMessage: 'No students enrolled yet', + }, + noStudentsHint: { + id: 'course.gradebook.GradebookIndex.noStudentsHint', + defaultMessage: 'Grades will appear here once students join the course.', + }, +}); + +const GradebookIndex: FC = () => { + const { t } = useTranslation(); + const dispatch = useAppDispatch(); + const { courseTitle } = useCourseContext(); + const { courseId: courseIdParam } = useParams(); + const courseId = parseInt(courseIdParam!, 10); + const [isLoading, setIsLoading] = useState(true); + + const assessments = useAppSelector(getAssessments); + const categories = useAppSelector(getCategories); + const tabs = useAppSelector(getTabs); + const students = useAppSelector(getStudents); + const submissions = useAppSelector(getSubmissions); + const gamificationEnabled = useAppSelector(getGamificationEnabled); + + useEffect(() => { + dispatch(fetchGradebook()) + .finally(() => setIsLoading(false)) + .catch(() => toast.error(t(translations.fetchFailure))); + }, [dispatch]); + + let content: JSX.Element; + if (isLoading) { + content = ; + } else if (students.length === 0) { + content = ( +
+ + + {t(translations.noStudents)} + + + {t(translations.noStudentsHint)} + +
+ ); + } else { + content = ( + <> + + + + ); + } + + return ( + + {content} + + ); +}; + +export default GradebookIndex; diff --git a/client/app/bundles/course/gradebook/selectors.ts b/client/app/bundles/course/gradebook/selectors.ts new file mode 100644 index 00000000000..fbe62e2611a --- /dev/null +++ b/client/app/bundles/course/gradebook/selectors.ts @@ -0,0 +1,24 @@ +import type { AppState } from 'store'; + +type GradebookState = AppState['gradebook']; + +function getLocalState(state: AppState): GradebookState { + return state.gradebook; +} + +export const getCategories = (state: AppState): GradebookState['categories'] => + getLocalState(state).categories; +export const getTabs = (state: AppState): GradebookState['tabs'] => + getLocalState(state).tabs; +export const getAssessments = ( + state: AppState, +): GradebookState['assessments'] => getLocalState(state).assessments; +export const getStudents = (state: AppState): GradebookState['students'] => + getLocalState(state).students; +export const getSubmissions = ( + state: AppState, +): GradebookState['submissions'] => getLocalState(state).submissions; +export const getGamificationEnabled = ( + state: AppState, +): GradebookState['gamificationEnabled'] => + getLocalState(state).gamificationEnabled; diff --git a/client/app/bundles/course/gradebook/store.ts b/client/app/bundles/course/gradebook/store.ts new file mode 100644 index 00000000000..00e3291032b --- /dev/null +++ b/client/app/bundles/course/gradebook/store.ts @@ -0,0 +1,63 @@ +import { produce } from 'immer'; +import type { GradebookData } from 'types/course/gradebook'; + +import type { + AssessmentData, + CategoryData, + StudentData, + SubmissionData, + TabData, +} from './types'; + +const SAVE_GRADEBOOK = 'course/gradebook/SAVE_GRADEBOOK'; + +interface GradebookState { + categories: CategoryData[]; + tabs: TabData[]; + assessments: AssessmentData[]; + students: StudentData[]; + submissions: SubmissionData[]; + gamificationEnabled: boolean; +} + +interface SaveGradebookAction { + type: typeof SAVE_GRADEBOOK; + payload: GradebookData; +} + +const initialState: GradebookState = { + categories: [], + tabs: [], + assessments: [], + students: [], + submissions: [], + gamificationEnabled: false, +}; + +const reducer = produce( + (draft: GradebookState, action: SaveGradebookAction) => { + switch (action.type) { + case SAVE_GRADEBOOK: { + draft.categories = action.payload.categories; + draft.tabs = action.payload.tabs; + draft.assessments = action.payload.assessments; + draft.students = action.payload.students; + draft.submissions = action.payload.submissions; + draft.gamificationEnabled = action.payload.gamificationEnabled; + break; + } + default: + break; + } + }, + initialState, +); + +export const actions = { + saveGradebook: (data: GradebookData): SaveGradebookAction => ({ + type: SAVE_GRADEBOOK, + payload: data, + }), +}; + +export default reducer; diff --git a/client/app/bundles/course/gradebook/types.ts b/client/app/bundles/course/gradebook/types.ts new file mode 100644 index 00000000000..f94aa7bf9c5 --- /dev/null +++ b/client/app/bundles/course/gradebook/types.ts @@ -0,0 +1,8 @@ +export type { + AssessmentData, + CategoryData, + GradebookData, + StudentData, + SubmissionData, + TabData, +} from 'types/course/gradebook'; diff --git a/client/app/bundles/course/statistics/operations.ts b/client/app/bundles/course/statistics/operations.ts index c2060ed41fe..1c071b810ef 100644 --- a/client/app/bundles/course/statistics/operations.ts +++ b/client/app/bundles/course/statistics/operations.ts @@ -1,8 +1,4 @@ -import { AxiosError } from 'axios'; -import { JobCompleted, JobErrored } from 'types/jobs'; - import CourseAPI from 'api/course'; -import pollJob from 'lib/helpers/jobHelpers'; import { AssessmentsStatistics, @@ -14,8 +10,6 @@ import { StudentsStatistics, } from './types'; -const DOWNLOAD_JOB_POLL_INTERVAL_MS = 2000; - export const fetchStatisticsIndex = async (): Promise => { const response = await CourseAPI.statistics.course.fetchStatisticsIndex(); return response.data; @@ -62,21 +56,3 @@ export const fetchCourseGetHelpActivity = async (params?: { await CourseAPI.statistics.course.fetchCourseGetHelpActivity(params); return response.data; }; - -export const downloadScoreSummary = ( - handleSuccess: (successData: JobCompleted) => void, - handleFailure: (error: JobErrored | AxiosError) => void, - assessmentIds: number[], -): void => { - CourseAPI.statistics.course - .downloadScoreSummary(assessmentIds) - .then((response) => { - pollJob( - response.data.jobUrl, - handleSuccess, - handleFailure, - DOWNLOAD_JOB_POLL_INTERVAL_MS, - ); - }) - .catch(handleFailure); -}; diff --git a/client/app/bundles/course/statistics/pages/StatisticsIndex/assessments/AssessmentsScoreSummaryDownload.tsx b/client/app/bundles/course/statistics/pages/StatisticsIndex/assessments/AssessmentsScoreSummaryDownload.tsx deleted file mode 100644 index 355bb97ee45..00000000000 --- a/client/app/bundles/course/statistics/pages/StatisticsIndex/assessments/AssessmentsScoreSummaryDownload.tsx +++ /dev/null @@ -1,115 +0,0 @@ -import { useState } from 'react'; -import { defineMessages } from 'react-intl'; -import { Button, Typography } from '@mui/material'; -import { AxiosError } from 'axios'; -import { JobCompleted, JobErrored } from 'types/jobs'; - -import { downloadScoreSummary } from 'course/statistics/operations'; -import { CourseAssessment } from 'course/statistics/types'; -import Prompt, { PromptText } from 'lib/components/core/dialogs/Prompt'; -import loadingToast, { LoadingToast } from 'lib/hooks/toast/loadingToast'; -import useTranslation from 'lib/hooks/useTranslation'; - -interface AssessmentsScoreSummaryDownloadProps { - assessments: CourseAssessment[]; -} - -const translations = defineMessages({ - selectedNUsers: { - id: 'course.statistics.StatisticsIndex.assessments.selectedNUsers', - defaultMessage: - 'Download Score Summary ({n, plural, =1 {# assessment} other {# assessments}})', - }, - download: { - id: 'course.statistics.StatisticsIndex.assessments.downloadCsv', - defaultMessage: 'Download', - }, - downloadCsvDialogTitle: { - id: 'course.statistics.StatisticsIndex.assessments.downloadCsv', - defaultMessage: 'Download Score Summary for the following Assessments?', - }, - downloadScoreSummarySuccess: { - id: 'course.statistics.StatisticsIndex.assessments.downloadScoreSummarySuccess', - defaultMessage: 'Successfully downloaded score summary', - }, - downloadScoreSummaryFailure: { - id: 'course.statistics.StatisticsIndex.assessments.downloadScoreSummaryFailure', - defaultMessage: 'An error occurred while downloading score summary', - }, - downloadScoreSummaryPending: { - id: 'course.statistics.StatisticsIndex.assessments.downloadScoreSummaryPending', - defaultMessage: - 'Please wait as your request to download is being processed', - }, -}); - -const AssessmentsScoreSummaryDownload = ( - props: AssessmentsScoreSummaryDownloadProps, -): JSX.Element => { - const { assessments } = props; - const { t } = useTranslation(); - - const [openDialog, setOpenDialog] = useState(false); - const [isDownloading, setIsDownloading] = useState(false); - - const handleSuccess = - (loadToast: LoadingToast) => - (successData: JobCompleted): void => { - window.location.href = successData.redirectUrl!; - loadToast.success(t(translations.downloadScoreSummarySuccess)); - setIsDownloading(false); - setOpenDialog(false); - }; - - const handleFailure = - (loadToast: LoadingToast) => - (error: JobErrored | AxiosError): void => { - const message = - error?.message || t(translations.downloadScoreSummaryFailure); - loadToast.error(message); - setIsDownloading(false); - }; - - const handleOnClick = (): void => { - setIsDownloading(true); - const loadToast = loadingToast(t(translations.downloadScoreSummaryPending)); - downloadScoreSummary( - handleSuccess(loadToast), - handleFailure(loadToast), - assessments.map((assessment) => assessment.id), - ); - }; - - return ( - <> - - - setOpenDialog(false)} - open={openDialog} - primaryColor="info" - primaryLabel={t(translations.download)} - title={t(translations.downloadCsvDialogTitle)} - > - - {assessments.map((assessment) => ( -
  • {assessment.title}
    • - ))} -     -     - - ); -}; - -export default AssessmentsScoreSummaryDownload; diff --git a/client/app/bundles/course/statistics/pages/StatisticsIndex/assessments/AssessmentsStatistics.tsx b/client/app/bundles/course/statistics/pages/StatisticsIndex/assessments/AssessmentsStatistics.tsx index f8a8a4ed78a..930eef2ed5d 100644 --- a/client/app/bundles/course/statistics/pages/StatisticsIndex/assessments/AssessmentsStatistics.tsx +++ b/client/app/bundles/course/statistics/pages/StatisticsIndex/assessments/AssessmentsStatistics.tsx @@ -12,6 +12,7 @@ const AssessmentsStatistics: FC = () => { {(data) => ( )} diff --git a/client/app/bundles/course/statistics/pages/StatisticsIndex/assessments/AssessmentsStatisticsTable.tsx b/client/app/bundles/course/statistics/pages/StatisticsIndex/assessments/AssessmentsStatisticsTable.tsx index 0b686edd539..450e1208850 100644 --- a/client/app/bundles/course/statistics/pages/StatisticsIndex/assessments/AssessmentsStatisticsTable.tsx +++ b/client/app/bundles/course/statistics/pages/StatisticsIndex/assessments/AssessmentsStatisticsTable.tsx @@ -1,6 +1,6 @@ import { FC } from 'react'; import { defineMessages } from 'react-intl'; -import { Typography } from '@mui/material'; +import { Box, Typography } from '@mui/material'; import { CourseAssessment } from 'course/statistics/types'; import Link from 'lib/components/core/Link'; @@ -14,13 +14,12 @@ import { getAssessmentStatisticsURL, getAssessmentWithCategoryURL, getAssessmentWithTabURL, + getCourseGradebookURL, } from 'lib/helpers/url-builders'; import { getCourseId } from 'lib/helpers/url-helpers'; import useTranslation from 'lib/hooks/useTranslation'; import { formatMiniDateTime, formatSecondsDuration } from 'lib/moment'; -import AssessmentsScoreSummaryDownload from './AssessmentsScoreSummaryDownload'; - const translations = defineMessages({ title: { id: 'course.statistics.StatisticsIndex.assessments.title', @@ -78,15 +77,26 @@ const translations = defineMessages({ id: 'course.statistics.StatisticsIndex.assessments.searchBar', defaultMessage: 'Search by Assessment Title, Tab, or Category', }, + subtitle: { + id: 'course.statistics.StatisticsIndex.assessments.subtitle', + defaultMessage: + 'To view and export individual student grades, open Gradebook.', + }, + subtitleDisabled: { + id: 'course.statistics.StatisticsIndex.assessments.subtitleDisabled', + defaultMessage: + 'To view and export individual student grades, enable Gradebook.', + }, }); interface Props { numStudents: number; assessments: CourseAssessment[]; + gradebookEnabled: boolean; } const AssessmentsStatisticsTable: FC = (props) => { - const { numStudents, assessments } = props; + const { numStudents, assessments, gradebookEnabled } = props; const courseId = getCourseId(); const { t } = useTranslation(); @@ -243,9 +253,26 @@ const AssessmentsStatisticsTable: FC = (props) => { return ( <> - - {t(translations.tableTitle, { numStudents })} - + + + {t(translations.tableTitle, { numStudents })} + + + {gradebookEnabled + ? t(translations.subtitle, { + url: (chunks) => ( + {chunks} + ), + }) + : t(translations.subtitleDisabled, { + url: (chunks) => ( + + {chunks} + + ), + })} + + = (props) => { } getRowEqualityData={(assessment): CourseAssessment => assessment} getRowId={(assessment): string => assessment.id.toString()} - indexing={{ indices: true, rowSelectable: true }} + indexing={{ indices: true }} pagination={{ rowsPerPage: [DEFAULT_TABLE_ROWS_PER_PAGE], showAllRows: true, @@ -285,15 +312,7 @@ const AssessmentsStatisticsTable: FC = (props) => { }, }, }} - toolbar={{ - show: true, - activeToolbar: (selectedAssessments): JSX.Element => ( - - ), - keepNative: true, - }} + toolbar={{ show: true }} /> ); diff --git a/client/app/bundles/course/statistics/pages/StatisticsIndex/assessments/__tests__/AssessmentsStatisticsTable.test.tsx b/client/app/bundles/course/statistics/pages/StatisticsIndex/assessments/__tests__/AssessmentsStatisticsTable.test.tsx new file mode 100644 index 00000000000..5eb96b0bd7b --- /dev/null +++ b/client/app/bundles/course/statistics/pages/StatisticsIndex/assessments/__tests__/AssessmentsStatisticsTable.test.tsx @@ -0,0 +1,58 @@ +import { render, screen } from 'test-utils'; + +import { CourseAssessment } from 'course/statistics/types'; + +import AssessmentsStatisticsTable from '../AssessmentsStatisticsTable'; + +const assessments: CourseAssessment[] = [ + { + id: 1, + title: 'Quiz 1', + startAt: new Date('2026-01-01T00:00:00Z'), + tab: { id: 1, title: 'Tab 1' }, + category: { id: 1, title: 'Category 1' }, + maximumGrade: 10, + numSubmitted: 2, + numAttempted: 3, + numLate: 1, + }, +]; + +const renderTable = (gradebookEnabled = true): void => { + render( + , + { at: ['/courses/1/statistics/assessments'] }, + ); +}; + +describe('', () => { + // Regression guard: an orphaned rowSelectable (selection with no activeToolbar) + // makes the toolbar render an empty 6.5rem bar on select, shifting every row down. + // Re-enabling row selection here brings that layout bug back. + it('keeps row selection off so selecting cannot shift the table down', async () => { + renderTable(); + await screen.findByText('Quiz 1'); + expect(screen.queryByRole('checkbox')).not.toBeInTheDocument(); + }); + + it('renders the native CSV download button', async () => { + renderTable(); + expect(await screen.findByTestId('DownloadIcon')).toBeInTheDocument(); + }); + + it('points to the Gradebook for individual student grades when enabled', async () => { + renderTable(true); + const link = await screen.findByRole('link', { name: /gradebook/i }); + expect(link).toHaveAttribute('href', '/courses/1/gradebook'); + }); + + it('points to course settings to enable Gradebook when it is disabled', async () => { + renderTable(false); + const link = await screen.findByRole('link', { name: /gradebook/i }); + expect(link).toHaveAttribute('href', '/courses/1/admin/components'); + }); +}); diff --git a/client/app/bundles/course/statistics/pages/StatisticsIndex/index.tsx b/client/app/bundles/course/statistics/pages/StatisticsIndex/index.tsx index d1dd39720a3..c1e230f8964 100644 --- a/client/app/bundles/course/statistics/pages/StatisticsIndex/index.tsx +++ b/client/app/bundles/course/statistics/pages/StatisticsIndex/index.tsx @@ -115,7 +115,10 @@ const StatisticsIndex: FC = () => { return ( - + { diff --git a/client/app/bundles/course/statistics/types.ts b/client/app/bundles/course/statistics/types.ts index 8dea0dfc4bd..076743f504c 100644 --- a/client/app/bundles/course/statistics/types.ts +++ b/client/app/bundles/course/statistics/types.ts @@ -137,6 +137,7 @@ export interface CourseAssessment { export interface AssessmentsStatistics { numStudents: number; assessments: CourseAssessment[]; + gradebookEnabled: boolean; } export interface CourseGetHelpActivity { diff --git a/client/app/bundles/course/translations.ts b/client/app/bundles/course/translations.ts index b92b165a744..c52ce359071 100644 --- a/client/app/bundles/course/translations.ts +++ b/client/app/bundles/course/translations.ts @@ -75,6 +75,10 @@ const translations = defineMessages({ id: 'course.componentTitles.course_forums_component', defaultMessage: 'Forums', }, + course_gradebook_component: { + id: 'course.componentTitles.course_gradebook_component', + defaultMessage: 'Gradebook', + }, course_groups_component: { id: 'course.componentTitles.course_groups_component', defaultMessage: 'Groups', diff --git a/client/app/bundles/course/user-invitations/components/tables/InvitationResultExistingTable.tsx b/client/app/bundles/course/user-invitations/components/tables/InvitationResultExistingTable.tsx index 51915776959..9f517937cf0 100644 --- a/client/app/bundles/course/user-invitations/components/tables/InvitationResultExistingTable.tsx +++ b/client/app/bundles/course/user-invitations/components/tables/InvitationResultExistingTable.tsx @@ -7,7 +7,6 @@ import { ColumnTemplate } from 'lib/components/table'; import Table from 'lib/components/table/Table'; import { DEFAULT_MINI_TABLE_ROWS_PER_PAGE, - DEFAULT_TABLE_ROWS_PER_PAGE, TIMELINE_ALGORITHMS, } from 'lib/constants/sharedConstants'; import useTranslation from 'lib/hooks/useTranslation'; diff --git a/client/app/bundles/course/user-invitations/components/tables/InvitationResultPrimaryTable.tsx b/client/app/bundles/course/user-invitations/components/tables/InvitationResultPrimaryTable.tsx index 3da6e4d2da0..5b2ad57a8ae 100644 --- a/client/app/bundles/course/user-invitations/components/tables/InvitationResultPrimaryTable.tsx +++ b/client/app/bundles/course/user-invitations/components/tables/InvitationResultPrimaryTable.tsx @@ -5,7 +5,6 @@ import { ColumnTemplate } from 'lib/components/table'; import Table from 'lib/components/table/Table'; import { DEFAULT_MINI_TABLE_ROWS_PER_PAGE, - DEFAULT_TABLE_ROWS_PER_PAGE, TIMELINE_ALGORITHMS, } from 'lib/constants/sharedConstants'; import useTranslation from 'lib/hooks/useTranslation'; diff --git a/client/app/bundles/users/store.ts b/client/app/bundles/users/store.ts index 230d7149d51..44ff5de5596 100644 --- a/client/app/bundles/users/store.ts +++ b/client/app/bundles/users/store.ts @@ -12,6 +12,8 @@ import { SaveCourseListAction, SaveInstanceListAction, SaveUserAction, + SET_CURRENT_USER_ID, + SetCurrentUserIdAction, } from './types'; const initialState: GlobalUserState = { @@ -33,6 +35,13 @@ const reducer = produce((draft: GlobalUserState, action: GlobalActionType) => { draft.user = userEntity; break; } + // Sets only the authenticated user's id, leaving name/imageUrl untouched. + // The course layout fetch knows the current user id but not the full profile; + // this is enough to namespace per-user client state (e.g. table column prefs). + case SET_CURRENT_USER_ID: { + draft.user.id = action.userId; + break; + } case SAVE_COURSE_LIST: { if (action.courses) { const coursesList = action.courses; @@ -67,6 +76,10 @@ export const actions = { return { type: SAVE_USER, user }; }, + setCurrentUserId: (userId: number): SetCurrentUserIdAction => { + return { type: SET_CURRENT_USER_ID, userId }; + }, + saveCourses: ( courses: UserCourseListData[], courseType: 'current' | 'completed', diff --git a/client/app/bundles/users/types.ts b/client/app/bundles/users/types.ts index f4cf9580e59..6a073977c78 100644 --- a/client/app/bundles/users/types.ts +++ b/client/app/bundles/users/types.ts @@ -12,6 +12,7 @@ import { // Action Names export const SAVE_USER = 'system/SAVE_USER'; +export const SET_CURRENT_USER_ID = 'system/SET_CURRENT_USER_ID'; export const SAVE_COURSE_LIST = 'system/SAVE_COURSE_LIST'; export const SAVE_INSTANCE_LIST = 'system/SAVE_INSTANCE_LIST'; @@ -21,6 +22,11 @@ export interface SaveUserAction { user: UserBasicListData; } +export interface SetCurrentUserIdAction { + type: typeof SET_CURRENT_USER_ID; + userId: number; +} + export interface SaveCourseListAction { type: typeof SAVE_COURSE_LIST; courses: UserCourseListData[]; @@ -34,6 +40,7 @@ export interface SaveInstanceListAction { export type GlobalActionType = | SaveUserAction + | SetCurrentUserIdAction | SaveCourseListAction | SaveInstanceListAction; diff --git a/client/app/lib/components/core/buttons/DownloadButton.tsx b/client/app/lib/components/core/buttons/DownloadButton.tsx index f241597144d..a9788884b08 100644 --- a/client/app/lib/components/core/buttons/DownloadButton.tsx +++ b/client/app/lib/components/core/buttons/DownloadButton.tsx @@ -24,7 +24,7 @@ const DownloadButton = (props: DownloadButtonProps): JSX.Element => ( > - {props.children} + {props.children} diff --git a/client/app/lib/components/core/dialogs/Prompt.tsx b/client/app/lib/components/core/dialogs/Prompt.tsx index 5330d10c7c4..32f4c7f251a 100644 --- a/client/app/lib/components/core/dialogs/Prompt.tsx +++ b/client/app/lib/components/core/dialogs/Prompt.tsx @@ -15,6 +15,7 @@ interface BasePromptProps { open?: boolean; title?: string | ReactNode; children?: string | ReactNode; + footer?: ReactNode; onClose?: () => void; onClosed?: () => void; disabled?: boolean; @@ -84,6 +85,8 @@ const Prompt = (props: PromptProps): JSX.Element => { )} + {props.footer} + {!props.cancel ? ( + )} + + {toolbar.onDirectExport && ( + + + + + + )} + + + + {/* Default-weights banner: weights are unconfigured, so the table is + showing the equal-split fallback. Keeps the Configure CTA for managers. */} + {showingDefaults && ( + + {canManageWeights + ? t(translations.defaultWeights) + : t(translations.defaultWeightsNoAccess)} + + )} + {/* Degenerate case: weights are 0 and there is nothing to default + (no tab has any assessment). */} + {allWeightsZero && !showingDefaults && ( + + {canManageWeights + ? t(translations.noWeightsConfigured) + : t(translations.noWeightsNoAccess)} + + )} + {/* A bounded maxHeight is what makes `stickyHeader` actually stick: + `overflowX: 'auto'` already promotes this container to a scroll + container on BOTH axes (CSS computes overflow-y to `auto` when + overflow-x isn't `visible`), so the sticky sticks relative + to THIS element, not the page. Without a height cap the container + grows to fit all rows and never scrolls internally, leaving the + header no scroll range. Capping the height makes the body scroll + within the container while the header (and the frozen Name/checkbox + columns, same scroll context) stay pinned. The cap subtracts the + chrome above the body — breadcrumb, page header, view tabs, + toolbar — plus the pagination below, so the table fills the + remaining viewport; shorter classes shrink to fit (no whitespace). */} + +
    { + // One definition for every grid line so horizontal and vertical + // separators share the same width and colour. + const gridLine = `1px solid ${theme.palette.divider}`; + return { + tableLayout: 'auto', + borderCollapse: 'separate', + borderSpacing: 0, + // Outer top + left edges. Interior lines come from each cell's + // own bottom + right, so every separator stays a single 1px + // (no doubling) and the whole table reads as one uniform grid. + borderTop: gridLine, + borderLeft: gridLine, + '& th, & td': { + boxSizing: 'border-box', + border: 0, + borderBottom: gridLine, + borderRight: gridLine, + py: 0.25, + px: 1, + lineHeight: 1.2, + height: 32, + }, + // MUI's default `.MuiTableRow-root:last-child th { border: 0 }` + // (specificity 0,2,1, the `:last-child` pseudo-class) outranks + // the `& th` rule above (0,1,1) and silently zeroes ALL borders + // on the weight row — it is the last in . Re-assert + // the grid lines with a higher-specificity selector so they + // survive through the "% of grade" row. + '& thead tr:last-of-type th': { + borderTop: gridLine, + borderBottom: gridLine, + borderRight: gridLine, + }, + }; + }} + > + + {/* Row 1: Checkbox + Student + Categories + Total */} + + + + + + {t(tableTranslations.name)} + + {showEmail && ( + + {t(tableTranslations.email)} + + )} + {showExternalId && ( + + {t(tableTranslations.externalId)} + + )} + {visibleCategories.map((cat) => ( + + {cat.title} + + ))} + + + {t(translations.total)} + {/* The policy moved out of the header label into this ⓘ — + the descriptive sentence stays available on demand after + the one-time banner is dismissed. */} + + + + + + + + + + {/* Row 2: Tab titles */} + + {resolvedTabs.map((tab) => ( + + {tab.title} + + ))} + + + {/* Row 3: Weight subheaders */} + + {resolvedTabs.map((tab) => ( + + {tabSubheaderLabel(tab)} + + ))} + + {totalWeight === 100 ? ( + totalWeightHeaderLabel + ) : ( + + + {displayMode === 'percent' + ? t(translations.percentTotalWarning, { + weight: totalWeight, + }) + : t(translations.outOfWeight, { + weight: totalWeight, + })} + + + )} + + + + + + {body.rows.map((row, idx) => { + const rowProps = body.forEachRow(row, idx); + const studentId = row.original.studentId; + const isExpanded = expandedIds.has(studentId); + return ( + + + {/* Body sticky-left cells sit at zIndex 1 — strictly + below the header's sticky cells (MUI gives every + stickyHeader cell zIndex 2). On a z-index tie the cell + later in the DOM (the body) wins, so matching z2 here + lets scrolled rows bleed up over the header in any + column the frozen Name cell (z4) doesn't cover — i.e. + the identity columns once they're toggled on. */} + + + + + toggleExpanded(studentId)} + size="small" + sx={{ + mr: 0.5, + p: 0.25, + }} + > + {isExpanded ? ( + + ) : ( + + )} + + {row.original.name} + + {showEmail && ( + {row.original.email} + )} + {showExternalId && ( + {row.original.externalId ?? ''} + )} + {row.original.subtotals.map((subtotal, i) => { + const weight = resolvedTabs[i].gradebookWeight ?? 0; + return ( + + {fmtDisplay( + tabDisplayValue(subtotal, weight), + columnPrecisions.tabs[i], + )} + + ); + })} + + {fmtDisplay( + totalDisplayValue(row.original.total), + columnPrecisions.total, + )} + + + {isExpanded && + (breakdownsByStudent.get(studentId) ?? []).flatMap( + (tb, tabIdx) => + tb.assessments.map((a) => { + const isExcluded = a.excluded; + // Weightage is always "% of grade" — it never + // follows the points/percent lens. + const weightText = t( + translations.percentOfGrade, + { + weight: + Math.round(a.effectiveWeight * 100) / 100, + }, + ); + const gradeText = + a.grade === null + ? `—/${a.maxGrade}` + : `${a.grade}/${a.maxGrade}`; + return ( + + {/* Empty checkbox cell so the breakdown row + carries the same checkbox | name divider (the + universal cell borderRight) as the rows above. */} + + {/* Title over a muted "raw mark · weightage" + subtitle, stacked and confined to the (sticky) + Name column. The breakdown row freezes the same + checkbox | Name region as the student rows above — + the identity columns get their own empty cells + after this — so the layout reads identically + whether identity columns are shown or not. A left + indent sits the title under the student name (past + the expand chevron), signalling these are that + student's assessments. */} + + {/* nowrap keeps the title on one line: its + max-content width then drives the table's auto + layout, expanding the (frozen) Name column to fit + the longest title. With the metadata line also + nowrap, every breakdown row is exactly 2 lines — + no fixed widths, no JS measurement. */} + + {a.title} + + {/* Muted metadata on its own line below the + title: raw mark · effective weightage, kept on + one line (nowrap). Weightage is always "% of + grade" — never routed through the points/percent + lens. */} + + {`${gradeText} · ${isExcluded ? t(translations.excluded) : weightText}`} + + + {/* One empty cell per visible identity column so + the grid lines stay aligned with the rows above. + These scroll with the table (only checkbox + Name + are frozen), matching the student rows. */} + {showEmail && } + {showExternalId && } + {resolvedTabs.map((tab, i) => { + const tabCellValue = isExcluded + ? '—' + : fmtDisplay( + breakdownDisplayValue(a), + columnPrecisions.tabs[i], + ); + return ( + + {i === tabIdx ? tabCellValue : ''} + + ); + })} + + + ); + }), + )} + + ); + })} + +
    + + {pagination && } + + + + {canManageWeights && ( + setConfigureOpen(false)} + open={configureOpen} + tabs={tabs} + /> + )} + + {toolbar.columnPicker && toolbar.commitColumnVisibility && ( + setPickerOpen(false)} + open={pickerOpen} + /> + )} + + ); +}; + +export default GradebookWeightedTable; diff --git a/client/app/bundles/course/gradebook/components/ProjectedTotalHint.tsx b/client/app/bundles/course/gradebook/components/ProjectedTotalHint.tsx new file mode 100644 index 00000000000..3ff0f0a4d2f --- /dev/null +++ b/client/app/bundles/course/gradebook/components/ProjectedTotalHint.tsx @@ -0,0 +1,51 @@ +import { FC } from 'react'; +import { defineMessages } from 'react-intl'; +import { Alert, Typography } from '@mui/material'; + +import { getUserEntity } from 'bundles/users/selectors'; +import { useAppSelector } from 'lib/hooks/store'; +import useDismissibleOnce from 'lib/hooks/useDismissibleOnce'; +import useTranslation from 'lib/hooks/useTranslation'; + +export const PROJECTED_TOTAL_POLICY_HINT_KEY = + 'gradebook_projected_total_policy_hint'; + +// Single source for the projected-total policy sentence: the one-time banner +// below and the ⓘ tooltip on the Total header (in GradebookWeightedTable) both +// render this exact message, so the explanation never drifts between the two. +export const projectedTotalPolicyTranslations = defineMessages({ + policy: { + id: 'course.gradebook.ProjectedTotalHint.policy', + defaultMessage: 'Totals count ungraded assessments as 0.', + }, +}); + +/** + * One-time, dismissable banner shown the first time a manager opens the weighted + * (Weighted total) gradebook view. It teaches the projected-total policy — ungraded + * assessments are counted as 0 — so a low total isn't mistaken for a bug. After + * dismissal the policy stays available via the ⓘ tooltip on the Total header. + * Dismissal is remembered per user via localStorage (see useDismissibleOnce). + */ +const ProjectedTotalHint: FC = () => { + const { t } = useTranslation(); + const userId = useAppSelector(getUserEntity).id; + const { dismissed, dismiss } = useDismissibleOnce( + PROJECTED_TOTAL_POLICY_HINT_KEY, + userId, + ); + + if (dismissed) return null; + + return ( +
    + + + {t(projectedTotalPolicyTranslations.policy)} + + +
    + ); +}; + +export default ProjectedTotalHint; diff --git a/client/app/bundles/course/gradebook/components/WeightedGradebookColumnTree.tsx b/client/app/bundles/course/gradebook/components/WeightedGradebookColumnTree.tsx new file mode 100644 index 00000000000..fb09b295390 --- /dev/null +++ b/client/app/bundles/course/gradebook/components/WeightedGradebookColumnTree.tsx @@ -0,0 +1,79 @@ +import { defineMessages } from 'react-intl'; +import { Chip } from '@mui/material'; + +import IndentedCheckbox from 'lib/components/core/IndentedCheckbox'; +import { + ColumnPickerRenderContext, + ColumnPickerTreeGroup, +} from 'lib/components/table'; +import useTranslation from 'lib/hooks/useTranslation'; +import tableTranslations from 'lib/translations/table'; + +import { STUDENT_INFO_COL_IDS, type StudentInfoColId } from '../constants'; + +const translations = defineMessages({ + studentInfo: { + id: 'course.gradebook.GradebookColumnTree.studentInfo', + defaultMessage: 'Student info', + }, + alwaysIncluded: { + id: 'course.gradebook.GradebookColumnTree.alwaysIncluded', + defaultMessage: 'Always included', + }, +}); + +const WeightedGradebookColumnTree = ({ + isVisible, + setVisible, + setManyVisible, +}: ColumnPickerRenderContext): JSX.Element => { + const { t } = useTranslation(); + const context: ColumnPickerRenderContext = { + isVisible, + setVisible, + setManyVisible, + }; + + return ( +
    + + {STUDENT_INFO_COL_IDS.map((id: StudentInfoColId) => + id === 'name' ? ( + + {t(tableTranslations[id])} + + + } + /> + ) : ( + setVisible(id, e.target.checked)} + /> + ), + )} + +
    + ); +}; + +export default WeightedGradebookColumnTree; diff --git a/client/app/bundles/course/gradebook/components/WeightedViewHint.tsx b/client/app/bundles/course/gradebook/components/WeightedViewHint.tsx new file mode 100644 index 00000000000..aaa54991392 --- /dev/null +++ b/client/app/bundles/course/gradebook/components/WeightedViewHint.tsx @@ -0,0 +1,65 @@ +import { FC } from 'react'; +import { defineMessages } from 'react-intl'; +import { Alert, Typography } from '@mui/material'; + +import { getUserEntity } from 'bundles/users/selectors'; +import Link from 'lib/components/core/Link'; +import { useAppSelector } from 'lib/hooks/store'; +import useDismissibleOnce from 'lib/hooks/useDismissibleOnce'; +import useTranslation from 'lib/hooks/useTranslation'; + +export const WEIGHTED_VIEW_HINT_KEY = 'gradebook_weighted_view_hint'; + +const translations = defineMessages({ + hint: { + id: 'course.gradebook.WeightedViewHint.hint', + defaultMessage: + 'Want a weighted total grade? You can set how much each tab counts toward each student’s overall grade and view the weighted total here. Turn it on in {link}.', + }, + settingsLink: { + id: 'course.gradebook.WeightedViewHint.settingsLink', + defaultMessage: 'Gradebook settings', + }, +}); + +interface Props { + courseId: number; +} + +/** + * One-time, dismissable nudge shown to managers in the (always-visible) gradebook view + * when the weighted view is turned off. It advertises the capability and links to the + * setting that enables it, since that setting is otherwise buried in course admin. + * Dismissal is remembered per user via localStorage (see useDismissibleOnce). + */ +const WeightedViewHint: FC = ({ courseId }) => { + const { t } = useTranslation(); + const userId = useAppSelector(getUserEntity).id; + const { dismissed, dismiss } = useDismissibleOnce( + WEIGHTED_VIEW_HINT_KEY, + userId, + ); + + if (dismissed) return null; + + return ( +
    + + + {t(translations.hint, { + link: ( + + {t(translations.settingsLink)} + + ), + })} + + +
    + ); +}; + +export default WeightedViewHint; diff --git a/client/app/bundles/course/gradebook/computeWeighted.ts b/client/app/bundles/course/gradebook/computeWeighted.ts new file mode 100644 index 00000000000..d4bb0424f70 --- /dev/null +++ b/client/app/bundles/course/gradebook/computeWeighted.ts @@ -0,0 +1,312 @@ +// client/app/bundles/course/gradebook/computeWeighted.ts +import { + AssessmentData, + StudentData, + SubmissionData, + TabData, +} from 'types/course/gradebook'; + +type GradeEntry = Pick; + +export interface WeightedRow { + studentId: number; + name: string; + email: string; + externalId: string | null; + subtotals: (number | null)[]; + total: number | null; +} + +export interface AssessmentContribution { + assessmentId: number; + title: string; + grade: number | null; + maxGrade: number; + points: number; // contribution to this tab's weighted-points cell + // Share of the overall grade this assessment carries, in percentage points. + // Equal mode: the tab's weight split evenly across its assessments. + // Custom mode: the assessment's own configured weight. + effectiveWeight: number; + excluded: boolean; +} + +export interface TabBreakdown { + tabId: number; + assessments: AssessmentContribution[]; +} + +type GradeLookup = Map; + +const gradeKey = (studentId: number, assessmentId: number): string => + `${studentId}:${assessmentId}`; + +// Index submissions by (student, assessment) once: O(submissions). +const buildGradeLookup = (submissions: GradeEntry[]): GradeLookup => { + const lookup: GradeLookup = new Map(); + submissions.forEach((s) => { + if (s.grade != null) + lookup.set(gradeKey(s.studentId, s.assessmentId), s.grade); + }); + return lookup; +}; + +// Group assessments by tab once: O(assessments). +const buildAssessmentsByTab = ( + assessments: AssessmentData[], +): Map => { + const byTab = new Map(); + assessments.forEach((a) => { + const list = byTab.get(a.tabId); + if (list) list.push(a); + else byTab.set(a.tabId, [a]); + }); + return byTab; +}; + +// Equal-weight formula: average of (grade/maxGrade) ratios over INCLUDED assessments. +// Excluded assessments are dropped from both numerator and count; ungraded included +// contribute 0. Returns null when no assessment is included. +const equalSubtotal = ( + studentId: number, + tabAssessments: AssessmentData[], + gradeLookup: GradeLookup, +): number | null => { + const included = tabAssessments.filter((a) => !a.gradebookExcluded); + if (included.length === 0) return null; + const ratios = included.map((a) => { + const grade = gradeLookup.get(gradeKey(studentId, a.id)); + return grade != null ? grade / a.maxGrade : 0; + }); + return ratios.reduce((acc, r) => acc + r, 0) / ratios.length; +}; + +// Custom-weight formula: Σ(grade_i/maxGrade_i × assessmentWeight_i) / tabWeight over +// INCLUDED assessments. Returns null if tabWeight=0 or no assessment is included; +// ungraded included assessments contribute 0. +const customSubtotal = ( + studentId: number, + tab: TabData, + tabAssessments: AssessmentData[], + gradeLookup: GradeLookup, +): number | null => { + const tabWeight = tab.gradebookWeight ?? 0; + if (tabWeight === 0) return null; + let numerator = 0; + let hasContributing = false; + tabAssessments.forEach((a) => { + if (a.gradebookExcluded) return; + const grade = gradeLookup.get(gradeKey(studentId, a.id)); + const assessmentWeight = a.gradebookWeight ?? 0; + if (grade != null) numerator += (grade / a.maxGrade) * assessmentWeight; + hasContributing = true; + }); + return hasContributing ? numerator / tabWeight : null; +}; + +// Single source of truth for the subtotal math, operating on prebuilt indexes. +const subtotalFromLookup = ( + studentId: number, + tab: TabData, + tabAssessments: AssessmentData[] | undefined, + gradeLookup: GradeLookup, +): number | null => { + if (!tabAssessments || tabAssessments.length === 0) return null; + if (tab.weightMode === 'custom') { + return customSubtotal(studentId, tab, tabAssessments, gradeLookup); + } + return equalSubtotal(studentId, tabAssessments, gradeLookup); +}; + +// Weighted, additive total from already-computed subtotals. +const totalFromSubtotals = ( + subtotals: (number | null)[], + tabs: TabData[], +): number | null => { + let contributingCount = 0; + let total = 0; + subtotals.forEach((sub, i) => { + if (sub == null) return; + contributingCount += 1; + total += (tabs[i].gradebookWeight ?? 0) * sub; + }); + return contributingCount > 0 ? total : null; +}; + +interface SubtotalArgs { + studentId: number; + tab: TabData; + assessments: AssessmentData[]; + submissions: GradeEntry[]; +} + +export const computeTabSubtotal = ({ + studentId, + tab, + assessments, + submissions, +}: SubtotalArgs): number | null => + subtotalFromLookup( + studentId, + tab, + assessments.filter((a) => a.tabId === tab.id), + buildGradeLookup(submissions), + ); + +interface TotalArgs { + studentId: number; + tabs: TabData[]; + assessments: AssessmentData[]; + submissions: GradeEntry[]; +} + +export const computeStudentTotal = ({ + studentId, + tabs, + assessments, + submissions, +}: TotalArgs): number | null => { + const gradeLookup = buildGradeLookup(submissions); + const assessmentsByTab = buildAssessmentsByTab(assessments); + const subtotals = tabs.map((tab) => + subtotalFromLookup( + studentId, + tab, + assessmentsByTab.get(tab.id), + gradeLookup, + ), + ); + return totalFromSubtotals(subtotals, tabs); +}; + +export const computeStudentBreakdown = ({ + studentId, + tabs, + assessments, + submissions, +}: TotalArgs): TabBreakdown[] => { + const gradeLookup = buildGradeLookup(submissions); + const assessmentsByTab = buildAssessmentsByTab(assessments); + return tabs.map((tab) => { + const list = assessmentsByTab.get(tab.id) ?? []; + const weight = tab.gradebookWeight ?? 0; + const includedCount = list.filter((a) => !a.gradebookExcluded).length; + + const contributions = list.map((a) => { + const excluded = !!a.gradebookExcluded; + const grade = gradeLookup.get(gradeKey(studentId, a.id)) ?? null; + const ratio = grade != null ? grade / a.maxGrade : 0; + let points: number; + let effectiveWeight: number; + if (excluded) { + points = 0; + effectiveWeight = 0; + } else if (tab.weightMode === 'custom') { + points = ratio * (a.gradebookWeight ?? 0); + effectiveWeight = a.gradebookWeight ?? 0; + } else { + points = includedCount > 0 ? (ratio / includedCount) * weight : 0; + effectiveWeight = includedCount > 0 ? weight / includedCount : 0; + } + return { + assessmentId: a.id, + title: a.title, + grade, + maxGrade: a.maxGrade, + points, + effectiveWeight, + excluded, + }; + }); + return { tabId: tab.id, assessments: contributions }; + }); +}; + +interface WeightedRowsArgs { + students: StudentData[]; + tabs: TabData[]; + assessments: AssessmentData[]; + submissions: GradeEntry[]; +} + +// Batch entry point used by the table: builds the indexes ONCE and reuses them +// across every student, computing each subtotal a single time. +export const computeWeightedRows = ({ + students, + tabs, + assessments, + submissions, +}: WeightedRowsArgs): WeightedRow[] => { + const gradeLookup = buildGradeLookup(submissions); + const assessmentsByTab = buildAssessmentsByTab(assessments); + return students.map((student) => { + const subtotals = tabs.map((tab) => + subtotalFromLookup( + student.id, + tab, + assessmentsByTab.get(tab.id), + gradeLookup, + ), + ); + return { + studentId: student.id, + name: student.name, + email: student.email, + externalId: student.externalId, + subtotals, + total: totalFromSubtotals(subtotals, tabs), + }; + }); +}; + +export const sumWeights = (tabs: TabData[]): number => + tabs.reduce((acc, t) => acc + (t.gradebookWeight ?? 0), 0); + +const r2 = (n: number): number => Math.round(n * 100) / 100; + +// Ids of tabs that have at least one assessment — only these are eligible for a +// default weight (an empty tab carries no grades, so weighting it is wasted). +const nonEmptyTabIds = ( + tabs: TabData[], + assessments: Pick[], +): number[] => { + const populated = new Set(assessments.map((a) => a.tabId)); + return tabs.filter((t) => populated.has(t.id)).map((t) => t.id); +}; + +// True when no tab weight has been configured (every weight is 0/null) yet there +// is at least one tab to weight — i.e. the table is showing the equal-split +// default rather than an instructor's configuration. Drives the "default weights" +// banner and dialog pre-fill so both read from one source of truth. +export const usingDefaultWeights = ( + tabs: TabData[], + assessments: Pick[], +): boolean => + sumWeights(tabs) === 0 && nonEmptyTabIds(tabs, assessments).length > 0; + +// When no weights are configured, distribute 100 equally across non-empty tabs so +// the weighted view is meaningful out of the box; the last such tab absorbs the +// rounding remainder so the result sums to exactly 100. Returns the input array +// unchanged (same reference) once any tab carries a weight, so a real configuration +// is never overwritten. +export const resolveTabWeights = ( + tabs: TabData[], + assessments: Pick[], +): TabData[] => { + if (sumWeights(tabs) !== 0) return tabs; + const ids = nonEmptyTabIds(tabs, assessments); + const n = ids.length; + if (n === 0) return tabs; + const base = r2(100 / n); + const weightById = new Map( + ids.map((id, i) => [id, i === n - 1 ? r2(100 - base * (n - 1)) : base]), + ); + return tabs.map((tab) => + weightById.has(tab.id) + ? { + ...tab, + gradebookWeight: weightById.get(tab.id), + weightMode: tab.weightMode ?? 'equal', + } + : tab, + ); +}; diff --git a/client/app/bundles/course/gradebook/operations.ts b/client/app/bundles/course/gradebook/operations.ts index 35790580ed0..ae2f962fbbf 100644 --- a/client/app/bundles/course/gradebook/operations.ts +++ b/client/app/bundles/course/gradebook/operations.ts @@ -1,4 +1,5 @@ import type { Operation } from 'store'; +import type { UpdateWeightsPayload } from 'types/course/gradebook'; import CourseAPI from 'api/course'; @@ -9,4 +10,11 @@ const fetchGradebook = (): Operation => async (dispatch) => { dispatch(actions.saveGradebook(response.data)); }; +export const updateGradebookWeights = + (weights: UpdateWeightsPayload['weights']): Operation => + async (dispatch) => { + const response = await CourseAPI.gradebook.updateWeights({ weights }); + dispatch(actions.updateTabWeights(response.data)); + }; + 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 6d90823b5e4..ae7fc55e029 100644 --- a/client/app/bundles/course/gradebook/pages/GradebookIndex/index.tsx +++ b/client/app/bundles/course/gradebook/pages/GradebookIndex/index.tsx @@ -1,8 +1,8 @@ -import { FC, useEffect, useState } from 'react'; +import { FC, useEffect, useState, useTransition } from 'react'; import { defineMessages } from 'react-intl'; -import { useParams } from 'react-router-dom'; +import { useParams, useSearchParams } from 'react-router-dom'; import { PeopleAlt } from '@mui/icons-material'; -import { Typography } from '@mui/material'; +import { Tab, Tabs, Typography } from '@mui/material'; import Page from 'lib/components/core/layouts/Page'; import LoadingIndicator from 'lib/components/core/LoadingIndicator'; @@ -12,15 +12,19 @@ import useTranslation from 'lib/hooks/useTranslation'; import { useCourseContext } from '../../../container/CourseLoader'; import GradebookTable from '../../components/GradebookTable'; +import GradebookWeightedTable from '../../components/GradebookWeightedTable'; import GradeLinkHint from '../../components/GradeLinkHint'; +import WeightedViewHint from '../../components/WeightedViewHint'; import fetchGradebook from '../../operations'; import { getAssessments, + getCanManageWeights, getCategories, getGamificationEnabled, getStudents, getSubmissions, getTabs, + getWeightedViewEnabled, } from '../../selectors'; const translations = defineMessages({ @@ -40,6 +44,14 @@ const translations = defineMessages({ id: 'course.gradebook.GradebookIndex.noStudentsHint', defaultMessage: 'Grades will appear here once students join the course.', }, + allAssessments: { + id: 'course.gradebook.GradebookIndex.allAssessments', + defaultMessage: 'All assessments', + }, + byWeight: { + id: 'course.gradebook.GradebookIndex.byWeight', + defaultMessage: 'Weighted total', + }, }); const GradebookIndex: FC = () => { @@ -48,7 +60,12 @@ const GradebookIndex: FC = () => { const { courseTitle } = useCourseContext(); const { courseId: courseIdParam } = useParams(); const courseId = parseInt(courseIdParam!, 10); + const [searchParams, setSearchParams] = useSearchParams(); const [isLoading, setIsLoading] = useState(true); + const [viewMode, setViewMode] = useState<'all' | 'weighted'>( + searchParams.get('view') === 'weighted' ? 'weighted' : 'all', + ); + const [isPending, startTransition] = useTransition(); const assessments = useAppSelector(getAssessments); const categories = useAppSelector(getCategories); @@ -56,6 +73,8 @@ const GradebookIndex: FC = () => { const students = useAppSelector(getStudents); const submissions = useAppSelector(getSubmissions); const gamificationEnabled = useAppSelector(getGamificationEnabled); + const weightedViewEnabled = useAppSelector(getWeightedViewEnabled); + const canManageWeights = useAppSelector(getCanManageWeights); useEffect(() => { dispatch(fetchGradebook()) @@ -78,27 +97,74 @@ const GradebookIndex: FC = () => { ); + } else if (weightedViewEnabled && viewMode === 'weighted') { + content = ( + + ); } else { content = ( - <> - - - + ); } return ( - {content} + {!isLoading && canManageWeights && !weightedViewEnabled && ( + + )} + {weightedViewEnabled && !isLoading && students.length > 0 && ( + + startTransition(() => { + setViewMode(v); + setSearchParams(v === 'weighted' ? { view: 'weighted' } : {}); + }) + } + TabIndicatorProps={{ style: { height: 2 } }} + value={viewMode} + > + + + + )} + {!isLoading && + students.length > 0 && + !(weightedViewEnabled && viewMode === 'weighted') && } +
    + {isPending && ( +
    + +
    + )} +
    + {content} +
    +
    ); }; diff --git a/client/app/bundles/course/gradebook/selectors.ts b/client/app/bundles/course/gradebook/selectors.ts index fbe62e2611a..0fb7d1398d5 100644 --- a/client/app/bundles/course/gradebook/selectors.ts +++ b/client/app/bundles/course/gradebook/selectors.ts @@ -22,3 +22,7 @@ export const getGamificationEnabled = ( state: AppState, ): GradebookState['gamificationEnabled'] => getLocalState(state).gamificationEnabled; +export const getWeightedViewEnabled = (state: AppState): boolean => + getLocalState(state).weightedViewEnabled; +export const getCanManageWeights = (state: AppState): boolean => + getLocalState(state).canManageWeights; diff --git a/client/app/bundles/course/gradebook/store.ts b/client/app/bundles/course/gradebook/store.ts index 00e3291032b..4777e16b3b6 100644 --- a/client/app/bundles/course/gradebook/store.ts +++ b/client/app/bundles/course/gradebook/store.ts @@ -1,5 +1,8 @@ import { produce } from 'immer'; -import type { GradebookData } from 'types/course/gradebook'; +import type { + GradebookData, + UpdateWeightsPayload, +} from 'types/course/gradebook'; import type { AssessmentData, @@ -10,6 +13,7 @@ import type { } from './types'; const SAVE_GRADEBOOK = 'course/gradebook/SAVE_GRADEBOOK'; +const UPDATE_TAB_WEIGHTS = 'course/gradebook/UPDATE_TAB_WEIGHTS'; interface GradebookState { categories: CategoryData[]; @@ -18,6 +22,8 @@ interface GradebookState { students: StudentData[]; submissions: SubmissionData[]; gamificationEnabled: boolean; + weightedViewEnabled: boolean; + canManageWeights: boolean; } interface SaveGradebookAction { @@ -25,6 +31,11 @@ interface SaveGradebookAction { payload: GradebookData; } +interface UpdateTabWeightsAction { + type: typeof UPDATE_TAB_WEIGHTS; + payload: UpdateWeightsPayload; +} + const initialState: GradebookState = { categories: [], tabs: [], @@ -32,10 +43,15 @@ const initialState: GradebookState = { students: [], submissions: [], gamificationEnabled: false, + weightedViewEnabled: false, + canManageWeights: false, }; const reducer = produce( - (draft: GradebookState, action: SaveGradebookAction) => { + ( + draft: GradebookState, + action: SaveGradebookAction | UpdateTabWeightsAction, + ) => { switch (action.type) { case SAVE_GRADEBOOK: { draft.categories = action.payload.categories; @@ -44,6 +60,43 @@ const reducer = produce( draft.students = action.payload.students; draft.submissions = action.payload.submissions; draft.gamificationEnabled = action.payload.gamificationEnabled; + draft.weightedViewEnabled = action.payload.weightedViewEnabled; + draft.canManageWeights = action.payload.canManageWeights; + break; + } + case UPDATE_TAB_WEIGHTS: { + action.payload.weights.forEach( + ({ + tabId, + weight, + weightMode, + assessmentWeights, + excludedAssessmentIds, + }) => { + const tab = draft.tabs.find((t) => t.id === tabId); + if (tab) { + tab.gradebookWeight = weight; + tab.weightMode = weightMode; + } + const excludedSet = new Set(excludedAssessmentIds ?? []); + const tabAssessments = draft.assessments.filter( + (a) => a.tabId === tabId, + ); + tabAssessments.forEach((a) => { + a.gradebookExcluded = excludedSet.has(a.id); + }); + if (weightMode === 'equal') { + tabAssessments.forEach((a) => { + a.gradebookWeight = null; + }); + } else if (assessmentWeights) { + assessmentWeights.forEach(({ assessmentId, weight: aw }) => { + const a = draft.assessments.find((x) => x.id === assessmentId); + if (a) a.gradebookWeight = aw; + }); + } + }, + ); break; } default: @@ -58,6 +111,12 @@ export const actions = { type: SAVE_GRADEBOOK, payload: data, }), + updateTabWeights: ( + payload: UpdateWeightsPayload, + ): UpdateTabWeightsAction => ({ + type: UPDATE_TAB_WEIGHTS, + payload, + }), }; export default reducer; diff --git a/client/app/bundles/course/gradebook/types.ts b/client/app/bundles/course/gradebook/types.ts index f94aa7bf9c5..b91689df872 100644 --- a/client/app/bundles/course/gradebook/types.ts +++ b/client/app/bundles/course/gradebook/types.ts @@ -5,4 +5,5 @@ export type { StudentData, SubmissionData, TabData, + UpdateWeightsPayload, } from 'types/course/gradebook'; diff --git a/client/app/lib/components/core/buttons/SegmentedSwitch.tsx b/client/app/lib/components/core/buttons/SegmentedSwitch.tsx new file mode 100644 index 00000000000..ad82714b64d --- /dev/null +++ b/client/app/lib/components/core/buttons/SegmentedSwitch.tsx @@ -0,0 +1,235 @@ +import { + KeyboardEvent, + ReactNode, + useLayoutEffect, + useRef, + useState, +} from 'react'; +import { Box, ButtonBase, Tooltip } from '@mui/material'; + +export interface SegmentedSwitchOption { + value: T; + /** Visible content. Keep it short — one word reads best in this control. */ + label: ReactNode; + /** Optional hint shown on hover/focus of the segment. */ + tooltip?: ReactNode; + /** + * Accessible name for the segment. Falls back to `label` when that is a + * plain string; supply this when `label` is an icon or other non-text node. + */ + ariaLabel?: string; +} + +interface SegmentedSwitchProps { + /** The currently selected option's value. */ + value: T; + /** The options, left to right. Designed for 2 but renders any count. */ + options: SegmentedSwitchOption[]; + /** Fired with the next value when a different segment is chosen. */ + onChange: (value: T) => void; + /** Accessible name for the whole control (the radiogroup). */ + ariaLabel: string; + disabled?: boolean; + /** + * Pass `self-stretch` to grow the switch to a taller row neighbour (e.g. a + * small `TextField`), so the two align without hardcoding a height. + */ + className?: string; +} + +// Sized to MUI `size="small"` controls: 13px text, ~30.75px tall. Pixels are +// resolved through the theme's `pxToRem` so the switch matches its siblings +// regardless of the app's `htmlFontSize` (Coursemology uses 10, so a hardcoded +// rem would render ~38% too small). `MIN_HEIGHT` is a floor — `self-stretch` +// lets the switch grow to match a taller neighbour in the same flex row. +const FONT_PX = 13; +const PADDING_X = 1.5; +const MIN_HEIGHT = 30.75; + +/** + * A compact, peer-state mode switcher: a pill track with the options side by + * side and a single elevated thumb that slides to the active one. + * + * Unlike a `Switch`, neither option reads as "off" — both are equally valid — + * and unlike `ToggleButtonGroup` it stays content-width, so it fits a packed + * toolbar or a dense prompt row. Use it when a binary choice has no default + * "on" state (e.g. Points vs. Percentage, Equal vs. Custom). + * + * Keyboard: arrows move and select; roving tabindex keeps it a single tab stop. + */ +const SegmentedSwitch = ( + props: SegmentedSwitchProps, +): JSX.Element => { + const { value, options, onChange, ariaLabel, disabled, className } = props; + + const containerRef = useRef(null); + const optionRefs = useRef<(HTMLButtonElement | null)[]>([]); + const [thumb, setThumb] = useState<{ left: number; width: number }>({ + left: 0, + width: 0, + }); + + const activeIndex = Math.max( + 0, + options.findIndex((o) => o.value === value), + ); + + // Slide the thumb under the active segment. Segments are content-width, so + // both offset and width are measured. Re-measure on selection change and on + // any resize (font load, container reflow) — a one-shot measure would leave + // the thumb stranded after either. + useLayoutEffect(() => { + const measure = (): void => { + const el = optionRefs.current[activeIndex]; + const container = containerRef.current; + if (!el || !container) return; + // `offsetLeft` is measured from the container's border box, but the thumb + // (`left: 0`) is anchored to its padding box — just inside the border. + // Subtract the border width (`clientLeft`) so the two share a coordinate + // system; without it the active-left thumb loses its inset and sits flush + // against the border. + setThumb({ + left: el.offsetLeft - container.clientLeft, + width: el.offsetWidth, + }); + }; + measure(); + const observer = new ResizeObserver(measure); + if (containerRef.current) observer.observe(containerRef.current); + return () => observer.disconnect(); + }, [activeIndex, options.length]); + + const select = (index: number): void => { + const next = options[index]; + if (next && next.value !== value) onChange(next.value); + }; + + const handleKeyDown = (event: KeyboardEvent): void => { + if (disabled) return; + const last = options.length - 1; + let next = activeIndex; + if (event.key === 'ArrowRight' || event.key === 'ArrowDown') { + next = activeIndex === last ? 0 : activeIndex + 1; + } else if (event.key === 'ArrowLeft' || event.key === 'ArrowUp') { + next = activeIndex === 0 ? last : activeIndex - 1; + } else { + return; + } + event.preventDefault(); + select(next); + optionRefs.current[next]?.focus(); + }; + + return ( + ({ + position: 'relative', + display: 'inline-flex', + alignItems: 'stretch', + minHeight: MIN_HEIGHT, + boxSizing: 'border-box', + p: '3px', + borderRadius: 999, + bgcolor: theme.palette.action.hover, + border: `1px solid ${theme.palette.divider}`, + lineHeight: 1, + opacity: disabled ? 0.5 : 1, + pointerEvents: disabled ? 'none' : 'auto', + })} + > + {/* The sliding thumb. Hidden until measured (width 0) to avoid a flash + from the top-left corner on first paint. */} + ({ + position: 'absolute', + top: '3px', + bottom: '3px', + left: 0, + width: thumb.width, + borderRadius: 999, + bgcolor: theme.palette.background.paper, + boxShadow: theme.shadows[1], + opacity: thumb.width === 0 ? 0 : 1, + transform: `translateX(${thumb.left}px)`, + transition: theme.transitions.create(['transform', 'width'], { + duration: 260, + easing: 'cubic-bezier(0.34, 1.36, 0.64, 1)', + }), + zIndex: 0, + })} + /> + {options.map((option, index) => { + const selected = index === activeIndex; + const label = + option.ariaLabel ?? + (typeof option.label === 'string' ? option.label : undefined); + const segment = ( + { + optionRefs.current[index] = el; + }} + aria-checked={selected} + aria-label={label} + disabled={disabled} + disableRipple + onClick={() => select(index)} + role="radio" + sx={(theme) => ({ + position: 'relative', + zIndex: 1, + fontFamily: 'inherit', + fontSize: theme.typography.pxToRem(FONT_PX), + fontWeight: selected ? 650 : 550, + letterSpacing: '0.01em', + color: selected + ? theme.palette.text.primary + : theme.palette.text.secondary, + px: PADDING_X, + // Height comes from the track's `minHeight` + `align-items: + // stretch`; the segment fills it and centers its label, so no + // vertical padding of its own. + py: 0, + borderRadius: 999, + whiteSpace: 'nowrap', + transition: theme.transitions.create('color', { duration: 180 }), + '&:hover': { color: theme.palette.text.primary }, + '&:focus-visible': { + outline: `2px solid ${theme.palette.primary.main}`, + outlineOffset: 2, + }, + })} + tabIndex={selected ? 0 : -1} + > + {option.label} + + ); + + return option.tooltip ? ( + + {segment} + + ) : ( + + {segment} + + ); + })} + + ); +}; + +export default SegmentedSwitch; diff --git a/client/app/lib/components/core/buttons/__test__/SegmentedSwitch.test.tsx b/client/app/lib/components/core/buttons/__test__/SegmentedSwitch.test.tsx new file mode 100644 index 00000000000..ee3eb271832 --- /dev/null +++ b/client/app/lib/components/core/buttons/__test__/SegmentedSwitch.test.tsx @@ -0,0 +1,106 @@ +import userEvent from '@testing-library/user-event'; +import { fireEvent, render, screen, within } from 'test-utils'; + +import SegmentedSwitch from '../SegmentedSwitch'; + +// Render synchronously without the real provider's locale-loading spinner +// (uses the manual mock at lib/components/wrappers/__mocks__/I18nProvider). +jest.mock('lib/components/wrappers/I18nProvider'); + +const OPTIONS = [ + { value: 'points', label: 'Points' }, + { value: 'percent', label: 'Percentage' }, +] as const; + +const setup = ( + value: 'points' | 'percent', + overrides: Partial< + Parameters>[0] + > = {}, +): { onChange: jest.Mock } => { + const onChange = jest.fn(); + render( + , + ); + return { onChange }; +}; + +describe('', () => { + it('renders a radiogroup with one radio per option, named by ariaLabel', () => { + setup('points'); + const group = screen.getByRole('radiogroup', { name: 'Display mode' }); + expect(within(group).getAllByRole('radio')).toHaveLength(2); + expect(screen.getByRole('radio', { name: 'Points' })).toBeInTheDocument(); + expect( + screen.getByRole('radio', { name: 'Percentage' }), + ).toBeInTheDocument(); + }); + + it('marks only the selected option aria-checked', () => { + setup('percent'); + expect(screen.getByRole('radio', { name: 'Points' })).toHaveAttribute( + 'aria-checked', + 'false', + ); + expect(screen.getByRole('radio', { name: 'Percentage' })).toHaveAttribute( + 'aria-checked', + 'true', + ); + }); + + it('keeps a single tab stop via roving tabindex', () => { + setup('points'); + expect(screen.getByRole('radio', { name: 'Points' })).toHaveAttribute( + 'tabindex', + '0', + ); + expect(screen.getByRole('radio', { name: 'Percentage' })).toHaveAttribute( + 'tabindex', + '-1', + ); + }); + + it('fires onChange with the chosen value when an inactive option is clicked', async () => { + const user = userEvent.setup(); + const { onChange } = setup('points'); + await user.click(screen.getByRole('radio', { name: 'Percentage' })); + expect(onChange).toHaveBeenCalledWith('percent'); + }); + + it('does not fire onChange when the already-active option is clicked', async () => { + const user = userEvent.setup(); + const { onChange } = setup('points'); + await user.click(screen.getByRole('radio', { name: 'Points' })); + expect(onChange).not.toHaveBeenCalled(); + }); + + it('moves selection with arrow keys', async () => { + const user = userEvent.setup(); + const { onChange } = setup('points'); + screen.getByRole('radio', { name: 'Points' }).focus(); + await user.keyboard('{ArrowRight}'); + expect(onChange).toHaveBeenCalledWith('percent'); + }); + + it('wraps arrow navigation at the ends', async () => { + const user = userEvent.setup(); + const { onChange } = setup('points'); + screen.getByRole('radio', { name: 'Points' }).focus(); + await user.keyboard('{ArrowLeft}'); + expect(onChange).toHaveBeenCalledWith('percent'); + }); + + it('disables every option and suppresses onChange when disabled', () => { + const { onChange } = setup('points', { disabled: true }); + const percent = screen.getByRole('radio', { name: 'Percentage' }); + expect(percent).toBeDisabled(); + fireEvent.click(percent); + expect(onChange).not.toHaveBeenCalled(); + }); +}); diff --git a/client/app/lib/components/wrappers/__mocks__/I18nProvider.tsx b/client/app/lib/components/wrappers/__mocks__/I18nProvider.tsx new file mode 100644 index 00000000000..71d61318e33 --- /dev/null +++ b/client/app/lib/components/wrappers/__mocks__/I18nProvider.tsx @@ -0,0 +1,16 @@ +import { ReactNode } from 'react'; +import { IntlProvider } from 'react-intl'; + +// Manual mock for lib/components/wrappers/I18nProvider. The real provider renders +// a LoadingIndicator while it async-loads locale messages, so suites that assert +// synchronously would only ever see the spinner. Activate this synchronous +// stand-in per-suite with `jest.mock('lib/components/wrappers/I18nProvider')`. +// It is NOT applied automatically — only when a test opts in — so suites relying +// on the real provider's loading transition are unaffected. +const I18nProvider = ({ children }: { children: ReactNode }): JSX.Element => ( + + {children} + +); + +export default I18nProvider; diff --git a/client/app/lib/containers/AppContainer/AppContainer.tsx b/client/app/lib/containers/AppContainer/AppContainer.tsx index 817c6e3ce40..f51baac146e 100644 --- a/client/app/lib/containers/AppContainer/AppContainer.tsx +++ b/client/app/lib/containers/AppContainer/AppContainer.tsx @@ -1,6 +1,9 @@ +import { useEffect } from 'react'; import { Outlet, useRouteError } from 'react-router-dom'; +import { actions } from 'bundles/users/store'; import NotificationPopup from 'lib/containers/NotificationPopup'; +import { useAppDispatch } from 'lib/hooks/store'; import { loader, useAppLoader } from './AppLoader'; import GlobalAnnouncements from './GlobalAnnouncements'; @@ -9,6 +12,19 @@ import ServerUnreachableBanner from './ServerUnreachableBanner'; const AppContainer = (): JSX.Element => { const data = useAppLoader(); const homeData = data.home; + const dispatch = useAppDispatch(); + + useEffect(() => { + if (homeData.user) { + dispatch( + actions.saveUser({ + id: homeData.user.id, + name: homeData.user.name, + imageUrl: homeData.user.avatarUrl, + }), + ); + } + }, [homeData.user?.id]); return (
    diff --git a/client/app/routers/course/admin.tsx b/client/app/routers/course/admin.tsx index f9e1e1028b1..a157415aaf6 100644 --- a/client/app/routers/course/admin.tsx +++ b/client/app/routers/course/admin.tsx @@ -119,6 +119,17 @@ const adminRouter: Translated = (_) => ({ ).default, }), }, + { + path: 'gradebook', + lazy: async (): Promise> => ({ + Component: ( + await import( + /* webpackChunkName: 'GradebookSettings' */ + 'course/admin/pages/GradebookSettings' + ) + ).default, + }), + }, { path: 'comments', lazy: async (): Promise> => ({ diff --git a/client/app/types/course/admin/gradebook.ts b/client/app/types/course/admin/gradebook.ts new file mode 100644 index 00000000000..13301111fa0 --- /dev/null +++ b/client/app/types/course/admin/gradebook.ts @@ -0,0 +1,9 @@ +export interface GradebookSettingsData { + weightedViewEnabled: boolean; +} + +export interface GradebookSettingsPostData { + settings_gradebook_component: { + weighted_view_enabled: GradebookSettingsData['weightedViewEnabled']; + }; +} diff --git a/client/app/types/course/gradebook.ts b/client/app/types/course/gradebook.ts index e57b2cdd6d2..c9009afbfe0 100644 --- a/client/app/types/course/gradebook.ts +++ b/client/app/types/course/gradebook.ts @@ -7,6 +7,8 @@ export interface TabData { id: number; title: string; categoryId: number; + gradebookWeight?: number; + weightMode?: 'equal' | 'custom'; } export interface AssessmentData { @@ -14,6 +16,8 @@ export interface AssessmentData { title: string; tabId: number; maxGrade: number; + gradebookWeight?: number | null; + gradebookExcluded?: boolean; } export interface StudentData { @@ -39,4 +43,16 @@ export interface GradebookData { students: StudentData[]; submissions: SubmissionData[]; gamificationEnabled: boolean; + weightedViewEnabled: boolean; + canManageWeights: boolean; +} + +export interface UpdateWeightsPayload { + weights: { + tabId: number; + weight: number; + weightMode?: 'equal' | 'custom'; + excludedAssessmentIds?: number[]; + assessmentWeights?: { assessmentId: number; weight: number }[]; + }[]; } diff --git a/client/jest.config.js b/client/jest.config.js index 947fb088890..38d6c438357 100644 --- a/client/jest.config.js +++ b/client/jest.config.js @@ -24,6 +24,7 @@ const config = { '^course(.*)$': '/app/bundles/course$1', '^store(.*)$': '/app/store$1', '^lodash-es(.*)$': 'lodash$1', + 'compiled-locales/.*\\.json$': '/app/__test__/mocks/localeMock.js', }, testPathIgnorePatterns: ['/node_modules/', '/dist/'], coveragePathIgnorePatterns: ['/node_modules/', '/__test__/'], diff --git a/client/locales/en.json b/client/locales/en.json index 5a536c15e2f..1d9bf1cce66 100644 --- a/client/locales/en.json +++ b/client/locales/en.json @@ -9268,5 +9268,164 @@ }, "lib.translations.table.column.newExternalId": { "defaultMessage": "New External ID" + }, + "course.admin.GradebookSettings.gradebookSettings": { + "defaultMessage": "Gradebook settings" + }, + "course.admin.GradebookSettings.weightedViewEnabled": { + "defaultMessage": "Enable weighted grade view" + }, + "course.admin.GradebookSettings.weightedViewEnabledHint": { + "defaultMessage": "Enables a \"Weighted total\" view in the gradebook where staff can configure per-tab weights and see a weighted Total column." + }, + "course.gradebook.ConfigureWeightsPrompt.allExcluded": { + "defaultMessage": "All assessments in \"{tab}\" are excluded — it contributes nothing to the total." + }, + "course.gradebook.ConfigureWeightsPrompt.allExcludedCount": { + "defaultMessage": "All {n} excluded" + }, + "course.gradebook.ConfigureWeightsPrompt.customMode": { + "defaultMessage": "Custom" + }, + "course.gradebook.ConfigureWeightsPrompt.customSum": { + "defaultMessage": "Assessment weights: {sum} / {total}" + }, + "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.descriptionDrop": { + "defaultMessage": "In Equal mode, optionally drop each student's N lowest-scoring assessments before averaging." + }, + "course.gradebook.ConfigureWeightsPrompt.descriptionExclusion": { + "defaultMessage": "Expand a tab to include or exclude individual assessments from grading." + }, + "course.gradebook.ConfigureWeightsPrompt.descriptionIntro": { + "defaultMessage": "Control how tabs and assessments count toward each student's total grade." + }, + "course.gradebook.ConfigureWeightsPrompt.descriptionModes": { + "defaultMessage": "Choose Equal (all assessments share the tab's weight) or Custom (set each assessment's share)." + }, + "course.gradebook.ConfigureWeightsPrompt.descriptionWeights": { + "defaultMessage": "Set each tab's weight — how much it contributes to the total (weights should sum to 100)." + }, + "course.gradebook.ConfigureWeightsPrompt.equalMode": { + "defaultMessage": "Equal" + }, + "course.gradebook.ConfigureWeightsPrompt.excluded": { + "defaultMessage": "Excluded" + }, + "course.gradebook.ConfigureWeightsPrompt.excludedCount": { + "defaultMessage": "{n} excluded" + }, + "course.gradebook.ConfigureWeightsPrompt.includeAssessment": { + "defaultMessage": "Include {assessment} in grade" + }, + "course.gradebook.ConfigureWeightsPrompt.modeAria": { + "defaultMessage": "{tab} weight mode" + }, + "course.gradebook.ConfigureWeightsPrompt.ofGrade": { + "defaultMessage": "{pct}% of grade" + }, + "course.gradebook.ConfigureWeightsPrompt.promptTitle": { + "defaultMessage": "Configure contributions" + }, + "course.gradebook.ConfigureWeightsPrompt.saveError": { + "defaultMessage": "Failed to save weights. Please try again." + }, + "course.gradebook.ConfigureWeightsPrompt.total": { + "defaultMessage": "Total: {sum}%" + }, + "course.gradebook.ConfigureWeightsPrompt.unbalanced": { + "defaultMessage": "Assessment weights for \"{tab}\" must sum to its tab total before saving." + }, + "course.gradebook.ConfigureWeightsPrompt.valueTooHigh": { + "defaultMessage": "Value must be at most 100" + }, + "course.gradebook.ConfigureWeightsPrompt.valueTooLow": { + "defaultMessage": "Value must be at least 0" + }, + "course.gradebook.ConfigureWeightsPrompt.weightsDoNotSum": { + "defaultMessage": "Weights do not sum to 100. Saving is allowed; Total may be inaccurate." + }, + "course.gradebook.GradebookIndex.allAssessments": { + "defaultMessage": "All assessments" + }, + "course.gradebook.GradebookIndex.byWeight": { + "defaultMessage": "Weighted total" + }, + "course.gradebook.GradebookWeightedTable.collapseRow": { + "defaultMessage": "Collapse {name}" + }, + "course.gradebook.GradebookWeightedTable.configureWeights": { + "defaultMessage": "Configure Weights" + }, + "course.gradebook.GradebookWeightedTable.defaultWeights": { + "defaultMessage": "Showing default weights — every tab counts equally. Click \"Configure Weights\" to set your own." + }, + "course.gradebook.GradebookWeightedTable.defaultWeightsNoAccess": { + "defaultMessage": "Showing default weights — every tab counts equally until weights are configured." + }, + "course.gradebook.GradebookWeightedTable.displayPercent": { + "defaultMessage": "Percentage" + }, + "course.gradebook.GradebookWeightedTable.displayPercentTooltip": { + "defaultMessage": "What fraction of each tab the student earned. 100% on a tab worth 20% = the student earned all 20 grade points from that tab." + }, + "course.gradebook.GradebookWeightedTable.displayPoints": { + "defaultMessage": "Points" + }, + "course.gradebook.GradebookWeightedTable.displayPointsTooltip": { + "defaultMessage": "How many grade points each tab contributes. Columns add up to the projected total." + }, + "course.gradebook.GradebookWeightedTable.downloadCsv": { + "defaultMessage": "Download as CSV" + }, + "course.gradebook.GradebookWeightedTable.email": { + "defaultMessage": "Email" + }, + "course.gradebook.GradebookWeightedTable.excluded": { + "defaultMessage": "Excluded" + }, + "course.gradebook.GradebookWeightedTable.expandRow": { + "defaultMessage": "Expand {name}" + }, + "course.gradebook.GradebookWeightedTable.name": { + "defaultMessage": "Name" + }, + "course.gradebook.GradebookWeightedTable.noWeightsConfigured": { + "defaultMessage": "No weights configured — all tab weights are 0. Click \"Configure Weights\" to assign weights." + }, + "course.gradebook.GradebookWeightedTable.noWeightsNoAccess": { + "defaultMessage": "No tab weights have been configured yet." + }, + "course.gradebook.GradebookWeightedTable.outOfWeight": { + "defaultMessage": "/{weight}" + }, + "course.gradebook.GradebookWeightedTable.percentOfGrade": { + "defaultMessage": "{weight}% of grade" + }, + "course.gradebook.GradebookWeightedTable.percentTotalExact": { + "defaultMessage": "100% total" + }, + "course.gradebook.GradebookWeightedTable.percentTotalWarning": { + "defaultMessage": "{weight}% total" + }, + "course.gradebook.GradebookWeightedTable.total": { + "defaultMessage": "Total" + }, + "course.gradebook.GradebookWeightedTable.searchStudents": { + "defaultMessage": "Search students" + }, + "course.gradebook.GradebookWeightedTable.weightsDoNotSum": { + "defaultMessage": "Weights do not sum to 100. Total may be inaccurate." + }, + "course.gradebook.TotalHint.policy": { + "defaultMessage": "Totals count ungraded assessments as 0." + }, + "course.gradebook.WeightedViewHint.hint": { + "defaultMessage": "Want a weighted total grade? You can set how much each tab counts toward each student’s overall grade and view the weighted total here. Turn it on in {link}." + }, + "course.gradebook.WeightedViewHint.settingsLink": { + "defaultMessage": "Gradebook settings" } -} +} \ No newline at end of file diff --git a/client/locales/ko.json b/client/locales/ko.json index 2855b1a3a4e..9f9c0c593cb 100644 --- a/client/locales/ko.json +++ b/client/locales/ko.json @@ -9253,5 +9253,164 @@ }, "lib.translations.table.column.newExternalId": { "defaultMessage": "새 외부 ID" + }, + "course.admin.GradebookSettings.gradebookSettings": { + "defaultMessage": "성적부 설정" + }, + "course.admin.GradebookSettings.weightedViewEnabled": { + "defaultMessage": "가중 성적 보기 사용" + }, + "course.admin.GradebookSettings.weightedViewEnabledHint": { + "defaultMessage": "성적부에 \"가중 총점\" 보기를 추가하여 교직원이 탭별 가중치를 설정하고 가중 총점 열을 확인할 수 있습니다." + }, + "course.gradebook.ConfigureWeightsPrompt.allExcluded": { + "defaultMessage": "“{tab}”의 모든 평가가 제외되어 총점에 반영되지 않습니다." + }, + "course.gradebook.ConfigureWeightsPrompt.allExcludedCount": { + "defaultMessage": "전체 {n}개 제외됨" + }, + "course.gradebook.ConfigureWeightsPrompt.customMode": { + "defaultMessage": "사용자 지정" + }, + "course.gradebook.ConfigureWeightsPrompt.customSum": { + "defaultMessage": "평가 가중치: {sum} / {total}" + }, + "course.gradebook.ConfigureWeightsPrompt.defaultsHint": { + "defaultMessage": "아직 설정된 가중치가 없습니다. 모든 탭이 동일한 비중으로 반영되도록 제안된 기본값입니다. 저장하여 확정하거나 아래에서 조정하세요." + }, + "course.gradebook.ConfigureWeightsPrompt.descriptionDrop": { + "defaultMessage": "동일 모드에서는 평균을 내기 전에 각 학생의 점수가 가장 낮은 평가 N개를 선택적으로 제외할 수 있습니다." + }, + "course.gradebook.ConfigureWeightsPrompt.descriptionExclusion": { + "defaultMessage": "탭을 펼쳐 개별 평가를 성적에 포함하거나 제외하세요." + }, + "course.gradebook.ConfigureWeightsPrompt.descriptionIntro": { + "defaultMessage": "각 탭과 평가가 학생의 총 성적에 어떻게 반영되는지 관리합니다." + }, + "course.gradebook.ConfigureWeightsPrompt.descriptionModes": { + "defaultMessage": "“동일”（모든 평가가 탭의 가중치를 동일하게 나눔）또는 “사용자 지정”（각 평가의 비중을 설정）을 선택하세요." + }, + "course.gradebook.ConfigureWeightsPrompt.descriptionWeights": { + "defaultMessage": "각 탭의 가중치, 즉 총 성적에 기여하는 비중을 설정하세요. 가중치 합계는 100이어야 합니다." + }, + "course.gradebook.ConfigureWeightsPrompt.equalMode": { + "defaultMessage": "동일" + }, + "course.gradebook.ConfigureWeightsPrompt.excluded": { + "defaultMessage": "제외됨" + }, + "course.gradebook.ConfigureWeightsPrompt.excludedCount": { + "defaultMessage": "{n}개 제외됨" + }, + "course.gradebook.ConfigureWeightsPrompt.includeAssessment": { + "defaultMessage": "{assessment} 성적에 포함" + }, + "course.gradebook.ConfigureWeightsPrompt.modeAria": { + "defaultMessage": "{tab} 가중치 모드" + }, + "course.gradebook.ConfigureWeightsPrompt.ofGrade": { + "defaultMessage": "성적의 {pct}%" + }, + "course.gradebook.ConfigureWeightsPrompt.promptTitle": { + "defaultMessage": "기여도 설정" + }, + "course.gradebook.ConfigureWeightsPrompt.saveError": { + "defaultMessage": "가중치를 저장하지 못했습니다. 다시 시도해 주세요." + }, + "course.gradebook.ConfigureWeightsPrompt.total": { + "defaultMessage": "합계: {sum}%" + }, + "course.gradebook.ConfigureWeightsPrompt.unbalanced": { + "defaultMessage": "저장하기 전에 “{tab}”의 평가 가중치 합계가 해당 탭의 총 가중치와 같아야 합니다." + }, + "course.gradebook.ConfigureWeightsPrompt.valueTooHigh": { + "defaultMessage": "값은 최대 100이어야 합니다" + }, + "course.gradebook.ConfigureWeightsPrompt.valueTooLow": { + "defaultMessage": "값은 최소 0이어야 합니다" + }, + "course.gradebook.ConfigureWeightsPrompt.weightsDoNotSum": { + "defaultMessage": "가중치 합계가 100이 아닙니다. 저장은 가능하지만 총점이 정확하지 않을 수 있습니다." + }, + "course.gradebook.GradebookIndex.allAssessments": { + "defaultMessage": "전체 평가" + }, + "course.gradebook.GradebookIndex.byWeight": { + "defaultMessage": "가중 총점" + }, + "course.gradebook.GradebookWeightedTable.collapseRow": { + "defaultMessage": "{name} 접기" + }, + "course.gradebook.GradebookWeightedTable.configureWeights": { + "defaultMessage": "가중치 설정" + }, + "course.gradebook.GradebookWeightedTable.defaultWeights": { + "defaultMessage": "기본 가중치를 표시하고 있습니다. 모든 탭이 동일하게 반영됩니다. \"가중치 설정\"을 클릭하여 직접 설정하세요." + }, + "course.gradebook.GradebookWeightedTable.defaultWeightsNoAccess": { + "defaultMessage": "기본 가중치를 표시하고 있습니다. 가중치가 설정되기 전까지 모든 탭이 동일하게 반영됩니다." + }, + "course.gradebook.GradebookWeightedTable.displayPercent": { + "defaultMessage": "백분율" + }, + "course.gradebook.GradebookWeightedTable.displayPercentTooltip": { + "defaultMessage": "학생이 각 탭에서 획득한 비율입니다. 비중이 20%인 탭에서 100%는 해당 탭의 20점을 모두 획득했음을 의미합니다." + }, + "course.gradebook.GradebookWeightedTable.displayPoints": { + "defaultMessage": "점수" + }, + "course.gradebook.GradebookWeightedTable.displayPointsTooltip": { + "defaultMessage": "각 탭이 기여하는 성적 점수입니다. 각 열의 합이 예상 총점이 됩니다." + }, + "course.gradebook.GradebookWeightedTable.downloadCsv": { + "defaultMessage": "CSV로 다운로드" + }, + "course.gradebook.GradebookWeightedTable.email": { + "defaultMessage": "이메일" + }, + "course.gradebook.GradebookWeightedTable.excluded": { + "defaultMessage": "제외됨" + }, + "course.gradebook.GradebookWeightedTable.expandRow": { + "defaultMessage": "{name} 펼치기" + }, + "course.gradebook.GradebookWeightedTable.name": { + "defaultMessage": "이름" + }, + "course.gradebook.GradebookWeightedTable.noWeightsConfigured": { + "defaultMessage": "설정된 가중치가 없습니다. 모든 탭의 가중치가 0입니다. \"가중치 설정\"을 클릭하여 가중치를 지정하세요." + }, + "course.gradebook.GradebookWeightedTable.noWeightsNoAccess": { + "defaultMessage": "아직 탭 가중치가 설정되지 않았습니다." + }, + "course.gradebook.GradebookWeightedTable.outOfWeight": { + "defaultMessage": "/{weight}" + }, + "course.gradebook.GradebookWeightedTable.percentOfGrade": { + "defaultMessage": "성적의 {weight}%" + }, + "course.gradebook.GradebookWeightedTable.percentTotalExact": { + "defaultMessage": "합계 100%" + }, + "course.gradebook.GradebookWeightedTable.percentTotalWarning": { + "defaultMessage": "합계 {weight}%" + }, + "course.gradebook.GradebookWeightedTable.total": { + "defaultMessage": "총점" + }, + "course.gradebook.GradebookWeightedTable.searchStudents": { + "defaultMessage": "학생 검색" + }, + "course.gradebook.GradebookWeightedTable.weightsDoNotSum": { + "defaultMessage": "가중치 합계가 100이 아닙니다. 총점이 정확하지 않을 수 있습니다." + }, + "course.gradebook.TotalHint.policy": { + "defaultMessage": "총점은 채점되지 않은 평가를 0점으로 계산합니다." + }, + "course.gradebook.WeightedViewHint.hint": { + "defaultMessage": "가중 총점이 필요하신가요? 각 탭이 학생의 전체 성적에 반영되는 비중을 설정하고 여기에서 가중 총점을 확인할 수 있습니다. {link}에서 활성화하세요." + }, + "course.gradebook.WeightedViewHint.settingsLink": { + "defaultMessage": "성적부 설정" } } diff --git a/client/locales/zh.json b/client/locales/zh.json index 41c456b03e7..9d5c753e3e3 100644 --- a/client/locales/zh.json +++ b/client/locales/zh.json @@ -9247,5 +9247,164 @@ }, "lib.translations.table.column.newExternalId": { "defaultMessage": "新外部编号" + }, + "course.admin.GradebookSettings.gradebookSettings": { + "defaultMessage": "成绩册设置" + }, + "course.admin.GradebookSettings.weightedViewEnabled": { + "defaultMessage": "启用加权成绩视图" + }, + "course.admin.GradebookSettings.weightedViewEnabledHint": { + "defaultMessage": "在成绩册中启用“加权总成绩”视图，教职员可以配置各标签页的权重并查看加权总成绩列。" + }, + "course.gradebook.ConfigureWeightsPrompt.allExcluded": { + "defaultMessage": "“{tab}”中的所有评估均已排除，且不会计入总成绩。" + }, + "course.gradebook.ConfigureWeightsPrompt.allExcludedCount": { + "defaultMessage": "已排除全部 {n} 个" + }, + "course.gradebook.ConfigureWeightsPrompt.customMode": { + "defaultMessage": "自定义" + }, + "course.gradebook.ConfigureWeightsPrompt.customSum": { + "defaultMessage": "评估权重：{sum} / {total}" + }, + "course.gradebook.ConfigureWeightsPrompt.defaultsHint": { + "defaultMessage": "尚未设置权重。以下为建议默认值，所有标签页均按相同比重计算。保存以确认，或在下方进行调整。" + }, + "course.gradebook.ConfigureWeightsPrompt.descriptionDrop": { + "defaultMessage": "在等权模式下，可选择在求平均分前剔除每位学生得分最低的 N 个评估。" + }, + "course.gradebook.ConfigureWeightsPrompt.descriptionExclusion": { + "defaultMessage": "展开标签页以将个别评估纳入或排除在评分之外。" + }, + "course.gradebook.ConfigureWeightsPrompt.descriptionIntro": { + "defaultMessage": "控制各标签页和评估如何计入每位学生的总成绩。" + }, + "course.gradebook.ConfigureWeightsPrompt.descriptionModes": { + "defaultMessage": "选择“等权”（所有评估平分该标签页的权重）或“自定义”（设置每个评估所占的份额）。" + }, + "course.gradebook.ConfigureWeightsPrompt.descriptionWeights": { + "defaultMessage": "设置每个标签页的权重，即其对总成绩的贡献比例（权重合计应为 100）。" + }, + "course.gradebook.ConfigureWeightsPrompt.equalMode": { + "defaultMessage": "等权" + }, + "course.gradebook.ConfigureWeightsPrompt.excluded": { + "defaultMessage": "已排除" + }, + "course.gradebook.ConfigureWeightsPrompt.excludedCount": { + "defaultMessage": "已排除 {n} 个" + }, + "course.gradebook.ConfigureWeightsPrompt.includeAssessment": { + "defaultMessage": "将 {assessment} 计入成绩" + }, + "course.gradebook.ConfigureWeightsPrompt.modeAria": { + "defaultMessage": "{tab} 权重模式" + }, + "course.gradebook.ConfigureWeightsPrompt.ofGrade": { + "defaultMessage": "占成绩的 {pct}%" + }, + "course.gradebook.ConfigureWeightsPrompt.promptTitle": { + "defaultMessage": "配置贡献比例" + }, + "course.gradebook.ConfigureWeightsPrompt.saveError": { + "defaultMessage": "保存权重失败。请重试。" + }, + "course.gradebook.ConfigureWeightsPrompt.total": { + "defaultMessage": "总计：{sum}%" + }, + "course.gradebook.ConfigureWeightsPrompt.unbalanced": { + "defaultMessage": "保存前，“{tab}”的评估权重合计必须等于该标签页的总权重。" + }, + "course.gradebook.ConfigureWeightsPrompt.valueTooHigh": { + "defaultMessage": "数值最多为 100" + }, + "course.gradebook.ConfigureWeightsPrompt.valueTooLow": { + "defaultMessage": "数值至少为 0" + }, + "course.gradebook.ConfigureWeightsPrompt.weightsDoNotSum": { + "defaultMessage": "权重合计不为 100。仍可保存；总成绩可能不准确。" + }, + "course.gradebook.GradebookIndex.allAssessments": { + "defaultMessage": "全部评估" + }, + "course.gradebook.GradebookIndex.byWeight": { + "defaultMessage": "加权总成绩" + }, + "course.gradebook.GradebookWeightedTable.collapseRow": { + "defaultMessage": "收起 {name}" + }, + "course.gradebook.GradebookWeightedTable.configureWeights": { + "defaultMessage": "配置权重" + }, + "course.gradebook.GradebookWeightedTable.defaultWeights": { + "defaultMessage": "正在显示默认权重——所有标签页比重相同。点击“配置权重”进行自定义设置。" + }, + "course.gradebook.GradebookWeightedTable.defaultWeightsNoAccess": { + "defaultMessage": "正在显示默认权重——在配置权重之前，所有标签页比重相同。" + }, + "course.gradebook.GradebookWeightedTable.displayPercent": { + "defaultMessage": "百分比" + }, + "course.gradebook.GradebookWeightedTable.displayPercentTooltip": { + "defaultMessage": "学生在各标签页所获得的比例。在比重为 20% 的标签页获得 100%，即表示学生获得了该标签页的全部 20 个成绩分。" + }, + "course.gradebook.GradebookWeightedTable.displayPoints": { + "defaultMessage": "分数" + }, + "course.gradebook.GradebookWeightedTable.displayPointsTooltip": { + "defaultMessage": "各标签页贡献的成绩分数。各列相加即为预计总成绩。" + }, + "course.gradebook.GradebookWeightedTable.downloadCsv": { + "defaultMessage": "下载为 CSV" + }, + "course.gradebook.GradebookWeightedTable.email": { + "defaultMessage": "电子邮件" + }, + "course.gradebook.GradebookWeightedTable.excluded": { + "defaultMessage": "已排除" + }, + "course.gradebook.GradebookWeightedTable.expandRow": { + "defaultMessage": "展开 {name}" + }, + "course.gradebook.GradebookWeightedTable.name": { + "defaultMessage": "姓名" + }, + "course.gradebook.GradebookWeightedTable.noWeightsConfigured": { + "defaultMessage": "尚未配置权重——所有标签页的权重均为 0。点击“配置权重”以分配权重。" + }, + "course.gradebook.GradebookWeightedTable.noWeightsNoAccess": { + "defaultMessage": "尚未配置任何标签页权重。" + }, + "course.gradebook.GradebookWeightedTable.outOfWeight": { + "defaultMessage": "/{weight}" + }, + "course.gradebook.GradebookWeightedTable.percentOfGrade": { + "defaultMessage": "占成绩的 {weight}%" + }, + "course.gradebook.GradebookWeightedTable.percentTotalExact": { + "defaultMessage": "合计 100%" + }, + "course.gradebook.GradebookWeightedTable.percentTotalWarning": { + "defaultMessage": "合计 {weight}%" + }, + "course.gradebook.GradebookWeightedTable.total": { + "defaultMessage": "总成绩" + }, + "course.gradebook.GradebookWeightedTable.searchStudents": { + "defaultMessage": "搜索学生" + }, + "course.gradebook.GradebookWeightedTable.weightsDoNotSum": { + "defaultMessage": "权重合计不为 100。总成绩可能不准确。" + }, + "course.gradebook.TotalHint.policy": { + "defaultMessage": "总成绩将未评分的评估按 0 分计算。" + }, + "course.gradebook.WeightedViewHint.hint": { + "defaultMessage": "需要加权总成绩吗？您可以设置每个标签页在每位学生总成绩中所占的比重，并在此查看加权总成绩。请在{link}中启用。" + }, + "course.gradebook.WeightedViewHint.settingsLink": { + "defaultMessage": "成绩册设置" } } diff --git a/config/locales/en/activerecord/errors.yml b/config/locales/en/activerecord/errors.yml index 95f0da1660a..713858a6dcb 100644 --- a/config/locales/en/activerecord/errors.yml +++ b/config/locales/en/activerecord/errors.yml @@ -97,6 +97,7 @@ en: autograded_no_partial_answer: 'There are updated answers which have not been re-submitted yet. Please re-submit all answers before finalising your submission.' course/assessment/tab: deletion: 'the last tab cannot be deleted' + custom_weights_mismatch: 'Custom assessment weights must sum to the tab total' course/condition: attributes: conditional: diff --git a/config/locales/ko/activerecord/errors.yml b/config/locales/ko/activerecord/errors.yml index 62c4016a70f..6b6c01e11c2 100644 --- a/config/locales/ko/activerecord/errors.yml +++ b/config/locales/ko/activerecord/errors.yml @@ -95,6 +95,7 @@ ko: autograded_no_partial_answer: '업데이트된 답변이 아직 다시 제출되지 않았습니다. 제출을 완료하기 전에 모든 답변을 다시 제출하세요.' course/assessment/tab: deletion: '마지막 탭은 삭제할 수 없습니다' + custom_weights_mismatch: '사용자 지정 평가 가중치의 합은 해당 탭의 총점과 같아야 합니다.' course/condition: attributes: conditional: diff --git a/config/locales/zh/activerecord/errors.yml b/config/locales/zh/activerecord/errors.yml index 5cbb439debc..6a654814f36 100644 --- a/config/locales/zh/activerecord/errors.yml +++ b/config/locales/zh/activerecord/errors.yml @@ -95,6 +95,7 @@ zh: autograded_no_partial_answer: '有一些更新的答案还没有重新提交。请在最后提交前重新提交所有答案。' course/assessment/tab: deletion: '最后一项无法被删除' + custom_weights_mismatch: '自定义评估权重的总和必须等于该标签的总分。' course/condition: attributes: conditional: diff --git a/config/routes.rb b/config/routes.rb index 9746e03277f..0ff051754ca 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -196,6 +196,9 @@ get 'leaderboard' => 'leaderboard_settings#edit' patch 'leaderboard' => 'leaderboard_settings#update' + get 'gradebook' => 'gradebook_settings#edit' + patch 'gradebook' => 'gradebook_settings#update' + get 'comments' => 'discussion/topic_settings#edit', as: 'topics' patch 'comments' => 'discussion/topic_settings#update' @@ -498,6 +501,7 @@ resource :gradebook, only: [] do get '/' => 'gradebook#index' + patch '/weights' => 'gradebook#update_weights' end scope module: :discussion do diff --git a/db/migrate/20260611000000_create_gradebook_contribution_tables.rb b/db/migrate/20260611000000_create_gradebook_contribution_tables.rb new file mode 100644 index 00000000000..bebf1873fb4 --- /dev/null +++ b/db/migrate/20260611000000_create_gradebook_contribution_tables.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true +class CreateGradebookContributionTables < ActiveRecord::Migration[7.2] + def change + create_table :course_gradebook_contributions do |t| + t.references :course, null: false, foreign_key: { to_table: :courses }, + index: { name: 'fk__course_gradebook_contributions_course_id' } + t.references :tab, null: true, + foreign_key: { to_table: :course_assessment_tabs, on_delete: :cascade }, + index: { unique: true, + name: 'index_course_gradebook_contributions_on_tab_id' } + t.decimal :weight, precision: 5, scale: 2, null: false, default: 0 + t.integer :weight_mode, null: false, default: 0 + t.integer :keep_highest, null: false, default: 0 + + t.references :creator, null: false, foreign_key: { to_table: :users }, + index: { name: 'fk__course_gradebook_contributions_creator_id' } + t.references :updater, null: false, foreign_key: { to_table: :users }, + index: { name: 'fk__course_gradebook_contributions_updater_id' } + t.timestamps null: false + end + + create_table :course_gradebook_assessment_contributions do |t| + t.references :assessment, null: false, + foreign_key: { to_table: :course_assessments, on_delete: :cascade }, + index: { unique: true, + name: 'index_cgac_on_assessment_id' } + t.decimal :weight, precision: 5, scale: 2, null: true + t.boolean :excluded, null: false, default: false + + t.references :creator, null: false, foreign_key: { to_table: :users }, + index: { name: 'fk__cgac_creator_id' } + t.references :updater, null: false, foreign_key: { to_table: :users }, + index: { name: 'fk__cgac_updater_id' } + t.timestamps null: false + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 1d320432f9a..d1a22b40e89 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_05_14_052933) do +ActiveRecord::Schema[7.2].define(version: 2026_06_11_000000) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" enable_extension "uuid-ossp" @@ -856,6 +856,35 @@ t.index ["updater_id"], name: "fk__course_forums_updater_id" end + create_table "course_gradebook_assessment_contributions", force: :cascade do |t| + t.bigint "assessment_id", null: false + t.decimal "weight", precision: 5, scale: 2 + t.boolean "excluded", default: false, 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 ["assessment_id"], name: "index_cgac_on_assessment_id", unique: true + t.index ["creator_id"], name: "fk__cgac_creator_id" + t.index ["updater_id"], name: "fk__cgac_updater_id" + end + + create_table "course_gradebook_contributions", force: :cascade do |t| + t.bigint "course_id", null: false + t.bigint "tab_id" + t.decimal "weight", precision: 5, scale: 2, default: "0.0", null: false + t.integer "weight_mode", default: 0, null: false + t.integer "keep_highest", default: 0, null: false + t.bigint "creator_id", null: false + t.bigint "updater_id", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["course_id"], name: "fk__course_gradebook_contributions_course_id" + t.index ["creator_id"], name: "fk__course_gradebook_contributions_creator_id" + t.index ["tab_id"], name: "index_course_gradebook_contributions_on_tab_id", unique: true + t.index ["updater_id"], name: "fk__course_gradebook_contributions_updater_id" + end + create_table "course_group_categories", force: :cascade do |t| t.bigint "course_id", null: false t.string "name", default: "", null: false @@ -1915,6 +1944,13 @@ add_foreign_key "course_forums", "courses", name: "fk_course_forums_course_id" add_foreign_key "course_forums", "users", column: "creator_id", name: "fk_course_forums_creator_id" add_foreign_key "course_forums", "users", column: "updater_id", name: "fk_course_forums_updater_id" + 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_contributions", "course_assessment_tabs", column: "tab_id", on_delete: :cascade + add_foreign_key "course_gradebook_contributions", "courses" + add_foreign_key "course_gradebook_contributions", "users", column: "creator_id" + add_foreign_key "course_gradebook_contributions", "users", column: "updater_id" add_foreign_key "course_group_categories", "courses" add_foreign_key "course_group_categories", "users", column: "creator_id" add_foreign_key "course_group_categories", "users", column: "updater_id" diff --git a/spec/controllers/course/admin/gradebook_settings_controller_spec.rb b/spec/controllers/course/admin/gradebook_settings_controller_spec.rb new file mode 100644 index 00000000000..0423499d2c9 --- /dev/null +++ b/spec/controllers/course/admin/gradebook_settings_controller_spec.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::Admin::GradebookSettingsController, type: :controller do + let(:instance) { Instance.default } + with_tenant(:instance) do + let(:course) { create(:course) } + let(:manager) { create(:course_manager, course: course) } + let(:teaching_assistant) { create(:course_teaching_assistant, course: course) } + + describe '#edit' do + context 'as manager' do + render_views + before { controller_sign_in(controller, manager.user) } + + it 'returns settings JSON' do + get :edit, params: { course_id: course.id }, format: :json + expect(response).to have_http_status(:ok) + body = JSON.parse(response.body) + expect(body).to include('weightedViewEnabled' => false) + end + end + + context 'as teaching assistant' do + before { controller_sign_in(controller, teaching_assistant.user) } + + it 'is denied' do + expect do + get :edit, params: { course_id: course.id }, format: :json + end.to raise_error(CanCan::AccessDenied) + end + end + end + + describe '#update' do + context 'as manager' do + render_views + before { controller_sign_in(controller, manager.user) } + + it 'updates weighted_view_enabled and returns 200' do + patch :update, + params: { course_id: course.id, + settings_gradebook_component: { weighted_view_enabled: true } }, + format: :json + expect(response).to have_http_status(:ok) + body = JSON.parse(response.body) + expect(body).to include('weightedViewEnabled' => true) + end + + it 'preserves existing tab gradebook_weights when toggling setting' do + category = create(:course_assessment_category, course: course) + tab = create(:course_assessment_tab, category: category) + contribution = create(:course_gradebook_contribution, tab: tab, course: course, weight: 50) + + patch :update, + params: { course_id: course.id, + settings_gradebook_component: { weighted_view_enabled: true } }, + format: :json + expect(contribution.reload.weight).to eq(50) + + patch :update, + params: { course_id: course.id, + settings_gradebook_component: { weighted_view_enabled: false } }, + format: :json + expect(contribution.reload.weight).to eq(50) + end + end + + context 'as teaching assistant' do + before { controller_sign_in(controller, teaching_assistant.user) } + + it 'is denied' do + expect do + patch :update, + params: { course_id: course.id, + settings_gradebook_component: { weighted_view_enabled: true } }, + format: :json + end.to raise_error(CanCan::AccessDenied) + end + end + end + end +end diff --git a/spec/controllers/course/gradebook_controller_spec.rb b/spec/controllers/course/gradebook_controller_spec.rb index a213e80f476..59397f4120d 100644 --- a/spec/controllers/course/gradebook_controller_spec.rb +++ b/spec/controllers/course/gradebook_controller_spec.rb @@ -200,5 +200,228 @@ end end end + + describe 'PATCH update_weights' do + let(:manager) { create(:course_manager, course: course) } + let(:ta) { create(:course_teaching_assistant, course: course) } + let(:student) { create(:course_student, course: course) } + let(:category) { create(:course_assessment_category, course: course) } + let!(:tab1) { create(:course_assessment_tab, category: category) } + let!(:tab2) { create(:course_assessment_tab, category: category) } + def weight_for(tab) + Course::Gradebook::Contribution.find_by(tab_id: tab.id)&.weight + end + + let(:valid_payload) do + { weights: [{ tabId: tab1.id, weight: 60 }, { tabId: tab2.id, weight: 40 }] } + end + + context 'as manager' do + before { controller_sign_in(controller, manager.user) } + + it 'updates and returns 200' do + patch :update_weights, params: { course_id: course.id, **valid_payload }, format: :json + expect(response).to have_http_status(:ok) + expect(weight_for(tab1)).to eq(60) + expect(weight_for(tab2)).to eq(40) + end + + it 'accepts sum < 100' do + patch :update_weights, + params: { course_id: course.id, weights: [tabId: tab1.id, weight: 30] }, + format: :json + expect(response).to have_http_status(:ok) + end + + it 'accepts sum > 100' do + patch :update_weights, + params: { course_id: course.id, + weights: [{ tabId: tab1.id, weight: 70 }, { tabId: tab2.id, weight: 70 }] }, + format: :json + expect(response).to have_http_status(:ok) + end + + it 'rejects negative with 422 and no partial write' do + create(:course_gradebook_contribution, tab: tab1, course: course, weight: 10) + patch :update_weights, + params: { course_id: course.id, + weights: [{ tabId: tab1.id, weight: 50 }, { tabId: tab2.id, weight: -1 }] }, + format: :json + expect(response).to have_http_status(:unprocessable_entity) + expect(weight_for(tab1)).to eq(10) + end + + it 'rejects >100 with 422' do + patch :update_weights, + params: { course_id: course.id, weights: [tabId: tab1.id, weight: 101] }, + format: :json + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'rejects foreign tab id with 422' do + other_course = create(:course) + other_tab = create(:course_assessment_tab, + category: create(:course_assessment_category, course: other_course)) + patch :update_weights, + params: { course_id: course.id, weights: [tabId: other_tab.id, weight: 50] }, + format: :json + expect(response).to have_http_status(:unprocessable_entity) + end + end + + context 'as TA' do + before { controller_sign_in(controller, ta.user) } + it 'is denied' do + expect do + patch :update_weights, params: { course_id: course.id, **valid_payload }, format: :json + end.to raise_error(CanCan::AccessDenied) + end + end + + context 'as student' do + before { controller_sign_in(controller, student.user) } + it 'is denied' do + expect do + patch :update_weights, params: { course_id: course.id, **valid_payload }, format: :json + end.to raise_error(CanCan::AccessDenied) + end + end + + context 'when setting is disabled' do + before { controller_sign_in(controller, manager.user) } + + it 'still allows update (storage independent of display)' do + patch :update_weights, params: { course_id: course.id, **valid_payload }, format: :json + expect(response).to have_http_status(:ok) + expect(weight_for(tab1)).to eq(60) + end + end + + describe '#update_weights with modes' do + render_views + + let(:category) { create(:course_assessment_category, course: course) } + let(:tab) { create(:course_assessment_tab, category: category) } + let!(:a1) { create(:assessment, course: course, tab: tab) } + let!(:a2) { create(:assessment, course: course, tab: tab) } + + before { controller_sign_in(controller, manager.user) } + + it 'persists custom mode + assessment weights and echoes them back' do + post :update_weights, as: :json, params: { + course_id: course.id, + weights: [{ + tabId: tab.id, weight: '50', weightMode: 'custom', + assessmentWeights: [ + { assessmentId: a1.id, weight: '30' }, + { assessmentId: a2.id, weight: '20' } + ] + }] + } + expect(response).to have_http_status(:ok) + body = JSON.parse(response.body) + entry = body['weights'].first + expect(entry['weightMode']).to eq('custom') + expect(entry['assessmentWeights']).to contain_exactly( + { 'assessmentId' => a1.id, 'weight' => 30.0 }, + { 'assessmentId' => a2.id, 'weight' => 20.0 } + ) + expect(a1.reload.gradebook_assessment_contribution.weight).to eq(30.0) + end + + it 'returns 422 when custom weights do not sum to the tab total' do + post :update_weights, as: :json, params: { + course_id: course.id, + weights: [{ + tabId: tab.id, weight: '50', weightMode: 'custom', + assessmentWeights: [{ assessmentId: a1.id, weight: '10' }] + }] + } + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'persists and echoes per-assessment exclusion in equal mode' do + post :update_weights, as: :json, params: { + course_id: course.id, + weights: [{ + tabId: tab.id, weight: '50', weightMode: 'equal', + excludedAssessmentIds: [a1.id] + }] + } + expect(response).to have_http_status(:ok) + expect(a1.reload.gradebook_assessment_contribution.excluded).to eq(true) + expect(a2.reload.gradebook_assessment_contribution.excluded).to eq(false) + entry = JSON.parse(response.body)['weights'].first + expect(entry['excludedAssessmentIds']).to eq([a1.id]) + end + end + end + + describe 'GET index — weighted view fields' do + render_views + let(:manager) { create(:course_manager, course: course) } + let(:ta) { create(:course_teaching_assistant, course: course) } + let(:category) { create(:course_assessment_category, course: course) } + let!(:tab) { create(:course_assessment_tab, category: category) } + let!(:contribution) { create(:course_gradebook_contribution, tab: tab, course: course, weight: 30) } + let!(:assessment) do + create(:course_assessment_assessment, :published_with_mcq_question, + course: course, tab: tab) + end + + context 'when setting is disabled (default)' do + before { controller_sign_in(controller, manager.user) } + + it 'returns weightedViewEnabled false and omits gradebookWeight per tab' do + get :index, params: { course_id: course.id }, format: :json + body = JSON.parse(response.body) + expect(body['weightedViewEnabled']).to eq(false) + tab_json = body['tabs'].find { |t| t['id'] == tab.id } + expect(tab_json).not_to have_key('gradebookWeight') + end + end + + context 'when setting is enabled' do + before do + ctx = Struct.new(:current_course, :key).new(course, Course::GradebookComponent.key) + Course::Settings::GradebookComponent.new(ctx).weighted_view_enabled = true + course.save! + end + + it 'includes weightedViewEnabled true and gradebookWeight per tab for manager' do + controller_sign_in(controller, manager.user) + get :index, params: { course_id: course.id }, format: :json + body = JSON.parse(response.body) + expect(body['weightedViewEnabled']).to eq(true) + expect(body['canManageWeights']).to eq(true) + tab_json = body['tabs'].find { |t| t['id'] == tab.id } + expect(tab_json['gradebookWeight']).to eq(30) + end + + it 'returns canManageWeights false for TA' do + controller_sign_in(controller, ta.user) + get :index, params: { course_id: course.id }, format: :json + body = JSON.parse(response.body) + expect(body['canManageWeights']).to eq(false) + end + + it 'serializes weightMode on tabs and gradebookWeight on assessments when weighted view is enabled' do + controller_sign_in(controller, manager.user) + get :index, params: { course_id: course.id }, format: :json + body = JSON.parse(response.body) + tab_json = body['tabs'].find { |t| t['id'] == tab.id } + expect(tab_json).to have_key('weightMode') + expect(body['assessments'].first).to have_key('gradebookWeight') + end + + it 'serializes gradebookExcluded on assessments when weighted view is enabled' do + controller_sign_in(controller, manager.user) + get :index, params: { course_id: course.id }, format: :json + body = JSON.parse(response.body) + expect(body['assessments'].first).to have_key('gradebookExcluded') + expect(body['assessments'].first['gradebookExcluded']).to eq(false) + end + end + end end end diff --git a/spec/factories/course_gradebook_assessment_contributions.rb b/spec/factories/course_gradebook_assessment_contributions.rb new file mode 100644 index 00000000000..6a0126e520f --- /dev/null +++ b/spec/factories/course_gradebook_assessment_contributions.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true +FactoryBot.define do + factory :course_gradebook_assessment_contribution, class: Course::Gradebook::AssessmentContribution.name do + assessment + excluded { false } + end +end diff --git a/spec/factories/course_gradebook_contributions.rb b/spec/factories/course_gradebook_contributions.rb new file mode 100644 index 00000000000..606994f4aa2 --- /dev/null +++ b/spec/factories/course_gradebook_contributions.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true +FactoryBot.define do + factory :course_gradebook_contribution, class: Course::Gradebook::Contribution.name do + association :tab, factory: :course_assessment_tab + course { tab.category.course } + weight { 0 } + weight_mode { :equal } + end +end diff --git a/spec/models/course/gradebook/assessment_contribution_spec.rb b/spec/models/course/gradebook/assessment_contribution_spec.rb new file mode 100644 index 00000000000..5bfe3af8635 --- /dev/null +++ b/spec/models/course/gradebook/assessment_contribution_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::Gradebook::AssessmentContribution do + let!(:instance) { Instance.default } + with_tenant(:instance) do + let(:course) { create(:course) } + let(:tab) { create(:course_assessment_tab, course: course) } + let(:assessment) { create(:assessment, course: course, tab: tab) } + + it { is_expected.to belong_to(:assessment) } + + it 'allows a nil weight (equal mode)' do + contribution = build(:course_gradebook_assessment_contribution, assessment: assessment, weight: nil) + expect(contribution).to be_valid + end + + it 'rejects a negative weight' do + contribution = build(:course_gradebook_assessment_contribution, assessment: assessment, weight: -1) + expect(contribution).not_to be_valid + end + + it 'defaults excluded to false' do + contribution = create(:course_gradebook_assessment_contribution, assessment: assessment) + expect(contribution.excluded).to eq(false) + end + + it 'enforces one row per assessment' do + create(:course_gradebook_assessment_contribution, assessment: assessment) + duplicate = build(:course_gradebook_assessment_contribution, assessment: assessment) + expect(duplicate).not_to be_valid + end + + it 'is destroyed when its assessment is destroyed' do + create(:course_gradebook_assessment_contribution, assessment: assessment) + expect { assessment.destroy! }.to change { described_class.count }.by(-1) + end + end +end diff --git a/spec/models/course/gradebook/contribution_spec.rb b/spec/models/course/gradebook/contribution_spec.rb new file mode 100644 index 00000000000..d4d433e028a --- /dev/null +++ b/spec/models/course/gradebook/contribution_spec.rb @@ -0,0 +1,213 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::Gradebook::Contribution do + let!(:instance) { Instance.default } + with_tenant(:instance) do + let(:course) { create(:course) } + let(:category) { create(:course_assessment_category, course: course) } + let(:tab) { create(:course_assessment_tab, category: category) } + + describe 'validations' do + it 'is valid with a tab and matching course' do + expect(build(:course_gradebook_contribution, tab: tab, course: course)).to be_valid + end + + it 'requires a tab' do + contribution = build(:course_gradebook_contribution, tab: tab, course: course) + contribution.tab = nil + expect(contribution).not_to be_valid + end + + it 'enforces one row per tab' do + create(:course_gradebook_contribution, tab: tab, course: course) + duplicate = build(:course_gradebook_contribution, tab: tab, course: course) + expect(duplicate).not_to be_valid + end + + it 'rejects a course that does not match the tab course' do + contribution = build(:course_gradebook_contribution, tab: tab) + contribution.course = create(:course) + expect(contribution).not_to be_valid + end + + it 'rejects a negative weight' do + contribution = build(:course_gradebook_contribution, tab: tab, course: course, weight: -1) + expect(contribution).not_to be_valid + end + + it 'rejects a weight above 100' do + contribution = build(:course_gradebook_contribution, tab: tab, course: course, weight: 101) + expect(contribution).not_to be_valid + end + + it 'rejects a negative keep_highest' do + contribution = build(:course_gradebook_contribution, tab: tab, course: course, keep_highest: -1) + expect(contribution).not_to be_valid + end + + it 'defaults weight 0, mode equal, keep_highest 0' do + contribution = create(:course_gradebook_contribution, tab: tab, course: course) + expect(contribution.weight).to eq(0) + expect(contribution.weight_mode).to eq('equal') + expect(contribution.keep_highest).to eq(0) + end + end + + describe 'dependent destroy' do + it 'is destroyed when its tab is destroyed' do + create(:course_assessment_tab, category: category) # sibling so the tab is deletable + create(:course_gradebook_contribution, tab: tab, course: course) + expect { tab.destroy! }.to change { described_class.count }.by(-1) + end + end + + describe '.bulk_update' do + let(:tab1) { create(:course_assessment_tab, category: category) } + let(:tab2) { create(:course_assessment_tab, category: category) } + + def weight_for(tab) + described_class.find_by(tab_id: tab.id)&.weight + end + + it 'upserts a contribution per given tab' do + described_class.bulk_update( + course: course, + updates: [{ tab_id: tab1.id, weight: 60 }, { tab_id: tab2.id, weight: 40 }] + ) + expect(weight_for(tab1)).to eq(60) + expect(weight_for(tab2)).to eq(40) + end + + it 'is transactional — an invalid value rolls everything back' do + create(:course_gradebook_contribution, tab: tab1, course: course, weight: 10) + create(:course_gradebook_contribution, tab: tab2, course: course, weight: 20) + expect do + described_class.bulk_update( + course: course, + updates: [{ tab_id: tab1.id, weight: 50 }, { tab_id: tab2.id, weight: 999 }] + ) + end.to raise_error(ActiveRecord::RecordInvalid) + expect(weight_for(tab1)).to eq(10) + expect(weight_for(tab2)).to eq(20) + end + + it 'rejects a foreign tab_id' do + other_course = create(:course) + other_tab = create(:course_assessment_tab, + category: create(:course_assessment_category, course: other_course)) + expect do + described_class.bulk_update(course: course, updates: [tab_id: other_tab.id, weight: 50]) + end.to raise_error(ActiveRecord::RecordNotFound) + end + + context 'with assessments in the tab' do + let(:tab) { create(:course_assessment_tab, category: category) } + let!(:a1) { create(:assessment, course: course, tab: tab) } + let!(:a2) { create(:assessment, course: course, tab: tab) } + + def assessment_weight(assessment) + assessment.reload.gradebook_assessment_contribution&.weight + end + + def excluded?(assessment) + assessment.reload.gradebook_assessment_contribution&.excluded + end + + it 'persists custom mode + assessment weights that sum to the tab total' do + described_class.bulk_update( + course: course, + updates: [ + tab_id: tab.id, weight: 50.0, weight_mode: 'custom', + assessment_weights: [ + { assessment_id: a1.id, weight: 30.0 }, + { assessment_id: a2.id, weight: 20.0 } + ] + ] + ) + expect(tab.reload.gradebook_contribution.weight_mode).to eq('custom') + expect(assessment_weight(a1)).to eq(30.0) + expect(assessment_weight(a2)).to eq(20.0) + end + + it 'raises RecordInvalid when custom weights do not sum to the tab total' do + expect do + described_class.bulk_update( + course: course, + updates: [ + tab_id: tab.id, weight: 50.0, weight_mode: 'custom', + assessment_weights: [ + { assessment_id: a1.id, weight: 30.0 }, + { assessment_id: a2.id, weight: 5.0 } + ] + ] + ) + end.to raise_error(ActiveRecord::RecordInvalid) + expect(assessment_weight(a1)).to be_nil # rolled back + end + + it 'nulls assessment weights when switching the tab to equal mode' do + create(:course_gradebook_assessment_contribution, assessment: a1, weight: 30.0) + described_class.bulk_update( + course: course, + updates: [tab_id: tab.id, weight: 50.0, weight_mode: 'equal'] + ) + expect(tab.reload.gradebook_contribution.weight_mode).to eq('equal') + expect(assessment_weight(a1)).to be_nil + end + + it 'persists per-assessment exclusion in equal mode' do + described_class.bulk_update( + course: course, + updates: [tab_id: tab.id, weight: 50.0, weight_mode: 'equal', + excluded_assessment_ids: [a1.id]] + ) + expect(excluded?(a1)).to eq(true) + expect(excluded?(a2)).to eq(false) + end + + it 'drops excluded assessments from the custom balance check and keeps their weight' do + described_class.bulk_update( + course: course, + updates: [ + tab_id: tab.id, weight: 30.0, weight_mode: 'custom', + excluded_assessment_ids: [a2.id], + assessment_weights: [ + { assessment_id: a1.id, weight: 30.0 }, + { assessment_id: a2.id, weight: 20.0 } + ] + ] + ) + expect(excluded?(a1)).to eq(false) + expect(excluded?(a2)).to eq(true) + expect(assessment_weight(a2)).to eq(20.0) # retained for restore, not zeroed + end + + it 'skips the custom balance check when every assessment is excluded' do + expect do + described_class.bulk_update( + course: course, + updates: [ + tab_id: tab.id, weight: 30.0, weight_mode: 'custom', + excluded_assessment_ids: [a1.id, a2.id], + assessment_weights: [ + { assessment_id: a1.id, weight: 0.0 }, + { assessment_id: a2.id, weight: 0.0 } + ] + ] + ) + end.not_to raise_error + end + + it 're-including a previously excluded assessment clears the flag' do + create(:course_gradebook_assessment_contribution, assessment: a1, excluded: true) + described_class.bulk_update( + course: course, + updates: [tab_id: tab.id, weight: 50.0, weight_mode: 'equal', excluded_assessment_ids: []] + ) + expect(excluded?(a1)).to eq(false) + end + end + end + end +end diff --git a/spec/models/course/gradebook_ability_spec.rb b/spec/models/course/gradebook_ability_spec.rb new file mode 100644 index 00000000000..7247c2084c8 --- /dev/null +++ b/spec/models/course/gradebook_ability_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::GradebookAbilityComponent do + let!(:instance) { Instance.default } + with_tenant(:instance) do + subject { Ability.new(user, course, course_user) } + let(:course) { create(:course) } + + context 'when the user is a Course Manager' do + let(:course_user) { create(:course_manager, course: course) } + let(:user) { course_user.user } + it { is_expected.to be_able_to(:read_gradebook, course) } + it { is_expected.to be_able_to(:manage_gradebook_weights, course) } + it { is_expected.to be_able_to(:manage_gradebook_settings, course) } + end + + context 'when the user is a Course Owner' do + let(:course_user) { create(:course_owner, course: course) } + let(:user) { course_user.user } + it { is_expected.to be_able_to(:read_gradebook, course) } + it { is_expected.to be_able_to(:manage_gradebook_weights, course) } + it { is_expected.to be_able_to(:manage_gradebook_settings, course) } + end + + context 'when the user is a Teaching Assistant' do + let(:course_user) { create(:course_teaching_assistant, course: course) } + let(:user) { course_user.user } + it { is_expected.to be_able_to(:read_gradebook, course) } + it { is_expected.not_to be_able_to(:manage_gradebook_weights, course) } + it { is_expected.not_to be_able_to(:manage_gradebook_settings, course) } + end + + context 'when the user is a Course Student' do + let(:course_user) { create(:course_student, course: course) } + let(:user) { course_user.user } + it { is_expected.not_to be_able_to(:read_gradebook, course) } + it { is_expected.not_to be_able_to(:manage_gradebook_weights, course) } + it { is_expected.not_to be_able_to(:manage_gradebook_settings, course) } + end + end +end diff --git a/spec/models/course/settings/gradebook_component_spec.rb b/spec/models/course/settings/gradebook_component_spec.rb new file mode 100644 index 00000000000..51e602521c5 --- /dev/null +++ b/spec/models/course/settings/gradebook_component_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::Settings::GradebookComponent do + let!(:instance) { Instance.default } + with_tenant(:instance) do + let(:course) { create(:course) } + let(:settings) do + context = OpenStruct.new(current_course: course, key: Course::GradebookComponent.key) + Course::Settings::GradebookComponent.new(context) + end + + describe '#weighted_view_enabled' do + it 'returns false by default' do + expect(settings.weighted_view_enabled).to eq(false) + end + end + + describe '#weighted_view_enabled=' do + it 'persists true when set to true' do + settings.weighted_view_enabled = true + course.save! + expect(settings.weighted_view_enabled).to eq(true) + end + + it 'persists false when set to false after being true' do + settings.weighted_view_enabled = true + course.save! + settings.weighted_view_enabled = false + course.save! + expect(settings.weighted_view_enabled).to eq(false) + end + + it 'handles string "1" as truthy' do + settings.weighted_view_enabled = '1' + expect(settings.weighted_view_enabled).to eq(true) + end + + it 'handles string "0" as falsy' do + settings.weighted_view_enabled = '0' + expect(settings.weighted_view_enabled).to eq(false) + end + end + end +end From 02def0fca128234cd6a3d862f58d5dce93b99525 Mon Sep 17 00:00:00 2001 From: lws49 Date: Fri, 12 Jun 2026 15:31:18 +0800 Subject: [PATCH 3/4] feat(gradebook): pin the audited student's summary row when expanded Expanding a student in the weighted gradebook now keeps that student's summary row pinned beneath the sticky header while their per-assessment breakdown is open, so the totals stay visible while scrolling the detail. The view becomes a single-open accordion (opening one student collapses any other), the focused row auto-scrolls to just under the header on expand (respecting prefers-reduced-motion), and focus stays on the expand/collapse toggle for keyboard users. --- .../__tests__/GradebookWeightedTable.test.tsx | 96 +++++ .../components/GradebookWeightedTable.tsx | 375 +++++++++++------- 2 files changed, 335 insertions(+), 136 deletions(-) diff --git a/client/app/bundles/course/gradebook/__tests__/GradebookWeightedTable.test.tsx b/client/app/bundles/course/gradebook/__tests__/GradebookWeightedTable.test.tsx index 9bc42a310ed..bac7a976f1f 100644 --- a/client/app/bundles/course/gradebook/__tests__/GradebookWeightedTable.test.tsx +++ b/client/app/bundles/course/gradebook/__tests__/GradebookWeightedTable.test.tsx @@ -1116,4 +1116,100 @@ describe('GradebookWeightedTable', () => { expect(cells[cells.length - 1]).toHaveTextContent('90%'); }); }); + + describe('single-open accordion', () => { + it('keeps only one student expanded at a time (opening another collapses the first)', async () => { + const user = userEvent.setup(); + renderWeighted({ + tabs: [makeTab(10, 'Missions', 1, 100)], + assessments: [makeAssessment(1, 'Mission 1', 10, 100)], + students: [makeStudent(1, 'Alice'), makeStudent(2, 'Bob')], + submissions: [makeSub(1, 1, 80), makeSub(2, 1, 60)], + }); + + await user.click(screen.getByRole('button', { name: /expand Alice/i })); + expect( + await screen.findByTestId(breakdownRowId(1, 10, 1)), + ).toBeInTheDocument(); + + // Opening Bob must collapse Alice. + await user.click(screen.getByRole('button', { name: /expand Bob/i })); + expect( + await screen.findByTestId(breakdownRowId(2, 10, 1)), + ).toBeInTheDocument(); + await waitFor(() => + expect( + screen.queryByTestId(breakdownRowId(1, 10, 1)), + ).not.toBeInTheDocument(), + ); + }); + + it('keeps focus on the toggle after expanding (keyboard users stay put)', async () => { + const user = userEvent.setup(); + renderWeighted({ + tabs: [makeTab(10, 'Missions', 1, 100)], + assessments: [makeAssessment(1, 'Mission 1', 10, 100)], + students: [makeStudent(1, 'Alice')], + submissions: [makeSub(1, 1, 80)], + }); + const toggle = screen.getByRole('button', { name: /expand Alice/i }); + await user.click(toggle); + // Same DOM button (label flips expand→collapse), so focus is retained. + expect( + screen.getByRole('button', { name: /collapse Alice/i }), + ).toHaveFocus(); + }); + }); + + describe('auto-scroll on expand', () => { + const one = { + tabs: [makeTab(10, 'Missions', 1, 100)], + assessments: [makeAssessment(1, 'Mission 1', 10, 100)], + students: [makeStudent(1, 'Alice')], + submissions: [makeSub(1, 1, 80)], + }; + + afterEach(() => { + // Restore the scrollTo we stub in (jsdom doesn't implement it). + delete (Element.prototype as unknown as { scrollTo?: unknown }).scrollTo; + }); + + it('smooth-scrolls the expanded row into view by default', async () => { + const scrollSpy = jest.fn(); + (Element.prototype as unknown as { scrollTo: unknown }).scrollTo = + scrollSpy; + const user = userEvent.setup(); + renderWeighted(one); + await user.click(screen.getByRole('button', { name: /expand Alice/i })); + await waitFor(() => expect(scrollSpy).toHaveBeenCalled()); + expect(scrollSpy.mock.calls[0][0]).toMatchObject({ behavior: 'smooth' }); + }); + + it('jumps instantly when the user prefers reduced motion', async () => { + const scrollSpy = jest.fn(); + (Element.prototype as unknown as { scrollTo: unknown }).scrollTo = + scrollSpy; + const originalMatchMedia = window.matchMedia; + window.matchMedia = ((query: string) => ({ + matches: query === '(prefers-reduced-motion: reduce)', + media: query, + onchange: null, + addEventListener: (): void => {}, + removeEventListener: (): void => {}, + addListener: (): void => {}, + removeListener: (): void => {}, + dispatchEvent: (): boolean => false, + })) as unknown as typeof window.matchMedia; + + try { + const user = userEvent.setup(); + renderWeighted(one); + await user.click(screen.getByRole('button', { name: /expand Alice/i })); + await waitFor(() => expect(scrollSpy).toHaveBeenCalled()); + expect(scrollSpy.mock.calls[0][0]).toMatchObject({ behavior: 'auto' }); + } finally { + window.matchMedia = originalMatchMedia; + } + }); + }); }); diff --git a/client/app/bundles/course/gradebook/components/GradebookWeightedTable.tsx b/client/app/bundles/course/gradebook/components/GradebookWeightedTable.tsx index 671a6c7fdbe..c3e8e0cef84 100644 --- a/client/app/bundles/course/gradebook/components/GradebookWeightedTable.tsx +++ b/client/app/bundles/course/gradebook/components/GradebookWeightedTable.tsx @@ -21,6 +21,8 @@ import { Tooltip, Typography, } from '@mui/material'; +import type { Theme } from '@mui/material/styles'; +import { lighten } from '@mui/material/styles'; import type { AssessmentData, CategoryData, @@ -40,7 +42,11 @@ import { DEFAULT_TABLE_ROWS_PER_PAGE } from 'lib/constants/sharedConstants'; import useTranslation from 'lib/hooks/useTranslation'; import tableTranslations from 'lib/translations/table'; -import type { AssessmentContribution, WeightedRow } from '../computeWeighted'; +import type { + AssessmentContribution, + TabBreakdown, + WeightedRow, +} from '../computeWeighted'; import { computeStudentBreakdown, computeWeightedRows, @@ -290,35 +296,59 @@ const GradebookWeightedTable = ({ return a.points; }; - const [expandedIds, setExpandedIds] = useState>(new Set()); + // Single-open accordion: auditing one student at a time. Replaces the former + // multi-expand Set so only the focused student's breakdown is on screen (and + // only one summary row ever pins under the header). + const [expandedId, setExpandedId] = useState(null); const toggleExpanded = (studentId: number): void => - setExpandedIds((prev) => { - const next = new Set(prev); - if (next.has(studentId)) next.delete(studentId); - else next.add(studentId); - return next; - }); + setExpandedId((prev) => (prev === studentId ? null : studentId)); - const breakdownsByStudent = useMemo( + const expandedBreakdown = useMemo( () => - new Map( - [...expandedIds].map((studentId) => [ - studentId, - computeStudentBreakdown({ - studentId, + expandedId === null + ? null + : computeStudentBreakdown({ + studentId: expandedId, tabs: resolvedTabs, assessments, submissions, }), - ]), - ), - [expandedIds, resolvedTabs, assessments, submissions], + [expandedId, resolvedTabs, assessments, submissions], ); + const containerRef = useRef(null); + const activeRowRef = useRef(null); const row1Ref = useRef(null); const row2Ref = useRef(null); + const row3Ref = useRef(null); const [row2Top, setRow2Top] = useState(0); const [row3Top, setRow3Top] = useState(0); + // Full header height = where the pinned summary row sticks, and the scroll + // offset that lands a focused row just beneath the header. + const [headerHeight, setHeaderHeight] = useState(0); + + // sx for a cell of the focused (expanded) student's summary row. `variant` + // selects the layering: 'checkbox' and 'name' are corner-sticky (top + the + // left freeze they already carry) and sit above the body's frozen column; + // 'data' cells pin on top only. The checkbox cell also carries the left + // accent bar that marks "this is the student you're auditing". + const pinnedCellSx = + (variant: 'checkbox' | 'name' | 'data') => + (theme: Theme): Record => { + const isLead = variant !== 'data'; + return { + position: 'sticky', + top: headerHeight, + zIndex: isLead ? 6 : 3, + // Opaque tint (not alpha) so scrolled body content can't bleed through + // the sticky cell. + backgroundColor: lighten(theme.palette.primary.light, 0.96), + // The checkbox cell carries the left accent bar marking the audited row. + ...(variant === 'checkbox' && { + boxShadow: `inset 3px 0 0 ${theme.palette.primary.main}`, + }), + }; + }; // Row-3 subheader for a tab: "Excluded" when the tab contributes nothing, // else the weight in the active lens ("/{w}" points / "{w}% of grade"). @@ -346,25 +376,54 @@ const GradebookWeightedTable = ({ useLayoutEffect(() => { const row1 = row1Ref.current; const row2 = row2Ref.current; - if (!row1 || !row2) return undefined; + const row3 = row3Ref.current; + if (!row1 || !row2 || !row3) return undefined; // Re-measure on every header-row resize, not just on mount. Expanding or // collapsing a row, switching display mode and showing/hiding columns all // reflow the header after mount; with a one-shot measurement rows 2–3 keep // a stale `top` and stay permanently dislodged from the rows above them. const measure = (): void => { - const h1 = row1.offsetHeight; + // getBoundingClientRect (subpixel) not offsetHeight (integer-rounded): + // rows are fractional (32px min + lineHeight content), and a rounded + // `top` lands the stuck rows 2-3 a fraction off — opening thin gaps + // between header rows (body bleeds through) and overshooting the single + // rowSpan=3 frozen-left cell so the right header reads a touch taller. + const h1 = row1.getBoundingClientRect().height; + const h2 = row2.getBoundingClientRect().height; + const h3 = row3.getBoundingClientRect().height; setRow2Top(h1); - setRow3Top(h1 + row2.offsetHeight); + setRow3Top(h1 + h2); + setHeaderHeight(h1 + h2 + h3); }; measure(); const observer = new ResizeObserver(measure); observer.observe(row1); observer.observe(row2); + observer.observe(row3); return () => observer.disconnect(); }, [visibleCategories, resolvedTabs]); + // On expand, glide the focused row to just beneath the header so its + // breakdown is guaranteed in view (even when the clicked row was near the + // bottom). getBoundingClientRect keeps this correct regardless of the row's + // offsetParent; scrollTo is optional-chained because jsdom lacks it. + useLayoutEffect(() => { + if (expandedId === null) return; + const container = containerRef.current; + const rowEl = activeRowRef.current; + if (!container || !rowEl) return; + const prefersReducedMotion = + window.matchMedia?.('(prefers-reduced-motion: reduce)').matches ?? false; + const delta = + rowEl.getBoundingClientRect().top - container.getBoundingClientRect().top; + container.scrollTo?.({ + top: container.scrollTop + delta - headerHeight, + behavior: prefersReducedMotion ? 'auto' : 'smooth', + }); + }, [expandedId, headerHeight]); + const rows = useMemo( () => computeWeightedRows({ @@ -622,6 +681,7 @@ const GradebookWeightedTable = ({ toolbar — plus the pagination below, so the table fills the remaining viewport; shorter classes shrink to fit (no whitespace). */} too — dropping the table's bottom edge and + // that row's right edge (the open bottom-right corner). The + // separator ABOVE it is owned by the previous row's borderBottom + // (that row isn't last-child, so it keeps it), so only the bottom + // and right edges need restoring here. Same (0,2,3) > (0,2,1). + '& tbody tr:last-of-type td': { + borderBottom: gridLine, + borderRight: gridLine, + }, + // The two frozen columns (checkbox + Name) are each their own + // sticky compositing layer; when scrolled, the next row's + // opaque sticky bg paints over the cell's borderBottom and the + // horizontal line vanishes in those columns only. Mirror the + // header fix (CLAUDE-tables.md): drop the coverable borderBottom + // and draw each separator as the lower cell's borderTop, which + // its own layer owns and always paints. Row 1's top line stays + // owned by the sticky header above it, so only rows 2+ get the + // borderTop — else both borders show at rest and read 2px thick. + '& tbody td:first-of-type, & tbody td:nth-of-type(2)': { + borderBottom: 'none', + }, + '& tbody tr:not(:first-of-type) td:first-of-type, & tbody tr:not(:first-of-type) td:nth-of-type(2)': + { + borderTop: gridLine, + }, }; }} > @@ -803,6 +889,7 @@ const GradebookWeightedTable = ({ {/* Row 3: Weight subheaders */} {resolvedTabs.map((tab) => ( @@ -842,10 +929,10 @@ const GradebookWeightedTable = ({ {body.rows.map((row, idx) => { const rowProps = body.forEachRow(row, idx); const studentId = row.original.studentId; - const isExpanded = expandedIds.has(studentId); + const isExpanded = expandedId === studentId; return ( - + {/* Body sticky-left cells sit at zIndex 1 — strictly below the header's sticky cells (MUI gives every stickyHeader cell zIndex 2). On a z-index tie the cell @@ -854,16 +941,19 @@ const GradebookWeightedTable = ({ column the frozen Name cell (z4) doesn't cover — i.e. the identity columns once they're toggled on. */} {showEmail && ( - {row.original.email} + + {row.original.email} + )} {showExternalId && ( - {row.original.externalId ?? ''} + + {row.original.externalId ?? ''} + )} {row.original.subtotals.map((subtotal, i) => { const weight = resolvedTabs[i].gradebookWeight ?? 0; return ( - + {fmtDisplay( tabDisplayValue(subtotal, weight), columnPrecisions.tabs[i], @@ -918,7 +1023,10 @@ const GradebookWeightedTable = ({ ); })} - + {fmtDisplay( totalDisplayValue(row.original.total), columnPrecisions.total, @@ -926,47 +1034,42 @@ const GradebookWeightedTable = ({ {isExpanded && - (breakdownsByStudent.get(studentId) ?? []).flatMap( - (tb, tabIdx) => - tb.assessments.map((a) => { - const isExcluded = a.excluded; - // Weightage is always "% of grade" — it never - // follows the points/percent lens. - const weightText = t( - translations.percentOfGrade, - { - weight: - Math.round(a.effectiveWeight * 100) / 100, - }, - ); - const gradeText = - a.grade === null - ? `—/${a.maxGrade}` - : `${a.grade}/${a.maxGrade}`; - return ( - + tb.assessments.map((a) => { + const isExcluded = a.excluded; + // Weightage is always "% of grade" — it never + // follows the points/percent lens. + const weightText = t(translations.percentOfGrade, { + weight: Math.round(a.effectiveWeight * 100) / 100, + }); + const gradeText = + a.grade === null + ? `—/${a.maxGrade}` + : `${a.grade}/${a.maxGrade}`; + return ( + + {/* Empty checkbox cell so the breakdown row + carries the same checkbox | name divider (the + universal cell borderRight) as the rows above. */} + - {/* Empty checkbox cell so the breakdown row - carries the same checkbox | name divider (the - universal cell borderRight) as the rows above. */} - - {/* Title over a muted "raw mark · weightage" + /> + {/* Title over a muted "raw mark · weightage" subtitle, stacked and confined to the (sticky) Name column. The breakdown row freezes the same checkbox | Name region as the student rows above — @@ -976,74 +1079,74 @@ const GradebookWeightedTable = ({ indent sits the title under the student name (past the expand chevron), signalling these are that student's assessments. */} - - {/* nowrap keeps the title on one line: its + + {/* nowrap keeps the title on one line: its max-content width then drives the table's auto layout, expanding the (frozen) Name column to fit the longest title. With the metadata line also nowrap, every breakdown row is exactly 2 lines — no fixed widths, no JS measurement. */} - - {a.title} - - {/* Muted metadata on its own line below the + + {a.title} + + {/* Muted metadata on its own line below the title: raw mark · effective weightage, kept on one line (nowrap). Weightage is always "% of grade" — never routed through the points/percent lens. */} - - {`${gradeText} · ${isExcluded ? t(translations.excluded) : weightText}`} - - - {/* One empty cell per visible identity column so + + {`${gradeText} · ${isExcluded ? t(translations.excluded) : weightText}`} + + + {/* One empty cell per visible identity column so the grid lines stay aligned with the rows above. These scroll with the table (only checkbox + Name are frozen), matching the student rows. */} - {showEmail && } - {showExternalId && } - {resolvedTabs.map((tab, i) => { - const tabCellValue = isExcluded - ? '—' - : fmtDisplay( - breakdownDisplayValue(a), - columnPrecisions.tabs[i], - ); - return ( - - {i === tabIdx ? tabCellValue : ''} - - ); - })} - - - ); - }), + {showEmail && } + {showExternalId && } + {resolvedTabs.map((tab, i) => { + const tabCellValue = isExcluded + ? '—' + : fmtDisplay( + breakdownDisplayValue(a), + columnPrecisions.tabs[i], + ); + return ( + + {i === tabIdx ? tabCellValue : ''} + + ); + })} + + + ); + }), )} ); From 07a42158d2237c052f995dfcf77c3268ba53150a Mon Sep 17 00:00:00 2001 From: lws49 Date: Tue, 16 Jun 2026 16:36:54 +0800 Subject: [PATCH 4/4] feat(gradebook): wire keepHighest through backend for keep-N feature - Permit keepHighest in update_weights_params - Parse keepHighest from request and pass to bulk_update via keep_highest - Echo keepHighest in serialize_weight_updates response - Serialize keepHighest from contribution in index.json.jbuilder - Add controller tests: round-trip persist and index response - Add model tests: bulk_update persists keep_highest in equal mode --- .../course/gradebook_controller.rb | 4 +- .../course/gradebook/index.json.jbuilder | 1 + .../__tests__/ConfigureWeightsPrompt.test.tsx | 189 ++++++++++++++++ .../__tests__/GradebookWeightedTable.test.tsx | 45 ++++ .../__tests__/computeWeighted.test.ts | 199 +++++++++++++++++ .../course/gradebook/__tests__/store.test.ts | 67 ++++++ .../components/ConfigureWeightsPrompt.tsx | 205 +++++++++++++++++- .../components/GradebookWeightedTable.tsx | 18 +- .../course/gradebook/computeWeighted.ts | 53 ++++- client/app/bundles/course/gradebook/store.ts | 2 + client/app/types/course/gradebook.ts | 2 + client/locales/en.json | 3 + db/schema.rb | 3 +- .../course/gradebook_controller_spec.rb | 30 +++ .../course/gradebook/contribution_spec.rb | 23 ++ 15 files changed, 820 insertions(+), 24 deletions(-) diff --git a/app/controllers/course/gradebook_controller.rb b/app/controllers/course/gradebook_controller.rb index b5ef6c71c2d..e23e7c0a819 100644 --- a/app/controllers/course/gradebook_controller.rb +++ b/app/controllers/course/gradebook_controller.rb @@ -47,6 +47,7 @@ def parse_weight_entry(entry) tab_id: entry[:tabId].to_i, weight: entry[:weight].to_f, weight_mode: entry[:weightMode] || 'equal', + keep_highest: entry[:keepHighest].to_i, excluded_assessment_ids: (entry[:excludedAssessmentIds] || []).map(&:to_i), assessment_weights: (entry[:assessmentWeights] || []).map do |aw| { assessment_id: aw[:assessmentId].to_i, weight: aw[:weight].to_f } @@ -56,7 +57,7 @@ def parse_weight_entry(entry) def update_weights_params params.permit( - weights: [:tabId, :weight, :weightMode, + weights: [:tabId, :weight, :weightMode, :keepHighest, excludedAssessmentIds: [], assessmentWeights: [:assessmentId, :weight]] ) end @@ -64,6 +65,7 @@ def update_weights_params def serialize_weight_updates(updates) updates.map do |u| entry = { tabId: u[:tab_id], weight: u[:weight], weightMode: u[:weight_mode].to_s, + keepHighest: u[:keep_highest], excludedAssessmentIds: u[:excluded_assessment_ids] } if u[:weight_mode].to_s == 'custom' entry[:assessmentWeights] = u[:assessment_weights].map do |aw| diff --git a/app/views/course/gradebook/index.json.jbuilder b/app/views/course/gradebook/index.json.jbuilder index b3cc4ec4d02..9f306850960 100644 --- a/app/views/course/gradebook/index.json.jbuilder +++ b/app/views/course/gradebook/index.json.jbuilder @@ -15,6 +15,7 @@ json.tabs @tabs do |tab| contribution = @tab_contributions[tab.id] json.gradebookWeight (contribution&.weight || 0).to_f json.weightMode(contribution&.weight_mode || 'equal') + json.keepHighest(contribution&.keep_highest || 0) end end diff --git a/client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx b/client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx index cce652258c0..3ea24d01d63 100644 --- a/client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx +++ b/client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx @@ -147,6 +147,7 @@ describe('', () => { tabId: 10, weight: 50, weightMode: 'custom', + keepHighest: 0, excludedAssessmentIds: [], assessmentWeights: [ { assessmentId: 101, weight: 25 }, @@ -157,6 +158,7 @@ describe('', () => { tabId: 11, weight: 50, weightMode: 'equal', + keepHighest: 0, excludedAssessmentIds: [], }, ]); @@ -404,6 +406,7 @@ describe('per-assessment exclusion', () => { expect(arg[0]).toMatchObject({ tabId: 10, weight: 50, + keepHighest: 0, excludedAssessmentIds: [101, 102], }); }); @@ -477,4 +480,190 @@ describe('per-assessment exclusion', () => { expect(screen.queryByText(/no weights set yet/i)).not.toBeInTheDocument(); }); }); + + it('shows assessment weights sum footer in custom mode', () => { + setup(); + fireEvent.click( + within(modeGroup('Assignments')).getByRole('radio', { name: /custom/i }), + ); + // Expand to see the footer + // custom mode auto-expands, so just check the footer + expect(screen.getByText(/Assessment weights:/i)).toBeInTheDocument(); + }); +}); + +describe('per-assessment exclusion (extended)', () => { + beforeEach(() => jest.clearAllMocks()); + + it('shows "Excluded" label in custom mode for excluded assessment', async () => { + setup({ + assessments: [ + { + id: 101, + title: A1, + tabId: 10, + maxGrade: 100, + gradebookExcluded: true, + }, + { id: 102, title: A2, tabId: 10, maxGrade: 50 }, + ], + tabs: [ + { + id: 10, + title: 'Assignments', + categoryId: 1, + gradebookWeight: 50, + weightMode: 'custom', + }, + ], + }); + fireEvent.click(screen.getAllByRole('button', { name: '' })[0]); + expect(await screen.findByText('Excluded')).toBeInTheDocument(); + }); + + it('does not show "Excluded" label in equal mode for excluded assessment', async () => { + setup({ + assessments: [ + { + id: 101, + title: A1, + tabId: 10, + maxGrade: 100, + gradebookExcluded: true, + }, + { id: 102, title: A2, tabId: 10, maxGrade: 50 }, + ], + }); + fireEvent.click(screen.getAllByRole('button', { name: '' })[0]); + // In equal mode excluded shows "Excluded" text too + expect(await screen.findByText('Excluded')).toBeInTheDocument(); + }); +}); + +describe('keep-highest control', () => { + const TOGGLE = 'Enable keep highest for Assignments'; + const INPUT = 'Keep highest for Assignments'; + const three = [ + { id: 101, title: A1, tabId: 10, maxGrade: 100 }, + { id: 102, title: A2, tabId: 10, maxGrade: 50 }, + { id: 103, title: 'Assignment 3', tabId: 10, maxGrade: 80 }, + ]; + + beforeEach(() => jest.clearAllMocks()); + + it('renders a keep-highest checkbox; number field hidden until checked', () => { + setup({ assessments: three }); + expect(screen.getByRole('checkbox', { name: TOGGLE })).toBeInTheDocument(); + expect( + screen.queryByRole('spinbutton', { name: INPUT }), + ).not.toBeInTheDocument(); + fireEvent.click(screen.getByRole('checkbox', { name: TOGGLE })); + expect(screen.getByRole('spinbutton', { name: INPUT })).toBeInTheDocument(); + }); + + it('shows a visible "Keep highest" text label next to the checkbox', () => { + setup({ assessments: three }); + expect(screen.getByText('Keep highest')).toBeInTheDocument(); + }); + + it('defaults the count to included − 1 when checked', () => { + setup({ assessments: three }); // 3 assessments -> default to 2 + fireEvent.click(screen.getByRole('checkbox', { name: TOGGLE })); + expect(screen.getByRole('spinbutton', { name: INPUT })).toHaveValue(2); + }); + + it('hides checkbox + field in custom mode', () => { + setup({ assessments: three }); + fireEvent.click( + within(modeGroup('Assignments')).getByRole('radio', { name: /custom/i }), + ); + expect( + screen.queryByRole('checkbox', { name: TOGGLE }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole('spinbutton', { name: INPUT }), + ).not.toBeInTheDocument(); + }); + + it('seeds the field (checkbox pre-checked) from tab.keepHighest', () => { + setup({ + assessments: three, + tabs: [ + { + id: 10, + title: 'Assignments', + categoryId: 1, + gradebookWeight: 50, + keepHighest: 2, + }, + { id: 11, title: 'Optional', categoryId: 1, gradebookWeight: 50 }, + ], + }); + expect(screen.getByRole('checkbox', { name: TOGGLE })).toBeChecked(); + expect(screen.getByRole('spinbutton', { name: INPUT })).toHaveValue(2); + }); + + it('disables the checkbox when only one assessment is included', () => { + setup({ + assessments: [{ id: 101, title: A1, tabId: 10, maxGrade: 100 }], + }); + expect(screen.getByRole('checkbox', { name: TOGGLE })).toBeDisabled(); + }); + + it('sends keepHighest in the save payload', async () => { + setup({ assessments: three }); + fireEvent.click(screen.getByRole('checkbox', { name: TOGGLE })); + fireEvent.click(screen.getByRole('button', { name: /save/i })); + await waitFor(() => + expect(operations.updateGradebookWeights).toHaveBeenCalled(), + ); + const arg = (operations.updateGradebookWeights as jest.Mock).mock + .calls[0][0]; + const tab10 = arg.find((e: { tabId: number }) => e.tabId === 10); + expect(tab10.keepHighest).toBe(2); + }); + + it('unchecking sends keepHighest 0', async () => { + setup({ + assessments: three, + tabs: [ + { + id: 10, + title: 'Assignments', + categoryId: 1, + gradebookWeight: 50, + keepHighest: 2, + }, + { id: 11, title: 'Optional', categoryId: 1, gradebookWeight: 50 }, + ], + }); + // checkbox is pre-checked; uncheck it + fireEvent.click(screen.getByRole('checkbox', { name: TOGGLE })); + fireEvent.click(screen.getByRole('button', { name: /save/i })); + await waitFor(() => + expect(operations.updateGradebookWeights).toHaveBeenCalled(), + ); + const arg = (operations.updateGradebookWeights as jest.Mock).mock + .calls[0][0]; + const tab10 = arg.find((e: { tabId: number }) => e.tabId === 10); + expect(tab10.keepHighest).toBe(0); + }); + + it('blocks saving on non-integer input but not on keep > included (overflow)', async () => { + setup({ assessments: three }); + fireEvent.click(screen.getByRole('checkbox', { name: TOGGLE })); + const input = screen.getByRole('spinbutton', { name: INPUT }); + + // overflow: keep=5 > included=3 -> warning but save NOT blocked + fireEvent.change(input, { target: { value: '5' } }); + expect(screen.getByRole('button', { name: /save/i })).not.toBeDisabled(); + expect( + screen.getByText(/keeps more assessments than it contains/i), + ).toBeInTheDocument(); + + // non-integer (0): should block saving + fireEvent.change(input, { target: { value: '0' } }); + expect(screen.getByRole('button', { name: /save/i })).toBeDisabled(); + expect(screen.getByText(/keep at least 1/i)).toBeInTheDocument(); + }); }); diff --git a/client/app/bundles/course/gradebook/__tests__/GradebookWeightedTable.test.tsx b/client/app/bundles/course/gradebook/__tests__/GradebookWeightedTable.test.tsx index bac7a976f1f..7fd58858549 100644 --- a/client/app/bundles/course/gradebook/__tests__/GradebookWeightedTable.test.tsx +++ b/client/app/bundles/course/gradebook/__tests__/GradebookWeightedTable.test.tsx @@ -1212,4 +1212,49 @@ describe('GradebookWeightedTable', () => { } }); }); + + describe('drop-lowest breakdown marker', () => { + // One equal tab, keepHighest=1, three graded assessments. The lowest (a1) drops. + const dropConfig = { + tabs: [{ ...makeTab(10, 'Missions', 1, 60), keepHighest: 1 }], + assessments: [ + makeAssessment(1, 'Mission 1', 10, 100), + makeAssessment(2, 'Mission 2', 10, 100), + makeAssessment(3, 'Mission 3', 10, 100), + ], + students: [makeStudent(1, 'Alice')], + submissions: [makeSub(1, 1, 30), makeSub(1, 2, 60), makeSub(1, 3, 90)], + }; + + it('labels the dropped assessment "Dropped (lowest)" (not "Excluded")', async () => { + const user = userEvent.setup(); + renderWeighted(dropConfig); + await user.click(screen.getByRole('button', { name: /expand Alice/i })); + const detail = await screen.findByTestId(breakdownRowId(1, 10, 1)); + expect( + within(detail).getByText(/Dropped \(lowest\)/), + ).toBeInTheDocument(); + expect(within(detail).queryByText(/Excluded/i)).not.toBeInTheDocument(); + }); + + it('shows 0 (not —) in the dropped assessment cell in points mode', async () => { + const user = userEvent.setup(); + renderWeighted(dropConfig); + await user.click(screen.getByRole('button', { name: /expand Alice/i })); + const detail = await screen.findByTestId(breakdownRowId(1, 10, 1)); + // Dropped contributes 0 points — distinct from excluded's em dash. + expect(within(detail).getByText('0')).toBeInTheDocument(); + expect(within(detail).queryByText('—')).not.toBeInTheDocument(); + }); + + it("shows the dropped assessment's own grade % in percentage mode", async () => { + const user = userEvent.setup(); + renderWeighted(dropConfig); + await user.click(screen.getByRole('radio', { name: /percentage/i })); + await user.click(screen.getByRole('button', { name: /expand Alice/i })); + const detail = await screen.findByTestId(breakdownRowId(1, 10, 1)); + // a1 = 30/100 = 30% — visible so the instructor sees the dropped grade. + expect(within(detail).getByText('30%')).toBeInTheDocument(); + }); + }); }); diff --git a/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts b/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts index c4f2ed866fa..dcd7cce5f68 100644 --- a/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts +++ b/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts @@ -686,3 +686,202 @@ describe('breakdown — exclusion', () => { expect(a.points).toBeCloseTo(60); }); }); + +// --------------------------------------------------------------------------- +// keep-N (keepHighest) — equal mode only +// --------------------------------------------------------------------------- + +// Three equal-mode assessments: a1=60/100=0.60, a2=80/100=0.80, a3=100/100=1.00 +// keepHighest:1 → keep top 1 (a3, ratio 1.0), drop a1 and a2 +const dropConfig = { + tab: { + id: 10, + title: 'Tab', + categoryId: 0, + gradebookWeight: 60, + keepHighest: 1, + }, + assessments: [ + { id: 1, tabId: 10, maxGrade: 100, title: 'A' }, + { id: 2, tabId: 10, maxGrade: 100, title: 'B' }, + { id: 3, tabId: 10, maxGrade: 100, title: 'C' }, + ], + submissions: subs([ + { studentId: 1, assessmentId: 1, grade: 60 }, + { studentId: 1, assessmentId: 2, grade: 80 }, + { studentId: 1, assessmentId: 3, grade: 100 }, + ]), +}; + +describe('equalSubtotal — keepHighest via computeTabSubtotal', () => { + it('keepHighest=0 (unset): keeps all assessments — same as default behavior', () => { + // (0.6 + 0.8 + 1.0) / 3 = 0.8 + expect( + computeTabSubtotal({ + studentId: 1, + tab: { id: 10, title: 'Tab', categoryId: 0, gradebookWeight: 60 }, + assessments: dropConfig.assessments, + submissions: dropConfig.submissions, + }), + ).toBeCloseTo(0.8); + }); + + it('keepHighest=1: averages only the single highest ratio', () => { + // top 1: a3 ratio=1.0 → subtotal = 1.0/1 = 1.0 + expect( + computeTabSubtotal({ + studentId: 1, + tab: dropConfig.tab, + assessments: dropConfig.assessments, + submissions: dropConfig.submissions, + }), + ).toBeCloseTo(1.0); + }); + + it('keepHighest > count: clamps to all assessments', () => { + // keepHighest=99 with 3 assessments → keep all 3 → (0.6+0.8+1.0)/3 = 0.8 + expect( + computeTabSubtotal({ + studentId: 1, + tab: { ...dropConfig.tab, keepHighest: 99 }, + assessments: dropConfig.assessments, + submissions: dropConfig.submissions, + }), + ).toBeCloseTo(0.8); + }); +}); + +describe('computeStudentBreakdown — keepHighest', () => { + it('dropped flag is true on lowest-ratio assessments, false on kept ones', () => { + const [tab] = computeStudentBreakdown({ + studentId: 1, + tabs: [dropConfig.tab], + assessments: dropConfig.assessments, + submissions: dropConfig.submissions, + }); + const a1 = tab.assessments.find((x) => x.assessmentId === 1)!; // ratio 0.60 — dropped + const a2 = tab.assessments.find((x) => x.assessmentId === 2)!; // ratio 0.80 — dropped + const a3 = tab.assessments.find((x) => x.assessmentId === 3)!; // ratio 1.00 — kept + + expect(a1.dropped).toBe(true); + expect(a2.dropped).toBe(true); + expect(a3.dropped).toBe(false); + }); + + it('dropped assessment has points=0 and effectiveWeight=0', () => { + const [tab] = computeStudentBreakdown({ + studentId: 1, + tabs: [dropConfig.tab], + assessments: dropConfig.assessments, + submissions: dropConfig.submissions, + }); + const a1 = tab.assessments.find((x) => x.assessmentId === 1)!; + const a2 = tab.assessments.find((x) => x.assessmentId === 2)!; + + expect(a1.points).toBe(0); + expect(a1.effectiveWeight).toBe(0); + expect(a2.points).toBe(0); + expect(a2.effectiveWeight).toBe(0); + }); + + it('kept assessment points/effectiveWeight are computed over keptCount=1', () => { + // tab weight=60, keptCount=1 → effectiveWeight=60/1=60; ratio=1.0 → points=(1.0/1)*60=60 + const [tab] = computeStudentBreakdown({ + studentId: 1, + tabs: [dropConfig.tab], + assessments: dropConfig.assessments, + submissions: dropConfig.submissions, + }); + const a3 = tab.assessments.find((x) => x.assessmentId === 3)!; + + expect(a3.effectiveWeight).toBeCloseTo(60); + expect(a3.points).toBeCloseTo(60); + }); + + it('excluded=false and dropped=false on kept assessment', () => { + const [tab] = computeStudentBreakdown({ + studentId: 1, + tabs: [dropConfig.tab], + assessments: dropConfig.assessments, + submissions: dropConfig.submissions, + }); + const a3 = tab.assessments.find((x) => x.assessmentId === 3)!; + expect(a3.excluded).toBe(false); + expect(a3.dropped).toBe(false); + }); + + it('no keepHighest (0): dropped=false for all assessments', () => { + const [tab] = computeStudentBreakdown({ + studentId: 1, + tabs: [{ id: 10, title: 'Tab', categoryId: 0, gradebookWeight: 60 }], + assessments: dropConfig.assessments, + submissions: dropConfig.submissions, + }); + tab.assessments.forEach((a) => expect(a.dropped).toBe(false)); + }); + + it('tie-break: equal ratios → lower id is dropped first', () => { + // a1 (id=1) and a2 (id=2) both have ratio 0.5; keepHighest=1. + // Tie-break ascending by id → a1 is ranked lower → a1 is dropped; a2 is kept. + const tieAssessments = [ + { id: 1, tabId: 10, maxGrade: 100, title: 'A' }, + { id: 2, tabId: 10, maxGrade: 100, title: 'B' }, + ]; + const [tab] = computeStudentBreakdown({ + studentId: 1, + tabs: [ + { + id: 10, + title: 'Tab', + categoryId: 0, + gradebookWeight: 60, + keepHighest: 1, + }, + ], + assessments: tieAssessments, + submissions: subs([ + { studentId: 1, assessmentId: 1, grade: 50 }, + { studentId: 1, assessmentId: 2, grade: 50 }, + ]), + }); + const a1 = tab.assessments.find((x) => x.assessmentId === 1)!; + const a2 = tab.assessments.find((x) => x.assessmentId === 2)!; + expect(a1.dropped).toBe(true); // lower id dropped + expect(a2.dropped).toBe(false); // higher id kept + }); +}); + +describe('computeWeightedRows — keepHighest', () => { + const keepStudent = [ + { + id: 1, + name: 'Alice', + email: 'alice@e.com', + externalId: null, + level: 1, + totalXp: 0, + }, + ]; + + it('subtotal for tab with keepHighest=1 equals average of the single top ratio', () => { + // top ratio among a1,a2,a3 = 1.0; subtotal = 1.0 + const rows = computeWeightedRows({ + students: keepStudent, + tabs: [dropConfig.tab], + assessments: dropConfig.assessments, + submissions: dropConfig.submissions, + }); + expect(rows[0].subtotals[0]).toBeCloseTo(1.0); + }); + + it('total reflects keepHighest subtotal multiplied by tab weight', () => { + // subtotal=1.0, tabWeight=60 → total=60 + const rows = computeWeightedRows({ + students: keepStudent, + tabs: [dropConfig.tab], + assessments: dropConfig.assessments, + submissions: dropConfig.submissions, + }); + expect(rows[0].total).toBeCloseTo(60); + }); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/store.test.ts b/client/app/bundles/course/gradebook/__tests__/store.test.ts index 97143ade931..a1a2f1a18cf 100644 --- a/client/app/bundles/course/gradebook/__tests__/store.test.ts +++ b/client/app/bundles/course/gradebook/__tests__/store.test.ts @@ -151,3 +151,70 @@ describe('UPDATE_TAB_WEIGHTS reducer', () => { ); }); }); + +describe('UPDATE_TAB_WEIGHTS — keepHighest', () => { + it('writes keepHighest onto the matching tab', () => { + const state = { + ...baseState, + tabs: [{ id: 10, title: 'T', categoryId: 1, gradebookWeight: 50 }], + }; + const next = reducer( + state, + actions.updateTabWeights({ + weights: [{ tabId: 10, weight: 50, keepHighest: 2 }], + }), + ); + expect(next.tabs[0].keepHighest).toBe(2); + }); + + it('defaults keepHighest to 0 when omitted from the payload', () => { + const state = { + ...baseState, + tabs: [ + { + id: 10, + title: 'T', + categoryId: 1, + gradebookWeight: 50, + keepHighest: 3, + }, + ], + }; + const next = reducer( + state, + actions.updateTabWeights({ + weights: [{ tabId: 10, weight: 50 }], + }), + ); + expect(next.tabs[0].keepHighest).toBe(0); + }); + + it('does not modify keepHighest on other tabs', () => { + const state = { + ...baseState, + tabs: [ + { + id: 10, + title: 'T1', + categoryId: 1, + gradebookWeight: 50, + keepHighest: 1, + }, + { + id: 11, + title: 'T2', + categoryId: 1, + gradebookWeight: 50, + keepHighest: 2, + }, + ], + }; + const next = reducer( + state, + actions.updateTabWeights({ + weights: [{ tabId: 10, weight: 50, keepHighest: 5 }], + }), + ); + expect(next.tabs[1].keepHighest).toBe(2); + }); +}); diff --git a/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx b/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx index 0783ab32f71..75d07fd63b6 100644 --- a/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx +++ b/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx @@ -51,10 +51,36 @@ const translations = defineMessages({ defaultMessage: "Choose Equal (all assessments share the tab's weight) or Custom (set each assessment's share).", }, - descriptionDrop: { - id: 'course.gradebook.ConfigureWeightsPrompt.descriptionDrop', + keepHighestLabel: { + id: 'course.gradebook.ConfigureWeightsPrompt.keepHighestLabel', + defaultMessage: 'Keep highest', + }, + keepHighestAria: { + id: 'course.gradebook.ConfigureWeightsPrompt.keepHighestAria', + defaultMessage: 'Keep highest for {tab}', + }, + keepHighestToggleAria: { + id: 'course.gradebook.ConfigureWeightsPrompt.keepHighestToggleAria', + defaultMessage: 'Enable keep highest for {tab}', + }, + keepInvalid: { + id: 'course.gradebook.ConfigureWeightsPrompt.keepInvalid', + defaultMessage: 'Keep at least 1 (whole number)', + }, + keepSubtitle: { + id: 'course.gradebook.ConfigureWeightsPrompt.keepSubtitle', + defaultMessage: + '— keeps highest {keep} of {included} · each counts as {pct}%', + }, + keepOverflowWarning: { + id: 'course.gradebook.ConfigureWeightsPrompt.keepOverflowWarning', defaultMessage: - "In Equal mode, optionally drop each student's N lowest-scoring assessments before averaging.", + '"{tab}" keeps more assessments than it contains — all of them will count.', + }, + descriptionKeep: { + id: 'course.gradebook.ConfigureWeightsPrompt.descriptionKeep', + defaultMessage: + "In Equal mode, optionally keep only each student's N highest-scoring assessments.", }, total: { id: 'course.gradebook.ConfigureWeightsPrompt.total', @@ -194,6 +220,13 @@ const ConfigureWeightsPrompt: FC = ({ const seedExclusions = (): Record => Object.fromEntries(assessments.map((a) => [a.id, !!a.gradebookExcluded])); + const seedKeepHighest = (): Record => + Object.fromEntries(resolvedTabs.map((tb) => [tb.id, tb.keepHighest ?? 0])); + const seedKeepOn = (): Record => + Object.fromEntries( + resolvedTabs.map((tb) => [tb.id, (tb.keepHighest ?? 0) > 0]), + ); + const [weights, setWeights] = useState>(seedWeights); const [modes, setModes] = useState>(seedModes); const [assessmentWeights, setAssessmentWeights] = useState< @@ -201,6 +234,9 @@ const ConfigureWeightsPrompt: FC = ({ >(seedAssessmentWeights); const [excluded, setExcluded] = useState>(seedExclusions); + const [keepHighest, setKeepHighest] = + useState>(seedKeepHighest); + const [keepOn, setKeepOn] = useState>(seedKeepOn); const [expanded, setExpanded] = useState>({}); const [submitting, setSubmitting] = useState(false); @@ -210,6 +246,8 @@ const ConfigureWeightsPrompt: FC = ({ setModes(seedModes()); setAssessmentWeights(seedAssessmentWeights()); setExcluded(seedExclusions()); + setKeepHighest(seedKeepHighest()); + setKeepOn(seedKeepOn()); setExpanded({}); } }, [open]); @@ -239,12 +277,42 @@ const ConfigureWeightsPrompt: FC = ({ const effectiveWeight = (tabId: number): number => isAllExcluded(tabId) ? 0 : weights[tabId] ?? 0; + const validateKeep = (value: number): string | null => + Number.isInteger(value) && value >= 1 ? null : t(translations.keepInvalid); + + // Returns the effective keep-N value when the feature is active for a tab, + // or null when not applicable (mode != equal, checkbox off, or only 1 included). + const activeKeepValue = (tabId: number): number | null => { + if ((modes[tabId] ?? 'equal') !== 'equal') return null; + if (!keepOn[tabId]) return null; + return keepHighest[tabId] ?? 0; + }; + + const handleKeepChange = (tabId: number, raw: string): void => { + const parsed = raw === '' ? 0 : Number(raw); + setKeepHighest((prev) => ({ ...prev, [tabId]: parsed })); + }; + const sum = tabs.reduce((acc, tb) => acc + effectiveWeight(tb.id), 0); const hasInvalid = Object.values(weights).some((w) => validate(w) !== null) || - Object.values(assessmentWeights).some((w) => validate(w) !== null); + Object.values(assessmentWeights).some((w) => validate(w) !== null) || + tabs.some((tb) => { + if ((modes[tb.id] ?? 'equal') !== 'equal') return false; + if (!keepOn[tb.id]) return false; + const includedCount = tabIncludedIds(tb.id).length; + if (includedCount <= 1) return false; + return validateKeep(keepHighest[tb.id] ?? 0) !== null; + }); const hasUnbalanced = tabs.some((tb) => isUnbalanced(tb.id)); + const keepOverflowTabs = tabs.filter((tb) => { + const v = activeKeepValue(tb.id); + if (!v) return false; + const included = tabIncludedIds(tb.id).length; + return included > 1 && v > included; + }); + const handleChange = (tabId: number, raw: string): void => { const parsed = raw === '' ? 0 : Number(raw); setWeights((prev) => ({ ...prev, [tabId]: parsed })); @@ -289,6 +357,11 @@ const ConfigureWeightsPrompt: FC = ({ tabId: tb.id, weight: weights[tb.id] ?? 0, weightMode: mode, + keepHighest: + activeKeepValue(tb.id) !== null && + tabIncludedIds(tb.id).length > 1 + ? keepHighest[tb.id] ?? 0 + : 0, excludedAssessmentIds: tabAssessmentIds(tb.id).filter( (id) => excluded[id], ), @@ -336,6 +409,7 @@ const ConfigureWeightsPrompt: FC = ({ translations.descriptionWeights, translations.descriptionExclusion, translations.descriptionModes, + translations.descriptionKeep, ].map((key) => ( = ({ const excludedCount = tabAssessments.length - includedCount; const pct = includedCount > 0 ? r2(value / includedCount) : 0; + // keep-highest vars + const keepValue = keepHighest[tb.id] ?? 0; + const keepDisabled = includedCount <= 1; + const keepEnabled = !keepDisabled && !!keepOn[tb.id]; + const keepErr = keepEnabled ? validateKeep(keepValue) : null; + const kept = Math.min(keepValue, includedCount); + const keepPct = kept > 0 ? r2(value / kept) : 0; + const isKeepOverflow = keepOverflowTabs.some( + (ot) => ot.id === tb.id, + ); + return (
    @@ -455,6 +540,104 @@ const ConfigureWeightsPrompt: FC = ({ {err} )} + {mode === 'equal' && !noAssessments && ( + <> +
    + { + if (keepEnabled) { + // Turn off: just toggle the on flag + setKeepOn((prev) => ({ + ...prev, + [tb.id]: false, + })); + } else { + // Turn on: seed default value if not already set + if (keepValue === 0) { + setKeepHighest((prev) => ({ + ...prev, + [tb.id]: Math.max(1, includedCount - 1), + })); + } + setKeepOn((prev) => ({ + ...prev, + [tb.id]: true, + })); + } + }} + size="small" + /> + + {t(translations.keepHighestLabel)} + + {keepEnabled && ( + + handleKeepChange(tb.id, e.target.value) + } + onKeyDown={(e) => { + if ( + ['.', ',', 'e', 'E', '-', '+'].includes( + e.key, + ) + ) + e.preventDefault(); + }} + size="small" + sx={{ width: 80, mx: 0.5 }} + type="number" + value={keepValue} + /> + )} + {keepEnabled && keepErr === null && ( + + {t(translations.keepSubtitle, { + keep: kept, + included: includedCount, + pct: keepPct.toFixed(2), + })} + + )} +
    + {keepEnabled && keepErr && ( + + {keepErr} + + )} + {isKeepOverflow && ( + + {t(translations.keepOverflowWarning, { + tab: tb.title, + })} + + )} + + )} {unbalanced && ( {t(translations.unbalanced, { tab: tb.title })} @@ -537,6 +720,14 @@ const ConfigureWeightsPrompt: FC = ({
    ); } + let caption = t(translations.ofGrade, { + pct: pct.toFixed(2), + }); + if (isExcluded) { + caption = t(translations.excluded); + } else if (keepEnabled) { + caption = '—'; + } return (
    = ({ color="text.disabled" variant="caption" > - {isExcluded - ? t(translations.excluded) - : t(translations.ofGrade, { - pct: pct.toFixed(2), - })} + {caption}
    ); diff --git a/client/app/bundles/course/gradebook/components/GradebookWeightedTable.tsx b/client/app/bundles/course/gradebook/components/GradebookWeightedTable.tsx index c3e8e0cef84..07d73b5f8df 100644 --- a/client/app/bundles/course/gradebook/components/GradebookWeightedTable.tsx +++ b/client/app/bundles/course/gradebook/components/GradebookWeightedTable.tsx @@ -166,6 +166,10 @@ const translations = defineMessages({ id: 'course.gradebook.GradebookWeightedTable.excluded', defaultMessage: 'Excluded', }, + dropped: { + id: 'course.gradebook.GradebookWeightedTable.dropped', + defaultMessage: 'Dropped (lowest)', + }, total: { id: 'course.gradebook.GradebookWeightedTable.total', defaultMessage: 'Total', @@ -1037,6 +1041,8 @@ const GradebookWeightedTable = ({ (expandedBreakdown ?? []).flatMap((tb, tabIdx) => tb.assessments.map((a) => { const isExcluded = a.excluded; + const isDropped = a.dropped; + const isInactive = isExcluded || isDropped; // Weightage is always "% of grade" — it never // follows the points/percent lens. const weightText = t(translations.percentOfGrade, { @@ -1046,13 +1052,19 @@ const GradebookWeightedTable = ({ a.grade === null ? `—/${a.maxGrade}` : `${a.grade}/${a.maxGrade}`; + let statusText = weightText; + if (isExcluded) { + statusText = t(translations.excluded); + } else if (isDropped) { + statusText = t(translations.dropped); + } return ( {/* Empty checkbox cell so the breakdown row @@ -1101,7 +1113,7 @@ const GradebookWeightedTable = ({ no fixed widths, no JS measurement. */} - {`${gradeText} · ${isExcluded ? t(translations.excluded) : weightText}`} + {`${gradeText} · ${statusText}`} {/* One empty cell per visible identity column so diff --git a/client/app/bundles/course/gradebook/computeWeighted.ts b/client/app/bundles/course/gradebook/computeWeighted.ts index d4bb0424f70..f490594d937 100644 --- a/client/app/bundles/course/gradebook/computeWeighted.ts +++ b/client/app/bundles/course/gradebook/computeWeighted.ts @@ -28,6 +28,7 @@ export interface AssessmentContribution { // Custom mode: the assessment's own configured weight. effectiveWeight: number; excluded: boolean; + dropped: boolean; // equal-mode keep-highest: ranked out for this student } export interface TabBreakdown { @@ -65,19 +66,25 @@ const buildAssessmentsByTab = ( // Equal-weight formula: average of (grade/maxGrade) ratios over INCLUDED assessments. // Excluded assessments are dropped from both numerator and count; ungraded included -// contribute 0. Returns null when no assessment is included. +// contribute 0. When keepN > 0, only the top keepN ratios are averaged. +// Returns null when no assessment is included. const equalSubtotal = ( studentId: number, + tab: TabData, tabAssessments: AssessmentData[], gradeLookup: GradeLookup, ): number | null => { const included = tabAssessments.filter((a) => !a.gradebookExcluded); if (included.length === 0) return null; + const keepN = tab.keepHighest ?? 0; const ratios = included.map((a) => { const grade = gradeLookup.get(gradeKey(studentId, a.id)); - return grade != null ? grade / a.maxGrade : 0; + return grade != null && a.maxGrade > 0 ? grade / a.maxGrade : 0; }); - return ratios.reduce((acc, r) => acc + r, 0) / ratios.length; + ratios.sort((x, y) => x - y); // ascending + const keep = keepN > 0 ? Math.min(keepN, included.length) : included.length; + const kept = ratios.slice(included.length - keep); // the `keep` highest + return kept.reduce((acc, r) => acc + r, 0) / kept.length; }; // Custom-weight formula: Σ(grade_i/maxGrade_i × assessmentWeight_i) / tabWeight over @@ -97,7 +104,8 @@ const customSubtotal = ( if (a.gradebookExcluded) return; const grade = gradeLookup.get(gradeKey(studentId, a.id)); const assessmentWeight = a.gradebookWeight ?? 0; - if (grade != null) numerator += (grade / a.maxGrade) * assessmentWeight; + if (grade != null && a.maxGrade > 0) + numerator += (grade / a.maxGrade) * assessmentWeight; hasContributing = true; }); return hasContributing ? numerator / tabWeight : null; @@ -114,7 +122,7 @@ const subtotalFromLookup = ( if (tab.weightMode === 'custom') { return customSubtotal(studentId, tab, tabAssessments, gradeLookup); } - return equalSubtotal(studentId, tabAssessments, gradeLookup); + return equalSubtotal(studentId, tab, tabAssessments, gradeLookup); }; // Weighted, additive total from already-computed subtotals. @@ -189,23 +197,47 @@ export const computeStudentBreakdown = ({ return tabs.map((tab) => { const list = assessmentsByTab.get(tab.id) ?? []; const weight = tab.gradebookWeight ?? 0; - const includedCount = list.filter((a) => !a.gradebookExcluded).length; + const included = list.filter((a) => !a.gradebookExcluded); + const includedCount = included.length; + + let droppedIds = new Set(); + let keptCount = includedCount; + if (tab.weightMode !== 'custom' && includedCount > 0) { + const keepN = tab.keepHighest ?? 0; + keptCount = keepN > 0 ? Math.min(keepN, includedCount) : includedCount; + if (keptCount < includedCount) { + const ranked = included + .map((a) => { + const grade = gradeLookup.get(gradeKey(studentId, a.id)); + return { + id: a.id, + ratio: grade != null && a.maxGrade > 0 ? grade / a.maxGrade : 0, + }; + }) + .sort((x, y) => x.ratio - y.ratio || x.id - y.id); // ascending: lowest first, tie-break by id + // Drop the lowest (includedCount − keptCount). + droppedIds = new Set( + ranked.slice(0, includedCount - keptCount).map((r) => r.id), + ); + } + } const contributions = list.map((a) => { const excluded = !!a.gradebookExcluded; + const dropped = droppedIds.has(a.id); const grade = gradeLookup.get(gradeKey(studentId, a.id)) ?? null; - const ratio = grade != null ? grade / a.maxGrade : 0; + const ratio = grade != null && a.maxGrade > 0 ? grade / a.maxGrade : 0; let points: number; let effectiveWeight: number; - if (excluded) { + if (excluded || dropped) { points = 0; effectiveWeight = 0; } else if (tab.weightMode === 'custom') { points = ratio * (a.gradebookWeight ?? 0); effectiveWeight = a.gradebookWeight ?? 0; } else { - points = includedCount > 0 ? (ratio / includedCount) * weight : 0; - effectiveWeight = includedCount > 0 ? weight / includedCount : 0; + points = keptCount > 0 ? (ratio / keptCount) * weight : 0; + effectiveWeight = keptCount > 0 ? weight / keptCount : 0; } return { assessmentId: a.id, @@ -215,6 +247,7 @@ export const computeStudentBreakdown = ({ points, effectiveWeight, excluded, + dropped, }; }); return { tabId: tab.id, assessments: contributions }; diff --git a/client/app/bundles/course/gradebook/store.ts b/client/app/bundles/course/gradebook/store.ts index 4777e16b3b6..c8267ed793e 100644 --- a/client/app/bundles/course/gradebook/store.ts +++ b/client/app/bundles/course/gradebook/store.ts @@ -70,6 +70,7 @@ const reducer = produce( tabId, weight, weightMode, + keepHighest, assessmentWeights, excludedAssessmentIds, }) => { @@ -77,6 +78,7 @@ const reducer = produce( if (tab) { tab.gradebookWeight = weight; tab.weightMode = weightMode; + tab.keepHighest = keepHighest ?? 0; } const excludedSet = new Set(excludedAssessmentIds ?? []); const tabAssessments = draft.assessments.filter( diff --git a/client/app/types/course/gradebook.ts b/client/app/types/course/gradebook.ts index c9009afbfe0..2d72bcb1c0c 100644 --- a/client/app/types/course/gradebook.ts +++ b/client/app/types/course/gradebook.ts @@ -9,6 +9,7 @@ export interface TabData { categoryId: number; gradebookWeight?: number; weightMode?: 'equal' | 'custom'; + keepHighest?: number; } export interface AssessmentData { @@ -52,6 +53,7 @@ export interface UpdateWeightsPayload { tabId: number; weight: number; weightMode?: 'equal' | 'custom'; + keepHighest?: number; excludedAssessmentIds?: number[]; assessmentWeights?: { assessmentId: number; weight: number }[]; }[]; diff --git a/client/locales/en.json b/client/locales/en.json index 1d9bf1cce66..bca35e87f7f 100644 --- a/client/locales/en.json +++ b/client/locales/en.json @@ -9383,6 +9383,9 @@ "course.gradebook.GradebookWeightedTable.email": { "defaultMessage": "Email" }, + "course.gradebook.GradebookWeightedTable.dropped": { + "defaultMessage": "Dropped (lowest)" + }, "course.gradebook.GradebookWeightedTable.excluded": { "defaultMessage": "Excluded" }, diff --git a/db/schema.rb b/db/schema.rb index d1a22b40e89..75ec3ce3e21 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_11_130000) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" enable_extension "uuid-ossp" @@ -564,6 +564,7 @@ t.integer "updater_id", null: false t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false + t.boolean "gradebook_excluded", default: false, null: false t.index ["category_id"], name: "fk__course_assessment_tabs_category_id" t.index ["creator_id"], name: "fk__course_assessment_tabs_creator_id" t.index ["updater_id"], name: "fk__course_assessment_tabs_updater_id" diff --git a/spec/controllers/course/gradebook_controller_spec.rb b/spec/controllers/course/gradebook_controller_spec.rb index 59397f4120d..aa94670ed19 100644 --- a/spec/controllers/course/gradebook_controller_spec.rb +++ b/spec/controllers/course/gradebook_controller_spec.rb @@ -297,6 +297,26 @@ def weight_for(tab) end end + describe '#update_weights keepHighest' do + render_views + + let(:category2) { create(:course_assessment_category, course: course) } + let(:tab) { create(:course_assessment_tab, category: category2) } + + before { controller_sign_in(controller, manager.user) } + + it 'persists keepHighest and echoes it back' do + post :update_weights, as: :json, params: { + course_id: course.id, + weights: [{ tabId: tab.id, weight: '50', weightMode: 'equal', keepHighest: 2 }] + } + expect(response).to have_http_status(:ok) + body = JSON.parse(response.body) + expect(body['weights'].first['keepHighest']).to eq(2) + expect(Course::Gradebook::Contribution.find_by(tab_id: tab.id).keep_highest).to eq(2) + end + end + describe '#update_weights with modes' do render_views @@ -421,6 +441,16 @@ def weight_for(tab) expect(body['assessments'].first).to have_key('gradebookExcluded') expect(body['assessments'].first['gradebookExcluded']).to eq(false) end + + it 'includes keepHighest in the weighted tabs response' do + contribution.update!(keep_highest: 3) + controller_sign_in(controller, manager.user) + get :index, params: { course_id: course.id }, format: :json + body = JSON.parse(response.body) + tab_json = body['tabs'].find { |t| t['id'] == tab.id } + expect(tab_json).to have_key('keepHighest') + expect(tab_json['keepHighest']).to eq(3) + end end end end diff --git a/spec/models/course/gradebook/contribution_spec.rb b/spec/models/course/gradebook/contribution_spec.rb index d4d433e028a..06e215a507c 100644 --- a/spec/models/course/gradebook/contribution_spec.rb +++ b/spec/models/course/gradebook/contribution_spec.rb @@ -208,6 +208,29 @@ def excluded?(assessment) expect(excluded?(a1)).to eq(false) end end + + context 'with keep_highest' do + it 'persists keep_highest in equal mode' do + described_class.bulk_update( + course: course, + updates: [{ tab_id: tab1.id, weight: 50, weight_mode: 'equal', keep_highest: 3 }] + ) + expect(described_class.find_by(tab_id: tab1.id).keep_highest).to eq(3) + end + + it 'accepts 0 as a valid keep_highest value' do + described_class.bulk_update( + course: course, + updates: [{ tab_id: tab1.id, weight: 50, weight_mode: 'equal', keep_highest: 5 }] + ) + expect(described_class.find_by(tab_id: tab1.id).keep_highest).to eq(5) + described_class.bulk_update( + course: course, + updates: [{ tab_id: tab1.id, weight: 50, weight_mode: 'equal', keep_highest: 0 }] + ) + expect(described_class.find_by(tab_id: tab1.id).keep_highest).to eq(0) + end + end end end end