Skip to content

Commit 7b74789

Browse files
committed
validate image size is a multiple of block size
1 parent 7bf337b commit 7b74789

4 files changed

Lines changed: 69 additions & 3 deletions

File tree

app/components/form/fields/FileField.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ import {
99
useController,
1010
type Control,
1111
type FieldPath,
12+
type FieldPathValue,
1213
type FieldValues,
14+
type Validate,
1315
} from 'react-hook-form'
1416

1517
import { FieldLabel } from '~/ui/lib/FieldLabel'
@@ -30,6 +32,7 @@ export function FileField<
3032
accept,
3133
description,
3234
disabled,
35+
validate,
3336
}: {
3437
id: string
3538
name: TName
@@ -40,11 +43,12 @@ export function FileField<
4043
accept?: string
4144
description?: string | React.ReactNode
4245
disabled?: boolean
46+
validate?: Validate<FieldPathValue<TFieldValues, TName>, TFieldValues>
4347
}) {
4448
const {
4549
field: { value: _, ...rest },
4650
fieldState: { error },
47-
} = useController({ name, control, rules: { required } })
51+
} = useController({ name, control, rules: { required, validate } })
4852
return (
4953
<div className="max-w-lg">
5054
<div className="mb-2">

app/components/form/fields/RadioField.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
type FieldPath,
1414
type FieldValues,
1515
type PathValue,
16+
type RegisterOptions,
1617
} from 'react-hook-form'
1718

1819
import { FieldLabel } from '~/ui/lib/FieldLabel'
@@ -41,6 +42,9 @@ export type RadioFieldProps<
4142
units?: string
4243
control: Control<TFieldValues>
4344
items: { value: PathValue<TFieldValues, TName>; label: string }[]
45+
/** Forwarded to react-hook-form's `useController`; use `deps` to trigger
46+
* validation on other fields when this one changes. */
47+
rules?: Pick<RegisterOptions<TFieldValues, TName>, 'deps'>
4448
} & (PathValue<TFieldValues, TName> extends string // this is wild lmao
4549
? { parseValue?: never }
4650
: {
@@ -63,10 +67,11 @@ export function RadioField<
6367
control,
6468
items,
6569
parseValue,
70+
rules,
6671
...props
6772
}: RadioFieldProps<TFieldValues, TName>) {
6873
const id = useId()
69-
const { field } = useController({ name, control })
74+
const { field } = useController({ name, control, rules })
7075
return (
7176
<div>
7277
<div className="mb-2">

app/forms/image-upload.tsx

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,9 @@ export default function ImageCreate() {
495495
setAllDone(true)
496496
}
497497

498-
const form = useForm({ defaultValues })
498+
// onChange mode so the file-size / block-size cross-validation surfaces
499+
// inline as soon as the user picks a file or changes block size
500+
const form = useForm({ defaultValues, mode: 'onChange' })
499501
const file = form.watch('imageFile')
500502
const blockSize = form.watch('blockSize')
501503

@@ -590,6 +592,8 @@ export default function ImageCreate() {
590592
units="Bytes"
591593
control={form.control}
592594
parseValue={(val) => parseInt(val, 10) as BlockSize}
595+
// re-run imageFile validation when block size changes
596+
rules={{ deps: ['imageFile'] }}
593597
items={[
594598
{ label: '512', value: 512 },
595599
{ label: '2048', value: 2048 },
@@ -605,6 +609,13 @@ export default function ImageCreate() {
605609
label="Image file"
606610
required
607611
control={form.control}
612+
// Crucible rejects bulk-write imports whose total size isn't a
613+
// multiple of the block size, so catch it before the long upload.
614+
validate={(f, { blockSize }) => {
615+
if (f && f.size % blockSize !== 0) {
616+
return `File size must be a multiple of the block size (${blockSize} bytes)`
617+
}
618+
}}
608619
/>
609620
{imageValidation && <BootableNotice {...imageValidation} />}
610621
</div>

test/e2e/image-upload.e2e.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,52 @@ test.describe('Image upload', () => {
101101
// TODO: changing name alone should cause error to disappear
102102
})
103103

104+
test('block size validation', async ({ page, browserName }) => {
105+
// eslint-disable-next-line playwright/no-skipped-test
106+
test.skip(browserName === 'webkit', 'safari. stop this')
107+
108+
await page.goto('/projects/mock-project/images-new')
109+
110+
await page.fill('role=textbox[name="Name"]', 'new-image')
111+
await page.fill('role=textbox[name="Description"]', 'image description')
112+
await page.fill('role=textbox[name="OS"]', 'Ubuntu')
113+
await page.fill('role=textbox[name="Version"]', 'Dapper Drake')
114+
115+
const sideModal = page.getByRole('dialog', { name: 'Upload image' })
116+
const uploadError = sideModal.getByText(/must be a multiple of the block size/i)
117+
const submit = page.getByRole('button', { name: 'Upload image' })
118+
const progressModal = page.getByRole('dialog', { name: 'Image upload progress' })
119+
120+
// 1000 bytes is not a multiple of any supported block size (512/2048/4096)
121+
await page.getByLabel('Image file').setInputFiles({
122+
name: 'my-image.iso',
123+
mimeType: 'application/octet-stream',
124+
buffer: Buffer.alloc(1000, 'a'),
125+
})
126+
127+
await expect(uploadError).toBeVisible()
128+
129+
// clicking submit does nothing — validation blocks it and the progress
130+
// modal never opens
131+
await submit.click()
132+
await expect(progressModal).toBeHidden()
133+
await expect(uploadError).toBeVisible()
134+
135+
// replace with an aligned file — error clears
136+
await page.getByLabel('Image file').setInputFiles({
137+
name: 'my-image.iso',
138+
mimeType: 'application/octet-stream',
139+
buffer: Buffer.alloc(2048, 'a'),
140+
})
141+
142+
await expect(uploadError).toBeHidden()
143+
144+
// switching block size to one that no longer divides the file brings it
145+
// back (exercises the cross-field `deps` re-validation path)
146+
await page.getByLabel('4096').click()
147+
await expect(uploadError).toBeVisible()
148+
})
149+
104150
test('form validation', async ({ page, browserName }) => {
105151
// eslint-disable-next-line playwright/no-skipped-test
106152
test.skip(browserName === 'webkit', 'safari. stop this')

0 commit comments

Comments
 (0)