diff --git a/.gitea/workflows/sonarqube.yml b/.gitea/workflows/sonarqube.yml index 11f4141..78b86c2 100755 --- a/.gitea/workflows/sonarqube.yml +++ b/.gitea/workflows/sonarqube.yml @@ -87,6 +87,20 @@ jobs: run: npm run build working-directory: frontend + # The frontend's build was the only thing this workspace ran, so the unit + # suite #188 added over the filter dimensions was run by nothing but the + # author's terminal. A suite CI never runs decays into a record of what + # the code used to do, and its value is highest exactly here: chips() is + # pure, and the end-to-end run reaches it only through a browser. + # + # Guarded and named in the gate like every suite: a failing test should + # fail the job at the end, not abort it and take the scan with it. + - name: Frontend unit tests + id: frontend_unit + continue-on-error: true + run: npm run test:unit + working-directory: frontend + - name: Check the Sonar tsconfig has not drifted run: node scripts/check-sonar-tsconfig.js @@ -233,6 +247,7 @@ jobs: - name: Fail if any guarded step failed if: >- always() && ( + steps.frontend_unit.outcome == 'failure' || steps.unit.outcome == 'failure' || steps.integration.outcome == 'failure' || steps.backend.outcome == 'failure' || diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index b0c8949..34e2cc6 100755 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -17,6 +17,7 @@ import { useCatalogue } from './useCatalogue'; import ItemCard from './components/ItemCard'; import BrandMark from './components/BrandMark'; import FilterBar from './components/filters/FilterBar'; +import { chipsFor } from './components/filters/dimension'; import { availabilityDimension, categoryDimension, @@ -149,8 +150,10 @@ export default function App() { }, [setSearchParams]); // The parent needs to know whether anything is filtering — for the empty - // state's wording — but has no chip row of its own to count. Computed the - // same way FilterBar computes its own tally, from the same dimensions. + // state's wording — but has no chip row of its own to count. Through the same + // chipsFor call FilterBar's tally goes through, not a second expression over + // the same dimensions: those agreed only by convention, which is the defect + // #188 exists to remove. const filterContext = { filters, onChange: applyFilters, @@ -158,7 +161,7 @@ export default function App() { tags: options?.tags ?? [], priceRange: options?.priceRange ?? null }; - const filtered = STOREFRONT_DIMENSIONS.some((dimension) => dimension.chips(filterContext).length > 0); + const filtered = chipsFor(STOREFRONT_DIMENSIONS, filterContext).length > 0; return ( diff --git a/frontend/src/components/filters/FilterBar.tsx b/frontend/src/components/filters/FilterBar.tsx index d2aeb90..438333b 100644 --- a/frontend/src/components/filters/FilterBar.tsx +++ b/frontend/src/components/filters/FilterBar.tsx @@ -5,6 +5,7 @@ import Grid from 'antd/es/grid'; import { FilterOutlined } from '@ant-design/icons'; import type { Category, Tag as ItemTag } from '../../api'; import type { ItemFilters } from '../../filters'; +import { chipsFor } from './dimension'; import type { FilterContext, FilterDimension } from './dimension'; import FilterChips from './FilterChips'; @@ -60,7 +61,11 @@ export default function FilterBar({ // Derived, not stored, and derived without rendering anything — the drawer // unmounts its sections when closed, so anything that needed them mounted // would lose the chips exactly when they matter. - const chips = dimensions.flatMap((dimension) => dimension.chips(context)); + // + // Through chipsFor rather than inline: the page asks the same question for its + // empty state, and one shared call is what stops the tally and the chip row + // from being two expressions that only happen to agree. + const chips = chipsFor(dimensions, context); const barDimensions = dimensions.filter((dimension) => dimension.placement === 'bar'); const drawerDimensions = dimensions.filter((dimension) => dimension.placement === 'drawer'); diff --git a/frontend/src/components/filters/dimension.ts b/frontend/src/components/filters/dimension.ts index 2ff8e42..67b4883 100644 --- a/frontend/src/components/filters/dimension.ts +++ b/frontend/src/components/filters/dimension.ts @@ -49,3 +49,17 @@ export interface FilterDimension { /** Empty when this dimension is filtering nothing. */ chips(ctx: FilterContext): Chip[]; } + +/** + * Every chip a set of dimensions reports, in the order the screen composed them. + * + * The single derivation both sides share. FilterBar's tally is this list's + * length, and a page's "is anything filtered" — which is what decides whether an + * empty grid reads as "no matches, here is the way out" or as an empty shop — is + * whether it is empty. Those were two separate expressions over two + * identically-built contexts until #188's review, agreeing only by convention. + * That is precisely the tally-versus-chip-row disagreement this design exists to + * make impossible, so there is one call and no second opinion. + */ +export const chipsFor = (dimensions: FilterDimension[], ctx: FilterContext): Chip[] => + dimensions.flatMap((dimension) => dimension.chips(ctx)); diff --git a/frontend/src/components/filters/standardDimensions.tsx b/frontend/src/components/filters/standardDimensions.tsx index 2668f28..9e57a28 100644 --- a/frontend/src/components/filters/standardDimensions.tsx +++ b/frontend/src/components/filters/standardDimensions.tsx @@ -234,6 +234,15 @@ export const favoritesDimension: FilterDimension = { : [] }; +/** + * The statuses themselves, as a multi-select. + * + * Mutually exclusive with availabilityDimension below, which is the preset + * alternative over the same `filters.status` field: a screen composes one or + * the other, never both. Composing both type-checks and renders two controls + * over one field, each fighting the other's writes and each contributing chips, + * so the tally counts the same filter twice. + */ export const statusDimension: FilterDimension = { key: 'status', placement: 'drawer', @@ -287,6 +296,11 @@ const SALE_STATE_LABELS: Record = { * makes and is worth having visible without opening anything. Before #188 it * was hand-written markup in App.tsx because the shared component had no way to * say "this belongs in the bar", which is the gap that design closed. + * + * Mutually exclusive with statusDimension above, which is the multi-select + * alternative over the same `filters.status` field: a screen composes one or + * the other, never both, or two controls write one field and it is counted + * twice. */ export const availabilityDimension: FilterDimension = { key: 'availability', @@ -295,6 +309,12 @@ export const availabilityDimension: FilterDimension = { render: ({ filters, onChange }) => ( { - const state = saleStateFromStatuses(filters.status, STOREFRONT_SALE_STATUSES, 'not-sold'); - if (state === 'not-sold') return []; + const statuses = filters.status; + // No preference is not a filter, whatever the control happens to read. + if (statuses === null) return []; + + // 'not-sold' unconditionally, deliberately unlike render above: that + // fallback follows favoritesOnly so the control tells the truth about what + // is on screen, but the customer never *chose* All, and a filter nobody + // chose must not produce a chip or count toward the tally. Unifying the two + // calls would give every signed-in favorites view a phantom All chip and a + // tally of 2. + const state = saleStateFromStatuses(statuses, STOREFRONT_SALE_STATUSES, 'not-sold'); + // saleStateFromStatuses reports a list matching no preset as its fallback, + // so asking twice with different fallbacks is what separates a real match + // from a fallback: the answers agree only when the list matched something. + const isPreset = state === saleStateFromStatuses(statuses, STOREFRONT_SALE_STATUSES, 'all'); + + // Only the actual default earns silence. filtersFromSearchParams accepts + // any non-empty subset of {available, reserved, sold} — seven lists, of + // which three are presets — and before #188 hasActiveFilters reported all + // seven as filtered. Without a chip for the other four, ?status=reserved is + // an empty grid reading "No items yet — check back soon" with no Clear + // filters button, and the only way out is editing the URL by hand. + if (isPreset && state === 'not-sold') return []; + return [ { key: 'availability', - label: SALE_STATE_LABELS[state], + // A known cosmetic wart, and the cheaper half of the trade: for a list + // matching no preset the Segmented still reads "Not sold" while this + // chip is showing, because there is no fourth position to move it to. + // A control one word out beats a dead end with no way back. + label: isPreset ? SALE_STATE_LABELS[state] : statuses.map(statusLabel).join(', '), onRemove: () => onChange({ ...filters, status: null }) } ]; diff --git a/frontend/tests/unit/filterDimensions.test.ts b/frontend/tests/unit/filterDimensions.test.ts index fe9fd7a..9a0fa29 100644 --- a/frontend/tests/unit/filterDimensions.test.ts +++ b/frontend/tests/unit/filterDimensions.test.ts @@ -105,6 +105,14 @@ describe('priceDimension', () => { expect(priceDimension.chips(contextFor({ maxPriceCents: 5000 }).ctx)).toHaveLength(1); }); + // formatPriceRange's real output, en dash and all. The only chip whose label + // text nothing asserted on, which is how a chip reading "$1000–$5000" — cents + // shown as dollars — would have reached a customer unnoticed. + it('labels the chip with the formatted range', () => { + const { ctx } = contextFor({ minPriceCents: 1000, maxPriceCents: 5000 }); + expect(priceDimension.chips(ctx)[0]?.label).toBe('$10–$50'); + }); + it('clears both ends when removed', () => { const { ctx, state } = contextFor({ minPriceCents: 1000, maxPriceCents: 5000 }); priceDimension.chips(ctx)[0]?.onRemove(); @@ -188,4 +196,30 @@ describe('availabilityDimension', () => { availabilityDimension.chips(ctx)[0]?.onRemove(); expect(state.latest?.status).toBeNull(); }); + + // filtersFromSearchParams accepts any non-empty subset of the three public + // statuses — seven lists, of which only three are presets. The other four + // report as the 'not-sold' fallback, so a chip keyed off the preset alone + // leaves ?status=reserved as an empty grid reading "No items yet" with no + // Clear filters button. hasActiveFilters covered all seven before #188; this + // is what replaces it. + it('reports one chip for a status list matching no preset', () => { + const chips = availabilityDimension.chips(contextFor({ status: ['reserved'] }).ctx); + expect(chips.map((c) => c.label)).toEqual(['Reserved']); + + // Several statuses read as a list rather than as a preset's name. + expect( + availabilityDimension + .chips(contextFor({ status: ['available', 'sold'] }).ctx) + .map((c) => c.label) + ).toEqual(['Available, Sold']); + }); + + it('clears the filter when a chip for no preset is removed', () => { + const { ctx, state } = contextFor({ status: ['reserved', 'sold'] }); + const chips = availabilityDimension.chips(ctx); + expect(chips).toHaveLength(1); + chips[0]?.onRemove(); + expect(state.latest?.status).toBeNull(); + }); });