Skip to content

Commit

Permalink
Fix errors caused by can not read property of undefined (#5174)
Browse files Browse the repository at this point in the history
* Fix errors from sentry

* Add changeset
  • Loading branch information
poulch committed Sep 23, 2024
1 parent a9c9600 commit a02dade
Show file tree
Hide file tree
Showing 13 changed files with 168 additions and 21 deletions.
5 changes: 5 additions & 0 deletions .changeset/quiet-roses-compete.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"saleor-dashboard": patch
---

Fix a group of errors caused by reading property of undefined
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { slugFromConditionValue } from "./ConditionValue";

describe("slugFromConditionValue", () => {
it("should return string when input is string", () => {
const result = slugFromConditionValue("test");

expect(result).toBe("test");
});

it("should return slug when input is ItemOption", () => {
const result = slugFromConditionValue({ label: "test", value: "test", slug: "test" });

expect(result).toBe("test");
});

it("should return slug when input is ItemOption array", () => {
const result = slugFromConditionValue([{ label: "test", value: "test", slug: "test" }]);

expect(result).toEqual(["test"]);
});

it("should return slug when input is tuple", () => {
const result = slugFromConditionValue(["test", "test"]);

expect(result).toEqual(["test", "test"]);
});

it("should return empty string when input is undefined", () => {
const result = slugFromConditionValue(undefined);

expect(result).toBe("");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export const isItemOptionArray = (x: ConditionValue): x is ItemOption[] =>
export const isTuple = (x: ConditionValue): x is [string, string] =>
Array.isArray(x) && x.length === 2 && (x as string[]).every(y => typeof y === "string");

export const slugFromConditionValue = (rawEntry: ConditionValue): string | string[] => {
export const slugFromConditionValue = (rawEntry?: ConditionValue): string | string[] => {
if (typeof rawEntry === "string") {
return rawEntry;
}
Expand All @@ -26,5 +26,5 @@ export const slugFromConditionValue = (rawEntry: ConditionValue): string | strin
return rawEntry.map(el => (typeof el === "string" ? el : el.slug));
}

return rawEntry.slug;
return rawEntry?.slug ?? "";
};
5 changes: 2 additions & 3 deletions src/components/SortableTree/hooks/useAnnouncement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,13 @@ export const useAnnouncement = <T extends DataTypePlaceholder>({
const activeIndex = clonedItems.findIndex(({ id }) => id === activeId);
const sortedItems = arrayMove(clonedItems, activeIndex, overIndex);
const previousItem = sortedItems[overIndex - 1];
const nextItem = sortedItems[overIndex + 1];

let announcement;
const movedVerb = eventName === "onDragEnd" ? "dropped" : "moved";
const nestedVerb = eventName === "onDragEnd" ? "dropped" : "nested";

if (!previousItem) {
const nextItem = sortedItems[overIndex + 1];

if (!previousItem && nextItem) {
announcement = `${activeId} was ${movedVerb} before ${nextItem.id}.`;
} else if (projected.depth > previousItem.depth) {
announcement = `${activeId} was ${nestedVerb} under ${previousItem.id}.`;
Expand Down
8 changes: 8 additions & 0 deletions src/orders/components/OrderListDatagrid/datagrid.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ describe("getCustomerCellContent", () => {
// Act
const result = getCustomerCellContent(data);

// Assert
expect(result.data).toEqual("-");
expect(result.displayData).toEqual("-");
});
it("should return - when no data", () => {
// Arrange & Act
const result = getCustomerCellContent(undefined);

// Assert
expect(result.data).toEqual("-");
expect(result.displayData).toEqual("-");
Expand Down
2 changes: 1 addition & 1 deletion src/orders/components/OrderListDatagrid/datagrid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export function getCustomerCellContent(
);
}

if (rowData.userEmail) {
if (rowData?.userEmail) {
return readonlyTextCell(rowData.userEmail);
}

Expand Down
13 changes: 4 additions & 9 deletions src/products/components/ProductUpdatePage/ProductUpdatePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import useStateFromProps from "@dashboard/hooks/useStateFromProps";
import { maybe } from "@dashboard/misc";
import ProductExternalMediaDialog from "@dashboard/products/components/ProductExternalMediaDialog";
import { ProductOrganization } from "@dashboard/products/components/ProductOrganization/ProductOrganization";
import { mapByChannel } from "@dashboard/products/components/ProductUpdatePage/utils";
import { defaultGraphiQLQuery } from "@dashboard/products/queries";
import { productImageUrl, productListUrl } from "@dashboard/products/urls";
import { ChoiceWithAncestors, getChoicesWithAncestors } from "@dashboard/products/utils/utils";
Expand Down Expand Up @@ -268,16 +269,10 @@ export const ProductUpdatePage: React.FC<ProductUpdatePageProps> = ({
onChange: handlers.changeChannels,
openModal: () => setChannelPickerOpen(true),
};
const listings = data.channels.updateChannels?.map<ChannelData>(listing => {
const channel = channels?.find(ac => ac.id === listing.channelId);

return {
...channel,
...listing,
id: listing.channelId,
currency: channel.currencyCode,
};
});
const byChannel = mapByChannel(channels);
const listings = data.channels.updateChannels?.map<ChannelData>(byChannel);

const entityType = getReferenceAttributeEntityTypeFromAttribute(
assignReferencesAttributeId,
data.attributes,
Expand Down
47 changes: 46 additions & 1 deletion src/products/components/ProductUpdatePage/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Locale } from "@dashboard/components/Locale";
import { ChannelFragment, ProductChannelListingAddInput } from "@dashboard/graphql";

import { parseCurrency } from "./utils";
import { mapByChannel, parseCurrency } from "./utils";

describe("parseCurrency", () => {
it("rounds down to 3 decimals in 3 digit currency - EN locale (dot)", () => {
Expand Down Expand Up @@ -70,3 +71,47 @@ describe("parseCurrency", () => {
expect(parsed).toBe(2123);
});
});

describe("mapByChannel", () => {
it("should map listing to channel data", () => {
// Arrange
const channels = [
{ id: "1", currencyCode: "USD" },
{ id: "2", currencyCode: "EUR" },
] as ChannelFragment[];
const listing: ProductChannelListingAddInput = { channelId: "2" };
const mapFunction = mapByChannel(channels);

// Act
const result = mapFunction(listing);

// Assert
expect(result).toEqual({ id: "2", currency: "EUR", currencyCode: "EUR", channelId: "2" });
});

it("should map listing to channel data when no channels", () => {
// Arrange
const channels = [] as ChannelFragment[];
const listing: ProductChannelListingAddInput = { channelId: "3" };
const mapFunction = mapByChannel(channels);

// Act
const result = mapFunction(listing);

// Assert
expect(result).toEqual({ channelId: "3", currency: undefined, id: "3" });
});

it("should map listing to channel data when channels undefined", () => {
// Arrange
const channels = undefined;
const listing: ProductChannelListingAddInput = { channelId: "3" };
const mapFunction = mapByChannel(channels);

// Act
const result = mapFunction(listing);

// Assert
expect(result).toEqual({ channelId: "3", currency: undefined, id: "3" });
});
});
20 changes: 19 additions & 1 deletion src/products/components/ProductUpdatePage/utils.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
// @ts-strict-ignore
import { ChannelData } from "@dashboard/channels/utils";
import {
DatagridChange,
DatagridChangeOpts,
} from "@dashboard/components/Datagrid/hooks/useDatagridChange";
import { Locale } from "@dashboard/components/Locale";
import { ProductFragment } from "@dashboard/graphql";
import {
ChannelFragment,
ProductChannelListingAddInput,
ProductFragment,
} from "@dashboard/graphql";

const getFractionDigits = (locale: Locale, currency: string) => {
try {
Expand Down Expand Up @@ -74,3 +79,16 @@ function getChannelCurrencyCodeById(

return "";
}

export function mapByChannel(channels?: ChannelFragment[]) {
return (listing: ProductChannelListingAddInput): ChannelData => {
const channel = channels?.find(ac => ac.id === listing.channelId);

return {
...channel,
...listing,
id: listing.channelId,
currency: channel?.currencyCode,
};
};
}
10 changes: 9 additions & 1 deletion src/products/utils/validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ describe("validateProductCreateData", () => {
// Assert
expect(errors).toEqual([]);
});

it("returns 'required' errors on product variant form if price is not provided", () => {
const intl = useIntl();

Expand Down Expand Up @@ -132,4 +131,13 @@ describe("validateProductCreateData", () => {
message: "This field cannot be blank",
});
});
it("returns empty errors when data is undefined", () => {
// Arrange
const data = undefined;
// Act
const errors = validateProductCreateData(data);

// Assert
expect(errors).toEqual([]);
});
});
6 changes: 5 additions & 1 deletion src/products/utils/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,13 @@ const createRequiredError = (
attributes: [],
});

export const validateProductCreateData = (data: ProductCreateData) => {
export const validateProductCreateData = (data?: ProductCreateData) => {
let errors: ProductErrorWithAttributesFragment[] = [];

if (!data) {
return errors;
}

if (!data.productType) {
errors = [...errors, createRequiredError("productType")];
}
Expand Down
24 changes: 24 additions & 0 deletions src/utils/handlers/metadataUpdateHelpers.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { areMetadataArraysEqual } from "./metadataUpdateHelpers";

describe("metadataUpdateHelpers", () => {
describe("areMetadataArraysEqual", () => {
it("should return false if before or after is undefined", () => {
expect(areMetadataArraysEqual(undefined, [])).toBe(false);
expect(areMetadataArraysEqual([], undefined)).toBe(false);
});

it("should return true if arrays are equal", () => {
const before = [{ key: "key1", value: "value1" }];
const after = [{ key: "key1", value: "value1" }];

expect(areMetadataArraysEqual(before, after)).toBe(true);
});

it("should return false if arrays are not equal", () => {
const before = [{ key: "key1", value: "value1" }];
const after = [{ key: "key2", value: "value2" }];

expect(areMetadataArraysEqual(before, after)).toBe(false);
});
});
});
12 changes: 10 additions & 2 deletions src/utils/handlers/metadataUpdateHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,13 @@ const removeTypename = ({ __typename, ...input }: GenericMetadataInput) => ({
...input,
});

export const areMetadataArraysEqual = (before: GenericMetadataInput[], after: MetadataInput[]) =>
isEqual(sortBy(before.map(removeTypename)), sortBy(after));
export const areMetadataArraysEqual = (
before?: GenericMetadataInput[],
after?: MetadataInput[],
) => {
if (!before || !after) {
return false;
}

return isEqual(sortBy(before.map(removeTypename)), sortBy(after));
};

0 comments on commit a02dade

Please sign in to comment.