docs: add collection library filtering implementation plan
Add comprehensive step-by-step plan for implementing library-aware filtering on collection detail pages. Purpose: - Preserve dashboard context when navigating to collection details - Support both filtered (single library) and unfiltered (all libraries) views - Maintain backward compatibility with existing URLs Plan includes: - Detailed code changes for dashboard.go, frontend.go, dashboard_test.go - Line-by-line modifications with before/after code snippets - Implementation order with 10 steps - Testing checklist for verification - Documentation requirements Follows PROJECT_GUIDELINES.md: - No cascading fix-up edits - Sequential implementation order - Post-edit verification steps - Test-driven approach with additions to dashboard_test.go - Documentation updates for user-facing feature This is a planning document only - no implementation changes yet.
This commit is contained in:
@@ -0,0 +1,291 @@
|
|||||||
|
# Collection Library Filtering Implementation Plan
|
||||||
|
|
||||||
|
## Overview
|
||||||
|
|
||||||
|
Implement library-aware filtering for collection detail pages to preserve context when navigating from the dashboard.
|
||||||
|
|
||||||
|
**Current Behavior:**
|
||||||
|
- Dashboard shows collections filtered by selected library
|
||||||
|
- Clicking "View All" navigates to `/collections/:id` showing ALL books across all libraries
|
||||||
|
- This causes UX confusion: user sees 2 books on dashboard, 5 on collection page
|
||||||
|
|
||||||
|
**Target Behavior:**
|
||||||
|
- Navigate to `/collections/:id` → Show ALL books in collection (maintains backward compatibility)
|
||||||
|
- Navigate to `/collections/:id?library_id=xxx` → Show books from that specific library only
|
||||||
|
- Dashboard "View All" links preserve library context
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Files to Modify
|
||||||
|
|
||||||
|
### 1. `internal/handlers/dashboard.go`
|
||||||
|
|
||||||
|
**Location:** Lines 188-193
|
||||||
|
|
||||||
|
**Current Code:**
|
||||||
|
```go
|
||||||
|
func getViewAllURL(collectionID string) string {
|
||||||
|
if collectionID != "" {
|
||||||
|
return "/collections/" + collectionID
|
||||||
|
}
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Change To:**
|
||||||
|
```go
|
||||||
|
func getViewAllURL(collectionID string) libraryID string) string {
|
||||||
|
if collectionID != "" {
|
||||||
|
if libraryID != "" {
|
||||||
|
return "/collections/" + collectionID + "?library_id=" + libraryID
|
||||||
|
}
|
||||||
|
return "/collections/" + collectionID
|
||||||
|
}
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Also Update Line 180:**
|
||||||
|
```go
|
||||||
|
// Current:
|
||||||
|
ViewAllURL: getViewAllURL(ds.CollectionID.String()),
|
||||||
|
|
||||||
|
// Change To:
|
||||||
|
ViewAllURL: getViewAllURL(ds.CollectionID.String(), currentLibraryID),
|
||||||
|
```
|
||||||
|
|
||||||
|
**Also Update Function Signature (Line 157):**
|
||||||
|
```go
|
||||||
|
// Current:
|
||||||
|
func BuildSections(sections []services.DashboardSection) []SectionData {
|
||||||
|
|
||||||
|
// Change To:
|
||||||
|
func BuildSections(sections []services.DashboardSection, currentLibraryID string) []SectionData {
|
||||||
|
```
|
||||||
|
|
||||||
|
**Also Update Call Site (frontend.go Lines 194, 195):**
|
||||||
|
```go
|
||||||
|
// Current:
|
||||||
|
sectionData := handlers.BuildSections(visibleSections)
|
||||||
|
allSectionsData := handlers.BuildSections(allSections)
|
||||||
|
|
||||||
|
// Change To:
|
||||||
|
sectionData := handlers.BuildSections(visibleSections, libraryID)
|
||||||
|
allSectionsData := handlers.BuildSections(allSections, libraryID)
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 2. `internal/router/frontend.go`
|
||||||
|
|
||||||
|
**Location:** Lines 360-378 (collection detail page handler)
|
||||||
|
|
||||||
|
**Current Code:**
|
||||||
|
```go
|
||||||
|
} else {
|
||||||
|
// User collection - fetch collection items
|
||||||
|
collItems, err := cfg.Queries.GetCollectionItems(c.Request().Context(), pgtype.UUID{Bytes: collUUID, Valid: true})
|
||||||
|
if err != nil {
|
||||||
|
books = []handlers.BookInfo{}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Convert to BookInfo format
|
||||||
|
bookCards := make([]handlers.BookInfo, len(collItems))
|
||||||
|
for i, item := range collItems {
|
||||||
|
itemUUID, _ := uuid.FromBytes(item.MediaItemID.Bytes[0:16])
|
||||||
|
bookCards[i] = handlers.BookInfo{
|
||||||
|
MediaItemID: itemUUID.String(),
|
||||||
|
Title: item.Title,
|
||||||
|
Author: getText(item.Author),
|
||||||
|
CoverImagePath: utils.ResolveMediaURL(item.LibraryID, item.CoverImagePath),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
books = bookCards
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Change To:**
|
||||||
|
```go
|
||||||
|
} else {
|
||||||
|
// User collection - check if library_id filter is present
|
||||||
|
libraryID := c.QueryParam("library_id")
|
||||||
|
|
||||||
|
if libraryID != "" {
|
||||||
|
// Filter by library - reuse dashboard query
|
||||||
|
libUUID, err := uuid.Parse(libraryID)
|
||||||
|
if err != nil {
|
||||||
|
return renderErrorPage(c, "Invalid library ID", "invalid_library_id")
|
||||||
|
}
|
||||||
|
|
||||||
|
// Use GetCollectionItemsForDashboard for library-filtered results
|
||||||
|
collItems, err := cfg.Queries.GetCollectionItemsForDashboard(c.Request().Context(),
|
||||||
|
database.GetCollectionItemsForDashboardParams{
|
||||||
|
CollectionID: pgtype.UUID{Bytes: collUUID, Valid: true},
|
||||||
|
LibraryID: pgtype.UUID{Bytes: libUUID, Valid: true},
|
||||||
|
Limit: 1000,
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
books = []handlers.BookInfo{}
|
||||||
|
} else {
|
||||||
|
// Convert to BookInfo format (non-excluded only)
|
||||||
|
var validItems []database.GetCollectionItemsForDashboardRow
|
||||||
|
for _, item := range collItems {
|
||||||
|
if !item.Excluded.Valid || !item.Excluded.Bool {
|
||||||
|
validItems = append(validItems, item)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
bookCards := make([]handlers.BookInfo, len(validItems))
|
||||||
|
for i, item := range validItems {
|
||||||
|
itemUUID, _ := uuid.FromBytes(item.ID.Bytes[0:16])
|
||||||
|
bookCards[i] = handlers.BookInfo{
|
||||||
|
MediaItemID: itemUUID.String(),
|
||||||
|
Title: item.Title,
|
||||||
|
Author: getText(item.Author),
|
||||||
|
CoverImagePath: utils.ResolveMediaURL(item.LibraryID, item.CoverImagePath),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
books = bookCards
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
// No library filter - show all books in collection
|
||||||
|
collItems, err := cfg.Queries.GetCollectionItems(c.Request().Context(), pgtype.UUID{Bytes: collUUID, Valid: true})
|
||||||
|
if err != nil {
|
||||||
|
books = []handlers.BookInfo{}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Convert to BookInfo format
|
||||||
|
bookCards := make([]handlers.BookInfo, len(collItems))
|
||||||
|
for i, item := range collItems {
|
||||||
|
itemUUID, _ := uuid.FromBytes(item.MediaItemID.Bytes[0:16])
|
||||||
|
bookCards[i] = handlers.BookInfo{
|
||||||
|
MediaItemID: itemUUID.String(),
|
||||||
|
Title: item.Title,
|
||||||
|
Author: getText(item.Author),
|
||||||
|
CoverImagePath: utils.ResolveMediaURL(item.LibraryID, item.CoverImagePath),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
books = bookCards
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 3. `internal/handlers/dashboard_test.go`
|
||||||
|
|
||||||
|
**Location:** Lines 60-66
|
||||||
|
|
||||||
|
**Current Code:**
|
||||||
|
```go
|
||||||
|
assert.Equal(t, "/section/continue-reading", section1.ViewAllURL, "ViewAllURL should match system collection")
|
||||||
|
assert.Equal(t, "", section2.ViewAllURL, "ViewAllURL should be empty for user collections")
|
||||||
|
```
|
||||||
|
|
||||||
|
**Add New Test Function:**
|
||||||
|
```go
|
||||||
|
func TestGetViewAllURLWithLibrary(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
collectionID string
|
||||||
|
libraryID string
|
||||||
|
expected string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "with library ID",
|
||||||
|
collectionID: "collection-123",
|
||||||
|
libraryID: "library-abc",
|
||||||
|
expected: "/collections/collection-123?library_id=library-abc",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "without library ID",
|
||||||
|
collectionID: "collection-123",
|
||||||
|
libraryID: "",
|
||||||
|
expected: "/collections/collection-123",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "empty collection ID",
|
||||||
|
collectionID: "",
|
||||||
|
libraryID: "library-abc",
|
||||||
|
expected: "",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
result := getViewAllURL(tt.collectionID, tt.libraryID)
|
||||||
|
assert.Equal(t, tt.expected, result, "getViewAllURL mismatch")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 4. `docs/user/collections.md` (Create if doesn't exist)
|
||||||
|
|
||||||
|
**Create documentation file:**
|
||||||
|
```markdown
|
||||||
|
# Collections
|
||||||
|
|
||||||
|
Collections allow you to organize books across multiple libraries.
|
||||||
|
|
||||||
|
## Viewing Collections
|
||||||
|
|
||||||
|
### From Collections Page
|
||||||
|
Navigate to `/collections` to see all your collections. Clicking on a collection shows ALL books in that collection across all libraries.
|
||||||
|
|
||||||
|
### From Dashboard
|
||||||
|
When viewing a specific library's dashboard, collections only show books from that library. Clicking "View All" preserves this context and shows only books from that library in the collection.
|
||||||
|
|
||||||
|
### Library Filtering
|
||||||
|
- **No filter** (`/collections/favorites`): Shows all books across all libraries
|
||||||
|
- **With filter** (`/collections/favorites?library_id=xxx`): Shows only books from the specified library
|
||||||
|
|
||||||
|
## Creating Collections
|
||||||
|
|
||||||
|
[Instructions for creating collections]
|
||||||
|
|
||||||
|
## Managing Collections
|
||||||
|
|
||||||
|
[Instructions for editing/deleting collections]
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Implementation Order
|
||||||
|
|
||||||
|
1. **Step 1:** Modify `dashboard.go` `getViewAllURL()` function signature and implementation
|
||||||
|
2. **Step 2:** Update `BuildSections()` to accept and pass `currentLibraryID`
|
||||||
|
3. **Step 3:** Update call sites in `frontend.go` dashboard handler
|
||||||
|
4. **Step 4:** Modify `frontend.go` collection detail handler to support library_id query parameter
|
||||||
|
5. **Step 5:** Add tests to `dashboard_test.go`
|
||||||
|
6. **Step 6:** Create/update documentation in `docs/user/collections.md`
|
||||||
|
7. **Step 7:** Rebuild TypeScript if needed (no changes expected)
|
||||||
|
8. **Step 8:** Rebuild container: `podman compose build`
|
||||||
|
9. **Step 9:** Restart container: `podman compose up -d`
|
||||||
|
10. **Step 10:** Test navigation from dashboard to collection detail page
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Testing Checklist
|
||||||
|
|
||||||
|
- [ ] Navigate to dashboard with Library A selected
|
||||||
|
- [ ] Click "View All" on a collection
|
||||||
|
- [ ] Verify URL includes `?library_id=library-a-id`
|
||||||
|
- [ ] Verify only books from Library A appear
|
||||||
|
- [ ] Navigate to collections page directly
|
||||||
|
- [ ] Click on same collection
|
||||||
|
- [ ] Verify URL has no library_id parameter
|
||||||
|
- [ ] Verify ALL books from all libraries appear
|
||||||
|
- [ ] Run tests: `go test ./internal/handlers/... -v`
|
||||||
|
- [ ] Verify no regressions in existing functionality
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Notes
|
||||||
|
|
||||||
|
- **Backward Compatibility:** URLs without `library_id` parameter work exactly as before
|
||||||
|
- **Reuses Existing Query:** `GetCollectionItemsForDashboard` already filters by library - no new SQL needed
|
||||||
|
- **Progressive Enhancement:** Works without JavaScript (server-side rendering)
|
||||||
|
- **Follows Guidelines:** No custom CSS, TypeScript only, service layer pattern maintained
|
||||||
Reference in New Issue
Block a user