diff --git a/app/controllers/course/statistics/assessments_controller.rb b/app/controllers/course/statistics/assessments_controller.rb index cc7afe09e58..dd186b610ea 100644 --- a/app/controllers/course/statistics/assessments_controller.rb +++ b/app/controllers/course/statistics/assessments_controller.rb @@ -4,7 +4,19 @@ class Course::Statistics::AssessmentsController < Course::Statistics::Controller include Course::Statistics::SubmissionsConcern include Course::Statistics::UsersConcern - def main_statistics + def assessment_statistics + @assessment = Course::Assessment.unscoped. + includes(programming_questions: [:language]). + calculated(:maximum_grade, :question_count). + find(assessment_params[:id]) + + load_ordered_questions + create_question_related_hash + + @assessment_autograded = @question_hash.any? { |_, (_, _, auto_gradable)| auto_gradable } + end + + def submission_statistics @assessment = Course::Assessment.unscoped. includes(programming_questions: [:language]). calculated(:maximum_grade, :question_count). @@ -16,10 +28,8 @@ def main_statistics load_course_user_students_info load_ordered_questions - fetch_all_ancestor_assessments create_question_related_hash - @assessment_autograded = @question_hash.any? { |_, (_, _, auto_gradable)| auto_gradable } @student_submissions_hash = fetch_hash_for_main_assessment(submissions, @all_students) end @@ -47,10 +57,8 @@ def live_feedback_statistics where(assessment_id: assessment_params[:id]) create_submission_question_id_hash(@assessment.questions) - load_course_user_students_info load_ordered_questions - create_student_live_feedback_hash end @@ -68,6 +76,10 @@ def live_feedback_history order(:created_at) end + def ancestor_info + fetch_all_ancestor_assessments + end + private def load_ordered_questions diff --git a/app/views/course/statistics/assessments/ancestor_info.json.jbuilder b/app/views/course/statistics/assessments/ancestor_info.json.jbuilder new file mode 100644 index 00000000000..fd42a335d57 --- /dev/null +++ b/app/views/course/statistics/assessments/ancestor_info.json.jbuilder @@ -0,0 +1,6 @@ +# frozen_string_literal: true +json.array! @ancestors do |ancestor| + json.id ancestor.id + json.title ancestor.title + json.courseTitle ancestor.course&.title +end diff --git a/app/views/course/statistics/assessments/ancestor_statistics.json.jbuilder b/app/views/course/statistics/assessments/ancestor_statistics.json.jbuilder index 0e96d2ab790..66d0ef768c8 100644 --- a/app/views/course/statistics/assessments/ancestor_statistics.json.jbuilder +++ b/app/views/course/statistics/assessments/ancestor_statistics.json.jbuilder @@ -6,4 +6,5 @@ end json.submissions @student_submissions_hash.each do |course_user, (submission, end_at)| json.partial! 'course_user', course_user: course_user json.partial! 'submission', submission: submission, end_at: end_at + json.maximumGrade @assessment.maximum_grade end diff --git a/app/views/course/statistics/assessments/assessment_statistics.json.jbuilder b/app/views/course/statistics/assessments/assessment_statistics.json.jbuilder new file mode 100644 index 00000000000..5b771ac8009 --- /dev/null +++ b/app/views/course/statistics/assessments/assessment_statistics.json.jbuilder @@ -0,0 +1,6 @@ +# frozen_string_literal: true +json.partial! 'assessment', assessment: @assessment, course: current_course +json.isAutograded @assessment_autograded +json.questionCount @assessment.question_count +json.questionIds @ordered_questions +json.liveFeedbackEnabled @assessment.programming_questions.any?(&:live_feedback_enabled) diff --git a/app/views/course/statistics/assessments/main_statistics.json.jbuilder b/app/views/course/statistics/assessments/submission_statistics.json.jbuilder similarity index 52% rename from app/views/course/statistics/assessments/main_statistics.json.jbuilder rename to app/views/course/statistics/assessments/submission_statistics.json.jbuilder index 35924a43df2..42f09cdb273 100644 --- a/app/views/course/statistics/assessments/main_statistics.json.jbuilder +++ b/app/views/course/statistics/assessments/submission_statistics.json.jbuilder @@ -1,16 +1,9 @@ # frozen_string_literal: true -json.assessment do - json.partial! 'assessment', assessment: @assessment, course: current_course - json.isAutograded @assessment_autograded - json.questionCount @assessment.question_count - json.questionIds @ordered_questions - json.liveFeedbackEnabled @assessment.programming_questions.any?(&:live_feedback_enabled) -end - -json.submissions @student_submissions_hash.each do |course_user, (submission, answers, end_at)| +json.array! @student_submissions_hash.each do |course_user, (submission, answers, end_at)| json.partial! 'course_user', course_user: course_user json.partial! 'submission', submission: submission, end_at: end_at + json.maximumGrade @assessment.maximum_grade json.groups @group_names_hash[course_user.id] do |name| json.name name end @@ -21,9 +14,3 @@ json.submissions @student_submissions_hash.each do |course_user, (submission, an end json.partial! 'attempt_status', answers: answers unless submission.nil? end - -json.ancestors @ancestors do |ancestor| - json.id ancestor.id - json.title ancestor.title - json.courseTitle ancestor.course&.title -end diff --git a/client/app/api/course/Statistics/AssessmentStatistics.ts b/client/app/api/course/Statistics/AssessmentStatistics.ts index 5a8f2a4e1e8..7e49d817e11 100644 --- a/client/app/api/course/Statistics/AssessmentStatistics.ts +++ b/client/app/api/course/Statistics/AssessmentStatistics.ts @@ -1,8 +1,10 @@ import { LiveFeedbackHistoryState } from 'types/course/assessment/submission/liveFeedback'; import { AncestorAssessmentStats, + AncestorInfo, AssessmentLiveFeedbackStatistics, - MainAssessmentStats, + MainAssessmentInfo, + MainSubmissionInfo, } from 'types/course/statistics/assessmentStatistics'; import { APIResponse } from 'api/types'; @@ -24,15 +26,23 @@ export default class AssessmentStatisticsAPI extends BaseCourseAPI { ancestorId: string | number, ): APIResponse { return this.client.get( - `${this.#urlPrefix}/${ancestorId}/ancestor_statistics/`, + `${this.#urlPrefix}/${ancestorId}/ancestor_statistics`, ); } - fetchMainStatistics( + fetchAssessmentStatistics( assessmentId: string | number, - ): APIResponse { + ): APIResponse { return this.client.get( - `${this.#urlPrefix}/${assessmentId}/main_statistics`, + `${this.#urlPrefix}/${assessmentId}/assessment_statistics`, + ); + } + + fetchSubmissionStatistics( + assessmentId: string | number, + ): APIResponse { + return this.client.get( + `${this.#urlPrefix}/${assessmentId}/submission_statistics`, ); } @@ -54,4 +64,10 @@ export default class AssessmentStatisticsAPI extends BaseCourseAPI { { params: { question_id: questionId, course_user_id: courseUserId } }, ); } + + fetchAncestorInfo( + assessmentId: number, + ): Promise> { + return this.client.get(`${this.#urlPrefix}/${assessmentId}/ancestor_info`); + } } diff --git a/client/app/bundles/course/assessment/operations/statistics.ts b/client/app/bundles/course/assessment/operations/statistics.ts index dc8d4d9381d..b21b15e47c0 100644 --- a/client/app/bundles/course/assessment/operations/statistics.ts +++ b/client/app/bundles/course/assessment/operations/statistics.ts @@ -1,9 +1,6 @@ import { AxiosError } from 'axios'; import { dispatch } from 'store'; -import { - AncestorAssessmentStats, - AssessmentLiveFeedbackStatistics, -} from 'types/course/statistics/assessmentStatistics'; +import { AncestorAssessmentStats } from 'types/course/statistics/assessmentStatistics'; import CourseAPI from 'api/course'; @@ -13,17 +10,39 @@ export const fetchAssessmentStatistics = async ( assessmentId: number, ): Promise => { try { - dispatch(actions.reset()); const response = - await CourseAPI.statistics.assessment.fetchMainStatistics(assessmentId); - const data = response.data; - dispatch( - actions.initialize({ - assessment: data.assessment, - submissions: data.submissions, - ancestors: data.ancestors, - }), - ); + await CourseAPI.statistics.assessment.fetchAssessmentStatistics( + assessmentId, + ); + dispatch(actions.setAssessmentStatistics(response.data)); + } catch (error) { + if (error instanceof AxiosError) throw error.response?.data?.errors; + throw error; + } +}; + +export const fetchSubmissionStatistics = async ( + assessmentId: number, +): Promise => { + try { + const response = + await CourseAPI.statistics.assessment.fetchSubmissionStatistics( + assessmentId, + ); + dispatch(actions.setSubmissionStatistics(response.data)); + } catch (error) { + if (error instanceof AxiosError) throw error.response?.data?.errors; + throw error; + } +}; + +export const fetchAncestorInfo = async ( + assessmentId: number, +): Promise => { + try { + const response = + await CourseAPI.statistics.assessment.fetchAncestorInfo(assessmentId); + dispatch(actions.setAncestorInfo(response.data)); } catch (error) { if (error instanceof AxiosError) throw error.response?.data?.errors; throw error; @@ -41,10 +60,15 @@ export const fetchAncestorStatistics = async ( export const fetchLiveFeedbackStatistics = async ( assessmentId: number, -): Promise => { - const response = - await CourseAPI.statistics.assessment.fetchLiveFeedbackStatistics( - assessmentId, - ); - return response.data; +): Promise => { + try { + const response = + await CourseAPI.statistics.assessment.fetchLiveFeedbackStatistics( + assessmentId, + ); + dispatch(actions.setLiveFeedbackStatistics(response.data)); + } catch (error) { + if (error instanceof AxiosError) throw error.response?.data?.errors; + throw error; + } }; diff --git a/client/app/bundles/course/assessment/pages/AssessmentStatistics/AssessmentStatisticsPage.tsx b/client/app/bundles/course/assessment/pages/AssessmentStatistics/AssessmentStatisticsPage.tsx index 2fc661c2dec..72567df0c00 100644 --- a/client/app/bundles/course/assessment/pages/AssessmentStatistics/AssessmentStatisticsPage.tsx +++ b/client/app/bundles/course/assessment/pages/AssessmentStatistics/AssessmentStatisticsPage.tsx @@ -89,16 +89,16 @@ const AssessmentStatisticsPage: FC = () => { const [tabValue, setTabValue] = useState('gradesPerQuestion'); const [includePhantom, setIncludePhantom] = useState(false); - const statistics = useAppSelector(getAssessmentStatistics); + const assessmentStatistics = useAppSelector(getAssessmentStatistics); const tabComponentMapping = tabMapping(includePhantom); return ( <> @@ -160,7 +160,7 @@ const AssessmentStatisticsPage: FC = () => { label={t(translations.duplicationHistory)} value="duplicationHistory" /> - {statistics.assessment?.liveFeedbackEnabled && ( + {assessmentStatistics?.liveFeedbackEnabled && ( { - const ancestors = useAppSelector(getAssessmentStatistics).ancestors; +const renderDuplicationHistoryStatistics = ( + ancestorInfo: AncestorInfo[], + parsedAssessmentId: number, + selectedAncestorId: number, + setSelectedAncestorId: Dispatch>, +): JSX.Element => ( + <> + +
+ +
+ +); +const DuplicationHistoryStatistics: FC = () => { const { assessmentId } = useParams(); const parsedAssessmentId = parseInt(assessmentId!, 10); - + const ancestorInfo = useAppSelector(getAncestorInfo); const [selectedAncestorId, setSelectedAncestorId] = useState(parsedAssessmentId); + const fetchAndSetAncestorInfo = async (): Promise => { + if (ancestorInfo.length > 0) return; + await fetchAncestorInfo(parsedAssessmentId); + }; + return ( - <> - -
- -
- + } while={fetchAndSetAncestorInfo}> + {renderDuplicationHistoryStatistics( + ancestorInfo, + parsedAssessmentId, + selectedAncestorId, + setSelectedAncestorId, + )} + ); }; diff --git a/client/app/bundles/course/assessment/pages/AssessmentStatistics/GradeDistribution/AncestorGradesChart.tsx b/client/app/bundles/course/assessment/pages/AssessmentStatistics/GradeDistribution/AncestorGradesChart.tsx index 0771548f4b4..e31769ab400 100644 --- a/client/app/bundles/course/assessment/pages/AssessmentStatistics/GradeDistribution/AncestorGradesChart.tsx +++ b/client/app/bundles/course/assessment/pages/AssessmentStatistics/GradeDistribution/AncestorGradesChart.tsx @@ -10,12 +10,14 @@ interface Props { const AncestorGradesChart: FC = (props) => { const { ancestorSubmissions } = props; - const totalGrades = - ancestorSubmissions - ?.filter((s) => s.totalGrade) - ?.map((s) => s.totalGrade) ?? []; + const gradedSubmissions = + ancestorSubmissions?.filter((s) => s.totalGrade) ?? []; + const totalGrades = gradedSubmissions.map((s) => + parseFloat(s.totalGrade as unknown as string), + ); + const maximumGrade = gradedSubmissions[0]?.maximumGrade ?? undefined; - return ; + return ; }; export default AncestorGradesChart; diff --git a/client/app/bundles/course/assessment/pages/AssessmentStatistics/GradeDistribution/MainGradesChart.tsx b/client/app/bundles/course/assessment/pages/AssessmentStatistics/GradeDistribution/MainGradesChart.tsx index bc558288273..3bc232560e1 100644 --- a/client/app/bundles/course/assessment/pages/AssessmentStatistics/GradeDistribution/MainGradesChart.tsx +++ b/client/app/bundles/course/assessment/pages/AssessmentStatistics/GradeDistribution/MainGradesChart.tsx @@ -1,8 +1,12 @@ -import { FC } from 'react'; +import { FC, useMemo } from 'react'; +import { useParams } from 'react-router-dom'; +import LoadingIndicator from 'lib/components/core/LoadingIndicator'; +import Preload from 'lib/components/wrappers/Preload'; import { useAppSelector } from 'lib/hooks/store'; -import { getAssessmentStatistics } from '../selectors'; +import { fetchSubmissionStatistics } from '../../../operations/statistics'; +import { getSubmissionStatistics } from '../selectors'; import GradesChart from './GradesChart'; @@ -10,24 +14,36 @@ interface Props { includePhantom: boolean; } -const MainGradesChart: FC = (props) => { - const { includePhantom } = props; - - const statistics = useAppSelector(getAssessmentStatistics); - const submissions = statistics.submissions; - const maximumGrade = statistics.assessment?.maximumGrade; - const nonNullSubmissions = submissions.filter((s) => s.totalGrade); - - const includedSubmissions = includePhantom - ? nonNullSubmissions - : nonNullSubmissions.filter((s) => !s.courseUser.isPhantom); - - const totalGrades = - includedSubmissions - ?.filter((s) => s.totalGrade) - ?.map((s) => s.totalGrade) ?? []; - - return ; +const MainGradesChart: FC = ({ includePhantom }) => { + const { assessmentId } = useParams(); + const parsedAssessmentId = parseInt(assessmentId!, 10); + + const submissionStatistics = useAppSelector(getSubmissionStatistics); + + const fetchAndSetSubmissionStatistics = async (): Promise => { + if (submissionStatistics.length > 0) return; + await fetchSubmissionStatistics(parsedAssessmentId); + }; + + const { maximumGrade, totalGrades } = useMemo(() => { + const filteredSubmissionStatistics = submissionStatistics.filter( + (s) => s.totalGrade && (includePhantom || !s.courseUser.isPhantom), + ); + + return { + maximumGrade: submissionStatistics[0]?.maximumGrade ?? 0, + totalGrades: filteredSubmissionStatistics.map((s) => s.totalGrade!), + }; + }, [submissionStatistics, includePhantom]); + + return ( + } + while={fetchAndSetSubmissionStatistics} + > + + + ); }; export default MainGradesChart; diff --git a/client/app/bundles/course/assessment/pages/AssessmentStatistics/LiveFeedbackHistory/LiveFeedbackHistoryPage.tsx b/client/app/bundles/course/assessment/pages/AssessmentStatistics/LiveFeedbackHistory/LiveFeedbackHistoryPage.tsx index 0c76d84d8f7..91613c546e6 100644 --- a/client/app/bundles/course/assessment/pages/AssessmentStatistics/LiveFeedbackHistory/LiveFeedbackHistoryPage.tsx +++ b/client/app/bundles/course/assessment/pages/AssessmentStatistics/LiveFeedbackHistory/LiveFeedbackHistoryPage.tsx @@ -5,7 +5,7 @@ import Accordion from 'lib/components/core/layouts/Accordion'; import { useAppSelector } from 'lib/hooks/store'; import useTranslation from 'lib/hooks/useTranslation'; -import { getLiveFeedbadkQuestionInfo } from '../selectors'; +import { getLiveFeedbackQuestionInfo } from '../selectors'; import translations from '../translations'; import LiveFeedbackDetails from './LiveFeedbackDetails'; @@ -17,7 +17,7 @@ interface Props { const LiveFeedbackHistoryPage: FC = (props) => { const { t } = useTranslation(); const { questionNumber } = props; - const question = useAppSelector(getLiveFeedbadkQuestionInfo); + const question = useAppSelector(getLiveFeedbackQuestionInfo); return ( <> diff --git a/client/app/bundles/course/assessment/pages/AssessmentStatistics/LiveFeedbackStatistics.tsx b/client/app/bundles/course/assessment/pages/AssessmentStatistics/LiveFeedbackStatistics.tsx index caca0143135..5dece59cc0f 100644 --- a/client/app/bundles/course/assessment/pages/AssessmentStatistics/LiveFeedbackStatistics.tsx +++ b/client/app/bundles/course/assessment/pages/AssessmentStatistics/LiveFeedbackStatistics.tsx @@ -1,42 +1,53 @@ -import { FC, useState } from 'react'; +import { FC } from 'react'; import { useParams } from 'react-router-dom'; -import { AssessmentLiveFeedbackStatistics } from 'types/course/statistics/assessmentStatistics'; -import { fetchLiveFeedbackStatistics } from 'course/assessment/operations/statistics'; +import { + fetchAssessmentStatistics, + fetchLiveFeedbackStatistics, +} from 'course/assessment/operations/statistics'; import LoadingIndicator from 'lib/components/core/LoadingIndicator'; import Preload from 'lib/components/wrappers/Preload'; +import { useAppSelector } from 'lib/hooks/store'; import LiveFeedbackStatisticsTable from './LiveFeedbackStatisticsTable'; +import { + getAssessmentStatistics, + getLiveFeedbackStatistics, +} from './selectors'; interface Props { includePhantom: boolean; } -const LiveFeedbackStatistics: FC = (props) => { +const LiveFeedbackStatistics: FC = ({ includePhantom }) => { const { assessmentId } = useParams(); const parsedAssessmentId = parseInt(assessmentId!, 10); - const { includePhantom } = props; - const [liveFeedbackStatistics, setLiveFeedbackStatistics] = useState< - AssessmentLiveFeedbackStatistics[] - >([]); - - const fetchAndSetLiveFeedbackStatistics = (): Promise => - fetchLiveFeedbackStatistics(parsedAssessmentId).then( - setLiveFeedbackStatistics, - ); + const assessmentStatistics = useAppSelector(getAssessmentStatistics); + const liveFeedbackStatistics = useAppSelector(getLiveFeedbackStatistics); + + const fetchAndSetAssessmentAndLiveFeedbackStatistics = + async (): Promise => { + const promises: Promise[] = []; + if (!assessmentStatistics) { + promises.push(fetchAssessmentStatistics(parsedAssessmentId)); + } + if (liveFeedbackStatistics.length === 0) { + promises.push(fetchLiveFeedbackStatistics(parsedAssessmentId)); + } + await Promise.all(promises); + }; return ( } - while={fetchAndSetLiveFeedbackStatistics} + while={fetchAndSetAssessmentAndLiveFeedbackStatistics} > - {(): JSX.Element => ( - - )} + ); }; diff --git a/client/app/bundles/course/assessment/pages/AssessmentStatistics/LiveFeedbackStatisticsTable.tsx b/client/app/bundles/course/assessment/pages/AssessmentStatistics/LiveFeedbackStatisticsTable.tsx index e07717bb797..de6e5ae1af7 100644 --- a/client/app/bundles/course/assessment/pages/AssessmentStatistics/LiveFeedbackStatisticsTable.tsx +++ b/client/app/bundles/course/assessment/pages/AssessmentStatistics/LiveFeedbackStatisticsTable.tsx @@ -1,7 +1,10 @@ import { FC, ReactNode, useEffect, useState } from 'react'; import { useParams } from 'react-router-dom'; import { Box, Typography } from '@mui/material'; -import { AssessmentLiveFeedbackStatistics } from 'types/course/statistics/assessmentStatistics'; +import { + AssessmentLiveFeedbackStatistics, + MainAssessmentInfo, +} from 'types/course/statistics/assessmentStatistics'; import SubmissionWorkflowState from 'course/assessment/submission/components/SubmissionWorkflowState'; import { workflowStates } from 'course/assessment/submission/constants'; @@ -11,27 +14,24 @@ import GhostIcon from 'lib/components/icons/GhostIcon'; import Table, { ColumnTemplate } from 'lib/components/table'; import { DEFAULT_TABLE_ROWS_PER_PAGE } from 'lib/constants/sharedConstants'; import { getEditSubmissionURL } from 'lib/helpers/url-builders'; -import { useAppSelector } from 'lib/hooks/store'; import useTranslation from 'lib/hooks/useTranslation'; import { getClassnameForLiveFeedbackCell } from './classNameUtils'; import LiveFeedbackHistoryIndex from './LiveFeedbackHistory'; -import { getAssessmentStatistics } from './selectors'; import translations from './translations'; import { getJointGroupsName } from './utils'; interface Props { includePhantom: boolean; + assessmentStatistics: MainAssessmentInfo | null; liveFeedbackStatistics: AssessmentLiveFeedbackStatistics[]; } const LiveFeedbackStatisticsTable: FC = (props) => { const { t } = useTranslation(); const { courseId, assessmentId } = useParams(); - const { includePhantom, liveFeedbackStatistics } = props; - - const statistics = useAppSelector(getAssessmentStatistics); - const assessment = statistics.assessment; + const { includePhantom, assessmentStatistics, liveFeedbackStatistics } = + props; const [parsedStatistics, setParsedStatistics] = useState< AssessmentLiveFeedbackStatistics[] @@ -59,15 +59,18 @@ const LiveFeedbackStatisticsTable: FC = (props) => { feedbackCounts[upperQuartilePercentileIndex]; setUpperQuartileFeedbackCount(upperQuartilePercentileValue); - const filteredStats = includePhantom - ? liveFeedbackStatistics - : liveFeedbackStatistics.filter((s) => !s.courseUser.isPhantom); - - filteredStats.forEach((stat) => { - stat.totalFeedbackCount = - stat.liveFeedbackCount?.reduce((sum, count) => sum + (count || 0), 0) ?? - 0; - }); + // Create new objects instead of modifying the original ones from Redux store + // This is necessary because Redux store objects are frozen and cannot be modified + const filteredStats = liveFeedbackStatistics + .filter((stat) => !stat.courseUser.isPhantom || props.includePhantom) + .map((stat) => ({ + ...stat, + totalFeedbackCount: + stat.liveFeedbackCount?.reduce( + (sum, count) => sum + (count || 0), + 0, + ) ?? 0, + })); setParsedStatistics( filteredStats.sort((a, b) => { @@ -113,38 +116,41 @@ const LiveFeedbackStatisticsTable: FC = (props) => { }; const statColumns: ColumnTemplate[] = - Array.from({ length: assessment?.questionCount ?? 0 }, (_, index) => { - return { - searchProps: { - getValue: (datum) => - datum.liveFeedbackCount?.[index]?.toString() ?? '', - }, - title: t(translations.questionIndex, { index: index + 1 }), - cell: (datum): ReactNode => { - return typeof datum.liveFeedbackCount?.[index] === 'number' - ? renderNonNullClickableLiveFeedbackCountCell( - datum.liveFeedbackCount?.[index], - datum.courseUser.id, - datum.questionIds[index], - index + 1, - ) - : null; - }, - sortable: true, - csvDownloadable: true, - className: 'text-right', - sortProps: { - sort: (a, b): number => { - const aValue = - a.liveFeedbackCount?.[index] ?? Number.MIN_SAFE_INTEGER; - const bValue = - b.liveFeedbackCount?.[index] ?? Number.MIN_SAFE_INTEGER; + Array.from( + { length: assessmentStatistics?.questionCount ?? 0 }, + (_, index) => { + return { + searchProps: { + getValue: (datum) => + datum.liveFeedbackCount?.[index]?.toString() ?? '', + }, + title: t(translations.questionIndex, { index: index + 1 }), + cell: (datum): ReactNode => { + return typeof datum.liveFeedbackCount?.[index] === 'number' + ? renderNonNullClickableLiveFeedbackCountCell( + datum.liveFeedbackCount?.[index], + datum.courseUser.id, + datum.questionIds[index], + index + 1, + ) + : null; + }, + sortable: true, + csvDownloadable: true, + className: 'text-right', + sortProps: { + sort: (a, b): number => { + const aValue = + a.liveFeedbackCount?.[index] ?? Number.MIN_SAFE_INTEGER; + const bValue = + b.liveFeedbackCount?.[index] ?? Number.MIN_SAFE_INTEGER; - return aValue - bValue; + return aValue - bValue; + }, }, - }, - }; - }); + }; + }, + ); const columns: ColumnTemplate[] = [ { @@ -261,7 +267,7 @@ const LiveFeedbackStatisticsTable: FC = (props) => { columns={columns} csvDownload={{ filename: t(translations.liveFeedbackFilename, { - assessment: assessment?.title ?? '', + assessment: assessmentStatistics?.title ?? '', }), }} data={parsedStatistics} diff --git a/client/app/bundles/course/assessment/pages/AssessmentStatistics/StudentAttemptCountTable.tsx b/client/app/bundles/course/assessment/pages/AssessmentStatistics/StudentAttemptCountTable.tsx index 79704955584..5397f1492f6 100644 --- a/client/app/bundles/course/assessment/pages/AssessmentStatistics/StudentAttemptCountTable.tsx +++ b/client/app/bundles/course/assessment/pages/AssessmentStatistics/StudentAttemptCountTable.tsx @@ -1,15 +1,21 @@ -import { FC, ReactNode, useState } from 'react'; +import { FC, ReactNode, useMemo, useState } from 'react'; import { useParams } from 'react-router-dom'; import { WorkflowState } from 'types/course/assessment/submission/submission'; import { MainSubmissionInfo } from 'types/course/statistics/assessmentStatistics'; +import { + fetchAssessmentStatistics, + fetchSubmissionStatistics, +} from 'course/assessment/operations/statistics'; import AllAttemptsPrompt from 'course/assessment/submission/components/AllAttempts'; import SubmissionWorkflowState from 'course/assessment/submission/components/SubmissionWorkflowState'; import { workflowStates } from 'course/assessment/submission/constants'; import Link from 'lib/components/core/Link'; +import LoadingIndicator from 'lib/components/core/LoadingIndicator'; import Note from 'lib/components/core/Note'; import GhostIcon from 'lib/components/icons/GhostIcon'; import Table, { ColumnTemplate } from 'lib/components/table'; +import Preload from 'lib/components/wrappers/Preload'; import { DEFAULT_TABLE_ROWS_PER_PAGE } from 'lib/constants/sharedConstants'; import TableLegends from 'lib/containers/TableLegends'; import { @@ -22,7 +28,7 @@ import useTranslation from 'lib/hooks/useTranslation'; import submissionTranslations from '../../submission/translations'; import { getClassNameForAttemptCountCell } from './classNameUtils'; -import { getAssessmentStatistics } from './selectors'; +import { getAssessmentStatistics, getSubmissionStatistics } from './selectors'; import translations from './translations'; import { getJointGroupsName } from './utils'; @@ -38,12 +44,77 @@ interface AnswerInfoState { workflowState?: WorkflowState | typeof workflowStates.Unstarted; } -const StudentAttemptCountTable: FC = (props) => { +const AttemptTableLegends: FC = () => { + const { t } = useTranslation(); + return ( + + ); +}; + +const AttemptsModal: FC<{ + open: boolean; + onClose: () => void; + answerInfo: AnswerInfoState; +}> = ({ open, onClose, answerInfo }) => { + const { t } = useTranslation(); + const { courseId, assessmentId } = useParams(); + return ( + + + {t(submissionTranslations.historyTitle, { + number: answerInfo.index, + studentName: answerInfo.studentName, + })} + + + + } + /> + ); +}; + +const StudentAttemptCountTable: FC = ({ includePhantom }) => { const { t } = useTranslation(); const { courseId, assessmentId } = useParams(); - const { includePhantom } = props; + const parsedAssessmentId = parseInt(assessmentId!, 10); + + const assessmentStatistics = useAppSelector(getAssessmentStatistics); + const submissionStatistics = useAppSelector(getSubmissionStatistics); - const statistics = useAppSelector(getAssessmentStatistics); const [openPastAnswers, setOpenPastAnswers] = useState(false); const [answerInfo, setAnswerInfo] = useState({ index: 0, @@ -52,33 +123,51 @@ const StudentAttemptCountTable: FC = (props) => { studentName: '', }); - const assessment = statistics.assessment; - const submissions = statistics.submissions; - - if (!assessment?.isAutograded) { - return ; - } + const fetchAndSetAssessmentAndSubmissionStatistics = + async (): Promise => { + const promises: Promise[] = []; + if (!assessmentStatistics) { + promises.push(fetchAssessmentStatistics(parsedAssessmentId)); + } + if (submissionStatistics.length === 0) { + promises.push(fetchSubmissionStatistics(parsedAssessmentId)); + } + await Promise.all(promises); + }; // since submissions come from Redux store, it is immutable, and hence // toggling between includePhantom status will render typeError if we // use submissions. Hence the reason of using slice in here, basically // creating a new array and use this instead for the display. - const filteredSubmissions = includePhantom - ? submissions.slice() - : submissions.slice().filter((s) => !s.courseUser.isPhantom); + const filteredAndSortedSubmissions = useMemo(() => { + return submissionStatistics + .filter((s) => includePhantom || !s.courseUser.isPhantom) + .slice() + .sort((a, b) => { + const phantomDiff = + Number(a.courseUser.isPhantom) - Number(b.courseUser.isPhantom); + return ( + phantomDiff || a.courseUser.name.localeCompare(b.courseUser.name) + ); + }); + }, [submissionStatistics, includePhantom]); - const sortedSubmission = filteredSubmissions - .sort((datum1, datum2) => - datum1.courseUser.name.localeCompare(datum2.courseUser.name), - ) - .sort( - (datum1, datum2) => - Number(datum1.courseUser.isPhantom) - - Number(datum2.courseUser.isPhantom), - ); + const handleClickAttemptCell = ( + index: number, + datum: MainSubmissionInfo, + ): void => { + setOpenPastAnswers(true); + setAnswerInfo({ + index: index + 1, + questionId: assessmentStatistics!.questionIds[index], + submissionId: datum.id, + studentName: datum.courseUser.name, + workflowState: datum.workflowState, + }); + }; // the case where the attempt count is null is handled separately inside the column - // (refer to the definition of answerColumns below) + // (refer to the definition of buildAnswerColumns below) const renderAttemptCountClickableCell = ( index: number, datum: MainSubmissionInfo, @@ -90,57 +179,43 @@ const StudentAttemptCountTable: FC = (props) => { return (
{ - setOpenPastAnswers(true); - setAnswerInfo({ - index: index + 1, - questionId: assessment!.questionIds[index], - submissionId: datum.id, - studentName: datum.courseUser.name, - workflowState: datum.workflowState, - }); - }} + onClick={(): void => handleClickAttemptCell(index, datum)} > {datum.attemptStatus![index].attemptCount}
); }; - const answerColumns: ColumnTemplate[] = Array.from( - { length: assessment?.questionCount ?? 0 }, - (_, index) => { - return { + const buildAnswerColumns = (): ColumnTemplate[] => { + return Array.from( + { length: assessmentStatistics?.questionCount ?? 0 }, + (_, index) => ({ + title: t(translations.questionIndex, { index: index + 1 }), + className: 'text-right', + sortable: true, + csvDownloadable: true, + sortProps: { undefinedPriority: 'last' }, searchProps: { getValue: (datum) => datum.attemptStatus?.[index]?.attemptCount?.toString() ?? undefined, }, - title: t(translations.questionIndex, { index: index + 1 }), - cell: (datum): ReactNode => { - return typeof datum.attemptStatus?.[index]?.attemptCount === - 'number' ? ( + cell: (datum): ReactNode => + typeof datum.attemptStatus?.[index]?.attemptCount === 'number' ? ( renderAttemptCountClickableCell(index, datum) ) : (
- ); - }, - sortable: true, - csvDownloadable: true, - className: 'text-right', - sortProps: { - undefinedPriority: 'last', - }, - }; - }, - ); + ), + }), + ); + }; - const columns: ColumnTemplate[] = [ + const baseColumns: ColumnTemplate[] = [ { - searchProps: { - getValue: (datum) => datum.courseUser.name, - }, title: t(translations.name), sortable: true, searchable: true, + csvDownloadable: true, + searchProps: { getValue: (datum) => datum.courseUser.name }, cell: (datum) => (
@@ -151,34 +226,30 @@ const StudentAttemptCountTable: FC = (props) => { )}
), - csvDownloadable: true, }, { - searchProps: { - getValue: (datum) => datum.courseUser.email, - }, title: t(translations.email), hidden: true, + csvDownloadable: true, + searchProps: { getValue: (datum) => datum.courseUser.email }, cell: (datum) => (
{datum.courseUser.email}
), - csvDownloadable: true, }, { - of: 'groups', title: t(translations.group), + of: 'groups', sortable: true, searchable: true, - searchProps: { - getValue: (datum) => getJointGroupsName(datum.groups), - }, - cell: (datum) => getJointGroupsName(datum.groups), csvDownloadable: true, + searchProps: { getValue: (datum) => getJointGroupsName(datum.groups) }, + cell: (datum) => getJointGroupsName(datum.groups), }, { - of: 'workflowState', title: t(translations.workflowState), + of: 'workflowState', sortable: true, + className: 'center', cell: (datum) => ( = (props) => { workflowState={datum.workflowState ?? workflowStates.Unstarted} /> ), - className: 'center', }, - ...answerColumns, ]; + const columns = useMemo( + () => [...baseColumns, ...buildAnswerColumns()], + [assessmentStatistics], + ); + return ( - <> - - - `data_${datum.courseUser.id} bg-slot-1 hover?:bg-slot-2 slot-1-white slot-2-neutral-100` - } - getRowEqualityData={(datum): MainSubmissionInfo => datum} - getRowId={(datum): string => datum.courseUser.id.toString()} - indexing={{ indices: true }} - pagination={{ - rowsPerPage: [DEFAULT_TABLE_ROWS_PER_PAGE], - showAllRows: true, - }} - search={{ searchPlaceholder: t(translations.nameGroupsSearchText) }} - toolbar={{ show: true }} - /> - setOpenPastAnswers(false)} - open={openPastAnswers} - questionId={answerInfo.questionId} - submissionId={answerInfo.submissionId} - title={ - - - {t(submissionTranslations.historyTitle, { - number: answerInfo.index, - studentName: answerInfo.studentName, - })} - - - - } - /> - + } + while={fetchAndSetAssessmentAndSubmissionStatistics} + > + {!assessmentStatistics?.isAutograded ? ( + + ) : ( + <> + +
+ `data_${datum.courseUser.id} bg-slot-1 hover?:bg-slot-2 slot-1-white slot-2-neutral-100` + } + getRowEqualityData={(datum) => datum} + getRowId={(datum) => datum.courseUser.id.toString()} + indexing={{ indices: true }} + pagination={{ + rowsPerPage: [DEFAULT_TABLE_ROWS_PER_PAGE], + showAllRows: true, + }} + search={{ + searchPlaceholder: t(translations.nameGroupsSearchText), + }} + /> + setOpenPastAnswers(false)} + open={openPastAnswers} + /> + + )} + ); }; diff --git a/client/app/bundles/course/assessment/pages/AssessmentStatistics/StudentGradesPerQuestionTable.tsx b/client/app/bundles/course/assessment/pages/AssessmentStatistics/StudentGradesPerQuestionTable.tsx index c1d5456e87e..a7f99eba11b 100644 --- a/client/app/bundles/course/assessment/pages/AssessmentStatistics/StudentGradesPerQuestionTable.tsx +++ b/client/app/bundles/course/assessment/pages/AssessmentStatistics/StudentGradesPerQuestionTable.tsx @@ -1,14 +1,23 @@ -import { FC, ReactNode, useState } from 'react'; +// the case where the grade is null is handled separately inside the column +// (refer to the definition of answerColumns below) + +import { FC, ReactNode, useMemo, useState } from 'react'; import { useParams } from 'react-router-dom'; import { WorkflowState } from 'types/course/assessment/submission/submission'; import { MainSubmissionInfo } from 'types/course/statistics/assessmentStatistics'; +import { + fetchAssessmentStatistics, + fetchSubmissionStatistics, +} from 'course/assessment/operations/statistics'; import SubmissionWorkflowState from 'course/assessment/submission/components/SubmissionWorkflowState'; import { workflowStates } from 'course/assessment/submission/constants'; import Prompt from 'lib/components/core/dialogs/Prompt'; import Link from 'lib/components/core/Link'; +import LoadingIndicator from 'lib/components/core/LoadingIndicator'; import GhostIcon from 'lib/components/icons/GhostIcon'; import Table, { ColumnTemplate } from 'lib/components/table'; +import Preload from 'lib/components/wrappers/Preload'; import { DEFAULT_TABLE_ROWS_PER_PAGE } from 'lib/constants/sharedConstants'; import TableLegends from 'lib/containers/TableLegends'; import { @@ -22,7 +31,7 @@ import submissionTranslations from '../../submission/translations'; import LastAttemptIndex from './AnswerDisplay/LastAttempt'; import { getClassNameForMarkCell } from './classNameUtils'; -import { getAssessmentStatistics } from './selectors'; +import { getAssessmentStatistics, getSubmissionStatistics } from './selectors'; import translations from './translations'; import { getJointGroupsName } from './utils'; @@ -39,11 +48,14 @@ interface AnswerInfoState { workflowState?: WorkflowState | typeof workflowStates.Unstarted; } -const StudentGradesPerQuestionTable: FC = (props) => { +const StudentGradesPerQuestionTable: FC = ({ includePhantom }) => { const { t } = useTranslation(); const { courseId, assessmentId } = useParams(); - const { includePhantom } = props; - const statistics = useAppSelector(getAssessmentStatistics); + const parsedAssessmentId = parseInt(assessmentId!, 10); + + const assessmentStatistics = useAppSelector(getAssessmentStatistics); + const submissionStatistics = useAppSelector(getSubmissionStatistics); + const [openAnswer, setOpenAnswer] = useState(false); const [answerDisplayInfo, setAnswerDisplayInfo] = useState({ index: 0, @@ -52,26 +64,35 @@ const StudentGradesPerQuestionTable: FC = (props) => { submissionId: 0, studentName: '', }); - const assessment = statistics.assessment; - const submissions = statistics.submissions; + + const fetchAndSetAssessmentAndSubmissionStatistics = + async (): Promise => { + const promises: Promise[] = []; + if (!assessmentStatistics) { + promises.push(fetchAssessmentStatistics(parsedAssessmentId)); + } + if (submissionStatistics.length === 0) { + promises.push(fetchSubmissionStatistics(parsedAssessmentId)); + } + await Promise.all(promises); + }; // since submissions come from Redux store, it is immutable, and hence // toggling between includePhantom status will render typeError if we // use submissions. Hence the reason of using slice in here, basically // creating a new array and use this instead for the display. - const filteredSubmissions = includePhantom - ? submissions.slice() - : submissions.slice().filter((s) => !s.courseUser.isPhantom); - - const sortedSubmission = filteredSubmissions - .sort((datum1, datum2) => - datum1.courseUser.name.localeCompare(datum2.courseUser.name), - ) - .sort( - (datum1, datum2) => - Number(datum1.courseUser.isPhantom) - - Number(datum2.courseUser.isPhantom), - ); + const filteredAndSortedSubmissions = useMemo(() => { + return submissionStatistics + .filter((s) => includePhantom || !s.courseUser.isPhantom) + .slice() + .sort((a, b) => { + const phantomDiff = + Number(a.courseUser.isPhantom) - Number(b.courseUser.isPhantom); + return ( + phantomDiff || a.courseUser.name.localeCompare(b.courseUser.name) + ); + }); + }, [submissionStatistics, includePhantom]); // the case where the grade is null is handled separately inside the column // (refer to the definition of answerColumns below) @@ -91,7 +112,7 @@ const StudentGradesPerQuestionTable: FC = (props) => { setAnswerDisplayInfo({ index: index + 1, answerId: datum.answers![index].lastAttemptAnswerId, - questionId: assessment!.questionIds[index], + questionId: assessmentStatistics!.questionIds[index], submissionId: datum.id, studentName: datum.courseUser.name, workflowState: datum.workflowState, @@ -112,7 +133,7 @@ const StudentGradesPerQuestionTable: FC = (props) => { }; const answerColumns: ColumnTemplate[] = Array.from( - { length: assessment?.questionCount ?? 0 }, + { length: assessmentStatistics?.questionCount ?? 0 }, (_, index) => { return { searchProps: { @@ -204,7 +225,10 @@ const StudentGradesPerQuestionTable: FC = (props) => { datum.workflowState === workflowStates.Graded || datum.workflowState === workflowStates.Published; return typeof datum.totalGrade === 'number' && isGradedOrPublished ? ( - renderTotalGradeCell(datum.totalGrade, assessment!.maximumGrade) + renderTotalGradeCell( + datum.totalGrade, + assessmentStatistics!.maximumGrade, + ) ) : (
); @@ -238,78 +262,83 @@ const StudentGradesPerQuestionTable: FC = (props) => { ]; return ( - <> - -
- `data_${datum.courseUser.id} bg-slot-1 hover?:bg-slot-2 slot-1-white slot-2-neutral-100` - } - getRowEqualityData={(datum): MainSubmissionInfo => datum} - getRowId={(datum): string => datum.courseUser.id.toString()} - indexing={{ indices: true }} - pagination={{ - rowsPerPage: [DEFAULT_TABLE_ROWS_PER_PAGE], - showAllRows: true, - }} - search={{ - searchPlaceholder: t(translations.nameGroupsGraderSearchText), - }} - toolbar={{ show: true }} - /> - setOpenAnswer(false)} - open={openAnswer} - title={ - - {t(submissionTranslations.historyTitle, { - number: answerDisplayInfo.index, - studentName: answerDisplayInfo.studentName, - })} - - - } - > - } + while={fetchAndSetAssessmentAndSubmissionStatistics} + > + <> + +
+ `data_${datum.courseUser.id} bg-slot-1 hover?:bg-slot-2 slot-1-white slot-2-neutral-100` + } + getRowEqualityData={(datum): MainSubmissionInfo => datum} + getRowId={(datum): string => datum.courseUser.id.toString()} + indexing={{ indices: true }} + pagination={{ + rowsPerPage: [DEFAULT_TABLE_ROWS_PER_PAGE], + showAllRows: true, + }} + search={{ + searchPlaceholder: t(translations.nameGroupsGraderSearchText), + }} + toolbar={{ show: true }} /> - - + setOpenAnswer(false)} + open={openAnswer} + title={ + + {t(submissionTranslations.historyTitle, { + number: answerDisplayInfo.index, + studentName: answerDisplayInfo.studentName, + })} + + + } + > + + + + ); }; diff --git a/client/app/bundles/course/assessment/pages/AssessmentStatistics/SubmissionStatus/MainSubmissionChart.tsx b/client/app/bundles/course/assessment/pages/AssessmentStatistics/SubmissionStatus/MainSubmissionChart.tsx index 69d782f71db..7ddbf499e59 100644 --- a/client/app/bundles/course/assessment/pages/AssessmentStatistics/SubmissionStatus/MainSubmissionChart.tsx +++ b/client/app/bundles/course/assessment/pages/AssessmentStatistics/SubmissionStatus/MainSubmissionChart.tsx @@ -1,6 +1,6 @@ import { useAppSelector } from 'lib/hooks/store'; -import { getAssessmentStatistics } from '../selectors'; +import { getSubmissionStatistics } from '../selectors'; import SubmissionStatusChart from './SubmissionStatusChart'; @@ -10,13 +10,12 @@ interface Props { const MainSubmissionChart = (props: Props): JSX.Element => { const { includePhantom } = props; - const statistics = useAppSelector(getAssessmentStatistics); - const submissions = statistics.submissions; + const submissionStatistics = useAppSelector(getSubmissionStatistics); const includedSubmissions = includePhantom - ? submissions - : submissions.filter((s) => !s.courseUser.isPhantom); + ? submissionStatistics + : submissionStatistics.filter((s) => !s.courseUser.isPhantom); return ; }; diff --git a/client/app/bundles/course/assessment/pages/AssessmentStatistics/SubmissionTimeAndGradeStatistics/MainSubmissionTimeAndGradeStatistics.tsx b/client/app/bundles/course/assessment/pages/AssessmentStatistics/SubmissionTimeAndGradeStatistics/MainSubmissionTimeAndGradeStatistics.tsx index a5e601e246a..8b9fa1d8e35 100644 --- a/client/app/bundles/course/assessment/pages/AssessmentStatistics/SubmissionTimeAndGradeStatistics/MainSubmissionTimeAndGradeStatistics.tsx +++ b/client/app/bundles/course/assessment/pages/AssessmentStatistics/SubmissionTimeAndGradeStatistics/MainSubmissionTimeAndGradeStatistics.tsx @@ -1,8 +1,12 @@ -import { FC } from 'react'; +import { FC, useMemo } from 'react'; +import { useParams } from 'react-router-dom'; +import LoadingIndicator from 'lib/components/core/LoadingIndicator'; +import Preload from 'lib/components/wrappers/Preload'; import { useAppSelector } from 'lib/hooks/store'; -import { getAssessmentStatistics } from '../selectors'; +import { fetchSubmissionStatistics } from '../../../operations/statistics'; +import { getSubmissionStatistics } from '../selectors'; import SubmissionTimeAndGradeChart from './SubmissionTimeAndGradeChart'; @@ -10,18 +14,33 @@ interface Props { includePhantom: boolean; } -const MainSubmissionTimeAndGradeStatistics: FC = (props) => { - const { includePhantom } = props; - const statistics = useAppSelector(getAssessmentStatistics); - - const submissions = statistics.submissions; - - const nonNullSubmissions = submissions.filter((s) => s.totalGrade); - const includedSubmissions = includePhantom - ? nonNullSubmissions - : nonNullSubmissions.filter((s) => !s.courseUser.isPhantom); - - return ; +const MainSubmissionTimeAndGradeStatistics: FC = ({ + includePhantom, +}) => { + const { assessmentId } = useParams(); + const parsedAssessmentId = parseInt(assessmentId!, 10); + + const submissionStatistics = useAppSelector(getSubmissionStatistics); + + const fetchAndSetSubmissionStatistics = async (): Promise => { + if (submissionStatistics.length > 0) return; + await fetchSubmissionStatistics(parsedAssessmentId); + }; + + const includedSubmissions = useMemo(() => { + return submissionStatistics.filter( + (s) => s.totalGrade && (includePhantom || !s.courseUser.isPhantom), + ); + }, [submissionStatistics, includePhantom]); + + return ( + } + while={fetchAndSetSubmissionStatistics} + > + + + ); }; export default MainSubmissionTimeAndGradeStatistics; diff --git a/client/app/bundles/course/assessment/pages/AssessmentStatistics/index.tsx b/client/app/bundles/course/assessment/pages/AssessmentStatistics/index.tsx index 4a6046dec04..23bd5b79b1f 100644 --- a/client/app/bundles/course/assessment/pages/AssessmentStatistics/index.tsx +++ b/client/app/bundles/course/assessment/pages/AssessmentStatistics/index.tsx @@ -1,9 +1,13 @@ +import { useEffect } from 'react'; import { defineMessages } from 'react-intl'; import { useParams } from 'react-router-dom'; -import { fetchAssessmentStatistics } from 'course/assessment/operations/statistics'; import LoadingIndicator from 'lib/components/core/LoadingIndicator'; import Preload from 'lib/components/wrappers/Preload'; +import { useAppDispatch } from 'lib/hooks/store'; + +import { fetchAssessmentStatistics } from '../../operations/statistics'; +import { statisticsActions } from '../../reducers/statistics'; import AssessmentStatisticsPage from './AssessmentStatisticsPage'; @@ -17,16 +21,22 @@ const translations = defineMessages({ const AssessmentStatistics = (): JSX.Element => { const { assessmentId } = useParams(); const parsedAssessmentId = parseInt(assessmentId!, 10); + const dispatch = useAppDispatch(); + + useEffect(() => { + // Reset statistics state when assessmentId changes + dispatch(statisticsActions.reset()); + }, [assessmentId, dispatch]); - const fetchAssessmentStatisticsDetails = (): Promise => + const fetchAndSetAssessmentStatistics = (): Promise => fetchAssessmentStatistics(parsedAssessmentId); return ( } - while={fetchAssessmentStatisticsDetails} + while={fetchAndSetAssessmentStatistics} > - {(): JSX.Element => } + ); }; diff --git a/client/app/bundles/course/assessment/pages/AssessmentStatistics/selectors.ts b/client/app/bundles/course/assessment/pages/AssessmentStatistics/selectors.ts index 1d8ee960e34..8506d7571ce 100644 --- a/client/app/bundles/course/assessment/pages/AssessmentStatistics/selectors.ts +++ b/client/app/bundles/course/assessment/pages/AssessmentStatistics/selectors.ts @@ -3,15 +3,33 @@ import { LiveFeedbackChatMessage, QuestionInfo, } from 'types/course/assessment/submission/liveFeedback'; -import { AssessmentStatisticsState } from 'types/course/statistics/assessmentStatistics'; +import { + AncestorInfo, + AssessmentLiveFeedbackStatistics, + MainAssessmentInfo, + MainSubmissionInfo, +} from 'types/course/statistics/assessmentStatistics'; export const getAssessmentStatistics = ( state: AppState, -): AssessmentStatisticsState => state.assessments.statistics; +): MainAssessmentInfo | null => + state.assessments.statistics.assessmentStatistics; + +export const getSubmissionStatistics = ( + state: AppState, +): MainSubmissionInfo[] => state.assessments.statistics.submissionStatistics; + +export const getAncestorInfo = (state: AppState): AncestorInfo[] => + state.assessments.statistics.ancestorInfo; + +export const getLiveFeedbackStatistics = ( + state: AppState, +): AssessmentLiveFeedbackStatistics[] => + state.assessments.statistics.liveFeedbackStatistics; export const getLiveFeedbackChatMessages = ( state: AppState, ): LiveFeedbackChatMessage[] => state.assessments.liveFeedback.messages; -export const getLiveFeedbadkQuestionInfo = (state: AppState): QuestionInfo => +export const getLiveFeedbackQuestionInfo = (state: AppState): QuestionInfo => state.assessments.liveFeedback.question; diff --git a/client/app/bundles/course/assessment/reducers/statistics.ts b/client/app/bundles/course/assessment/reducers/statistics.ts index 1de14dbc227..d3823001083 100644 --- a/client/app/bundles/course/assessment/reducers/statistics.ts +++ b/client/app/bundles/course/assessment/reducers/statistics.ts @@ -4,9 +4,10 @@ import { AssessmentStatisticsState } from 'types/course/statistics/assessmentSta import { processAssessment, processSubmission } from '../utils/statisticsUtils'; const initialState: AssessmentStatisticsState = { - assessment: null, - submissions: [], - ancestors: [], + submissionStatistics: [], + assessmentStatistics: null, + liveFeedbackStatistics: [], + ancestorInfo: [], }; export const statisticsSlice = createSlice({ @@ -14,9 +15,39 @@ export const statisticsSlice = createSlice({ initialState, reducers: { initialize: (state, action: PayloadAction) => { - state.assessment = processAssessment(action.payload.assessment); - state.submissions = action.payload.submissions.map(processSubmission); - state.ancestors = action.payload.ancestors; + state.assessmentStatistics = processAssessment( + action.payload.assessmentStatistics, + ); + state.submissionStatistics = + action.payload.submissionStatistics.map(processSubmission); + state.liveFeedbackStatistics = action.payload.liveFeedbackStatistics; + state.ancestorInfo = action.payload.ancestorInfo; + }, + setSubmissionStatistics: ( + state, + action: PayloadAction, + ) => { + state.submissionStatistics = action.payload.map(processSubmission); + }, + setAssessmentStatistics: ( + state, + action: PayloadAction, + ) => { + state.assessmentStatistics = action.payload; + }, + setLiveFeedbackStatistics: ( + state, + action: PayloadAction< + AssessmentStatisticsState['liveFeedbackStatistics'] + >, + ) => { + state.liveFeedbackStatistics = action.payload; + }, + setAncestorInfo: ( + state, + action: PayloadAction, + ) => { + state.ancestorInfo = action.payload; }, reset: () => { return initialState; diff --git a/client/app/bundles/course/assessment/submission/containers/RubricExplanation.tsx b/client/app/bundles/course/assessment/submission/containers/RubricExplanation.tsx index 9cdac3ace2a..8f413a5670c 100644 --- a/client/app/bundles/course/assessment/submission/containers/RubricExplanation.tsx +++ b/client/app/bundles/course/assessment/submission/containers/RubricExplanation.tsx @@ -1,4 +1,4 @@ -import { FC, useState } from 'react'; +import { FC } from 'react'; import { Chip, MenuItem, Select, Typography } from '@mui/material'; import { AnswerRubricGradeData } from 'types/course/assessment/question/rubric-based-responses'; import { diff --git a/client/app/bundles/course/assessment/submission/containers/RubricPanelRow.tsx b/client/app/bundles/course/assessment/submission/containers/RubricPanelRow.tsx index b6d91d25172..729d64299c9 100644 --- a/client/app/bundles/course/assessment/submission/containers/RubricPanelRow.tsx +++ b/client/app/bundles/course/assessment/submission/containers/RubricPanelRow.tsx @@ -30,6 +30,7 @@ interface RubricPanelRowProps { question: SubmissionQuestionData<'RubricBasedResponse'>; category: RubricBasedResponseCategoryQuestionData; categoryGrades: Record; + // eslint-disable-next-line react/no-unused-prop-types setIsFirstRendering: (isFirstRendering: boolean) => void; } diff --git a/client/app/bundles/course/assessment/utils/statisticsUtils.js b/client/app/bundles/course/assessment/utils/statisticsUtils.js index 551c9f1a54b..fc376377201 100644 --- a/client/app/bundles/course/assessment/utils/statisticsUtils.js +++ b/client/app/bundles/course/assessment/utils/statisticsUtils.js @@ -12,6 +12,10 @@ const processAnswer = (answer) => ({ export const processSubmission = (submission) => { const totalGrade = submission.totalGrade != null ? parseFloat(submission.totalGrade) : null; + const maximumGrade = + submission.maximumGrade != null + ? parseFloat(submission.maximumGrade) + : null; const answers = submission.answers != null ? submission.answers.map(processAnswer) : null; const submittedAt = @@ -26,6 +30,7 @@ export const processSubmission = (submission) => { ...submission, answers, totalGrade, + maximumGrade, submittedAt, endAt, dayDifference, diff --git a/client/app/types/course/statistics/assessmentStatistics.ts b/client/app/types/course/statistics/assessmentStatistics.ts index 9601c36192a..68869e9a20f 100644 --- a/client/app/types/course/statistics/assessmentStatistics.ts +++ b/client/app/types/course/statistics/assessmentStatistics.ts @@ -9,14 +9,14 @@ interface AssessmentInfo { url: string; } -interface MainAssessmentInfo extends AssessmentInfo { +export interface MainAssessmentInfo extends AssessmentInfo { isAutograded: boolean; questionCount: number; questionIds: number[]; liveFeedbackEnabled: boolean; } -interface AncestorAssessmentInfo extends AssessmentInfo {} +export interface AncestorAssessmentInfo extends AssessmentInfo {} interface UserInfo { id: number; @@ -49,6 +49,7 @@ interface SubmissionInfo { submittedAt?: string; endAt?: string; totalGrade?: number | null; + maximumGrade?: number | null; } export interface MainSubmissionInfo extends SubmissionInfo { @@ -72,9 +73,10 @@ export interface AncestorInfo { } export interface MainAssessmentStats { - assessment: MainAssessmentInfo | null; - submissions: MainSubmissionInfo[]; - ancestors: AncestorInfo[]; + ancestorInfo: AncestorInfo[]; + assessmentStatistics: MainAssessmentInfo | null; + submissionStatistics: MainSubmissionInfo[]; + liveFeedbackStatistics: AssessmentLiveFeedbackStatistics[]; } export interface AncestorAssessmentStats { diff --git a/config/routes.rb b/config/routes.rb index 039e15fc526..72b3974f219 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -467,7 +467,9 @@ get 'course/progression' => 'aggregate#course_progression' get 'course/performance' => 'aggregate#course_performance' get 'user/:user_id/learning_rate_records' => 'users#learning_rate_records' - get 'assessment/:id/main_statistics' => 'assessments#main_statistics' + get 'assessment/:id/ancestor_info' => 'assessments#ancestor_info' + get 'assessment/:id/assessment_statistics' => 'assessments#assessment_statistics' + get 'assessment/:id/submission_statistics' => 'assessments#submission_statistics' get 'assessment/:id/ancestor_statistics' => 'assessments#ancestor_statistics' get 'assessment/:id/live_feedback_statistics' => 'assessments#live_feedback_statistics' get 'assessment/:id/live_feedback_history' => 'assessments#live_feedback_history' diff --git a/spec/controllers/course/statistics/assessment_controller_spec.rb b/spec/controllers/course/statistics/assessment_controller_spec.rb index f4f7b1a4dbf..3ee04251a9a 100644 --- a/spec/controllers/course/statistics/assessment_controller_spec.rb +++ b/spec/controllers/course/statistics/assessment_controller_spec.rb @@ -36,23 +36,23 @@ assessment: assessment, course: course, creator: teaching_assistant.user) end - describe '#main_statistics' do + describe '#assessment_statistics' do render_views - subject { get :main_statistics, as: :json, params: { course_id: course, id: assessment.id } } + subject { get :assessment_statistics, as: :json, params: { course_id: course, id: assessment.id } } - context 'when the Normal User get the main statistics data' do + context 'when the Normal User get the assessment statistics data' do let(:user) { create(:user) } before { controller_sign_in(controller, user) } it { expect { subject }.to raise_exception(CanCan::AccessDenied) } end - context 'when the Course Student get the main statistics data' do + context 'when the Course Student get the assessment statistics data' do let(:user) { create(:course_student, course: course).user } before { controller_sign_in(controller, user) } it { expect { subject }.to raise_exception(CanCan::AccessDenied) } end - context 'when the Course Manager get the main statistics data' do + context 'when the Course Manager get the assessment statistics data' do let(:user) { create(:course_manager, course: course).user } before { controller_sign_in(controller, user) } @@ -60,62 +60,142 @@ expect(subject).to have_http_status(:success) json_result = JSON.parse(response.body) + expect(json_result['title']).to eq(course.assessments.first.title) + expect(json_result['questionCount']).to eq(json_result['questionIds'].length) + end + end + + context 'when the administrator get the assessment statistics data' do + let(:administrator) { create(:administrator) } + before { controller_sign_in(controller, administrator) } + + it 'returns OK with right information and 2 ancestors (both current and its predecassors)' do + expect(subject).to have_http_status(:success) + json_result = JSON.parse(response.body) + + expect(json_result['title']).to eq(course.assessments.first.title) + expect(json_result['questionCount']).to eq(json_result['questionIds'].length) + end + end + end + + describe '#submission_statistics' do + render_views + subject { get :submission_statistics, as: :json, params: { course_id: course, id: assessment.id } } + + context 'when the Normal User get the submission statistics data' do + let(:user) { create(:user) } + before { controller_sign_in(controller, user) } + it { expect { subject }.to raise_exception(CanCan::AccessDenied) } + end + + context 'when the Course Student get the submission statistics data' do + let(:user) { create(:course_student, course: course).user } + before { controller_sign_in(controller, user) } + it { expect { subject }.to raise_exception(CanCan::AccessDenied) } + end + + context 'when the Course Manager get the submission statistics data' do + let(:user) { create(:course_manager, course: course).user } + before { controller_sign_in(controller, user) } + + it 'returns OK with right number of submissions being displayed' do + expect(subject).to have_http_status(:success) + json_result = JSON.parse(response.body) + # all the students data will be included here, including the non-existing submission one - expect(json_result['submissions'].count).to eq(4) + expect(json_result.count).to eq(4) # showing the correct workflow state - expect(json_result['submissions'][0]['workflowState']).to eq('published') - expect(json_result['submissions'][1]['workflowState']).to eq('attempting') - expect(json_result['submissions'][2]['workflowState']).to eq('graded') - expect(json_result['submissions'][3]['workflowState']).to eq('unstarted') + expect(json_result[0]['workflowState']).to eq('published') + expect(json_result[1]['workflowState']).to eq('attempting') + expect(json_result[2]['workflowState']).to eq('graded') + expect(json_result[3]['workflowState']).to eq('unstarted') # only graded and published submissions' answers will be included in the stats - expect(json_result['submissions'][0]['answers']).not_to be_nil - expect(json_result['submissions'][1]['answers']).to be_nil - expect(json_result['submissions'][2]['answers']).not_to be_nil - expect(json_result['submissions'][3]['answers']).to be_nil + expect(json_result[0]['answers']).not_to be_nil + expect(json_result[1]['answers']).to be_nil + expect(json_result[2]['answers']).not_to be_nil + expect(json_result[3]['answers']).to be_nil # only graded and published submissions' answers will be included in the stats - expect(json_result['submissions'][0]['courseUser']['role']).to eq('student') - expect(json_result['submissions'][1]['courseUser']['role']).to eq('student') - expect(json_result['submissions'][2]['courseUser']['role']).to eq('student') - expect(json_result['submissions'][3]['courseUser']['role']).to eq('student') - - # only 1 ancestor will be returned (current) as no permission for ancestor assessment - expect(json_result['ancestors'].count).to eq(1) + expect(json_result[0]['courseUser']['role']).to eq('student') + expect(json_result[1]['courseUser']['role']).to eq('student') + expect(json_result[2]['courseUser']['role']).to eq('student') + expect(json_result[3]['courseUser']['role']).to eq('student') end end - context 'when the administrator get the main statistics data' do + context 'when the administrator get the submission statistics data' do let(:administrator) { create(:administrator) } before { controller_sign_in(controller, administrator) } - it 'returns OK with right information and 2 ancestors (both current and its predecassors)' do + it 'returns OK with right information' do expect(subject).to have_http_status(:success) json_result = JSON.parse(response.body) # all the students data will be included here, including the non-existing submission one - expect(json_result['submissions'].count).to eq(4) + expect(json_result.count).to eq(4) # showing the correct workflow state - expect(json_result['submissions'][0]['workflowState']).to eq('published') - expect(json_result['submissions'][1]['workflowState']).to eq('attempting') - expect(json_result['submissions'][2]['workflowState']).to eq('graded') - expect(json_result['submissions'][3]['workflowState']).to eq('unstarted') + expect(json_result[0]['workflowState']).to eq('published') + expect(json_result[1]['workflowState']).to eq('attempting') + expect(json_result[2]['workflowState']).to eq('graded') + expect(json_result[3]['workflowState']).to eq('unstarted') # only graded and published submissions' answers will be included in the stats - expect(json_result['submissions'][0]['answers']).not_to be_nil - expect(json_result['submissions'][1]['answers']).to be_nil - expect(json_result['submissions'][2]['answers']).not_to be_nil - expect(json_result['submissions'][3]['answers']).to be_nil + expect(json_result[0]['answers']).not_to be_nil + expect(json_result[1]['answers']).to be_nil + expect(json_result[2]['answers']).not_to be_nil + expect(json_result[3]['answers']).to be_nil # only graded and published submissions' answers will be included in the stats - expect(json_result['submissions'][0]['courseUser']['role']).to eq('student') - expect(json_result['submissions'][1]['courseUser']['role']).to eq('student') - expect(json_result['submissions'][2]['courseUser']['role']).to eq('student') - expect(json_result['submissions'][3]['courseUser']['role']).to eq('student') + expect(json_result[0]['courseUser']['role']).to eq('student') + expect(json_result[1]['courseUser']['role']).to eq('student') + expect(json_result[2]['courseUser']['role']).to eq('student') + expect(json_result[3]['courseUser']['role']).to eq('student') + end + end + end + + describe '#ancestor_info' do + render_views + subject { get :ancestor_info, as: :json, params: { course_id: course, id: assessment.id } } + + context 'when the Normal User get the ancestor info data' do + let(:user) { create(:user) } + before { controller_sign_in(controller, user) } + it { expect { subject }.to raise_exception(CanCan::AccessDenied) } + end + + context 'when the Course Student get the ancestor info data' do + let(:user) { create(:course_student, course: course).user } + before { controller_sign_in(controller, user) } + it { expect { subject }.to raise_exception(CanCan::AccessDenied) } + end + + context 'when the Course Manager get the ancestor info data' do + let(:user) { create(:course_manager, course: course).user } + before { controller_sign_in(controller, user) } + + it 'returns OK with only 1 ancestor' do + expect(subject).to have_http_status(:success) + json_result = JSON.parse(response.body) + + # only 1 ancestor will be returned (current) as no permission for ancestor assessment + expect(json_result.count).to eq(1) + end + end + + context 'when the administrator get the ancestor info data' do + let(:administrator) { create(:administrator) } + before { controller_sign_in(controller, administrator) } + + it 'returns OK with right information and 2 ancestors (both current and its predecessors)' do + expect(subject).to have_http_status(:success) + json_result = JSON.parse(response.body) - expect(json_result['ancestors'].count).to eq(2) + expect(json_result.count).to eq(2) end end end