Assessments tab: total student submission counts #23353
Conversation
| @@ -101,6 +107,7 @@ export default connect(state => ({ | |||
| assessmentList: getCurrentScriptAssessmentList(state), | |||
| scriptId: state.scriptSelection.scriptId, | |||
| assessmentId: state.sectionAssessments.assessmentId, | |||
| totalStudentCount: getTotalStudentCount(state), | |||
There was a problem hiding this comment.
It looks like this component only cares about totalStudentCount so it can pass it through to child components, but the child components are themselves redux-connected and could retrieve this information themselves. Thoughts on just letting MCAssessmentsOverviewContainer and MCSurveyOverviewContainer ask for the total student count directly?
| * Selector function that takes in the state and a boolean indicating | ||
| * if the current assessment is a survey. | ||
| * Returns an integer total count of the number of students who have submitted | ||
| * the current assessment. |
There was a problem hiding this comment.
I'd prefer JSDoc @returns for consistency.
/**
* @returns {number} count of the number of students who have submitted
* the current assessment.
*/| * Returns an integer total count of the number of students who have submitted | ||
| * the current assessment. | ||
| */ | ||
| export const getTotalSubmissionForCurrentAssessment = (state, isSurvey) => { |
There was a problem hiding this comment.
Should Submissions be plural in this name? I'd also consider naming this countSubmissionsForCurrentAssessment which strongly implies that the return value is an integer and hints that some nontrivial computation is going on when you call this method, not just a simple lookup in our state tree.
Updates overview headers for surveys and assessments to include submission and student counts.
Total students comes from
sectionDataReduxand is a simple length operation. Total submissions comes fromsectionAssessmentsReduxand requires some iteration.