Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 90 additions & 0 deletions packages/dashboard/e2e/tests/catalog/collections.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -366,3 +366,93 @@ test.describe('Issue #3548: Collection facet filter boolean args', () => {
expect(facetFilter?.args).toEqual(expect.arrayContaining([{ name: 'containsAny', value: 'false' }]));
});
});

// OSS-567 — a structurally different page (configurable-operation `filters` array,
// no update transform). Editing only the name must send just id + translations,
// leaving the `filters` replace-array untouched so a concurrent filter change
// can't be clobbered.
test.describe('collection update sends only changed fields (OSS-567)', () => {
let collectionId: string;

test.beforeAll(async ({ browser }) => {
const page = await browser.newPage();
const client = new VendureAdminClient(page);
await client.login();
const { facetValues } = await client.gql(
`query { facetValues(options: { take: 1 }) { items { id } } }`,
);
const facetValueId = facetValues.items[0].id as string;
const { createCollection } = await client.gql(
`mutation ($input: CreateCollectionInput!) { createCollection(input: $input) { id } }`,
{
input: {
translations: [
{
languageCode: 'en',
name: 'OSS567 Collection',
slug: `oss567-collection-${Date.now()}`,
description: '',
},
],
filters: [
{
code: 'facet-value-filter',
arguments: [
{ name: 'facetValueIds', value: `["${facetValueId}"]` },
{ name: 'containsAny', value: 'false' },
],
},
],
},
},
);
collectionId = createCollection.id;
await page.close();
});

test('editing only the name submits just id + translations, not filters', async ({ page }) => {
await page.goto(`/collections/${collectionId}`);
const nameField = page
.getByRole('main')
.locator('[data-slot="field"]')
.filter({
has: page.locator('[data-slot="field-label"]').getByText('Name', { exact: true }),
})
.getByRole('textbox')
.first();
await expect(nameField).toBeVisible({ timeout: 10_000 });
const newName = `OSS567 Collection ${Date.now()}`;
await nameField.fill(newName);

const updateRequest = page.waitForRequest(
req => req.method() === 'POST' && (req.postData() ?? '').includes('mutation UpdateCollection('),
{ timeout: 15_000 },
);
await page.getByRole('button', { name: 'Update' }).click();
const input = (await updateRequest).postDataJSON()?.variables?.input;

expect(input).toBeTruthy();
expect(Object.keys(input).sort()).toEqual(['id', 'translations']);
expect(input.filters).toBeUndefined();
expect(input.inheritFilters).toBeUndefined();
expect(input.translations?.[0]?.name).toBe(newName);

await expect(
page
.locator('[data-sonner-toast]')
.filter({ hasText: /updated/i })
.first(),
).toBeVisible({ timeout: 10_000 });
});

test.afterAll(async ({ browser }) => {
if (!collectionId) return;
const page = await browser.newPage();
const client = new VendureAdminClient(page);
await client.login();
await client.gql(`mutation ($id: ID!) { deleteCollection(id: $id) { result } }`, {
id: collectionId,
});
await page.close();
});
});
62 changes: 62 additions & 0 deletions packages/dashboard/e2e/tests/catalog/product-variants.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -376,3 +376,65 @@ test.describe('variant option group edit link', () => {
});
});
});

// OSS-567 — the form engine must submit only the fields the user actually
// changed, so an untouched field's stale page-load value cannot silently
// overwrite a concurrent edit by another admin or the API.
test.describe('variant update sends only changed fields (OSS-567)', () => {
test('editing only the SKU submits just id + sku', async ({ page }) => {
// Open a Laptop variant detail page (seed data — has facets, assets, translations).
await page.goto('/product-variants');
await expect(page.getByRole('heading', { name: 'Product Variants' })).toBeVisible({
timeout: 10_000,
});
await page
.locator('table')
.getByRole('button', { name: /Laptop/ })
.first()
.click();
await expect(page).toHaveURL(/\/product-variants\/[^/]+$/);

// Locate the SKU field on the detail form and change only that.
const skuField = page
.getByRole('main')
.locator('[data-slot="field"]')
.filter({
has: page.locator('[data-slot="field-label"]').getByText('SKU', { exact: true }),
})
.getByRole('textbox');
await expect(skuField).toBeVisible({ timeout: 10_000 });
const newSku = `OSS567-${Date.now()}`;
await skuField.fill(newSku);

// Capture the update mutation payload. `mutation UpdateProductVariant(` (with the
// paren) avoids matching the plural `UpdateProductVariants` mutation.
const updateRequest = page.waitForRequest(
req =>
req.method() === 'POST' && (req.postData() ?? '').includes('mutation UpdateProductVariant('),
{ timeout: 15_000 },
);
await page.getByRole('button', { name: 'Update' }).click();
const request = await updateRequest;
const input = request.postDataJSON()?.variables?.input;

// Only the non-nullable id and the changed sku should be present — every other
// replace-semantics field (facetValueIds, assetIds, trackInventory, translations,
// price, …) must be omitted.
expect(input).toBeTruthy();
expect(Object.keys(input).sort()).toEqual(['id', 'sku']);
expect(input.sku).toBe(newSku);
expect(input.facetValueIds).toBeUndefined();
expect(input.assetIds).toBeUndefined();
expect(input.trackInventory).toBeUndefined();
expect(input.translations).toBeUndefined();
expect(input.price).toBeUndefined();

// Confirm the save succeeded.
await expect(
page
.locator('[data-sonner-toast]')
.filter({ hasText: /updated/i })
.first(),
).toBeVisible({ timeout: 10_000 });
});
});
47 changes: 47 additions & 0 deletions packages/dashboard/e2e/tests/catalog/products.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,3 +120,50 @@ test.describe('Product detail features', () => {
// If no custom fields configured in the fixture, this test passes silently
});
});

