Skip to content

Commit e2f8d01

Browse files
adi-herwana-nusadi-herwana-nus
committed
feat(text-response): bug fixes
- render cells with formula differently and do not randomize by default - clarify random number/date values are rounded down - misc form fixes and refactoring - fix RAND() in sheet function behavior - remove dead code in client/app/lib/types.js
1 parent e856d25 commit e2f8d01

15 files changed

Lines changed: 176 additions & 132 deletions

File tree

‎client/app/bundles/course/assessment/question/text-responses/components/Solution.tsx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ const Solution: FC<SolutionProps> = ({
7070
return null;
7171
}
7272

73+
// Check spreadsheet existence to prevent early rendering causing issues (isRandomizationEnabled checkbox showing weird state)
74+
const shouldRenderSpreadsheetFields =
75+
solution.solutionType === 'spreadsheet_formula' && solution.spreadsheet;
76+
7377
return (
7478
<section
7579
className={`flex border-0 border-b border-solid border-neutral-200 last:border-b-0 ${
@@ -172,7 +176,7 @@ const Solution: FC<SolutionProps> = ({
172176
/>
173177
</div>
174178
</div>
175-
{solution.solutionType === 'spreadsheet_formula' && (
179+
{shouldRenderSpreadsheetFields && (
176180
<>
177181
<div className="flex flex-col space-y-1">
178182
<Typography variant="body1">

‎client/app/bundles/course/assessment/question/text-responses/components/SpreadsheetRandomizationManager.tsx‎

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,19 @@ import {
1313
import {
1414
CellRandomConfig,
1515
CellRandomConfigBody,
16-
SpreadsheetCellValue,
1716
} from 'types/course/assessment/question/text-responses';
1817

19-
import SpreadsheetPreview, {
20-
getCellKey,
18+
import { SpreadsheetCellValueView } from 'lib/components/form/fields/SingleFileInput/SpreadsheetCellValueView';
19+
import SpreadsheetPreview from 'lib/components/form/fields/SingleFileInput/SpreadsheetPreview';
20+
import {
2121
GridData,
22-
parseSpreadsheet,
23-
renderCellValue,
2422
SheetGrid,
25-
} from 'lib/components/form/fields/SingleFileInput/SpreadsheetPreview';
23+
SpreadsheetCellValue,
24+
} from 'lib/components/form/fields/SingleFileInput/types';
25+
import {
26+
getCellKey,
27+
parseSpreadsheet,
28+
} from 'lib/components/form/fields/SingleFileInput/utils';
2629
import AZIcon from 'lib/components/icons/AZIcon';
2730
import OneNineIcon from 'lib/components/icons/OneNineIcon';
2831
import RandomizeIcon from 'lib/components/icons/RandomizeIcon';
@@ -179,7 +182,9 @@ const CellOverrideContent: FC<{
179182
case 'override':
180183
return (
181184
<span className="flex-1 text-blue-800">
182-
{cellConfig.override?.value ?? renderCellValue(cellValue)}
185+
<SpreadsheetCellValueView
186+
cellValue={cellConfig.override?.value ?? cellValue}
187+
/>
183188
</span>
184189
);
185190
case 'numeric':
@@ -188,18 +193,22 @@ const CellOverrideContent: FC<{
188193
cellConfig.numeric?.max !== undefined
189194
) {
190195
const min = cellConfig.numeric.roundToInteger
191-
? Math.round(cellConfig.numeric.min)
196+
? Math.floor(cellConfig.numeric.min)
192197
: cellConfig.numeric.min;
193198
const max = cellConfig.numeric.roundToInteger
194-
? Math.round(cellConfig.numeric.max)
199+
? Math.floor(cellConfig.numeric.max)
195200
: cellConfig.numeric.max;
196201
return (
197202
<span className="flex-1 text-blue-800">
198203
{min} - {max}
199204
</span>
200205
);
201206
}
202-
return <span className="flex-1">{renderCellValue(cellValue)}</span>;
207+
return (
208+
<span className="flex-1">
209+
<SpreadsheetCellValueView cellValue={cellValue} />
210+
</span>
211+
);
203212
case 'date':
204213
if (
205214
cellConfig.date?.min !== undefined &&
@@ -212,16 +221,24 @@ const CellOverrideContent: FC<{
212221
</span>
213222
);
214223
}
215-
return <span className="flex-1">{renderCellValue(cellValue)}</span>;
224+
return (
225+
<span className="flex-1">
226+
<SpreadsheetCellValueView cellValue={cellValue} />
227+
</span>
228+
);
216229
case 'string':
217230
case 'shuffle':
218231
return (
219232
<span className="flex-1 text-blue-800">
220-
{renderCellValue(cellValue)}
233+
<SpreadsheetCellValueView cellValue={cellValue} />
221234
</span>
222235
);
223236
default:
224-
return <span className="flex-1">{renderCellValue(cellValue)}</span>;
237+
return (
238+
<span className="flex-1">
239+
<SpreadsheetCellValueView cellValue={cellValue} />
240+
</span>
241+
);
225242
}
226243
};
227244

@@ -239,6 +256,7 @@ const SpreadsheetRandomizationManager: FC<Props> = ({
239256
const [gridData, setGridData] = useState<GridData | null>(null);
240257
const popoverActionsRef = useRef<PopoverActions>(null);
241258
const spreadsheetRef = useRef<HTMLDivElement>(null);
259+
const fileRef = useRef<File | undefined>(undefined);
242260

243261
const updateCellConfigs = (newState: CellRandomConfigState): void => {
244262
setCellConfigs(newState);
@@ -258,9 +276,17 @@ const SpreadsheetRandomizationManager: FC<Props> = ({
258276
}
259277
};
260278

279+
// If there is a previous file, we always override because old config structure is no longer valid.
280+
// If there are initial variables, it implies this is the first render editing a spreadsheet formula, so don't override.
281+
// If there are no initial variables, this triggers when a file is uploaded for the first time creating new spreadsheet formula solution.
261282
useEffect(() => {
262-
if (!file) return;
263-
initSpreadsheetDefaultConfigFromFile(cellConfigs === undefined);
283+
initSpreadsheetDefaultConfigFromFile(
284+
Boolean(fileRef.current) || !initialVariables,
285+
);
286+
fileRef.current = file;
287+
return (): void => {
288+
fileRef.current = undefined;
289+
};
264290
}, [file]);
265291

266292
const isEditableElement = (el: Element | null): boolean => {

‎client/app/bundles/course/assessment/question/text-responses/utils.tsx‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@ import {
22
AttachmentType,
33
CellRandomConfig,
44
CellRandomConfigBody,
5-
SpreadsheetCellValue,
65
TextResponseQuestionFormData,
76
} from 'types/course/assessment/question/text-responses';
87

8+
import { SpreadsheetCellValue } from 'lib/components/form/fields/SingleFileInput/types';
9+
910
export const getAttachmentTypeFromMaxAttachment = (
1011
maxAttachments: number | undefined,
1112
): AttachmentType => {
@@ -57,7 +58,10 @@ export const getDefaultRandomizationMode = (
5758
if (cellValue instanceof Date) return 'date';
5859
if (typeof cellValue === 'number' || !Number.isNaN(Number(cellValue)))
5960
return 'numeric';
60-
return cellValue.length > 0 ? 'string' : 'off';
61+
if (typeof cellValue === 'string') {
62+
return cellValue.startsWith('=') ? 'off' : 'string';
63+
}
64+
return 'off';
6165
};
6266

6367
export function getDefaultRandomizationConfig<

‎client/app/bundles/course/assessment/translations.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1174,11 +1174,11 @@ const translations = defineMessages({
11741174
},
11751175
roundToInteger: {
11761176
id: 'course.assessment.question.textResponses.roundToInteger',
1177-
defaultMessage: 'Round to nearest integer',
1177+
defaultMessage: 'Round down to nearest integer',
11781178
},
11791179
roundToDay: {
11801180
id: 'course.assessment.question.textResponses.roundToDay',
1181-
defaultMessage: 'Round to nearest day (00:00)',
1181+
defaultMessage: 'Round down to day start (00:00)',
11821182
},
11831183
grade: {
11841184
id: 'course.assessment.question.textResponses.grade',

‎client/app/lib/components/form/fields/SingleFileInput/SpreadsheetCellValueView.tsx‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { FC } from 'react';
2+
3+
import { formatRawDate } from 'lib/moment';
4+
5+
import { SpreadsheetCellValue } from './types';
6+
7+
export const SpreadsheetCellValueView: FC<{
8+
cellValue: SpreadsheetCellValue;
9+
}> = ({ cellValue }) => {
10+
if (cellValue === undefined) return '';
11+
if (typeof cellValue === 'string') {
12+
if (cellValue.startsWith('=')) {
13+
return <i>{cellValue}</i>;
14+
}
15+
return cellValue;
16+
}
17+
if (typeof cellValue === 'number') return String(cellValue);
18+
if (cellValue instanceof Date) return formatRawDate(cellValue);
19+
return String(cellValue);
20+
};

‎client/app/lib/components/form/fields/SingleFileInput/SpreadsheetPreview.tsx‎

Lines changed: 8 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import { FC, useEffect, useState } from 'react';
2-
import { SpreadsheetCellValue } from 'types/course/assessment/question/text-responses';
3-
import * as XLSX from 'xlsx';
42

53
import useTranslation from 'lib/hooks/useTranslation';
6-
import { formatRawDate } from 'lib/moment';
74

5+
import { SpreadsheetCellValueView } from './SpreadsheetCellValueView';
86
import translations from './translations';
7+
import { GridData, SpreadsheetCellValue } from './types';
8+
import { getCellKey, parseSpreadsheet } from './utils';
99

1010
interface SpreadsheetPreviewProps {
1111
file: File | null;
@@ -27,89 +27,6 @@ interface SpreadsheetPreviewProps {
2727
) => React.ReactNode;
2828
}
2929

30-
export interface SheetGrid {
31-
headers: string[];
32-
rows: SpreadsheetCellValue[][];
33-
rowOffset: number;
34-
colOffset: number;
35-
}
36-
37-
export interface GridData {
38-
sheetNames: string[];
39-
sheets: SheetGrid[];
40-
}
41-
42-
export const getCellKey = (
43-
rowIdx: number,
44-
colIdx: number,
45-
sheetName: string,
46-
): string => `${sheetName}!${XLSX.utils.encode_cell({ r: rowIdx, c: colIdx })}`;
47-
48-
const parseCellValue = (cell: XLSX.CellObject): SpreadsheetCellValue => {
49-
if (cell?.v === null || cell?.v === undefined) return undefined;
50-
if (cell.f !== undefined) return cell.f;
51-
if (cell.t === 'd' && cell.v instanceof Date) return cell.v;
52-
if (typeof cell.v === 'number') return cell.v;
53-
return String(cell.v);
54-
};
55-
56-
const parseSheetGrid = (worksheet: XLSX.WorkSheet): SheetGrid => {
57-
const ref = worksheet['!ref'];
58-
if (!ref) return { headers: [], rows: [], rowOffset: 0, colOffset: 0 };
59-
60-
const range = XLSX.utils.decode_range(ref);
61-
const numCols = range.e.c - range.s.c + 1;
62-
const numRows = range.e.r - range.s.r + 1;
63-
const headers: string[] = [];
64-
const allRows: SpreadsheetCellValue[][] = [];
65-
66-
for (let c = 0; c < numCols; c++) {
67-
headers.push(XLSX.utils.encode_col(range.s.c + c));
68-
}
69-
70-
for (let r = 0; r < numRows; r++) {
71-
const row: SpreadsheetCellValue[] = [];
72-
for (let c = 0; c < numCols; c++) {
73-
const rowIdx = range.s.r + r;
74-
const colIdx = range.s.c + c;
75-
const cell = worksheet[XLSX.utils.encode_cell({ r: rowIdx, c: colIdx })];
76-
77-
row.push(parseCellValue(cell));
78-
}
79-
allRows.push(row);
80-
}
81-
82-
const lastNonBlank = allRows.reduce(
83-
(last, row, idx) =>
84-
row.some((c) => c !== undefined && String(c).trim() !== '') ? idx : last,
85-
-1,
86-
);
87-
const rows = lastNonBlank >= 0 ? allRows.slice(0, lastNonBlank + 1) : [];
88-
return { headers, rows, rowOffset: range.s.r, colOffset: range.s.c };
89-
};
90-
91-
export const parseSpreadsheet = async (file: File): Promise<GridData> => {
92-
const buffer = await file.arrayBuffer();
93-
const workbook = XLSX.read(buffer, {
94-
cellDates: true,
95-
cellNF: true,
96-
cellFormula: true,
97-
UTC: true,
98-
});
99-
const sheets = workbook.SheetNames.map((name) =>
100-
parseSheetGrid(workbook.Sheets[name]),
101-
);
102-
return { sheetNames: workbook.SheetNames, sheets };
103-
};
104-
105-
export const renderCellValue = (cellValue: SpreadsheetCellValue): string => {
106-
if (cellValue === undefined) return '';
107-
if (typeof cellValue === 'string') return cellValue;
108-
if (typeof cellValue === 'number') return String(cellValue);
109-
if (cellValue instanceof Date) return formatRawDate(cellValue);
110-
return String(cellValue);
111-
};
112-
11330
const SpreadsheetPreview: FC<SpreadsheetPreviewProps> = ({
11431
file,
11532
activeCellKey,
@@ -163,7 +80,6 @@ const SpreadsheetPreview: FC<SpreadsheetPreviewProps> = ({
16380
<div className="flex min-w-0 flex-col">
16481
<div
16582
className={`overflow-auto border border-solid border-neutral-300 ${multiSheet ? 'rounded-t' : 'rounded'}`}
166-
style={{ maxHeight: 320 }}
16783
>
16884
<table
16985
className="border-collapse cursor-default select-none text-md"
@@ -223,9 +139,11 @@ const SpreadsheetPreview: FC<SpreadsheetPreviewProps> = ({
223139
);
224140
}}
225141
>
226-
{renderCell
227-
? renderCell(cellValue, cellKey)
228-
: renderCellValue(cellValue)}
142+
{renderCell ? (
143+
renderCell(cellValue, cellKey)
144+
) : (
145+
<SpreadsheetCellValueView cellValue={cellValue} />
146+
)}
229147
</td>
230148
);
231149
})}

‎client/app/lib/components/form/fields/SingleFileInput/types.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
export type SpreadsheetCellValue = string | number | Date | undefined;
2+
3+
export interface SheetGrid {
4+
headers: string[];
5+
rows: SpreadsheetCellValue[][];
6+
rowOffset: number;
7+
colOffset: number;
8+
}
9+
10+
export interface GridData {
11+
sheetNames: string[];
12+
sheets: SheetGrid[];
13+
}

0 commit comments

Comments
 (0)