Harden product type tab count deduping and add missing tests (#6722)

* Harden product type tab count deduping and add missing tests

* Default attribute list group-by-type on and fix test types.

New sessions see type tabs on product and model attribute lists by default, with ripple copy updated accordingly. Test helpers use NonNullable aliases for nullable GraphQL query fields.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Extract messages

---------

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: mirekm

locale/defaultMessages.json

@@ -501,6 +501,10 @@
     "context": "countries list menu label when no countries are assigned",
     "string": "There are no countries assigned"
   },
+  "0a93bd": {
+    "context": "success message after unassigning attributes from a model type",
+    "string": "Model type updated"
+  },
   "0dCGBW": {
     "context": "button",
     "string": "Create structure"
@@ -4009,6 +4013,10 @@
     "context": "onboarding step title",
     "string": "Create a new product"
   },
+  "IEkGwG": {
+    "context": "success message after unassigning attributes from a product type",
+    "string": "Product type updated"
+  },
   "IFWHn0": {
     "context": "error message",
     "string": "Billing address is not set"

src/attributes/components/AttributeListPage/AttributeListPage.tsx

@@ -69,7 +69,7 @@ const AttributeListPage = ({
   onSelectAttributesIds,
   hidePagination = false,
   canGroupByType = false,
-  groupByType = false,
+  groupByType = true,
   onGroupByTypeChange,
   showTypeTabs = false,
   types,

src/attributes/components/AttributeListPage/messages.ts

@@ -23,4 +23,14 @@ export const attributeListPageMessages = defineMessages({
       "Attributes are removed from this model type only. They are not deleted and remain available for other model types. Models that use other types are not affected.",
     description: "helper text in unassign dialog on model attributes list, type tab",
   },
+  productTypeUpdated: {
+    id: "IEkGwG",
+    defaultMessage: "Product type updated",
+    description: "success message after unassigning attributes from a product type",
+  },
+  modelTypeUpdated: {
+    id: "0a93bd",
+    defaultMessage: "Model type updated",
+    description: "success message after unassigning attributes from a model type",
+  },
 });

src/attributes/hooks/useAttributeGroupByType.ts

@@ -5,7 +5,7 @@ const ATTRIBUTE_LIST_GROUP_BY_TYPE_KEY = "attributeList.groupByType";
 export const useAttributeGroupByType = () => {
   const [groupByType, setGroupByType] = useLocalStorage<boolean>(
     ATTRIBUTE_LIST_GROUP_BY_TYPE_KEY,
-    false,
+    true,
   );
 
   return {

src/attributes/ripples/attributeListGroupByType.tsx

@@ -9,10 +9,10 @@ const settingsIconStyle: CSSProperties = {
 
 const attributeListGroupByTypeContextual = (
   <Text color="default2">
-    Turn on <em>Group by type</em> in the view menu{" "}
-    <Settings size={16} aria-hidden style={settingsIconStyle} /> to browse attributes by product or
-    model type. Your filters stay above the tabs, and you can unassign attributes from a single type
-    when one tab is selected.
+    Product and model attribute lists are grouped by type. Use the tabs to browse attributes per
+    type, or turn off <em>Group by type</em> in the view menu{" "}
+    <Settings size={16} aria-hidden style={settingsIconStyle} />. Your filters stay above the tabs,
+    and you can unassign attributes from a single type when one tab is selected.
   </Text>
 );
 
@@ -25,6 +25,6 @@ export const rippleAttributeListGroupByType: Ripple = {
     oneLiner: "Group attributes by type",
     contextual: attributeListGroupByTypeContextual,
     global:
-      "On product and model attribute lists, open the view menu next to Create attribute and choose Group by type. The list adds tabs for each type so you can see which attributes belong to it. Filters and search stay above the tabs. When a single type tab is active, you can unassign selected attributes from that type without deleting them shop-wide.",
+      "On product and model attribute lists, attributes are grouped by type with tabs for each type. Filters and search stay above the tabs. When a single type tab is active, you can unassign selected attributes from that type without deleting them shop-wide. Turn off Group by type in the view menu next to Create attribute if you prefer the flat list.",
   },
 };

src/attributes/utils/computeTypeTabCounts.test.ts

@@ -1,5 +1,55 @@
-import { computePageTypeTabCounts } from "@dashboard/attributes/utils/computeTypeTabCounts";
-import { type PageTypeListWithAssignedAttributeCountsQuery } from "@dashboard/graphql";
+import {
+  computePageTypeTabCounts,
+  computeProductTypeTabCounts,
+} from "@dashboard/attributes/utils/computeTypeTabCounts";
+import {
+  type PageTypeListWithAssignedAttributeCountsQuery,
+  ProductTypeKindEnum,
+  type ProductTypeListWithAssignedAttributeCountsQuery,
+} from "@dashboard/graphql";
+
+type ProductTypesConnection = NonNullable<
+  ProductTypeListWithAssignedAttributeCountsQuery["productTypes"]
+>;
+type ProductTypeNode = ProductTypesConnection["edges"][number]["node"];
+
+const emptyPageInfo = {
+  __typename: "PageInfo" as const,
+  endCursor: null,
+  hasNextPage: false,
+  hasPreviousPage: false,
+  startCursor: null,
+};
+
+const createProductTypeTabCountsQuery = (
+  productType: ProductTypeNode,
+): ProductTypeListWithAssignedAttributeCountsQuery => ({
+  __typename: "Query",
+  productTypes: {
+    __typename: "ProductTypeCountableConnection",
+    edges: [
+      {
+        __typename: "ProductTypeCountableEdge",
+        node: productType,
+      },
+    ],
+    pageInfo: emptyPageInfo,
+  },
+});
+
+const createProductTypeNode = (
+  attributes: Pick<ProductTypeNode, "productAttributes" | "variantAttributes">,
+): ProductTypeNode => ({
+  __typename: "ProductType",
+  id: "pt-1",
+  name: "Default",
+  slug: "default",
+  kind: ProductTypeKindEnum.NORMAL,
+  hasVariants: true,
+  isShippingRequired: true,
+  taxClass: null,
+  ...attributes,
+});
 
 describe("computePageTypeTabCounts", () => {
   it("should build counts from page type attribute ids", () => {
@@ -20,13 +70,7 @@ describe("computePageTypeTabCounts", () => {
             },
           },
         ],
-        pageInfo: {
-          __typename: "PageInfo",
-          endCursor: null,
-          hasNextPage: false,
-          hasPreviousPage: false,
-          startCursor: null,
-        },
+        pageInfo: emptyPageInfo,
       },
     };
 
@@ -39,3 +83,50 @@ describe("computePageTypeTabCounts", () => {
     });
   });
 });
+
+describe("computeProductTypeTabCounts", () => {
+  it("should build counts from product and variant attribute ids", () => {
+    // Arrange
+    const data = createProductTypeTabCountsQuery(
+      createProductTypeNode({
+        productAttributes: [
+          { __typename: "Attribute", id: "a-1" },
+          { __typename: "Attribute", id: "a-2" },
+        ],
+        variantAttributes: [{ __typename: "Attribute", id: "a-3" }],
+      }),
+    );
+
+    // Act
+    const result = computeProductTypeTabCounts(data);
+
+    // Assert
+    expect(result).toEqual({
+      "pt-1": { value: 3, hasMore: false },
+    });
+  });
+
+  it("should dedupe attribute ids shared between product and variant attributes", () => {
+    // Arrange
+    const data = createProductTypeTabCountsQuery(
+      createProductTypeNode({
+        productAttributes: [
+          { __typename: "Attribute", id: "a-1" },
+          { __typename: "Attribute", id: "a-2" },
+        ],
+        variantAttributes: [
+          { __typename: "Attribute", id: "a-2" },
+          { __typename: "Attribute", id: "a-3" },
+        ],
+      }),
+    );
+
+    // Act
+    const result = computeProductTypeTabCounts(data);
+
+    // Assert
+    expect(result).toEqual({
+      "pt-1": { value: 3, hasMore: false },
+    });
+  });
+});

src/attributes/utils/computeTypeTabCounts.ts

@@ -1,3 +1,4 @@
+import { countUniqueAttributeIds } from "@dashboard/attributes/utils/countUniqueAttributeIds";
 import {
   type PageTypeListWithAssignedAttributeCountsQuery,
   type ProductTypeListWithAssignedAttributeCountsQuery,
@@ -27,12 +28,9 @@ export const computeProductTypeTabCounts = (
   const counts: Record<string, ModelTypeTabCount> = {};
 
   data?.productTypes?.edges?.forEach(({ node }) => {
-    const ids = new Set<string>();
-
-    node.productAttributes?.forEach(attribute => ids.add(attribute.id));
-    node.variantAttributes?.forEach(attribute => ids.add(attribute.id));
-
-    counts[node.id] = toTabCount(ids.size);
+    counts[node.id] = toTabCount(
+      countUniqueAttributeIds([node.productAttributes, node.variantAttributes]),
+    );
   });
 
   return counts;

src/attributes/utils/countUniqueAttributeIds.test.ts

@@ -0,0 +1,27 @@
+import { countUniqueAttributeIds } from "@dashboard/attributes/utils/countUniqueAttributeIds";
+
+describe("countUniqueAttributeIds", () => {
+  it("should count unique ids across attribute lists", () => {
+    // Arrange
+    const productAttributes = [{ id: "a-1" }, { id: "a-2" }];
+    const variantAttributes = [{ id: "a-3" }];
+
+    // Act
+    const result = countUniqueAttributeIds([productAttributes, variantAttributes]);
+
+    // Assert
+    expect(result).toBe(3);
+  });
+
+  it("should dedupe ids shared between attribute lists", () => {
+    // Arrange
+    const productAttributes = [{ id: "a-1" }, { id: "a-2" }];
+    const variantAttributes = [{ id: "a-2" }, { id: "a-3" }];
+
+    // Act
+    const result = countUniqueAttributeIds([productAttributes, variantAttributes]);
+
+    // Assert
+    expect(result).toBe(3);
+  });
+});

src/attributes/utils/countUniqueAttributeIds.ts

@@ -0,0 +1,15 @@
+interface AttributeId {
+  id: string;
+}
+
+export const countUniqueAttributeIds = (
+  attributeLists: Array<Array<AttributeId> | null | undefined>,
+): number => {
+  const ids = new Set<string>();
+
+  attributeLists.forEach(attributeList => {
+    attributeList?.forEach(attribute => ids.add(attribute.id));
+  });
+
+  return ids.size;
+};

src/attributes/views/AttributeList/AttributeList.tsx

@@ -369,8 +369,8 @@ const AttributeList = ({ params }: AttributeListProps) => {
       status: "success",
       text: intl.formatMessage(
         assignedTypeKind === "product"
-          ? { id: "6j4TUi", defaultMessage: "Product type updated" }
-          : { id: "GVGaij", defaultMessage: "Model type updated" },
+          ? attributeListPageMessages.productTypeUpdated
+          : attributeListPageMessages.modelTypeUpdated,
       ),
     });
     clearRowSelection();