fix: consolidate worksheet validation constants and increase MAX_PAGES to 100
**Problem**: Worksheet seeds were not persisting because configs with >20 pages
failed Zod schema validation (max was 20), even though the code elsewhere allowed
up to 2000 total problems. When validation failed, the config fell back to defaults,
losing the saved seed.
**Changes**:
1. Created `constants/validation.ts` with centralized limits:
- MAX_PAGES: 100 (was 20 in schemas)
- MAX_TOTAL_PROBLEMS: 2000
- MAX_PROBLEMS_PER_PAGE: 100
- MAX_COLS: 10
- DIGIT_RANGE: { MIN: 1, MAX: 5 }
- FONT_SIZE: { MIN: 8, MAX: 32 }
- Helper function validateWorksheetLimits()
2. Updated all Zod schema versions (V1-V4) in `config-schemas.ts` to use constants
3. Updated runtime validation in `validation.ts` to use constants
4. Enhanced settings API (`route.ts`) to:
- Validate worksheet limits before saving
- Validate against Zod schema
- Return clear error messages with actionable guidance
5. Enhanced auto-save hook (`useWorksheetAutoSave.ts`) to:
- Return saveError state
- Surface validation errors to user
- Clear errors on successful save
6. Removed TEN-FRAMES DEBUG logging from displayRules.ts
**Impact**: Worksheet configs with 21-100 pages will now pass validation and
persist correctly, including the seed. Users will see clear error messages if
their config exceeds limits.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Thomas Hallock
parent
8fb68334d5
commit
0f3ec369bf
|
|
@ -6,7 +6,12 @@ import {
|
|||
parseAdditionConfig,
|
||||
serializeAdditionConfig,
|
||||
defaultAdditionConfig,
|
||||
additionConfigSchema,
|
||||
} from '@/app/create/worksheets/config-schemas'
|
||||
import {
|
||||
WORKSHEET_LIMITS,
|
||||
validateWorksheetLimits,
|
||||
} from '@/app/create/worksheets/constants/validation'
|
||||
|
||||
/**
|
||||
* GET /api/worksheets/settings?type=addition
|
||||
|
|
@ -105,6 +110,33 @@ export async function POST(req: NextRequest) {
|
|||
)
|
||||
}
|
||||
|
||||
// Validate worksheet limits before saving
|
||||
const validation = validateWorksheetLimits(config.problemsPerPage, config.pages)
|
||||
if (!validation.valid) {
|
||||
return NextResponse.json(
|
||||
{
|
||||
success: false,
|
||||
error: validation.error,
|
||||
},
|
||||
{ status: 400 }
|
||||
)
|
||||
}
|
||||
|
||||
// Validate against schema (this will check all field types and ranges)
|
||||
const schemaValidation = additionConfigSchema.safeParse({ ...config, version: 4 })
|
||||
if (!schemaValidation.success) {
|
||||
const errorMessages = schemaValidation.error.issues
|
||||
.map((err) => `${err.path.join('.')}: ${err.message}`)
|
||||
.join(', ')
|
||||
return NextResponse.json(
|
||||
{
|
||||
success: false,
|
||||
error: `Invalid configuration: ${errorMessages}`,
|
||||
},
|
||||
{ status: 400 }
|
||||
)
|
||||
}
|
||||
|
||||
// Serialize config (adds version automatically)
|
||||
const configJson = serializeAdditionConfig(config)
|
||||
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import { z } from 'zod'
|
||||
import { getProfileFromConfig } from './difficultyProfiles'
|
||||
import { WORKSHEET_LIMITS } from './constants/validation'
|
||||
|
||||
/**
|
||||
* Versioned worksheet config schemas with type-safe validation and migration
|
||||
|
|
@ -29,9 +30,9 @@ const ADDITION_CURRENT_VERSION = 4
|
|||
*/
|
||||
export const additionConfigV1Schema = z.object({
|
||||
version: z.literal(1),
|
||||
problemsPerPage: z.number().int().min(1).max(100),
|
||||
cols: z.number().int().min(1).max(10),
|
||||
pages: z.number().int().min(1).max(20),
|
||||
problemsPerPage: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_PROBLEMS_PER_PAGE),
|
||||
cols: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_COLS),
|
||||
pages: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_PAGES),
|
||||
orientation: z.enum(['portrait', 'landscape']),
|
||||
name: z.string(),
|
||||
pAnyStart: z.number().min(0).max(1),
|
||||
|
|
@ -44,7 +45,11 @@ export const additionConfigV1Schema = z.object({
|
|||
showCellBorder: z.boolean(),
|
||||
showTenFrames: z.boolean(),
|
||||
showTenFramesForAll: z.boolean(),
|
||||
fontSize: z.number().int().min(8).max(32),
|
||||
fontSize: z
|
||||
.number()
|
||||
.int()
|
||||
.min(WORKSHEET_LIMITS.FONT_SIZE.MIN)
|
||||
.max(WORKSHEET_LIMITS.FONT_SIZE.MAX),
|
||||
})
|
||||
|
||||
export type AdditionConfigV1 = z.infer<typeof additionConfigV1Schema>
|
||||
|
|
@ -55,9 +60,9 @@ export type AdditionConfigV1 = z.infer<typeof additionConfigV1Schema>
|
|||
*/
|
||||
export const additionConfigV2Schema = z.object({
|
||||
version: z.literal(2),
|
||||
problemsPerPage: z.number().int().min(1).max(100),
|
||||
cols: z.number().int().min(1).max(10),
|
||||
pages: z.number().int().min(1).max(20),
|
||||
problemsPerPage: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_PROBLEMS_PER_PAGE),
|
||||
cols: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_COLS),
|
||||
pages: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_PAGES),
|
||||
orientation: z.enum(['portrait', 'landscape']),
|
||||
name: z.string(),
|
||||
pAnyStart: z.number().min(0).max(1),
|
||||
|
|
@ -128,7 +133,11 @@ export const additionConfigV2Schema = z.object({
|
|||
difficultyProfile: z.string().optional(),
|
||||
|
||||
// V2: Keep fontSize and showTenFramesForAll for now (may refactor later)
|
||||
fontSize: z.number().int().min(8).max(32),
|
||||
fontSize: z
|
||||
.number()
|
||||
.int()
|
||||
.min(WORKSHEET_LIMITS.FONT_SIZE.MIN)
|
||||
.max(WORKSHEET_LIMITS.FONT_SIZE.MAX),
|
||||
showTenFramesForAll: z.boolean(),
|
||||
})
|
||||
|
||||
|
|
@ -144,12 +153,16 @@ const additionConfigV3BaseSchema = z.object({
|
|||
version: z.literal(3),
|
||||
|
||||
// Core worksheet settings
|
||||
problemsPerPage: z.number().int().min(1).max(100),
|
||||
cols: z.number().int().min(1).max(10),
|
||||
pages: z.number().int().min(1).max(20),
|
||||
problemsPerPage: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_PROBLEMS_PER_PAGE),
|
||||
cols: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_COLS),
|
||||
pages: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_PAGES),
|
||||
orientation: z.enum(['portrait', 'landscape']),
|
||||
name: z.string(),
|
||||
fontSize: z.number().int().min(8).max(32),
|
||||
fontSize: z
|
||||
.number()
|
||||
.int()
|
||||
.min(WORKSHEET_LIMITS.FONT_SIZE.MIN)
|
||||
.max(WORKSHEET_LIMITS.FONT_SIZE.MAX),
|
||||
|
||||
// Regrouping probabilities (shared between modes)
|
||||
pAnyStart: z.number().min(0).max(1),
|
||||
|
|
@ -265,18 +278,30 @@ const additionConfigV4BaseSchema = z.object({
|
|||
version: z.literal(4),
|
||||
|
||||
// Core worksheet settings
|
||||
problemsPerPage: z.number().int().min(1).max(100),
|
||||
cols: z.number().int().min(1).max(10),
|
||||
pages: z.number().int().min(1).max(20),
|
||||
problemsPerPage: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_PROBLEMS_PER_PAGE),
|
||||
cols: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_COLS),
|
||||
pages: z.number().int().min(1).max(WORKSHEET_LIMITS.MAX_PAGES),
|
||||
orientation: z.enum(['portrait', 'landscape']),
|
||||
name: z.string(),
|
||||
fontSize: z.number().int().min(8).max(32),
|
||||
fontSize: z
|
||||
.number()
|
||||
.int()
|
||||
.min(WORKSHEET_LIMITS.FONT_SIZE.MIN)
|
||||
.max(WORKSHEET_LIMITS.FONT_SIZE.MAX),
|
||||
|
||||
// V4: Digit range for problem generation
|
||||
digitRange: z
|
||||
.object({
|
||||
min: z.number().int().min(1).max(5),
|
||||
max: z.number().int().min(1).max(5),
|
||||
min: z
|
||||
.number()
|
||||
.int()
|
||||
.min(WORKSHEET_LIMITS.DIGIT_RANGE.MIN)
|
||||
.max(WORKSHEET_LIMITS.DIGIT_RANGE.MAX),
|
||||
max: z
|
||||
.number()
|
||||
.int()
|
||||
.min(WORKSHEET_LIMITS.DIGIT_RANGE.MIN)
|
||||
.max(WORKSHEET_LIMITS.DIGIT_RANGE.MAX),
|
||||
})
|
||||
.refine((data) => data.min <= data.max, {
|
||||
message: 'min must be less than or equal to max',
|
||||
|
|
|
|||
|
|
@ -0,0 +1,73 @@
|
|||
/**
|
||||
* Validation constants for worksheet generation
|
||||
*
|
||||
* These constants define the limits for worksheet configuration.
|
||||
* Keep these in sync across:
|
||||
* - Zod schemas (config-schemas.ts)
|
||||
* - Runtime validation (validation.ts)
|
||||
* - UI components (forms, sliders, etc.)
|
||||
*/
|
||||
|
||||
export const WORKSHEET_LIMITS = {
|
||||
/** Maximum total problems across all pages */
|
||||
MAX_TOTAL_PROBLEMS: 2000,
|
||||
|
||||
/** Maximum problems per page */
|
||||
MAX_PROBLEMS_PER_PAGE: 100,
|
||||
|
||||
/** Maximum number of pages */
|
||||
MAX_PAGES: 100,
|
||||
|
||||
/** Maximum columns per page */
|
||||
MAX_COLS: 10,
|
||||
|
||||
/** Minimum/maximum digit range for problems */
|
||||
DIGIT_RANGE: {
|
||||
MIN: 1,
|
||||
MAX: 5,
|
||||
},
|
||||
|
||||
/** Font size limits */
|
||||
FONT_SIZE: {
|
||||
MIN: 8,
|
||||
MAX: 32,
|
||||
},
|
||||
} as const
|
||||
|
||||
/**
|
||||
* Validate that worksheet config doesn't exceed limits
|
||||
*
|
||||
* IMPORTANT: problemsPerPage * pages must not exceed MAX_TOTAL_PROBLEMS
|
||||
*/
|
||||
export function validateWorksheetLimits(
|
||||
problemsPerPage: number,
|
||||
pages: number
|
||||
): {
|
||||
valid: boolean
|
||||
error?: string
|
||||
} {
|
||||
const total = problemsPerPage * pages
|
||||
|
||||
if (total > WORKSHEET_LIMITS.MAX_TOTAL_PROBLEMS) {
|
||||
return {
|
||||
valid: false,
|
||||
error: `Total problems (${total}) exceeds maximum of ${WORKSHEET_LIMITS.MAX_TOTAL_PROBLEMS}`,
|
||||
}
|
||||
}
|
||||
|
||||
if (problemsPerPage > WORKSHEET_LIMITS.MAX_PROBLEMS_PER_PAGE) {
|
||||
return {
|
||||
valid: false,
|
||||
error: `Problems per page (${problemsPerPage}) exceeds maximum of ${WORKSHEET_LIMITS.MAX_PROBLEMS_PER_PAGE}`,
|
||||
}
|
||||
}
|
||||
|
||||
if (pages > WORKSHEET_LIMITS.MAX_PAGES) {
|
||||
return {
|
||||
valid: false,
|
||||
error: `Pages (${pages}) exceeds maximum of ${WORKSHEET_LIMITS.MAX_PAGES}`,
|
||||
}
|
||||
}
|
||||
|
||||
return { valid: true }
|
||||
}
|
||||
|
|
@ -68,7 +68,7 @@ export function resolveDisplayForProblem(
|
|||
rules: DisplayRules,
|
||||
problem: AnyProblemMeta
|
||||
): ResolvedDisplayOptions {
|
||||
const resolved = {
|
||||
return {
|
||||
showCarryBoxes: evaluateRule(rules.carryBoxes, problem),
|
||||
showAnswerBoxes: evaluateRule(rules.answerBoxes, problem),
|
||||
showPlaceValueColors: evaluateRule(rules.placeValueColors, problem),
|
||||
|
|
@ -78,15 +78,4 @@ export function resolveDisplayForProblem(
|
|||
showBorrowNotation: evaluateRule(rules.borrowNotation, problem),
|
||||
showBorrowingHints: evaluateRule(rules.borrowingHints, problem),
|
||||
}
|
||||
|
||||
// DEBUG: Ten-frames evaluation
|
||||
console.log('[TEN-FRAMES DEBUG]', {
|
||||
rule: rules.tenFrames,
|
||||
requiresRegrouping:
|
||||
'requiresRegrouping' in problem ? problem.requiresRegrouping : problem.requiresBorrowing,
|
||||
regroupCount: 'regroupCount' in problem ? problem.regroupCount : problem.borrowCount,
|
||||
resolved: resolved.showTenFrames,
|
||||
})
|
||||
|
||||
return resolved
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import { extractConfigFields } from '../utils/extractConfigFields'
|
|||
interface UseWorksheetAutoSaveReturn {
|
||||
isSaving: boolean
|
||||
lastSaved: Date | null
|
||||
saveError: string | null
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -26,6 +27,7 @@ export function useWorksheetAutoSave(
|
|||
): UseWorksheetAutoSaveReturn {
|
||||
const [isSaving, setIsSaving] = useState(false)
|
||||
const [lastSaved, setLastSaved] = useState<Date | null>(null)
|
||||
const [saveError, setSaveError] = useState<string | null>(null)
|
||||
|
||||
// Store the previous formState for auto-save to detect real changes
|
||||
const prevAutoSaveFormStateRef = useRef(formState)
|
||||
|
|
@ -45,6 +47,7 @@ export function useWorksheetAutoSave(
|
|||
const timer = setTimeout(async () => {
|
||||
console.log('[useWorksheetAutoSave] Attempting to save settings...')
|
||||
setIsSaving(true)
|
||||
setSaveError(null) // Clear previous errors
|
||||
try {
|
||||
// Extract persisted config fields (includes seed/prngAlgorithm, excludes date and derived state)
|
||||
const config = extractConfigFields(formState)
|
||||
|
|
@ -64,15 +67,24 @@ export function useWorksheetAutoSave(
|
|||
if (data.success) {
|
||||
console.log('[useWorksheetAutoSave] ✓ Settings saved successfully')
|
||||
setLastSaved(new Date())
|
||||
setSaveError(null)
|
||||
} else if (data.error) {
|
||||
// Validation error from server
|
||||
console.error('[useWorksheetAutoSave] Validation error:', data.error)
|
||||
setSaveError(data.error)
|
||||
} else {
|
||||
console.log('[useWorksheetAutoSave] Save skipped')
|
||||
}
|
||||
} else {
|
||||
const errorText = await response.text()
|
||||
console.error('[useWorksheetAutoSave] Save failed with status:', response.status)
|
||||
setSaveError(`Failed to save settings (${response.status}): ${errorText}`)
|
||||
}
|
||||
} catch (error) {
|
||||
// Silently fail - settings persistence is not critical
|
||||
// Surface error to user
|
||||
const errorMessage = error instanceof Error ? error.message : 'Unknown error'
|
||||
console.error('[useWorksheetAutoSave] Settings save error:', error)
|
||||
setSaveError(`Failed to save settings: ${errorMessage}`)
|
||||
} finally {
|
||||
setIsSaving(false)
|
||||
}
|
||||
|
|
@ -84,5 +96,6 @@ export function useWorksheetAutoSave(
|
|||
return {
|
||||
isSaving,
|
||||
lastSaved,
|
||||
saveError,
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import type {
|
|||
} from '@/app/create/worksheets/types'
|
||||
import type { DisplayRules } from './displayRules'
|
||||
import { getSkillById } from './skills'
|
||||
import { WORKSHEET_LIMITS } from './constants/validation'
|
||||
|
||||
/**
|
||||
* Get current date formatted as "Month Day, Year"
|
||||
|
|
@ -28,14 +29,14 @@ export function validateWorksheetConfig(formState: WorksheetFormState): Validati
|
|||
|
||||
// Validate total (must be positive, reasonable limit)
|
||||
const total = formState.total ?? 20
|
||||
if (total < 1 || total > 2000) {
|
||||
errors.push('Total problems must be between 1 and 2000')
|
||||
if (total < 1 || total > WORKSHEET_LIMITS.MAX_TOTAL_PROBLEMS) {
|
||||
errors.push(`Total problems must be between 1 and ${WORKSHEET_LIMITS.MAX_TOTAL_PROBLEMS}`)
|
||||
}
|
||||
|
||||
// Validate cols and auto-calculate rows
|
||||
const cols = formState.cols ?? 4
|
||||
if (cols < 1 || cols > 10) {
|
||||
errors.push('Columns must be between 1 and 10')
|
||||
if (cols < 1 || cols > WORKSHEET_LIMITS.MAX_COLS) {
|
||||
errors.push(`Columns must be between 1 and ${WORKSHEET_LIMITS.MAX_COLS}`)
|
||||
}
|
||||
|
||||
// Auto-calculate rows to fit all problems
|
||||
|
|
@ -60,18 +61,32 @@ export function validateWorksheetConfig(formState: WorksheetFormState): Validati
|
|||
|
||||
// Validate fontSize
|
||||
const fontSize = formState.fontSize ?? 16
|
||||
if (fontSize < 8 || fontSize > 32) {
|
||||
errors.push('Font size must be between 8 and 32')
|
||||
if (fontSize < WORKSHEET_LIMITS.FONT_SIZE.MIN || fontSize > WORKSHEET_LIMITS.FONT_SIZE.MAX) {
|
||||
errors.push(
|
||||
`Font size must be between ${WORKSHEET_LIMITS.FONT_SIZE.MIN} and ${WORKSHEET_LIMITS.FONT_SIZE.MAX}`
|
||||
)
|
||||
}
|
||||
|
||||
// V4: Validate digitRange (min and max must be 1-5, min <= max)
|
||||
// Note: Same range applies to both addition and subtraction
|
||||
const digitRange = formState.digitRange ?? { min: 2, max: 2 }
|
||||
if (!digitRange.min || digitRange.min < 1 || digitRange.min > 5) {
|
||||
errors.push('Digit range min must be between 1 and 5')
|
||||
if (
|
||||
!digitRange.min ||
|
||||
digitRange.min < WORKSHEET_LIMITS.DIGIT_RANGE.MIN ||
|
||||
digitRange.min > WORKSHEET_LIMITS.DIGIT_RANGE.MAX
|
||||
) {
|
||||
errors.push(
|
||||
`Digit range min must be between ${WORKSHEET_LIMITS.DIGIT_RANGE.MIN} and ${WORKSHEET_LIMITS.DIGIT_RANGE.MAX}`
|
||||
)
|
||||
}
|
||||
if (!digitRange.max || digitRange.max < 1 || digitRange.max > 5) {
|
||||
errors.push('Digit range max must be between 1 and 5')
|
||||
if (
|
||||
!digitRange.max ||
|
||||
digitRange.max < WORKSHEET_LIMITS.DIGIT_RANGE.MIN ||
|
||||
digitRange.max > WORKSHEET_LIMITS.DIGIT_RANGE.MAX
|
||||
) {
|
||||
errors.push(
|
||||
`Digit range max must be between ${WORKSHEET_LIMITS.DIGIT_RANGE.MIN} and ${WORKSHEET_LIMITS.DIGIT_RANGE.MAX}`
|
||||
)
|
||||
}
|
||||
if (digitRange.min > digitRange.max) {
|
||||
errors.push('Digit range min cannot be greater than max')
|
||||
|
|
@ -101,7 +116,7 @@ export function validateWorksheetConfig(formState: WorksheetFormState): Validati
|
|||
const pages = formState.pages ?? 1
|
||||
|
||||
// Determine mode (default to 'smart' if not specified)
|
||||
const mode = formState.mode ?? 'smart'
|
||||
const mode: 'smart' | 'manual' | 'mastery' = formState.mode ?? 'smart'
|
||||
|
||||
// Shared fields for both modes
|
||||
const sharedFields = {
|
||||
|
|
@ -232,7 +247,7 @@ export function validateWorksheetConfig(formState: WorksheetFormState): Validati
|
|||
if (addSkill?.recommendedScaffolding && subSkill?.recommendedScaffolding) {
|
||||
// Merge user's displayRules with skill's recommended scaffolding
|
||||
// User's displayRules take precedence for problemNumbers and cellBorders (layout options)
|
||||
const userDisplayRules = formState.displayRules || {}
|
||||
const userDisplayRules: Partial<DisplayRules> = formState.displayRules || {}
|
||||
|
||||
config = {
|
||||
...baseConfig,
|
||||
|
|
|
|||
Loading…
Reference in New Issue