Skip to content

Commit a635eb2

Browse files
biggamesmallworldbiggamesmallworld
authored
fix(dashboard): Fix validation & UX issues in product variant option groups (vendurehq#4458)
1 parent 0dc2bfe commit a635eb2

File tree

7 files changed

+118
-38
lines changed

7 files changed

+118
-38
lines changed

‎docs/docs/reference/dashboard/list-views/paginated-list-data-table.mdx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
title: "PaginatedListDataTable"
33
generated: true
44
---
5-
<GenerationInfo sourceFile="packages/dashboard/src/lib/components/shared/paginated-list-data-table.tsx" sourceLine="340" packageName="@vendure/dashboard" since="3.4.0" />
5+
<GenerationInfo sourceFile="packages/dashboard/src/lib/components/shared/paginated-list-data-table.tsx" sourceLine="342" packageName="@vendure/dashboard" since="3.4.0" />
66

77
A wrapper around the [DataTable](/reference/dashboard/list-views/data-table#datatable) component, which automatically configures functionality common to
88
list queries that implement the `PaginatedList` interface, which is the common way of representing lists
@@ -117,7 +117,7 @@ Parameters
117117

118118
<MemberInfo kind="parameter" type={`Readonly<<a href='/reference/dashboard/list-views/paginated-list-data-table#paginatedlistdatatableprops'>PaginatedListDataTableProps</a><T, U, V, AC>>`} />
119119

120-
<GenerationInfo sourceFile="packages/dashboard/src/lib/components/shared/paginated-list-data-table.tsx" sourceLine="176" packageName="@vendure/dashboard" since="3.4.0" />
120+
<GenerationInfo sourceFile="packages/dashboard/src/lib/components/shared/paginated-list-data-table.tsx" sourceLine="178" packageName="@vendure/dashboard" since="3.4.0" />
121121

122122
Props to configure the [PaginatedListDataTable](/reference/dashboard/list-views/paginated-list-data-table#paginatedlistdatatable) component.
123123

‎packages/dashboard/src/app/routes/_authenticated/_products/components/add-option-group-dialog.tsx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ export function AddOptionGroupDialog({
8181

8282
toast.success(t`Successfully created option group`);
8383
setOpen(false);
84+
form.reset();
8485
onSuccess?.();
8586
} catch (error) {
8687
toast.error(t`Failed to create option group`, {
@@ -90,7 +91,10 @@ export function AddOptionGroupDialog({
9091
};
9192

9293
return (
93-
<Dialog open={open} onOpenChange={setOpen}>
94+
<Dialog open={open} onOpenChange={(isOpen) => {
95+
setOpen(isOpen);
96+
if (!isOpen) form.reset();
97+
}}>
9498
<DialogTrigger asChild>
9599
<Button variant="outline">
96100
<Plus className="mr-2 h-4 w-4" />

‎packages/dashboard/src/app/routes/_authenticated/_products/components/create-product-variants-dialog.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,8 @@ export function CreateProductVariantsDialog({
140140
[],
141141
);
142142
const createCount = Object.values(variantData?.variants ?? {}).filter(v => v.enabled).length;
143+
const hasInvalidOptionGroups =
144+
variantData?.optionGroups.some(g => !g.name || g.values.length === 0) ?? false;
143145

144146
return (
145147
<>
@@ -174,7 +176,8 @@ export function CreateProductVariantsDialog({
174176
createOptionGroupMutation.isPending ||
175177
addOptionGroupToProductMutation.isPending ||
176178
createProductVariantsMutation.isPending ||
177-
createCount === 0
179+
createCount === 0 ||
180+
hasInvalidOptionGroups
178181
}
179182
>
180183
{createOptionGroupMutation.isPending ||

‎packages/dashboard/src/app/routes/_authenticated/_products/components/option-groups-editor.tsx‎

Lines changed: 14 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -39,21 +39,6 @@ export interface OptionGroupConfiguration {
3939
optionGroups: SingleOptionGroup[];
4040
}
4141

42-
function validateOptionGroup(group: any): SingleOptionGroup | null {
43-
if (!group || typeof group.name !== 'string' || !Array.isArray(group.values)) {
44-
return null;
45-
}
46-
47-
const validValues = group.values
48-
.filter((v: any): v is NonNullable<typeof v> => !!v)
49-
.filter((v: any) => typeof v.value === 'string' && typeof v.id === 'string')
50-
.map((v: any) => ({
51-
value: v.value,
52-
id: v.id,
53-
}));
54-
55-
return validValues.length > 0 ? { name: group.name, values: validValues } : null;
56-
}
5742

5843
interface SingleOptionGroupEditorProps {
5944
control: Control<any>;
@@ -68,7 +53,7 @@ export function SingleOptionGroupEditor({
6853
}: Readonly<SingleOptionGroupEditorProps>) {
6954
const { fields, append, remove } = useFieldArray({
7055
control,
71-
name: [fieldArrayPath, 'values'].join('.'),
56+
name: fieldArrayPath ? `${fieldArrayPath}.values` : 'values',
7257
});
7358

7459
return (
@@ -77,7 +62,7 @@ export function SingleOptionGroupEditor({
7762
<div>
7863
<FormFieldWrapper
7964
control={control}
80-
name={[fieldArrayPath, 'name'].join('.')}
65+
name={fieldArrayPath ? `${fieldArrayPath}.name` : 'name'}
8166
label={<Trans>Option Group Name</Trans>}
8267
render={({ field }) => <Input placeholder="e.g. Size" {...field} />}
8368
/>
@@ -86,7 +71,7 @@ export function SingleOptionGroupEditor({
8671
<div>
8772
<FormFieldWrapper
8873
control={control}
89-
name="values"
74+
name={fieldArrayPath ? `${fieldArrayPath}.values` : 'values'}
9075
label={<Trans>Option Values</Trans>}
9176
render={({ field }) => (
9277
<OptionValueInput
@@ -132,15 +117,17 @@ export function OptionGroupsEditor({ onChange, initialGroups = [] }: Readonly<Op
132117
useEffect(() => {
133118
const subscription = form.watch(value => {
134119
if (value?.optionGroups) {
135-
const validOptionGroups = value.optionGroups
136-
.map(validateOptionGroup)
137-
.filter((group): group is SingleOptionGroup => group !== null);
138-
139-
const filteredData: OptionGroupConfiguration = {
140-
optionGroups: validOptionGroups,
141-
};
142-
143-
onChange?.(filteredData);
120+
const allOptionGroups: SingleOptionGroup[] = value.optionGroups
121+
.filter((g): g is NonNullable<typeof g> => !!g)
122+
.map(g => ({
123+
name: g.name ?? '',
124+
values: (g.values ?? [])
125+
.filter((v): v is NonNullable<typeof v> => !!v)
126+
.filter(v => typeof v.value === 'string' && typeof v.id === 'string')
127+
.map(v => ({ value: v.value!, id: v.id! })),
128+
}));
129+
130+
onChange?.({ optionGroups: allOptionGroups });
144131
}
145132
});
146133

‎packages/dashboard/src/app/routes/_authenticated/_products/components/option-value-input.tsx‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import { Badge } from '@/vdb/components/ui/badge.js';
22
import { Button } from '@/vdb/components/ui/button.js';
33
import { Input } from '@/vdb/components/ui/input.js';
4+
import { useLingui } from '@lingui/react/macro';
45
import { X } from 'lucide-react';
56
import { useState } from 'react';
7+
import { toast } from 'sonner';
68

79
interface OptionValue {
810
value: string;
@@ -23,12 +25,18 @@ export function OptionValueInput({
2325
disabled = false,
2426
}: Readonly<OptionValueInputProps>) {
2527
const [newValue, setNewValue] = useState('');
28+
const { t } = useLingui();
2629

2730
const handleAddValue = () => {
28-
if (newValue.trim() && !fields.some(f => f.value === newValue.trim())) {
29-
onAdd({ value: newValue.trim(), id: Date.now().toString() });
30-
setNewValue('');
31+
const trimmed = newValue.trim();
32+
if (!trimmed) return;
33+
const normalized = trimmed.toLowerCase().normalize();
34+
if (fields.some(f => f.value.toLowerCase().normalize() === normalized)) {
35+
toast.error(t`Duplicate value "${trimmed}" already exists`);
36+
return;
3137
}
38+
onAdd({ value: trimmed, id: Date.now().toString() });
39+
setNewValue('');
3240
};
3341

3442
const handleKeyPress = (e: React.KeyboardEvent) => {

‎packages/dashboard/src/app/routes/_authenticated/_products/products.graphql.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,11 @@ export const productDetailFragment = graphql(
5454
id
5555
code
5656
name
57+
options {
58+
id
59+
code
60+
name
61+
}
5762
}
5863
facetValues {
5964
id
@@ -290,17 +295,19 @@ export const deleteProductVariantDocument = graphql(`
290295
`);
291296

292297
export const removeOptionGroupFromProductDocument = graphql(`
293-
mutation RemoveOptionGroupFromProduct($productId: ID!, $optionGroupId: ID!) {
294-
removeOptionGroupFromProduct(productId: $productId, optionGroupId: $optionGroupId) {
298+
mutation RemoveOptionGroupFromProduct($productId: ID!, $optionGroupId: ID!, $force: Boolean) {
299+
removeOptionGroupFromProduct(productId: $productId, optionGroupId: $optionGroupId, force: $force) {
295300
... on Product {
301+
__typename
296302
id
297303
optionGroups {
298304
id
299305
code
300306
name
301307
}
302308
}
303-
... on ErrorResult {
309+
... on ProductOptionInUseError {
310+
__typename
304311
errorCode
305312
message
306313
}

‎packages/dashboard/src/app/routes/_authenticated/_products/products_.$id_.variants.tsx‎

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,16 @@
1+
import { ConfirmationDialog } from '@/vdb/components/shared/confirmation-dialog.js';
12
import { ErrorPage } from '@/vdb/components/shared/error-page.js';
23
import { FormFieldWrapper } from '@/vdb/components/shared/form-field-wrapper.js';
4+
import {
5+
AlertDialog,
6+
AlertDialogAction,
7+
AlertDialogCancel,
8+
AlertDialogContent,
9+
AlertDialogDescription,
10+
AlertDialogFooter,
11+
AlertDialogHeader,
12+
AlertDialogTitle,
13+
} from '@/vdb/components/ui/alert-dialog.js';
314
import { Badge } from '@/vdb/components/ui/badge.js';
415
import { Button } from '@/vdb/components/ui/button.js';
516
import {
@@ -184,9 +195,25 @@ function ManageProductVariants() {
184195
},
185196
});
186197

198+
const [forceRemoveGroupId, setForceRemoveGroupId] = useState<string | null>(null);
199+
187200
const removeOptionGroupMutation = useMutation({
201+
mutationFn: api.mutate(removeOptionGroupFromProductDocument),
202+
onSuccess: (result: any, variables: any) => {
203+
const removeResult = result?.removeOptionGroupFromProduct;
204+
if (removeResult && '__typename' in removeResult && removeResult.__typename === 'ProductOptionInUseError') {
205+
setForceRemoveGroupId(variables.optionGroupId);
206+
return;
207+
}
208+
toast.success(t`Option group removed`);
209+
refetch();
210+
},
211+
});
212+
213+
const forceRemoveOptionGroupMutation = useMutation({
188214
mutationFn: api.mutate(removeOptionGroupFromProductDocument),
189215
onSuccess: () => {
216+
setForceRemoveGroupId(null);
190217
toast.success(t`Option group removed`);
191218
refetch();
192219
},
@@ -270,7 +297,7 @@ function ManageProductVariants() {
270297
</label>
271298
<Input value={group.name} disabled />
272299
</div>
273-
<div className="col-span-8">
300+
<div className="col-span-7">
274301
<label className="text-sm font-medium">
275302
<Trans>Option values</Trans>
276303
</label>
@@ -287,6 +314,24 @@ function ManageProductVariants() {
287314
/>
288315
</div>
289316
</div>
317+
<div className="col-span-1 flex items-end justify-end">
318+
<ConfirmationDialog
319+
title={t`Remove option group`}
320+
description={t`Are you sure you want to remove this option group from the product?`}
321+
onConfirm={() => removeOptionGroupMutation.mutate({
322+
productId: id,
323+
optionGroupId: group.id,
324+
})}
325+
>
326+
<Button
327+
size="icon"
328+
variant="ghost"
329+
disabled={removeOptionGroupMutation.isPending}
330+
>
331+
<Trash2 className="h-4 w-4 text-destructive" />
332+
</Button>
333+
</ConfirmationDialog>
334+
</div>
290335
</div>
291336
))
292337
)}
@@ -425,6 +470,32 @@ function ManageProductVariants() {
425470
/>
426471
</PageBlock>
427472
</PageLayout>
473+
<AlertDialog open={!!forceRemoveGroupId} onOpenChange={(open) => { if (!open) setForceRemoveGroupId(null); }}>
474+
<AlertDialogContent>
475+
<AlertDialogHeader>
476+
<AlertDialogTitle><Trans>Force remove option group</Trans></AlertDialogTitle>
477+
<AlertDialogDescription>
478+
<Trans>This option group is in use by existing variants. Force removing it may affect those variants. Are you sure?</Trans>
479+
</AlertDialogDescription>
480+
</AlertDialogHeader>
481+
<AlertDialogFooter>
482+
<AlertDialogCancel onClick={() => setForceRemoveGroupId(null)}>
483+
<Trans>Cancel</Trans>
484+
</AlertDialogCancel>
485+
<AlertDialogAction onClick={() => {
486+
if (forceRemoveGroupId) {
487+
forceRemoveOptionGroupMutation.mutate({
488+
productId: id,
489+
optionGroupId: forceRemoveGroupId,
490+
force: true,
491+
});
492+
}
493+
}}>
494+
<Trans>Force remove</Trans>
495+
</AlertDialogAction>
496+
</AlertDialogFooter>
497+
</AlertDialogContent>
498+
</AlertDialog>
428499
</Page>
429500
);
430501
}

0 commit comments

Comments
 (0)