fix(books): remove redundant Shelf Status facet from filter drawer [shot:filter-drawer] (bookshelf-onx3v) #1215

Merged

zombor merged 2 commits from bd-bookshelf-onx3v into main 2026-07-24 00:54:00 +00:00

Conversation 36 Commits 2 Files changed 23 +18 -579

zombor commented 2026-07-24 00:06:45 +00:00

Owner

Copy link

Summary

• The filter drawer showed two nearly identical facets — "Read Status" and "Shelf Status" — both exposing UNREAD/READING/FINISHED counts.
• Root cause: FacetShelfStatusCounts delegated to FacetStatusCounts and buildShelfStatusGroup literally called buildStatusGroup, producing the same SQL predicate. The "Shelf Status" facet was entirely redundant.
• Fix: remove fshelfstatus URL param, FacetShelfStatuses/ShelfStatuses fields from all structs, FacetShelfStatusCounts function, buildShelfStatusGroup, the template section, and all JS references (fshelfstatus, FACET_PARAMS entry, _paramName mapping).

Before: Filter drawer had "Read Status" and "Shelf Status" sections showing identical UNREAD/READING/FINISHED counts.
After: Filter drawer has only "Read Status" — the authoritative per-user read status filter.

Test plan

• make test passes — all unit tests green
• make build passes — no compile errors
• make lint passes — no new lint issues in this worktree
• internal/books tests: removed FacetShelfStatusCounts, FacetShelfStatuses, ShelfStatuses, fshelfstatus test cases; kept all other facet tests
• JS Vitest: removed "maps shelfstatus → fshelfstatus" test and fshelfstatus=UNREAD from the all-params test
• Screenshot: CI auto-posts via [shot:filter-drawer] title marker using existing filter drawer journey

Closes bead bookshelf-onx3v on merge.

## Summary - The filter drawer showed two nearly identical facets — "Read Status" and "Shelf Status" — both exposing UNREAD/READING/FINISHED counts. - Root cause: `FacetShelfStatusCounts` delegated to `FacetStatusCounts` and `buildShelfStatusGroup` literally called `buildStatusGroup`, producing the same SQL predicate. The "Shelf Status" facet was entirely redundant. - Fix: remove `fshelfstatus` URL param, `FacetShelfStatuses`/`ShelfStatuses` fields from all structs, `FacetShelfStatusCounts` function, `buildShelfStatusGroup`, the template section, and all JS references (`fshelfstatus`, `FACET_PARAMS` entry, `_paramName` mapping). **Before:** Filter drawer had "Read Status" and "Shelf Status" sections showing identical UNREAD/READING/FINISHED counts. **After:** Filter drawer has only "Read Status" — the authoritative per-user read status filter. ## Test plan - [x] `make test` passes — all unit tests green - [x] `make build` passes — no compile errors - [x] `make lint` passes — no new lint issues in this worktree - [x] `internal/books` tests: removed `FacetShelfStatusCounts`, `FacetShelfStatuses`, `ShelfStatuses`, `fshelfstatus` test cases; kept all other facet tests - [x] JS Vitest: removed `"maps shelfstatus → fshelfstatus"` test and `fshelfstatus=UNREAD` from the all-params test - [x] Screenshot: CI auto-posts via `[shot:filter-drawer]` title marker using existing filter drawer journey Closes bead bookshelf-onx3v on merge.

filter-drawer-shelf-filtered.png

75 KiB

filter-drawer-fantasy-filtered.png

74 KiB

filter-drawer-genre-expanded.png

76 KiB

filter-drawer-cross-facet-series-narrowed.png

74 KiB

filter-drawer-fantasy-filtered.png

74 KiB

filter-drawer-range-expanded.png

74 KiB

filter-drawer-range-filtered.png

72 KiB

filter-drawer-value-search.png

72 KiB

filter-drawer-chips-combinator.png

75 KiB

filter-drawer-chip-removed.png

76 KiB

filter-drawer-shelf-open.png

76 KiB

filter-drawer-shelf-filtered.png

75 KiB

filter-drawer-character-filtered.png

73 KiB

filter-drawer-chips-combinator.png

75 KiB

filter-drawer-genre-expanded.png

76 KiB

filter-drawer-chip-removed.png

76 KiB

