Skip to content

Commit

Permalink
Fix incorrect indexing in concurrent variant datagrid bulk mutations (#…
Browse files Browse the repository at this point in the history 
…4390)

* Fix multiple problems regarding datagrid row indexing

* Add changeset

* Trigger e2e

* Add unit test case for getBulkVariantUpdateInputs

* Fix changeset typo

* Clarify comment
  • Loading branch information
@Droniu
Droniu authored Nov 3, 2023
1 parent 28dafdc commit b7c2a96
Show file tree
Hide file tree
Showing 16 changed files with 316 additions and 135 deletions.
5 changes: 5 additions & 0 deletions .changeset/late-cooks-kiss.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"saleor-dashboard": patch
---

Fix incorrect indexing in concurrent variant datagrid bulk mutations
2 changes: 1 addition & 1 deletion src/products/components/ProductVariants/utils.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ export function getData({
const change = changes.current[getChangeIndex(columnId, row)]?.data;
const dataRow = added.includes(row)
? undefined
: variants[row + removed.filter(r => r <= row).length];
: variants.filter((_, index) => !removed.includes(index))[row];

switch (columnId) {
case "name":
Expand Down
176 changes: 176 additions & 0 deletions src/products/fixtures.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1044,6 +1044,182 @@ export const product: (
quantityLimitPerCustomer: null,
__typename: "ProductVariant",
},
{
id: "UHJvZHVjdFZhcmlhbnQ6MjA0",
sku: "76432981",
name: "750ml",
margin: 0.25,
attributes: [
{
attribute: {
id: "QXR0cmlidXRlOjE2",
name: "Bottle Size",
__typename: "Attribute",
},
values: [
{
id: "QXR0cmlidXRlVmFsdWU6NTU=",
name: "750ml",
plainText: "",
richText: "",
slug: "",
reference: "",
boolean: false,
date: "",
dateTime: "",
value: "",
file: {
__typename: "File",
url: "",
contentType: "",
},
__typename: "AttributeValue",
},
],
__typename: "SelectedAttribute",
},
],
media: [
{
id: "2",
type: ProductMediaType.VIDEO,
url: "randomVideoUrl",
__typename: "ProductMedia",
},
],
stocks: [
{
id: "U3RvY2s6MTYz",
quantity: 600,
quantityAllocated: 50,
warehouse: {
id: "V2FyZWhvdXNlOjEwM2VjNzY2LTA1NmItNDU2My05YjQzLTUxYmU5ZGJmNGEzYQ==",
name: "Warehouse-123",
__typename: "Warehouse",
},
__typename: "Stock",
},
],
trackInventory: true,
preorder: null,
channelListings: [
{
id: "UHJvZHVjdFZhcmlhbnRDaSAD3w2FubmVsTGlzdGluZzoyNzU=",
channel: {
id: "Q2hhbm5lbDox",
name: "Channel-EUR",
currencyCode: "EUR",
__typename: "Channel",
},
price: {
amount: 7.5,
currency: "EUR",
__typename: "Money",
},
costPrice: {
amount: 2.5,
currency: "EUR",
__typename: "Money",
},
preorderThreshold: {
quantity: null,
soldUnits: 0,
__typename: "PreorderThreshold",
},
__typename: "ProductVariantChannelListing",
},
],
quantityLimitPerCustomer: 5,
__typename: "ProductVariant",
},
{
id: "UHJvZHVjdFZhcmlhbnQ6MjA1",
sku: "12345678",
name: "1 Liter",
margin: 0.15,
attributes: [
{
attribute: {
id: "QXR0cmlidXRlOjE3",
name: "Bottle Size",
__typename: "Attribute",
},
values: [
{
id: "QXR0cmlidXRlVmFsdWU6NjU=",
name: "1 Liter",
plainText: "",
richText: "",
slug: "",
reference: "",
boolean: false,
date: "",
dateTime: "",
value: "",
file: {
__typename: "File",
url: "",
contentType: "",
},
__typename: "AttributeValue",
},
],
__typename: "SelectedAttribute",
},
],
media: [
{
id: "3",
type: ProductMediaType.IMAGE,
url: "randomImageUrl",
__typename: "ProductMedia",
},
],
stocks: [
{
id: "U3RvY2s6MTY0",
quantity: 800,
quantityAllocated: 100,
warehouse: {
id: "V2FyZWhvdXNlOjExNmQ2NGYyLTZhOGYtNGE4MC1iNmJkLTk1MDg4YTliZDEwYQ==",
name: "Warehouse-456",
__typename: "Warehouse",
},
__typename: "Stock",
},
],
trackInventory: true,
preorder: null,
channelListings: [
{
id: "UHJvZHVjdFZhcmlhbnRDaSAD3w2FubmVsTGlzdGluZzoyNzY=",
channel: {
id: "Q2hhbm5lbDoy",
name: "Channel-GBP",
currencyCode: "GBP",
__typename: "Channel",
},
price: {
amount: 10.0,
currency: "GBP",
__typename: "Money",
},
costPrice: {
amount: 2.0,
currency: "GBP",
__typename: "Money",
},
preorderThreshold: {
quantity: null,
soldUnits: 0,
__typename: "PreorderThreshold",
},
__typename: "ProductVariantChannelListing",
},
],
quantityLimitPerCustomer: null,
__typename: "ProductVariant",
},
],
visibleInListings: true,
weight: {
Expand Down
64 changes: 0 additions & 64 deletions src/products/utils/datagrid.test.ts
View file

This file was deleted.

6 changes: 1 addition & 5 deletions src/products/utils/datagrid.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,4 @@ export const getColumnName = (column: string) => {
export const isCurrentRow = (
datagridChangeIndex: number,
variantIndex: number,
datagridRemovedRowsIds: number[],
) =>
datagridChangeIndex ===
variantIndex +
datagridRemovedRowsIds.filter(index => index <= variantIndex).length;
) => datagridChangeIndex === variantIndex;
6 changes: 3 additions & 3 deletions src/products/views/ProductUpdate/handlers/data/attributes.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe("getAttributeData", () => {
];

// Act
const attributes = getAttributeData(changeData, 1, [], variantAttributes);
const attributes = getAttributeData(changeData, 1, variantAttributes);

// Assert
expect(attributes).toEqual([
Expand All @@ -42,7 +42,7 @@ describe("getAttributeData", () => {
];

// Act
const attributes = getAttributeData(changeData, 2, [], variantAttributes);
const attributes = getAttributeData(changeData, 2, variantAttributes);

// Assert
expect(attributes).toEqual([]);
Expand All @@ -56,7 +56,7 @@ describe("getAttributeData", () => {
];

// Act
const attributes = getAttributeData(changeData, 1, [], variantAttributes);
const attributes = getAttributeData(changeData, 1, variantAttributes);

// Assert
expect(attributes).toEqual([]);
Expand Down
3 changes: 1 addition & 2 deletions src/products/views/ProductUpdate/handlers/data/attributes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,10 @@ import { byAttributeName } from "../utils";
export function getAttributeData(
data: DatagridChange[],
currentIndex: number,
removedIds: number[],
variantAttributes: VariantAttributeFragment[],
) {
return data
.filter(change => isCurrentRow(change.row, currentIndex, removedIds))
.filter(change => isCurrentRow(change.row, currentIndex))
.filter(byHavingAnyAttribute)
.map(toAttributeData(variantAttributes));
}
Expand Down
9 changes: 4 additions & 5 deletions src/products/views/ProductUpdate/handlers/data/channel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export function getUpdateVariantChannelInputs(
variant: ProductFragment["variants"][number],
): ProductVariantChannelListingUpdateInput {
return data.updates
.filter(byCurrentRowByIndex(index, data))
.filter(byCurrentRowByIndex(index))
.map(availabilityToChannelColumn)
.filter(byChannelColumn)
.reduce(byColumn, [])
Expand All @@ -39,18 +39,17 @@ export function getVariantChannelsInputs(
index: number,
): ProductVariantChannelListingAddInput[] {
return data.updates
.filter(byCurrentRowByIndex(index, data))
.filter(byCurrentRowByIndex(index))
.map(availabilityToChannelColumn)
.filter(byChannelColumn)
.reduce(byColumn, [])
.map(dataGridChangeToFlatChannel)
.filter(byNotNullPrice);
}

function byCurrentRowByIndex(index: number, data: DatagridChangeOpts) {
function byCurrentRowByIndex(index: number) {
return (change: DatagridChange) => {
const totalRemoved = data.removed.filter(r => r <= index).length;
return change.row === index + totalRemoved;
return change.row === index;
};
}

Expand Down
6 changes: 3 additions & 3 deletions src/products/views/ProductUpdate/handlers/data/name.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe("getNameData", () => {
];

// Act
const name = getNameData(changeData, 1, []);
const name = getNameData(changeData, 1);

// Assert
expect(name).toEqual("Joe");
Expand All @@ -24,7 +24,7 @@ describe("getNameData", () => {
];

// Act
const name = getNameData(changeData, 1, []);
const name = getNameData(changeData, 1);

// Assert
expect(name).toEqual(undefined);
Expand All @@ -37,7 +37,7 @@ describe("getNameData", () => {
];

// Act
const name = getNameData(changeData, 1, []);
const name = getNameData(changeData, 1);

// Assert
expect(name).toEqual(undefined);
Expand Down
4 changes: 1 addition & 3 deletions src/products/views/ProductUpdate/handlers/data/name.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@ import { isCurrentRow } from "@dashboard/products/utils/datagrid";
export function getNameData(
data: DatagridChange[],
currentIndex: number,
removedIds: number[],
): string | undefined {
return data.find(
change =>
change.column === "name" &&
isCurrentRow(change.row, currentIndex, removedIds),
change.column === "name" && isCurrentRow(change.row, currentIndex),
)?.data;
}
Loading

0 comments on commit b7c2a96

Please sign in to comment.