From c1841fc444d0a1f58b987c01dc64064a0f6ae821 Mon Sep 17 00:00:00 2001 From: adi-herwana-nus Date: Sat, 30 May 2026 17:19:05 +0800 Subject: [PATCH 1/3] feat(text-response): add regex/spreadsheet formula solution types - created spreadsheets table to capture spreadsheet-specific fields - added spreadsheet-formula-specific configuration section to solutions form --- .../course/assessment/question/controller.rb | 4 +- .../question/text_responses_controller.rb | 10 +- .../question/text_response_solution.rb | 17 +- .../text_response_solution_spreadsheet.rb | 30 ++++ .../_solution_details.json.jbuilder | 14 ++ .../Assessment/Question/TextResponse.ts | 9 +- client/app/assets/icons/randomize.svg | 1 + .../__test__/operations.test.ts | 100 +++++++---- .../text-responses/commons/validations.ts | 38 ++++- .../text-responses/components/Solution.tsx | 158 ++++++++++++++++-- .../components/SpreadsheetManagerPrompt.tsx | 105 ++++++++++++ .../question/text-responses/operations.ts | 109 +++++++++--- .../question/text-responses/utils.tsx | 48 ++++++ .../bundles/course/assessment/translations.ts | 60 +++++++ .../form/fields/SingleFileInput/index.tsx | 4 +- .../lib/components/icons/RandomizeIcon.tsx | 16 ++ .../assessment/question/text-responses.ts | 74 +++++++- client/locales/en.json | 42 +++++ client/locales/ko.json | 42 +++++ client/locales/zh.json | 42 +++++ ...spreadsheets_to_text_response_solutions.rb | 16 ++ db/schema.rb | 15 +- 22 files changed, 860 insertions(+), 94 deletions(-) create mode 100644 app/models/course/assessment/question/text_response_solution_spreadsheet.rb create mode 100644 client/app/assets/icons/randomize.svg create mode 100644 client/app/bundles/course/assessment/question/text-responses/components/SpreadsheetManagerPrompt.tsx create mode 100644 client/app/lib/components/icons/RandomizeIcon.tsx create mode 100644 db/migrate/20260423065615_add_test_spreadsheets_to_text_response_solutions.rb diff --git a/app/controllers/course/assessment/question/controller.rb b/app/controllers/course/assessment/question/controller.rb index e0be1f5ba31..15f2d157c21 100644 --- a/app/controllers/course/assessment/question/controller.rb +++ b/app/controllers/course/assessment/question/controller.rb @@ -45,7 +45,9 @@ def load_question_assessment_for(question) end def update_skill_ids_if_params_present(question_assessment_params) - skill_ids_params = question_assessment_params[:skill_ids] unless question_assessment_params[:skill_ids].nil? + return if question_assessment_params.nil? || question_assessment_params[:skill_ids].nil? + + skill_ids_params = question_assessment_params[:skill_ids] @question_assessment.skill_ids = skill_ids_params unless skill_ids_params.nil? end diff --git a/app/controllers/course/assessment/question/text_responses_controller.rb b/app/controllers/course/assessment/question/text_responses_controller.rb index d3a6ca2789e..0205b21fec2 100644 --- a/app/controllers/course/assessment/question/text_responses_controller.rb +++ b/app/controllers/course/assessment/question/text_responses_controller.rb @@ -88,7 +88,15 @@ def text_response_question_params ) else permitted_params.concat( - [solutions_attributes: [:_destroy, :id, :solution_type, :solution, :grade, :explanation]] + [solutions_attributes: [ + :_destroy, :id, :solution_type, :solution, :grade, :explanation, + test_spreadsheet_attributes: [ + :id, :file, :_destroy, :is_randomization_enabled, :num_random_tests, + :is_random_seed_fixed, :test_random_seed, + :is_timestamp_fixed, :test_timestamp, + :variables + ] + ]] ) end params.require(:question_text_response).permit(*permitted_params) diff --git a/app/models/course/assessment/question/text_response_solution.rb b/app/models/course/assessment/question/text_response_solution.rb index 74767c14314..4c0890b5639 100644 --- a/app/models/course/assessment/question/text_response_solution.rb +++ b/app/models/course/assessment/question/text_response_solution.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true class Course::Assessment::Question::TextResponseSolution < ApplicationRecord - enum :solution_type, [:exact_match, :keyword] + enum :solution_type, [:exact_match, :keyword, :regex, :spreadsheet_formula] before_validation :strip_whitespace before_save :sanitize_explanation @@ -12,9 +12,24 @@ class Course::Assessment::Question::TextResponseSolution < ApplicationRecord belongs_to :question, class_name: 'Course::Assessment::Question::TextResponse', inverse_of: :solutions + has_one :test_spreadsheet, class_name: 'Course::Assessment::Question::TextResponseSolutionSpreadsheet', + inverse_of: :solution, dependent: :destroy, autosave: true + accepts_nested_attributes_for :test_spreadsheet, allow_destroy: true + + def test_spreadsheet_attributes=(attributes) + if ActiveRecord::Type::Boolean.new.cast(attributes[:_destroy]) + test_spreadsheet&.mark_for_destruction + else + spreadsheet = test_spreadsheet || build_test_spreadsheet + spreadsheet.assign_params(attributes) + end + end def initialize_duplicate(duplicator, other) self.question = duplicator.duplicate(other.question) + if other.test_spreadsheet && other.spreadsheet_formula? + self.test_spreadsheet = duplicator.duplicate(other.test_spreadsheet) + end end private diff --git a/app/models/course/assessment/question/text_response_solution_spreadsheet.rb b/app/models/course/assessment/question/text_response_solution_spreadsheet.rb new file mode 100644 index 00000000000..44261d538fa --- /dev/null +++ b/app/models/course/assessment/question/text_response_solution_spreadsheet.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true +class Course::Assessment::Question::TextResponseSolutionSpreadsheet < ApplicationRecord + belongs_to :solution, class_name: 'Course::Assessment::Question::TextResponseSolution', + inverse_of: :test_spreadsheet + + has_one_attachment + + validates :attachment_reference, presence: true + + def assign_params(params) + assign_attributes( + params.slice(:is_randomization_enabled, :num_random_tests, :is_random_seed_fixed, + :test_random_seed, :is_timestamp_fixed, :test_timestamp) + ) + self.variables = JSON.parse(params[:variables]) if params[:variables] + self.file = params[:file] if params[:file] + end + + def initialize_duplicate(duplicator, other) + assign_attributes(other.attributes.slice('is_randomization_enabled', 'is_random_seed_fixed', + 'test_random_seed', 'is_timestamp_fixed', + 'test_timestamp', 'num_random_tests', 'variables')) + self.attachment = duplicator.duplicate(other.attachment) + end + + def container_filename + ext = attachment&.name ? File.extname(attachment.name) : '' + "sheet_#{id}#{ext}" + end +end diff --git a/app/views/course/assessment/question/text_responses/_solution_details.json.jbuilder b/app/views/course/assessment/question/text_responses/_solution_details.json.jbuilder index 6ee5c0857cc..e3f65309252 100644 --- a/app/views/course/assessment/question/text_responses/_solution_details.json.jbuilder +++ b/app/views/course/assessment/question/text_responses/_solution_details.json.jbuilder @@ -5,4 +5,18 @@ json.solutions question.solutions do |sol| json.solution sol.solution json.grade sol.grade json.explanation sol.explanation + json.spreadsheet do + json.id sol.test_spreadsheet.id + json.file do + json.name sol.test_spreadsheet.attachment.name + json.url attachment_reference_path(sol.test_spreadsheet.attachment) + end + json.isRandomizationEnabled sol.test_spreadsheet.is_randomization_enabled + json.isRandomSeedFixed sol.test_spreadsheet.is_random_seed_fixed + json.testRandomSeed sol.test_spreadsheet.test_random_seed + json.isTimestampFixed sol.test_spreadsheet.is_timestamp_fixed + json.testTimestamp sol.test_spreadsheet.test_timestamp + json.numRandomTests sol.test_spreadsheet.num_random_tests + json.variables sol.test_spreadsheet.variables + end if sol.test_spreadsheet end diff --git a/client/app/api/course/Assessment/Question/TextResponse.ts b/client/app/api/course/Assessment/Question/TextResponse.ts index 56b06b466d6..b09898564e9 100644 --- a/client/app/api/course/Assessment/Question/TextResponse.ts +++ b/client/app/api/course/Assessment/Question/TextResponse.ts @@ -1,7 +1,4 @@ -import { - TextResponseFormData, - TextResponsePostData, -} from 'types/course/assessment/question/text-responses'; +import { TextResponseFormData } from 'types/course/assessment/question/text-responses'; import { APIResponse, JustRedirect } from 'api/types'; @@ -26,11 +23,11 @@ export default class TextResponseAPI extends BaseAPI { return this.client.get(`${this.#urlPrefix}/${id}/edit`); } - create(data: TextResponsePostData): APIResponse { + create(data: FormData): APIResponse { return this.client.post(`${this.#urlPrefix}`, data); } - update(id: number, data: TextResponsePostData): APIResponse { + update(id: number, data: FormData): APIResponse { return this.client.patch(`${this.#urlPrefix}/${id}`, data); } } diff --git a/client/app/assets/icons/randomize.svg b/client/app/assets/icons/randomize.svg new file mode 100644 index 00000000000..d2a1b7e1df8 --- /dev/null +++ b/client/app/assets/icons/randomize.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/client/app/bundles/course/assessment/question/text-responses/__test__/operations.test.ts b/client/app/bundles/course/assessment/question/text-responses/__test__/operations.test.ts index b7f3b21106f..1ceb5bbd338 100644 --- a/client/app/bundles/course/assessment/question/text-responses/__test__/operations.test.ts +++ b/client/app/bundles/course/assessment/question/text-responses/__test__/operations.test.ts @@ -68,22 +68,16 @@ describe('solutions_attributes request payload', () => { ]), ); - expect(spy).toHaveBeenCalledWith( - expect.objectContaining({ - question_text_response: expect.objectContaining({ - solutions_attributes: [ - expect.objectContaining({ - id: 1, - solution: 'exact answer', - solution_type: 'exact_match', - grade: 5, - explanation: 'well done', - _destroy: undefined, - }), - ], - }), - }), - ); + expect(spy).toHaveBeenCalledWith(expect.any(FormData)); + const fd = spy.mock.calls[0][0] as FormData; + const base = 'question_text_response[solutions_attributes][0]'; + expect(fd.get(`${base}[id]`)).toBe('1'); + expect(fd.get(`${base}[solution]`)).toBe('exact answer'); + expect(fd.get(`${base}[solution_type]`)).toBe('exact_match'); + expect(fd.get(`${base}[grade]`)).toBe('5'); + expect(fd.get(`${base}[explanation]`)).toBe('well done'); + // not deleted — _destroy absent + expect(fd.get(`${base}[_destroy]`)).toBeNull(); }); it('omits id for draft (unsaved) solutions', async () => { @@ -106,18 +100,15 @@ describe('solutions_attributes request payload', () => { ]), ); - expect(spy).toHaveBeenCalledWith( - expect.objectContaining({ - question_text_response: expect.objectContaining({ - solutions_attributes: [ - expect.objectContaining({ id: undefined, solution: 'new answer' }), - ], - }), - }), - ); + expect(spy).toHaveBeenCalledWith(expect.any(FormData)); + const fd = spy.mock.calls[0][0] as FormData; + const base = 'question_text_response[solutions_attributes][0]'; + // draft: true causes id to be omitted (undefined → not appended) + expect(fd.get(`${base}[id]`)).toBeNull(); + expect(fd.get(`${base}[solution]`)).toBe('new answer'); }); - it('sends _destroy: true for solutions marked toBeDeleted', async () => { + it('sends _destroy for solutions marked toBeDeleted', async () => { const spy = jest.spyOn( CourseAPI.assessment.question.textResponse, 'update', @@ -138,15 +129,54 @@ describe('solutions_attributes request payload', () => { ]), ); - expect(spy).toHaveBeenCalledWith( - questionId, - expect.objectContaining({ - question_text_response: expect.objectContaining({ - solutions_attributes: [ - expect.objectContaining({ id: 2, _destroy: true }), - ], - }), - }), + expect(spy).toHaveBeenCalledWith(questionId, expect.any(FormData)); + const fd = spy.mock.calls[0][1] as FormData; + const base = 'question_text_response[solutions_attributes][0]'; + expect(fd.get(`${base}[id]`)).toBe('2'); + // booleans are encoded as '1' (true) / '0' (false) + expect(fd.get(`${base}[_destroy]`)).toBe('1'); + }); + + it('sends spreadsheet attributes for spreadsheet_formula solutions', async () => { + const spy = jest.spyOn( + CourseAPI.assessment.question.textResponse, + 'create', ); + mock.onPost().reply(200, { redirectUrl }); + + await create( + makeData([ + { + id: 1, + solution: '', + solutionType: 'spreadsheet_formula', + grade: 10, + explanation: '', + spreadsheet: { + isRandomizationEnabled: false, + isRandomSeedFixed: true, + randomSeed: 42, + isTimestampFixed: false, + testTimestamp: new Date(), + numRandomTests: 4, + variables: [], + file: { name: 'sheet.xlsx', url: '' }, + }, + }, + ]), + ); + + expect(spy).toHaveBeenCalledWith(expect.any(FormData)); + const fd = spy.mock.calls[0][0] as FormData; + const ss = 'question_text_response[solutions_attributes][0][test_spreadsheet_attributes]'; + expect(fd.get(`${ss}[is_randomization_enabled]`)).toBe('0'); + expect(fd.get(`${ss}[is_random_seed_fixed]`)).toBe('1'); + expect(fd.get(`${ss}[test_random_seed]`)).toBe('42'); + expect(fd.get(`${ss}[is_timestamp_fixed]`)).toBe('0'); + // null values are not appended + expect(fd.get(`${ss}[test_timestamp]`)).toBeNull(); + expect(fd.get(`${ss}[num_random_tests]`)).toBe('4'); + // variables serialised as JSON string + expect(fd.get(`${ss}[variables]`)).toBe('[]'); }); }); diff --git a/client/app/bundles/course/assessment/question/text-responses/commons/validations.ts b/client/app/bundles/course/assessment/question/text-responses/commons/validations.ts index f062f3b9709..cbc9babb54b 100644 --- a/client/app/bundles/course/assessment/question/text-responses/commons/validations.ts +++ b/client/app/bundles/course/assessment/question/text-responses/commons/validations.ts @@ -9,11 +9,24 @@ import { commonQuestionFieldsValidation } from '../../components/CommonQuestionF const solutionSchema = object({ solutionType: string().required(formTranslations.required), - solution: string().when('toBeDeleted', { - is: true, - then: string().notRequired(), - otherwise: string().required(formTranslations.required), - }), + solution: string() + .when('toBeDeleted', { + is: true, + then: string().notRequired(), + otherwise: string().required(formTranslations.required), + }) + .test('valid-regex', translations.invalidRegex, function (value) { + if (this.parent.toBeDeleted || this.parent.solutionType !== 'regex') + return true; + if (!value) return true; + try { + // eslint-disable-next-line no-new + new RegExp(value); + return true; + } catch { + return false; + } + }), grade: number().when('toBeDeleted', { is: true, then: number().notRequired(), @@ -23,6 +36,21 @@ const solutionSchema = object({ }), explanation: string().nullable(), toBeDeleted: bool(), + spreadsheet: object() + .nullable() + .when(['toBeDeleted', 'solutionType'], { + is: (toBeDeleted: boolean, solutionType: string) => + !toBeDeleted && solutionType === 'spreadsheet_formula', + then: object({ + file: object({ name: string(), url: string() }) + .nullable() + .test( + 'spreadsheet-file-required', + translations.testSpreadsheetRequired, + (value) => Boolean(value?.name), + ), + }), + }), }); export const questionSchema = ( diff --git a/client/app/bundles/course/assessment/question/text-responses/components/Solution.tsx b/client/app/bundles/course/assessment/question/text-responses/components/Solution.tsx index 53ce5713790..bdcc643854e 100644 --- a/client/app/bundles/course/assessment/question/text-responses/components/Solution.tsx +++ b/client/app/bundles/course/assessment/question/text-responses/components/Solution.tsx @@ -1,15 +1,28 @@ +import { FC, useEffect, useState } from 'react'; import { Controller, useFormContext } from 'react-hook-form'; import { Undo } from '@mui/icons-material'; -import { Alert, IconButton, Select, Tooltip } from '@mui/material'; +import { + Alert, + Button, + IconButton, + Select, + Tooltip, + Typography, +} from '@mui/material'; import FormHelperText from '@mui/material/FormHelperText'; import { TextResponseEditableFormData } from 'types/course/assessment/question/text-responses'; import CKEditorRichText from 'lib/components/core/fields/CKEditorRichText'; +import FormCheckboxField from 'lib/components/form/fields/CheckboxField'; +import FormSingleFileInput from 'lib/components/form/fields/SingleFileInput'; import FormTextField from 'lib/components/form/fields/TextField'; import { formatErrorMessage } from 'lib/components/form/fields/utils/mapError'; import useTranslation from 'lib/hooks/useTranslation'; import translations from '../../../translations'; +import { generateRandomSeed } from '../utils'; + +import SpreadsheetManagerPrompt from './SpreadsheetManagerPrompt'; interface SolutionProps { index: number; @@ -18,23 +31,44 @@ interface SolutionProps { disabled?: boolean; } -const Solution = ({ +const Solution: FC = ({ index, disabled, onDelete, onUndoDelete, -}: SolutionProps): JSX.Element => { +}: SolutionProps) => { const { t } = useTranslation(); - const { control, watch } = useFormContext(); + const { control, watch, setValue } = + useFormContext(); + + const solution = watch(`solutions.${index}`); + + const [isAdvancedPromptOpen, setIsAdvancedPromptOpen] = useState(false); - const toBeDeleted = watch(`solutions.${index}.toBeDeleted`); - const isDraft = watch(`solutions.${index}.draft`); + useEffect(() => { + if (solution?.solutionType === 'spreadsheet_formula') { + setValue(`solutions.${index}.spreadsheet`, { + isRandomizationEnabled: false, + isRandomSeedFixed: false, + randomSeed: generateRandomSeed(), + isTimestampFixed: false, + testTimestamp: new Date(), + numRandomTests: 2, + file: { name: '', url: '' }, + ...(solution.spreadsheet ?? {}), + }); + } + }, [solution?.solutionType]); + + if (!solution) { + return null; + } return (
@@ -46,9 +80,10 @@ const Solution = ({ render={({ field, fieldState }): JSX.Element => ( <> {fieldState.error && ( @@ -74,11 +113,12 @@ const Solution = ({ name={`solutions.${index}.grade`} render={({ field, fieldState }): JSX.Element => ( )} /> @@ -94,8 +134,8 @@ const Solution = ({ render={({ field, fieldState }): JSX.Element => ( <>