filter-drawer-fantasy-filtered.png

74 KiB

filter-drawer-range-expanded.png

74 KiB

filter-drawer-range-filtered.png

72 KiB

filter-drawer-shelf-open.png

76 KiB

filter-drawer-shelf-filtered.png

75 KiB

filter-drawer-character-filtered.png

73 KiB

filter-drawer-value-search.png

72 KiB

filter-drawer-cross-facet-series-narrowed.png

74 KiB

filter-drawer-character-filtered.png

73 KiB

filter-drawer-range-expanded.png

75 KiB

filter-drawer-range-filtered.png

72 KiB

filter-drawer-chips-combinator.png

76 KiB

filter-drawer-chip-removed.png

77 KiB

filter-drawer-cross-facet-series-narrowed.png

74 KiB

filter-drawer-value-search.png

72 KiB

filter-drawer-shelf-open.png

76 KiB

filter-drawer-genre-expanded.png

77 KiB

zombor added 1 commit 2026-07-24 00:06:45 +00:00

fix(books): remove redundant Shelf Status facet from filter drawer ... 7ca434bf85

FacetShelfStatusCounts / buildShelfStatusGroup / FacetShelfStatuses were
semantically identical to FacetStatusCounts / buildStatusGroup / FacetStatuses:
buildShelfStatusGroup literally called buildStatusGroup with the same args,
and appendScopePredicates scoped both to the current shelf anyway. The "Shelf
Status" drawer facet (fshelfstatus URL param) showed UNREAD/READING/FINISHED
counts that perfectly matched the existing "Read Status" facet — confusing users
who saw two near-identical panels.

Remove FacetShelfStatusCounts, buildShelfStatusGroup, FacetShelfStatuses,
ShelfStatuses, the "shelfstatus" facet registration in wire.go, the template
section, and all JS references. Read Status continues to work unchanged.

Closes bead bookshelf-onx3v on merge.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

forgejo-actions commented 2026-07-24 00:13:16 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-genre-expanded)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-genre-expanded) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:13:20 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-cross-facet-series-narrowed)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-cross-facet-series-narrowed) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:13:21 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-fantasy-filtered)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-fantasy-filtered) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:13:26 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-range-expanded)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-range-expanded) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:13:30 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-range-filtered)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-range-filtered) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:13:36 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-value-search)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-value-search) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:13:38 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-chips-combinator)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-chips-combinator) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:13:42 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-chip-removed)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-chip-removed) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:14:28 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-shelf-open)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-shelf-open) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:14:34 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-shelf-filtered)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-shelf-filtered) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:14:39 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-character-filtered)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-character-filtered) Books list filter drawer — Genre facet expanded, filter applied. 

zombor added 1 commit 2026-07-24 00:23:32 +00:00

fix(e2e): remove e2e browser tests for removed Shelf Status facet ... e49bcefe28

The Shelf Status facet was removed in the prior commit (bookshelf-onx3v).
Two e2e browser journey tests that exercised the shelfstatus facet endpoint
and DOM element now fail because the facet no longer exists in the template
or wire.go registration.

Remove:
- The "expanding Shelf Status facet does not show 'Could not load filters.'"
  It block from the lazy-fetch/localStorage/range journey
- The entire "Journey: Filter Drawer — shelf status facet scoped to selected shelf"
  Describe block (4 Its: opens drawer, expand-no-shelf, navigate-with-fshelf,
  per-shelf counts screenshot)

Update the lazy-fetch journey title and BeforeAll comment to drop stale
references to Shelf Status.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

forgejo-actions commented 2026-07-24 00:30:56 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-chips-combinator)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-chips-combinator) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:30:58 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-genre-expanded)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-genre-expanded) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:31:01 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-chip-removed)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-chip-removed) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:31:02 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-fantasy-filtered)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-fantasy-filtered) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:31:07 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-range-expanded)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-range-expanded) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:31:11 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-range-filtered)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-range-filtered) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:31:13 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-shelf-open)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-shelf-open) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:31:20 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-shelf-filtered)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-shelf-filtered) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:31:25 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-character-filtered)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-character-filtered) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:31:42 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-value-search)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-value-search) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:31:45 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-cross-facet-series-narrowed)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-cross-facet-series-narrowed) Books list filter drawer — Genre facet expanded, filter applied. 