// OSS-567 — the changed-fields-only update behaviour is framework-wide, not just
// the variant page. Editing only the (translated) product name must send just
// `id` + `translations`, leaving replace-semantics fields (facetValueIds, assetIds,
// enabled) untouched so they can't clobber a concurrent edit.
test.describe('product update sends only changed fields (OSS-567)', () => {
test('editing only the name submits just id + translations', async ({ page }) => {
await page.goto('/products');
await expect(page.locator('table')).toBeVisible();
await page.getByPlaceholder('Filter...').fill('Laptop');
await page.waitForResponse(resp => resp.url().includes('/admin-api') && resp.status() === 200);
await page.locator('table tbody tr').first().getByRole('button').first().click();
await expect(page).toHaveURL(/\/products\/.+/);

const nameField = page
.getByRole('main')
.locator('[data-slot="field"]')
.filter({
has: page.locator('[data-slot="field-label"]').getByText('Product name', { exact: true }),
})
.getByRole('textbox');
await expect(nameField).toBeVisible({ timeout: 10_000 });
const newName = `Laptop OSS567 ${Date.now()}`;
await nameField.fill(newName);

const updateRequest = page.waitForRequest(
req => req.method() === 'POST' && (req.postData() ?? '').includes('mutation UpdateProduct('),
{ timeout: 15_000 },
);
await page.getByRole('button', { name: 'Update' }).click();
const input = (await updateRequest).postDataJSON()?.variables?.input;

expect(input).toBeTruthy();
expect(Object.keys(input).sort()).toEqual(['id', 'translations']);
expect(input.facetValueIds).toBeUndefined();
expect(input.assetIds).toBeUndefined();
expect(input.enabled).toBeUndefined();
expect(input.translations?.[0]?.name).toBe(newName);

await expect(
page
.locator('[data-sonner-toast]')
.filter({ hasText: /updated/i })
.first(),
).toBeVisible({ timeout: 10_000 });
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,29 @@ import {
} from './form-schema-tools.js';
import {
convertEmptyStringsToNull,
getChangedTopLevelFields,
removeEmptyIdFields,
stripNullNullableFields,
transformRelationFields,
} from './utils.js';

/**
* @description
* Metadata passed as the second argument to a {@link GeneratedFormOptions} `onSubmit`
* handler, describing the outcome of the form submission.
*
* @since 3.8.0
*/
export interface GeneratedFormSubmitMeta {
/**
* @description
* The set of top-level input field names the user actually changed relative to
* the loaded entity (plus non-nullable fields which are always included).
* `undefined` when creating a new entity (there is no baseline to diff against).
*/
changedFields?: ReadonlySet<string>;
}

// Stable empty array reference used as a fallback when the server config
// (and therefore `availableLanguages`) has not yet loaded — keeps memo
// dependencies stable across renders.
Expand Down Expand Up @@ -63,6 +81,7 @@ export interface GeneratedFormOptions<
>;
onSubmit?: (
values: VarName extends keyof VariablesOf<T> ? VariablesOf<T>[VarName] : VariablesOf<T>,
meta?: GeneratedFormSubmitMeta,
) => void;
}

Expand Down Expand Up @@ -186,15 +205,26 @@ export function useGeneratedForm<
return;
}

const onSubmitWrapper = (values: any) => {
const onSubmitWrapper = (rawValues: any) => {
let processed = convertEmptyStringsToNull(
removeEmptyIdFields(values, updateFields),
removeEmptyIdFields(rawValues, updateFields),
updateFields,
);
if (!entity) {
processed = stripNullNullableFields(processed, updateFields);
}
onSubmit(processed);
// Compute which top-level fields actually changed relative to the
// loaded entity, so that consumers (e.g. useDetailPage) can omit
// untouched fields from update mutations and avoid clobbering
// concurrent edits. Diff the raw values against the `values` memo,
// which is the exact baseline the form was last reset to. Only
// meaningful for updates (an existing entity provides the baseline).
const meta: GeneratedFormSubmitMeta = {
changedFields: entity
? getChangedTopLevelFields(rawValues, values, updateFields)
: undefined,
};
onSubmit(processed, meta);
};
form.handleSubmit(onSubmitWrapper)(event);
};
Expand Down
Loading
Loading