Skip to content

Shopping list store select renders blank when an item's price data fails to load #168

Description

@OffCrazyFreak

Symptom

On a shopping list, the per-item store select ("Trgovina") stays on its placeholder and its dropdown is empty. No error, no toast, no skeleton. The row otherwise renders normally, so it reads as "the feature is broken" rather than "this data is missing".

Observed roughly 2026-08-01 on a freshly registered account with no pinned stores set. Production main at that point was f49d4c70f98d1c1c03382436a3fa05470a6e1eb6.

Why the store is fragile in the first place

Two things combine, and neither is obvious from reading one file.

1. Adding a product never persists a store. frontend/src/app/products/utils/shopping-list-item-request.ts:23 returns early before chainCode is attached:

if (!data.isChecked) return request;

The store field in the add modal is only rendered when "already bought" is ticked (frontend/src/app/products/components/forms/add-to-shopping-list-form.tsx:135). So a normal add writes the item with chainCode = NULL, and nothing on the backend backfills it.

2. The store you see is therefore filled entirely client-side, after the fact. The effect at frontend/src/components/custom/store-chain/store-chain-select.tsx:61:

if (!value && defaultValue && !disabled && autoSelectedForRef.current !== defaultValue) {
  autoSelectedForRef.current = defaultValue;
  setDisplayValue(defaultValue);
  onChange(defaultValue);   // fires the PUT that persists it
}

defaultValue is cheapestStores[item.id], computed in frontend/src/app/(user)/shopping-lists/[id]/hooks/use-shopping-list-data.ts:56-91.

Where it breaks

That memo silently skips any item whose product it could not fetch:

const product = productsByEan.get(item.ean);
if (!product) continue;

An item that misses gets no cheapestStore, no averagePrice and no storePrices. defaultValue is then undefined, so the auto-select effect never fires, and availableChainCodes in the select is empty too. The result is a blank, optionless dropdown and zero feedback.

The fetch path is strict at both hops, so this is easy to trigger. frontend/src/lib/cijene-api/utils/with-cijene-route.ts:26 returns 502 when the upstream payload does not match the zod schema exactly, and frontend/src/lib/cijene-api/queries.ts:68 parses it a second time client-side. One unexpected field from the upstream price API and the item loses its store with no trace.

Two regressions that were live in that exact window

Both were fixed on 2026-08-02 in 208d302ed1bb7f18aa26ddc521ca92ccc4eba532, after the sighting. They are listed here so whoever picks this up does not re-report them as new:

  • EAN key mismatch. productsByEan was built from the EAN the upstream API echoed back, but looked up by the EAN stored on the list item. Any normalisation upstream (leading zero, UPC-A widened to EAN-13) made the lookup miss. This one hits some items and not others, which matches the sighting best.
  • Unguarded productData.chains. getStorePricesFromProduct and findCheapestStoreFromProduct iterated chains with no nullish guard, inside the useMemo. This one takes down every item at once.

Still live on dev today

  • The ref latch never retries. autoSelectedForRef is set before the write is confirmed. If the PUT fails, the optimistic patch rolls back, value returns to null, but the ref already equals defaultValue, so the effect will not fire again for the life of that mount. One failed write leaves a blank store until a reload.
  • pickCheapestChain can return null on valid data. frontend/src/app/(user)/shopping-lists/utils/item-price-utils.ts:24 compares parseFloat(avg_price) against Infinity. If every chain's avg_price is unparseable, NaN < Infinity is false every time and no chain is ever picked.

Recommended fix

Distinguish "prices still loading" from "prices unavailable", and render the second one explicitly instead of an empty select.

Sketch:

  • In use-shopping-list-data.ts, alongside cheapestStores / averagePrices / storePrices, return the set of item ids whose product query resolved to an error rather than data. useQueries already exposes per-result state in combine, and it is already index-aligned with eans, so this is a small addition to the existing combine.
  • Thread that through shopping-list-items.tsx into shopping-list-item.tsx as a prop.
  • In StoreChainSelect, when the item is in that set, render a disabled trigger with an explicit reason rather than the Trgovina placeholder. Croatian copy, second person, ungendered: something like Cijene nisu dostupne.
  • Keep the row layout identical so nothing shifts.
  • Accessibility: the trigger needs an accessible name that states the reason, and the state change should not be announced as an error since it is a data gap, not a user mistake.

Why not the alternatives

Not "make the auto-select retry instead of latching" (on its own). The latch is deliberate. 967db89b2c9199ef95265584ef51bf83bb0fdf91 added it because the effect re-ran on rollback and re-selected the default, spamming requests and toasts. Loosening it means re-solving that, and getting the bound wrong puts the toast storm back. It also addresses a different failure: price data loaded fine but the write bounced. That failure is narrower than it looks, because the update carries OFFLINE_MUTATION_KEYS.shoppingListItemUpdate, so an offline write is queued and replayed rather than lost, and a genuine server error already clears on reload. Worth doing later as its own small commit, with the retry bounded to a single re-attempt per defaultValue rather than an unlatched effect.

Not "persist the cheapest chain at add time on the server". It would remove the client round trip, but it needs a backend change and a decision about what the server does when the upstream price API is down at write time. It also freezes a choice that is meant to track current prices. Larger, and it does not make the failure visible when it does happen, which is the actual complaint here.

Not "loosen the zod schemas so a 502 becomes partial data". That trades a visible gap for silently wrong prices, which is worse in a price comparison app.

Verification for whoever picks this up

Please confirm or contradict the analysis before changing anything.

  1. On a list with several items, force one product-by-EAN response to fail (block /api/cijene/products/<ean> in devtools, or point one item at an EAN the upstream API does not carry). Confirm that item's select renders blank and optionless while its neighbours populate normally.
  2. Confirm the item's chainCode is NULL in the database after a plain add, so the client effect really is the only writer.
  3. Check whether the upstream API ever returns a normalised EAN that differs from the one we requested. If it does, the fix in 208d302e was necessary rather than precautionary, and that is worth recording.

Notes

  • The "fresh account with no preferences" detail in the original sighting looks incidental. findCheapestStoreFromProduct skips the pinned-store branch entirely when there are none and falls through to the global cheapest chain, which is correct. The "just signed up, no JWT yet" theory is also undercut by the 401 retry with forced token refresh at frontend/src/lib/api/api-base.ts:126. The plausible connection is that a fresh account has an empty React Query cache, so every product fetch had to go out live instead of hitting a 6 hour stale entry, which widens the window for a 502 to land.
  • Related earlier fixes in this component, for context: b3965edf8357203ca711a822365a51ae4b798559 and 967db89b2c9199ef95265584ef51bf83bb0fdf91.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions