Merge pull request #123 from beatz174-bit/codex/fix-filter-list-by-packaging-control-q6lm5i

Fix packaging filter enablement and coverage

e2e/picklist.spec.ts

@@ -264,6 +264,37 @@ test.describe('Active pick list', () => {
     await expect(showPickedToggle).toBeChecked();
   });
 
+  test('enables packaging filters when mixed pick statuses are visible and keeps the selection', async ({ page }) => {
+    await navigateToNewPickList(page);
+
+    await addProductToPickList(page, additionalProduct);
+    await addProductToPickList(page, secondaryProduct);
+
+    await page.getByRole('button', { name: /Switch to carton packaging/i }).first().click();
+
+    const statusToggles = page.getByLabel('Toggle picked status');
+    await statusToggles.first().check();
+
+    const cartonsRadio = page.getByRole('radio', { name: 'Cartons' });
+    const unitsRadio = page.getByRole('radio', { name: 'Units' });
+
+    await expect(cartonsRadio).toBeEnabled();
+    await expect(unitsRadio).toBeEnabled();
+
+    await unitsRadio.click();
+    await expect(unitsRadio).toBeChecked();
+    await expect(cartonsRadio).not.toBeChecked();
+    await expect(page.getByText(secondaryProduct)).toBeVisible();
+    await expect(page.getByText(additionalProduct)).toHaveCount(0);
+
+    const showPickedToggle = page.getByRole('checkbox', { name: 'Show picked' });
+    await showPickedToggle.click();
+
+    await expect(cartonsRadio).toBeDisabled();
+    await expect(unitsRadio).toBeDisabled();
+    await expect(page.getByRole('radio', { name: 'All' })).toBeChecked();
+  });
+
   test('toggles packaging type and persists the selection', async ({ page }) => {
     await navigateToNewPickList(page);
     await addProductToPickList(page, additionalProduct);

src/screens/ActivePickListScreen.test.tsx

@@ -442,7 +442,7 @@ describe('ActivePickListScreen product search', () => {
     expect(screen.getByRole('radio', { name: /units/i })).toBeDisabled();
   });
 
-  it('keeps packaging filters enabled when hiding picked items with mixed statuses', async () => {
+  it('enables and disables packaging filters based on the visible item statuses', async () => {
     pickItemsMock.mockReturnValue([
       {
         id: 'item-1',
@@ -476,13 +476,20 @@ describe('ActivePickListScreen product search', () => {
       </MemoryRouter>,
     );
 
-    expect(screen.getByRole('radio', { name: /cartons/i })).toBeEnabled();
+    const cartonsRadio = screen.getByRole('radio', { name: /cartons/i });
-    expect(screen.getByRole('radio', { name: /units/i })).toBeEnabled();
+    const unitsRadio = screen.getByRole('radio', { name: /units/i });
+    expect(cartonsRadio).toBeEnabled();
+    expect(unitsRadio).toBeEnabled();
 
-    await user.click(screen.getByLabelText(/show picked/i));
+    const togglePicked = screen.getByLabelText(/show picked/i);
+    await user.click(togglePicked);
 
-    expect(screen.getByRole('radio', { name: /cartons/i })).toBeEnabled();
+    expect(cartonsRadio).toBeDisabled();
-    expect(screen.getByRole('radio', { name: /units/i })).toBeEnabled();
+    expect(unitsRadio).toBeDisabled();
+
+    await user.click(togglePicked);
+    expect(cartonsRadio).toBeEnabled();
+    expect(unitsRadio).toBeEnabled();
   });
 
   it('resets the filter when the selected packaging type is unavailable', async () => {
@@ -595,7 +602,7 @@ describe('ActivePickListScreen product search', () => {
     expect(screen.getByText('Cola')).toBeVisible();
   });
 
-  it('enables packaging filters when there are picked and unpicked items', async () => {
+  it('filters the visible list by packaging type and keeps the selection active', async () => {
     pickItemsMock.mockReturnValue([
       {
         id: 'item-1',
@@ -629,11 +636,18 @@ describe('ActivePickListScreen product search', () => {
       </MemoryRouter>,
     );
 
-    const showPickedToggle = screen.getByLabelText(/show picked/i);
+    const cartonsRadio = screen.getByRole('radio', { name: /cartons/i });
-    await user.click(showPickedToggle);
+    const unitsRadio = screen.getByRole('radio', { name: /units/i });
 
-    expect(screen.getByRole('radio', { name: /cartons/i })).not.toBeDisabled();
+    await user.click(unitsRadio);
-    expect(screen.getByRole('radio', { name: /units/i })).not.toBeDisabled();
+    expect(unitsRadio).toBeChecked();
+    expect(screen.getByText('Chips')).toBeVisible();
+    expect(screen.queryByText('Cola')).not.toBeInTheDocument();
+
+    await user.click(cartonsRadio);
+    expect(cartonsRadio).toBeChecked();
+    expect(screen.getByText('Cola')).toBeVisible();
+    expect(screen.queryByText('Chips')).not.toBeInTheDocument();
   });
 
   it('disables packaging filters when all items share the same status', () => {

src/screens/ActivePickListScreen.tsx

@@ -43,17 +43,22 @@ export const ActivePickListScreen = () => {
     setItemState(items);
   }, [items]);
 
+  const itemsVisibleByStatus = useMemo(
+    () => (showPicked ? itemState : itemState.filter((item) => item.status !== 'picked')),
+    [itemState, showPicked],
+  );
+
   const allItemsPicked = useMemo(
     () => itemState.length > 0 && itemState.every((item) => item.status === 'picked'),
     [itemState],
   );
   const hasUnpickedItems = useMemo(
-    () => itemState.some((item) => item.status !== 'picked'),
+    () => itemsVisibleByStatus.some((item) => item.status !== 'picked'),
-    [itemState],
+    [itemsVisibleByStatus],
   );
   const hasPickedItems = useMemo(
-    () => itemState.some((item) => item.status === 'picked'),
+    () => itemsVisibleByStatus.some((item) => item.status === 'picked'),
-    [itemState],
+    [itemsVisibleByStatus],
   );
   const hasMixedPickStatuses = hasPickedItems && hasUnpickedItems;
 
@@ -71,13 +76,8 @@ export const ActivePickListScreen = () => {
     [areas, pickList?.area_id],
   );
 
-  const visibleItemsByStatus = useMemo(
-    () => (showPicked ? itemState : itemState.filter((item) => item.status !== 'picked')),
-    [itemState, showPicked],
-  );
-
   const sortedItems = useMemo(() => {
-    return [...visibleItemsByStatus].sort((a, b) => {
+    return [...itemsVisibleByStatus].sort((a, b) => {
       const productA = productMap.get(a.product_id);
       const productB = productMap.get(b.product_id);
 
@@ -98,7 +98,7 @@ export const ActivePickListScreen = () => {
 
       return timeA - timeB;
     });
-  }, [visibleItemsByStatus, productMap]);
+  }, [itemsVisibleByStatus, productMap]);
 
   const sortedProducts = useMemo(() => {
     const dedupedById = new Map<string, Product>();
@@ -141,12 +141,12 @@ export const ActivePickListScreen = () => {
   }, [pickList?.categories, sortedProducts]);
 
   const hasCartonItems = useMemo(
-    () => itemState.some((item) => item.is_carton),
+    () => itemsVisibleByStatus.some((item) => item.is_carton),
-    [itemState],
+    [itemsVisibleByStatus],
   );
   const hasUnitItems = useMemo(
-    () => itemState.some((item) => !item.is_carton),
+    () => itemsVisibleByStatus.some((item) => !item.is_carton),
-    [itemState],
+    [itemsVisibleByStatus],
   );
   const packagingTypeCount = Number(hasCartonItems) + Number(hasUnitItems);
   const singlePackagingType = packagingTypeCount === 1;