zombor commented 2026-07-24 00:37:04 +00:00

Author

Owner

Copy link

UI Review — bookshelf-onx3v (PR #1215)

Reviewed rendered screenshots (second batch, comments 14922–14934) and the diff against main.

Screenshot observations

filter-drawer-shelf-open — Drawer open with Shelf facet expanded. Panel order reads: Read Status → File Format → Category → Author → Series → Publisher → Language → Tag → Rating → Page Count → Published Year → Metadata Score → Library → Shelf (expanded, MySuperShelf (2)) → Age Rating → Content Rating. No "Shelf Status" panel present. No orphaned heading, empty section, stray divider, or spacing anomaly where the panel used to sit. The Shelf→Age Rating transition is clean.

filter-drawer-genre-expanded — Category expanded. "Shelf Status" remains absent. Panel spacing and typography are visually uniform throughout. Shelf is visible (collapsed) at the foot of the drawer with no gap artifacts.

filter-drawer-chips-combinator — Active fantasy filter chip, Category expanded. Panel structure unchanged and correct. No double Read Status entry.

Source cross-check

Diff removes exactly:

• The <div class="facet-category" data-facet="shelfstatus">…Shelf Status…</div> block from templates/pages/books_index.html
• {{range .Filter.FacetShelfStatuses}}&fshelfstatus=…{{end}} from the two URL-building locations (infinite-scroll sentinel + load-more link)
• params.delete("fshelfstatus"), the "fshelfstatus" entry from FACET_PARAMS, and the facet === "shelfstatus" mapping branch in filter_drawer_controller.js

No inline style= introduced. No bespoke classes introduced. All retained facet panels continue to use the existing canonical facet-category / facet-category-toggle / facet-values classes. No CSS changes in this PR.

Findings

None.

---

REVIEW VERDICT: 0 blocker, 0 major, 0 minor

## UI Review — bookshelf-onx3v (PR #1215) Reviewed rendered screenshots (second batch, comments 14922–14934) and the diff against `main`. ### Screenshot observations **filter-drawer-shelf-open** — Drawer open with Shelf facet expanded. Panel order reads: Read Status → File Format → Category → Author → Series → Publisher → Language → Tag → Rating → Page Count → Published Year → Metadata Score → Library → **Shelf** (expanded, MySuperShelf (2)) → Age Rating → Content Rating. No "Shelf Status" panel present. No orphaned heading, empty section, stray divider, or spacing anomaly where the panel used to sit. The Shelf→Age Rating transition is clean. **filter-drawer-genre-expanded** — Category expanded. "Shelf Status" remains absent. Panel spacing and typography are visually uniform throughout. Shelf is visible (collapsed) at the foot of the drawer with no gap artifacts. **filter-drawer-chips-combinator** — Active fantasy filter chip, Category expanded. Panel structure unchanged and correct. No double Read Status entry. ### Source cross-check Diff removes exactly: - The `<div class="facet-category" data-facet="shelfstatus">…Shelf Status…</div>` block from `templates/pages/books_index.html` - `{{range .Filter.FacetShelfStatuses}}&fshelfstatus=…{{end}}` from the two URL-building locations (infinite-scroll sentinel + load-more link) - `params.delete("fshelfstatus")`, the `"fshelfstatus"` entry from `FACET_PARAMS`, and the `facet === "shelfstatus"` mapping branch in `filter_drawer_controller.js` No inline `style=` introduced. No bespoke classes introduced. All retained facet panels continue to use the existing canonical `facet-category` / `facet-category-toggle` / `facet-values` classes. No CSS changes in this PR. ### Findings None. --- REVIEW VERDICT: 0 blocker, 0 major, 0 minor

zombor commented 2026-07-24 00:37:54 +00:00

Author

Owner

Copy link

Review: Remove Duplicate Shelf Status Facet (bookshelf-onx3v)

Verification Results

1. Removal Completeness ✓

• FacetShelfStatuses field removed from Filter struct (books.go:171-176)
• ShelfStatuses field removed from ActiveFacets struct (books.go:315)
• FacetShelfStatusCounts() function completely removed (facet_store.go:397-420)
• buildShelfStatusGroup() function completely removed (store.go:841-843)
• fshelfstatus param validation removed (facet_handler.go:365-367)
• shelfstatus facet mapping removed from wire (wire.go:893)
• Template HTML element removed (templates/pages/books_index.html:988-993)
• JS controller handling removed (filter_drawer_controller.js:906, 923)
• All URL param forwarding for fshelfstatus removed
• No remaining references to any removed symbols (verified: zero additions of ShelfStatus/shelfstatus)

2. Duplicate Verification ✓

Confirmed buildShelfStatusGroup was identical to buildStatusGroup:

// OLD:
func buildShelfStatusGroup(statuses []string) facetGroup {
    return buildStatusGroup(statuses)
}

100% duplicate logic — no user-facing distinction from regular Read Status facet.

3. Backward Compatibility (URL Degradation) ✓

• Existing bookmarks/shares with fshelfstatus=... params will degrade gracefully
• validateFacetParams() only processes explicitly listed param names; unknown params are silently ignored (no 400 error)
• No SQL changes; filter logic unaffected

4. Other Facets Unaffected ✓

• FacetShelves (shelf membership filter) remains intact and functional
• FacetStatuses (read status facet) unchanged
• Cross-facet narrowing logic correctly updated (buildCrossFacetGroups line 1031-1433)
• bookFilterNeedsUBP() correctly updated to exclude FacetShelfStatuses check
• No changes to store predicates, SQL, or indexes

5. Test Updates Correct ✓

Removed (appropriately):

• 194-line browser e2e journey "shelf status facet scoped to selected shelf" (e2e/browser/journey_filter_drawer_test.go:79-265)
• Shelf-status regression guard test from cross-facet journey (e2e/browser:33-71)
• 120-line FacetShelfStatusCounts unit test suite (facet_store_test.go:492-612)
• FacetShelfStatusCounts cross-facet test (facet_store_test.go:450-483)
• FacetShelfStatuses from HasActiveFilter test (filter_has_active_test.go:621-623)
• fshelfstatus param validation tests (handler_test.go:734-752)
• FacetShelfStatuses from service forwarding test (service_test.go:2795-2814)
• FacetShelfStatuses from filtered_ids parity test (filtered_ids_store_test.go:688-702)
• All JS controller tests for shelfstatus (filter_drawer_controller.test.js:952-957, 944)

Retained & Passing:

• All FacetStatusCounts tests (core predicate logic unchanged)
• All shelf (FacetShelves) tests
• All other facet tests
• CI: all 7 checks passing (Unit/Race/Coverage/Lint/Integration/E2E API/E2E Browser)

6. Comment Consistency ✓

Stale reference updated in facet_handler.go:356:

// OLD: "fstatus/fshelfstatus: each value must be a valid read status"
// NEW: "fstatus: each value must be a valid read status"

Summary

This is a surgical removal of a 100% redundant facet. The logic was identical to the global Read Status facet; the shelf-status-specific wrapper only added shelf-context gating (return empty if no shelf selected). With that gate removed, the facet is just a duplicate UI control for the same predicate. The removal is complete, consistent, correctly tested, and backward-compatible.

REVIEW VERDICT: 0 blocker, 0 major, 0 minor

# Review: Remove Duplicate Shelf Status Facet (bookshelf-onx3v) ## Verification Results ### 1. Removal Completeness ✓ - `FacetShelfStatuses` field removed from Filter struct (books.go:171-176) - `ShelfStatuses` field removed from ActiveFacets struct (books.go:315) - `FacetShelfStatusCounts()` function completely removed (facet_store.go:397-420) - `buildShelfStatusGroup()` function completely removed (store.go:841-843) - `fshelfstatus` param validation removed (facet_handler.go:365-367) - `shelfstatus` facet mapping removed from wire (wire.go:893) - Template HTML element removed (templates/pages/books_index.html:988-993) - JS controller handling removed (filter_drawer_controller.js:906, 923) - All URL param forwarding for `fshelfstatus` removed - No remaining references to any removed symbols (verified: zero additions of ShelfStatus/shelfstatus) ### 2. Duplicate Verification ✓ Confirmed `buildShelfStatusGroup` was identical to `buildStatusGroup`: ```go // OLD: func buildShelfStatusGroup(statuses []string) facetGroup { return buildStatusGroup(statuses) } ``` 100% duplicate logic — no user-facing distinction from regular Read Status facet. ### 3. Backward Compatibility (URL Degradation) ✓ - Existing bookmarks/shares with `fshelfstatus=...` params will degrade gracefully - `validateFacetParams()` only processes explicitly listed param names; unknown params are silently ignored (no 400 error) - No SQL changes; filter logic unaffected ### 4. Other Facets Unaffected ✓ - `FacetShelves` (shelf membership filter) remains intact and functional - `FacetStatuses` (read status facet) unchanged - Cross-facet narrowing logic correctly updated (buildCrossFacetGroups line 1031-1433) - `bookFilterNeedsUBP()` correctly updated to exclude FacetShelfStatuses check - No changes to store predicates, SQL, or indexes ### 5. Test Updates Correct ✓ **Removed (appropriately):** - 194-line browser e2e journey "shelf status facet scoped to selected shelf" (e2e/browser/journey_filter_drawer_test.go:79-265) - Shelf-status regression guard test from cross-facet journey (e2e/browser:33-71) - 120-line FacetShelfStatusCounts unit test suite (facet_store_test.go:492-612) - FacetShelfStatusCounts cross-facet test (facet_store_test.go:450-483) - FacetShelfStatuses from HasActiveFilter test (filter_has_active_test.go:621-623) - fshelfstatus param validation tests (handler_test.go:734-752) - FacetShelfStatuses from service forwarding test (service_test.go:2795-2814) - FacetShelfStatuses from filtered_ids parity test (filtered_ids_store_test.go:688-702) - All JS controller tests for shelfstatus (filter_drawer_controller.test.js:952-957, 944) **Retained & Passing:** - All FacetStatusCounts tests (core predicate logic unchanged) - All shelf (FacetShelves) tests - All other facet tests - CI: all 7 checks passing (Unit/Race/Coverage/Lint/Integration/E2E API/E2E Browser) ### 6. Comment Consistency ✓ Stale reference updated in facet_handler.go:356: ``` // OLD: "fstatus/fshelfstatus: each value must be a valid read status" // NEW: "fstatus: each value must be a valid read status" ``` ## Summary This is a surgical removal of a 100% redundant facet. The logic was identical to the global Read Status facet; the shelf-status-specific wrapper only added shelf-context gating (return empty if no shelf selected). With that gate removed, the facet is just a duplicate UI control for the same predicate. The removal is complete, consistent, correctly tested, and backward-compatible. REVIEW VERDICT: 0 blocker, 0 major, 0 minor

zombor commented 2026-07-24 00:37:58 +00:00

Author

Owner

Copy link

Security Review — PR #1215 (bd-bookshelf-onx3v)

Scope: removal of the redundant "Shelf Status" facet (fshelfstatus param / FacetShelfStatuses / FacetShelfStatusCounts / buildShelfStatusGroup) across Go, template, and JS layers.

Multi-user scoping — intact. The removed FacetShelfStatusCounts was a thin wrapper delegating to FacetStatusCounts (identical per-user user_id/userLibraryIDs scoping); it added no unique scoping clause. The remaining facets keep their scoping: FacetHandler still resolves userLibraryIDs via resolveLibraryIDs(userID, ...) (facet_handler.go:77) fail-closed, and the shelf facet retains its ownership predicate buildShelfGroup(names, userID) (store.go) plus cross-facet buildCrossFacetStatusGroup(..., StatusUserID) / buildShelfGroup(..., ShelvesUserID). The removal drops only the duplicate status predicate, not any scoping clause.

Leftover fshelfstatus param — safe. validateFacetParams no longer reads q["fshelfstatus"], so on /books the param is now silently ignored (standard Go — unread query params never trigger a query or error). On /books/facets, an unknown ?facet=shelfstatus hits the guard at facet_handler.go:72 (facet != "status" && rangeCounts[facet]==nil && userRangeCounts[facet]==nil) → returns ErrValidation (400). No unscoped query, no existence/error leak.

Injection surface — unchanged. Remaining param parsing/validation (allowlist for fstatus/fformat, max-count/max-length caps for free-text facets, validateValueSearch) is untouched. No new dynamic SQL.

Ownership / shelf-visibility — no check removed. FacetShelfCounts (with its shelf.user_id = userID join) remains wired (wire.go). Only the read-status duplicate was removed.

Removal is symmetric and complete across all layers (Filter, ActiveFacets, params structs, validation, cross-facet builder, predicate builder, wire map, JS controller, template, tests); git grep finds no dangling shelfstatus/FacetShelfStatuses references in production code.

REVIEW VERDICT: 0 blocker, 0 major, 0 minor

## Security Review — PR #1215 (bd-bookshelf-onx3v) Scope: removal of the redundant "Shelf Status" facet (`fshelfstatus` param / `FacetShelfStatuses` / `FacetShelfStatusCounts` / `buildShelfStatusGroup`) across Go, template, and JS layers. **Multi-user scoping — intact.** The removed `FacetShelfStatusCounts` was a thin wrapper delegating to `FacetStatusCounts` (identical per-user `user_id`/`userLibraryIDs` scoping); it added no unique scoping clause. The remaining facets keep their scoping: `FacetHandler` still resolves `userLibraryIDs` via `resolveLibraryIDs(userID, ...)` (facet_handler.go:77) fail-closed, and the shelf facet retains its ownership predicate `buildShelfGroup(names, userID)` (store.go) plus cross-facet `buildCrossFacetStatusGroup(..., StatusUserID)` / `buildShelfGroup(..., ShelvesUserID)`. The removal drops only the duplicate status predicate, not any scoping clause. **Leftover `fshelfstatus` param — safe.** `validateFacetParams` no longer reads `q["fshelfstatus"]`, so on `/books` the param is now silently ignored (standard Go — unread query params never trigger a query or error). On `/books/facets`, an unknown `?facet=shelfstatus` hits the guard at facet_handler.go:72 (`facet != "status" && rangeCounts[facet]==nil && userRangeCounts[facet]==nil`) → returns `ErrValidation` (400). No unscoped query, no existence/error leak. **Injection surface — unchanged.** Remaining param parsing/validation (allowlist for `fstatus`/`fformat`, max-count/max-length caps for free-text facets, `validateValueSearch`) is untouched. No new dynamic SQL. **Ownership / shelf-visibility — no check removed.** `FacetShelfCounts` (with its `shelf.user_id = userID` join) remains wired (wire.go). Only the read-status duplicate was removed. Removal is symmetric and complete across all layers (Filter, ActiveFacets, params structs, validation, cross-facet builder, predicate builder, wire map, JS controller, template, tests); `git grep` finds no dangling `shelfstatus`/`FacetShelfStatuses` references in production code. REVIEW VERDICT: 0 blocker, 0 major, 0 minor

zombor force-pushed bd-bookshelf-onx3v from e49bcefe28 to 168f8a0d86 2026-07-24 00:44:11 +00:00 Compare

forgejo-actions commented 2026-07-24 00:51:15 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-shelf-open)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-shelf-open) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:51:20 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-genre-expanded)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-genre-expanded) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:51:23 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-shelf-filtered)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-shelf-filtered) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:51:28 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-fantasy-filtered)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-fantasy-filtered) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:51:33 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-character-filtered)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-character-filtered) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:51:36 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-range-expanded)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-range-expanded) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:51:45 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-range-filtered)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-range-filtered) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:51:50 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-chips-combinator)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-chips-combinator) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:51:56 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-chip-removed)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-chip-removed) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:52:00 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-cross-facet-series-narrowed)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-cross-facet-series-narrowed) Books list filter drawer — Genre facet expanded, filter applied. 

forgejo-actions commented 2026-07-24 00:52:02 +00:00

Copy link

Filter Drawer screenshot (filter-drawer-value-search)

Books list filter drawer — Genre facet expanded, filter applied.

**Filter Drawer screenshot** (filter-drawer-value-search) Books list filter drawer — Genre facet expanded, filter applied. 

zombor merged commit 890fcaa474 into main 2026-07-24 00:54:00 +00:00

zombor referenced this pull request from a commit 2026-07-24 00:54:05 +00:00

Merge pull request 'fix(books): remove redundant Shelf Status facet from filter drawer [shot:filter-drawer] (bookshelf-onx3v)' (#1215) from bd-bookshelf-onx3v into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

zombor/pergamum!1215

No description provided.