Skip to content

Commit e04f7f0

Browse files
authored
Fix incorrect assessment overview memoization (#3304)
* Move assessment overview card to separate file Also refactor to React.FC. * Refactor interact button and move to separate file * Fix deprecations * Remove unused index param * Fix incorrect memoization * Fix deprecations * Update snapshots
1 parent 9101d2e commit e04f7f0

File tree

4 files changed

+278
-208
lines changed

4 files changed

+278
-208
lines changed

src/commons/assessment/Assessment.tsx

Lines changed: 34 additions & 205 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,9 @@
11
import {
22
Button,
3-
Card,
43
Collapse,
54
Dialog,
65
DialogBody,
76
DialogFooter,
8-
Elevation,
9-
H4,
10-
H6,
11-
Icon,
12-
IconName,
137
Intent,
148
NonIdealState,
159
Position,
@@ -18,31 +12,26 @@ import {
1812
Tooltip
1913
} from '@blueprintjs/core';
2014
import { IconNames } from '@blueprintjs/icons';
21-
import classNames from 'classnames';
2215
import { sortBy } from 'lodash';
2316
import React, { useEffect, useMemo, useState } from 'react';
2417
import { useDispatch } from 'react-redux';
25-
import { Navigate, NavLink, useLoaderData, useParams } from 'react-router';
18+
import { Navigate, useLoaderData, useParams } from 'react-router';
2619
import { numberRegExp } from 'src/features/academy/AcademyTypes';
2720
import Messages, { sendToWebview } from 'src/features/vscode/messages';
28-
import classes from 'src/styles/Academy.module.scss';
2921

30-
import defaultCoverImage from '../../assets/default_cover_image.jpg';
3122
import SessionActions from '../application/actions/SessionActions';
3223
import { Role } from '../application/ApplicationTypes';
3324
import AssessmentWorkspace, {
3425
AssessmentWorkspaceProps
3526
} from '../assessmentWorkspace/AssessmentWorkspace';
3627
import ContentDisplay from '../ContentDisplay';
3728
import ControlButton from '../ControlButton';
38-
import Markdown from '../Markdown';
39-
import NotificationBadge from '../notificationBadge/NotificationBadge';
40-
import { filterNotificationsByAssessment } from '../notificationBadge/NotificationBadgeHelper';
4129
import Constants from '../utils/Constants';
42-
import { beforeNow, getPrettyDate, getPrettyDateAfterHours } from '../utils/DateHelper';
43-
import { useResponsive, useSession, useTypedSelector } from '../utils/Hooks';
44-
import { assessmentTypeLink, convertParamToInt } from '../utils/ParamParseHelper';
30+
import { beforeNow } from '../utils/DateHelper';
31+
import { useSession, useTypedSelector } from '../utils/Hooks';
32+
import { convertParamToInt } from '../utils/ParamParseHelper';
4533
import AssessmentNotFound from './AssessmentNotFound';
34+
import AssessmentOverviewCard from './AssessmentOverviewCard';
4635
import {
4736
AssessmentConfiguration,
4837
AssessmentOverview,
@@ -52,7 +41,6 @@ import {
5241

5342
const Assessment: React.FC = () => {
5443
const params = useParams<AssessmentWorkspaceParams>();
55-
const { isMobileBreakpoint } = useResponsive();
5644
const [betchaAssessment, setBetchaAssessment] = useState<AssessmentOverview | null>(null);
5745
const [showClosedAssessments, setShowClosedAssessments] = useState(false);
5846
const [showOpenedAssessments, setShowOpenedAssessments] = useState(true);
@@ -91,7 +79,7 @@ const Assessment: React.FC = () => {
9179

9280
const sortAssessments = (assessments: AssessmentOverview[]) => sortBy(assessments, [a => -a.id]);
9381

94-
const makeSubmissionButton = (overview: AssessmentOverview, index: number) => (
82+
const makeSubmissionButton = (overview: AssessmentOverview) => (
9583
<Tooltip
9684
disabled={overview.status === AssessmentStatuses.attempted}
9785
content={'You can finalize after saving an answer for each question!'}
@@ -101,7 +89,7 @@ const Assessment: React.FC = () => {
10189
disabled={overview.status !== AssessmentStatuses.attempted}
10290
icon={IconNames.CONFIRM}
10391
intent={overview.status === AssessmentStatuses.attempted ? Intent.DANGER : Intent.NONE}
104-
minimal={true}
92+
variant="minimal"
10593
// intentional: each listing renders its own version of onClick
10694
// tslint:disable-next-line:jsx-no-lambda
10795
onClick={() => setBetchaAssessment(overview)}
@@ -112,166 +100,6 @@ const Assessment: React.FC = () => {
112100
</Tooltip>
113101
);
114102

115-
const makeAssessmentInteractButton = (overview: AssessmentOverview) => {
116-
let icon: IconName;
117-
let label: string;
118-
let optionalLabel: string = '';
119-
120-
switch (overview.status) {
121-
case AssessmentStatuses.not_attempted:
122-
icon = IconNames.PLAY;
123-
label = 'Attempt';
124-
break;
125-
case AssessmentStatuses.attempting:
126-
icon = IconNames.PLAY;
127-
label = 'Continue';
128-
optionalLabel = ' Attempt';
129-
break;
130-
case AssessmentStatuses.attempted:
131-
icon = IconNames.EDIT;
132-
label = 'Review';
133-
optionalLabel = ' Attempt';
134-
break;
135-
case AssessmentStatuses.submitted:
136-
icon = IconNames.EYE_OPEN;
137-
label = 'Review';
138-
optionalLabel = ' Submission';
139-
break;
140-
default:
141-
// If we reach this case, backend data did not fit IAssessmentOverview
142-
icon = IconNames.PLAY;
143-
label = 'Review';
144-
break;
145-
}
146-
return (
147-
<NavLink
148-
to={`/courses/${courseId}/${assessmentTypeLink(overview.type)}/${overview.id.toString()}/${
149-
Constants.defaultQuestionId
150-
}`}
151-
>
152-
<Button
153-
icon={icon}
154-
minimal={true}
155-
onClick={() =>
156-
dispatch(
157-
SessionActions.acknowledgeNotifications(filterNotificationsByAssessment(overview.id))
158-
)
159-
}
160-
>
161-
<span data-testid="Assessment-Attempt-Button">{label}</span>
162-
<span className="custom-hidden-xxxs">{optionalLabel}</span>
163-
</Button>
164-
</NavLink>
165-
);
166-
};
167-
168-
/**
169-
* Create a series of cards to display IAssessmentOverviews.
170-
* @param {AssessmentOverview} overview the assessment overview to display
171-
* @param {number} index a unique number for this card (required for sequential rendering).
172-
* See {@link https://reactjs.org/docs/lists-and-keys.html#keys}
173-
* @param renderAttemptButton will only render the attempt button if true, regardless
174-
* of attempt status.
175-
* @param notifications the notifications to be passed in.
176-
*/
177-
const makeOverviewCard = (
178-
overview: AssessmentOverview,
179-
index: number,
180-
renderAttemptButton: boolean,
181-
renderGradingTooltip: boolean
182-
) => {
183-
return (
184-
<div key={index}>
185-
<Card className="row listing" elevation={Elevation.ONE}>
186-
<div className={classNames('listing-picture', !isMobileBreakpoint && 'col-xs-3')}>
187-
<NotificationBadge
188-
className="badge"
189-
notificationFilter={filterNotificationsByAssessment(overview.id)}
190-
large={true}
191-
/>
192-
<img
193-
alt="Assessment"
194-
className={`cover-image-${overview.status}`}
195-
src={overview.coverImage ? overview.coverImage : defaultCoverImage}
196-
/>
197-
</div>
198-
<div className={classNames('listing-text', !isMobileBreakpoint && 'col-xs-9')}>
199-
{makeOverviewCardTitle(overview, index, renderGradingTooltip)}
200-
<div className={classes['listing-xp']}>
201-
<H6>
202-
{overview.isGradingPublished
203-
? `XP: ${overview.xp} / ${overview.maxXp}`
204-
: `Max XP: ${overview.maxXp}`}
205-
</H6>
206-
{overview.earlySubmissionXp > 0 && (
207-
<Tooltip
208-
content={`Max XP ends on ${getPrettyDateAfterHours(overview.openAt, overview.hoursBeforeEarlyXpDecay)}`}
209-
>
210-
<Icon icon={IconNames.InfoSign} />
211-
</Tooltip>
212-
)}
213-
</div>
214-
<div className="listing-description">
215-
<Markdown content={overview.shortSummary} />
216-
</div>
217-
{overview.maxTeamSize > 1 ? (
218-
<div className="listing-team_information">
219-
<H6> This is a team assessment. </H6>
220-
</div>
221-
) : (
222-
<div>
223-
<H6> This is an individual assessment. </H6>
224-
</div>
225-
)}
226-
<div className="listing-footer">
227-
<div>
228-
<Text className="listing-due-date">
229-
<Icon className="listing-due-icon" size={12} icon={IconNames.CALENDAR} />
230-
{`${beforeNow(overview.openAt) ? 'Opened' : 'Opens'}: ${getPrettyDate(
231-
overview.openAt
232-
)}`}
233-
</Text>
234-
{beforeNow(overview.openAt) && (
235-
<Text className="listing-due-date">
236-
<Icon className="listing-due-icon" size={12} icon={IconNames.TIME} />
237-
{`Due: ${getPrettyDate(overview.closeAt)}`}
238-
</Text>
239-
)}
240-
</div>
241-
<div className="listing-button">
242-
{renderAttemptButton ? makeAssessmentInteractButton(overview) : null}
243-
</div>
244-
</div>
245-
</div>
246-
</Card>
247-
</div>
248-
);
249-
};
250-
251-
const makeOverviewCardTitle = (
252-
overview: AssessmentOverview,
253-
index: number,
254-
renderProgressStatus: boolean
255-
) => (
256-
<div className="listing-header">
257-
<Text ellipsize={true}>
258-
<H4 className="listing-title">
259-
{overview.title}
260-
{overview.private ? (
261-
<Tooltip
262-
className="listing-title-tooltip"
263-
content="This assessment is password-protected."
264-
>
265-
<Icon icon="lock" />
266-
</Tooltip>
267-
) : null}
268-
{renderProgressStatus ? showGradingTooltip(overview.isGradingPublished) : null}
269-
</H4>
270-
</Text>
271-
<div className="listing-button">{makeSubmissionButton(overview, index)}</div>
272-
</div>
273-
);
274-
275103
// Rendering Logic
276104
const assessmentConfigToLoad = useLoaderData() as AssessmentConfiguration;
277105
const assessmentOverviews = useMemo(
@@ -326,7 +154,15 @@ const Assessment: React.FC = () => {
326154
const isOverviewUpcoming = (overview: AssessmentOverview) =>
327155
!beforeNow(overview.closeAt) && !beforeNow(overview.openAt);
328156
const upcomingCards = sortAssessments(assessmentOverviews.filter(isOverviewUpcoming)).map(
329-
(overview, index) => makeOverviewCard(overview, index, role !== Role.Student, false)
157+
overview => (
158+
<AssessmentOverviewCard
159+
key={overview.id}
160+
overview={overview}
161+
renderAttemptButton={role !== Role.Student}
162+
renderGradingTooltip={false}
163+
makeSubmissionButton={makeSubmissionButton}
164+
/>
165+
)
330166
);
331167

332168
/** Opened assessments, that are released and can be attempted. */
@@ -336,14 +172,30 @@ const Assessment: React.FC = () => {
336172
overview.status !== AssessmentStatuses.submitted;
337173
const openedCards = sortAssessments(
338174
assessmentOverviews.filter(overview => isOverviewOpened(overview))
339-
).map((overview, index) => makeOverviewCard(overview, index, true, false));
175+
).map(overview => (
176+
<AssessmentOverviewCard
177+
key={overview.id}
178+
overview={overview}
179+
renderAttemptButton
180+
renderGradingTooltip={false}
181+
makeSubmissionButton={makeSubmissionButton}
182+
/>
183+
));
340184

341185
/** Closed assessments, that are past the due date or cannot be attempted further. */
342186
const closedCards = sortAssessments(
343187
assessmentOverviews.filter(
344188
overview => !isOverviewOpened(overview) && !isOverviewUpcoming(overview)
345189
)
346-
).map((overview, index) => makeOverviewCard(overview, index, true, true));
190+
).map(overview => (
191+
<AssessmentOverviewCard
192+
key={overview.id}
193+
overview={overview}
194+
renderAttemptButton
195+
renderGradingTooltip
196+
makeSubmissionButton={makeSubmissionButton}
197+
/>
198+
));
347199

348200
/** Render cards */
349201
const upcomingCardsCollapsible = (
@@ -448,29 +300,6 @@ const Assessment: React.FC = () => {
448300
);
449301
};
450302

451-
const showGradingTooltip = (isGradingPublished: boolean) => {
452-
let iconName: IconName;
453-
let intent: Intent;
454-
let tooltip: string;
455-
456-
if (isGradingPublished) {
457-
iconName = IconNames.TICK;
458-
intent = Intent.SUCCESS;
459-
tooltip = 'Fully graded';
460-
} else {
461-
// shh, hide actual grading progress from users even if graded
462-
iconName = IconNames.TIME;
463-
intent = Intent.WARNING;
464-
tooltip = 'Grading in progress';
465-
}
466-
467-
return (
468-
<Tooltip className="listing-title-tooltip" content={tooltip} placement={Position.RIGHT}>
469-
<Icon icon={iconName} intent={intent} />
470-
</Tooltip>
471-
);
472-
};
473-
474303
const collapseButton = (label: string, isOpen: boolean, toggleFunc: () => void) => (
475304
<ControlButton
476305
label={label}

0 commit comments

Comments
 (0)