Merge pull request #136 from beatz174-bit/codex/fix-packaging-type-filter-on-pick-list-page

Fix packaging filter behavior on pick list

e2e/picklist.spec.ts

@@ -264,16 +264,14 @@ 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 }) => {
+  test('disables packaging filters when statuses are mixed and filters by packaging when enabled', 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 cartonToggle = page.getByRole('button', { name: /Switch to carton packaging/i }).first();
-
+    await cartonToggle.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' });
@@ -283,16 +281,25 @@ test.describe('Active pick list', () => {
 
     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);
+    await expect(page.getByText(secondaryProduct)).toBeVisible();
 
-    const showPickedToggle = page.getByRole('checkbox', { name: 'Show picked' });
+    await cartonsRadio.click();
-    await showPickedToggle.click();
+    await expect(cartonsRadio).toBeChecked();
+    await expect(page.getByText(secondaryProduct)).toHaveCount(0);
+    await expect(page.getByText(additionalProduct)).toBeVisible();
+
+    const statusToggles = page.getByLabel('Toggle picked status');
+    await statusToggles.first().check();
 
     await expect(cartonsRadio).toBeDisabled();
     await expect(unitsRadio).toBeDisabled();
     await expect(page.getByRole('radio', { name: 'All' })).toBeChecked();
+
+    const showPickedToggle = page.getByRole('checkbox', { name: 'Show picked' });
+    await showPickedToggle.click();
+    await expect(cartonsRadio).toBeDisabled();
+    await expect(unitsRadio).toBeDisabled();
   });
 
   test('toggles packaging type and persists the selection', async ({ page }) => {

src/screens/ActivePickListScreen.test.tsx

@@ -500,7 +500,7 @@ describe('ActivePickListScreen product search', () => {
     expect(itemLabels).toEqual(['Apple Juice', 'Chips', 'Cola', 'Cola']);
   });
 
-  it('keeps packaging filters enabled when items share a single packaging type but mixed statuses', () => {
+  it('disables packaging filters when the visible list has mixed pick statuses', () => {
     pickItemsMock.mockReturnValue([
       {
         id: 'item-1',
@@ -532,11 +532,11 @@ describe('ActivePickListScreen product search', () => {
       </MemoryRouter>,
     );
 
-    expect(screen.getByRole('radio', { name: /cartons/i })).toBeEnabled();
+    expect(screen.getByRole('radio', { name: /cartons/i })).toBeDisabled();
-    expect(screen.getByRole('radio', { name: /units/i })).toBeEnabled();
+    expect(screen.getByRole('radio', { name: /units/i })).toBeDisabled();
   });
 
-  it('enables and disables packaging filters based on the visible item statuses', async () => {
+  it('disables packaging filters when show picked is unchecked', async () => {
     pickItemsMock.mockReturnValue([
       {
         id: 'item-1',
@@ -544,7 +544,7 @@ describe('ActivePickListScreen product search', () => {
         product_id: 'prod-1',
         quantity: 1,
         is_carton: true,
-        status: 'picked',
+        status: 'pending',
         created_at: 0,
         updated_at: 0,
       },
@@ -580,10 +580,6 @@ describe('ActivePickListScreen product search', () => {
 
     expect(cartonsRadio).toBeDisabled();
     expect(unitsRadio).toBeDisabled();
-
-    await user.click(togglePicked);
-    expect(cartonsRadio).toBeEnabled();
-    expect(unitsRadio).toBeEnabled();
   });
 
   it('resets the packaging filter when show picked is unchecked', async () => {
@@ -594,7 +590,7 @@ describe('ActivePickListScreen product search', () => {
         product_id: 'prod-1',
         quantity: 1,
         is_carton: true,
-        status: 'picked',
+        status: 'pending',
         created_at: 0,
         updated_at: 0,
       },
@@ -638,7 +634,7 @@ describe('ActivePickListScreen product search', () => {
         product_id: 'prod-1',
         quantity: 1,
         is_carton: true,
-        status: 'picked',
+        status: 'pending',
         created_at: 0,
         updated_at: 0,
       },
@@ -730,7 +726,7 @@ describe('ActivePickListScreen product search', () => {
         product_id: 'prod-1',
         quantity: 1,
         is_carton: true,
-        status: 'picked',
+        status: 'pending',
         created_at: 0,
         updated_at: 0,
       },
@@ -770,49 +766,7 @@ describe('ActivePickListScreen product search', () => {
     expect(screen.queryByText('Chips')).not.toBeInTheDocument();
   });
 
-  it('retains a chosen packaging filter when statuses are mixed but packaging types are not', async () => {
+  it('keeps packaging filters enabled when all items share the same status', () => {
-    pickItemsMock.mockReturnValue([
-      {
-        id: 'item-1',
-        pick_list_id: 'list-1',
-        product_id: 'prod-1',
-        quantity: 1,
-        is_carton: false,
-        status: 'pending',
-        created_at: 0,
-        updated_at: 0,
-      },
-      {
-        id: 'item-2',
-        pick_list_id: 'list-1',
-        product_id: 'prod-2',
-        quantity: 1,
-        is_carton: false,
-        status: 'picked',
-        created_at: 0,
-        updated_at: 0,
-      },
-    ]);
-
-    const user = userEvent.setup();
-
-    render(
-      <MemoryRouter initialEntries={['/pick-lists/1']}>
-        <Routes>
-          <Route path="/pick-lists/:id" element={<ActivePickListScreen />} />
-        </Routes>
-      </MemoryRouter>,
-    );
-
-    const unitsRadio = screen.getByRole('radio', { name: /units/i });
-    await user.click(unitsRadio);
-
-    expect(unitsRadio).toBeChecked();
-    expect(screen.getByText('Cola')).toBeVisible();
-    expect(screen.getByText('Chips')).toBeVisible();
-  });
-
-  it('disables packaging filters when all items share the same status', () => {
     pickItemsMock.mockReturnValue([
       {
         id: 'item-1',
@@ -844,11 +798,11 @@ describe('ActivePickListScreen product search', () => {
       </MemoryRouter>,
     );
 
-    expect(screen.getByRole('radio', { name: /cartons/i })).toBeDisabled();
+    expect(screen.getByRole('radio', { name: /cartons/i })).toBeEnabled();
-    expect(screen.getByRole('radio', { name: /units/i })).toBeDisabled();
+    expect(screen.getByRole('radio', { name: /units/i })).toBeEnabled();
   });
 
-  it('disables packaging filters when every item is picked', () => {
+  it('keeps packaging filters enabled when every item is picked', () => {
     pickItemsMock.mockReturnValue([
       {
         id: 'item-1',
@@ -880,8 +834,8 @@ describe('ActivePickListScreen product search', () => {
       </MemoryRouter>,
     );
 
-    expect(screen.getByRole('radio', { name: /cartons/i })).toBeDisabled();
+    expect(screen.getByRole('radio', { name: /cartons/i })).toBeEnabled();
-    expect(screen.getByRole('radio', { name: /units/i })).toBeDisabled();
+    expect(screen.getByRole('radio', { name: /units/i })).toBeEnabled();
   });
 
   it('disables show picked toggle when all items are picked', async () => {

src/screens/ActivePickListScreen.tsx

@@ -50,13 +50,13 @@ export const ActivePickListScreen = () => {
     [itemState, showPicked],
   );
 
-  const hasPickedItemsInList = useMemo(
+  const hasPickedItemsVisible = useMemo(
-    () => itemState.some((item) => item.status === 'picked'),
+    () => itemsVisibleByStatus.some((item) => item.status === 'picked'),
-    [itemState],
+    [itemsVisibleByStatus],
   );
-  const hasUnpickedItemsInList = useMemo(
+  const hasUnpickedItemsVisible = useMemo(
-    () => itemState.some((item) => item.status !== 'picked'),
+    () => itemsVisibleByStatus.some((item) => item.status !== 'picked'),
-    [itemState],
+    [itemsVisibleByStatus],
   );
   const hasCartonItems = useMemo(
     () => itemsVisibleByStatus.some((item) => item.is_carton),
@@ -67,12 +67,12 @@ export const ActivePickListScreen = () => {
     [itemsVisibleByStatus],
   );
   const allItemsPicked = useMemo(
-    () => itemState.length > 0 && !hasUnpickedItemsInList,
+    () => itemState.length > 0 && itemState.every((item) => item.status === 'picked'),
-    [hasUnpickedItemsInList, itemState.length],
+    [itemState],
   );
-  const allItemsUnpicked = useMemo(
+  const packagingFiltersDisabled = useMemo(
-    () => itemState.length > 0 && !hasPickedItemsInList,
+    () => !showPicked || (hasPickedItemsVisible && hasUnpickedItemsVisible),
-    [hasPickedItemsInList, itemState.length],
+    [hasPickedItemsVisible, hasUnpickedItemsVisible, showPicked],
   );
 
   const productMap = useMemo(() => {
@@ -156,7 +156,6 @@ export const ActivePickListScreen = () => {
     );
   }, [pickList?.categories, sortedProducts]);
 
-  const packagingFiltersDisabled = !showPicked || allItemsPicked || allItemsUnpicked;
   const appliedItemFilter = useMemo(() => {
     if (packagingFiltersDisabled) {
       return 'all';