diff --git a/COLLECTION_LIBRARY_FILTERING_PLAN.md b/COLLECTION_LIBRARY_FILTERING_PLAN.md deleted file mode 100644 index 28080e8..0000000 --- a/COLLECTION_LIBRARY_FILTERING_PLAN.md +++ /dev/null @@ -1,291 +0,0 @@ -# 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 diff --git a/IMPLEMENTATION_COLLECTION_FIX.md b/IMPLEMENTATION_COLLECTION_FIX.md deleted file mode 100644 index 4b57b7a..0000000 --- a/IMPLEMENTATION_COLLECTION_FIX.md +++ /dev/null @@ -1,1446 +0,0 @@ -# Implementation Plan: Collection Detail Page Fix + library_id Filter - -## Overview - -Fix the broken `/collections/:id` page (inline JS bug) and add library_id support to the search API. This is a **full-stack task** involving: -- Backend: Search API enhancement, WebSocket permission fix -- Database: SQL query modification -- Frontend: TypeScript conversion, UI improvements - -Uses hybrid approach: minimal TypeScript for client-only features, HTMX-like patterns for CRUD operations, WebSocket for real-time sync. - ---- - -## Files to Modify - -| File | Changes | -|------|---------| -| `internal/sync/websocket.go` | Add user-scoped broadcast method | -| `internal/handlers/collections.go` | Use user-scoped broadcasts | -| `internal/database/queries.sql` | Add library_id filter to search | -| `internal/handlers/media.go` | Add library_id param to search handler | -| `templates/collections.templ` | Remove inline JS, add data attributes, add toggle UI | -| `web/src/collections.ts` | Add TypeScript with WebSocket support | -| `internal/router/frontend.go` | Pass libraryID to template | -| `cmd/server/tests/collections_test.go` | Add integration tests | -| `docs/developer/api/search.md` | Document library_id parameter | -| `bruno/collections/search-with-library-filter.yml` | API test for new parameter | - ---- - -## Git Commit Strategy - -Use multiple logical commits: - -1. **feat(websocket): Add user-scoped broadcasting** - - `internal/sync/websocket.go` - - `internal/handlers/collections.go` - - Tests for user-scoped broadcasts - -2. **feat(database): Add library_id filter to search queries** - - `internal/database/queries.sql` - - Regenerate `internal/database/queries.sql.go` - - Verify params struct updated - -3. **feat(api): Add library_id parameter to search endpoint** - - `internal/handlers/media.go` - - Integration tests for library filtering - -4. **refactor(templates): Remove inline JS from collection detail** - - `templates/collections.templ` - - `internal/router/frontend.go` - - `web/src/collections.ts` - -5. **feat(frontend): Add library filter toggle UI** - - `templates/collections.templ` - - `web/src/collections.ts` - -6. **docs(api): Document library_id search parameter** - - `docs/developer/api/search.md` - - `bruno/collections/search-with-library-filter.yml` - ---- - -## Step 1: Add User-Scoped Broadcasting (WebSocket Fix) - -### File: `internal/sync/websocket.go` - -**Add after line 94** (after existing `Broadcast` method): - -```go -// BroadcastToUser sends a message to all connections for a specific user -func (m *ConnectionManager) BroadcastToUser(userID string, msg BroadcastMessage) { - m.mu.RLock() - defer m.mu.RUnlock() - - for _, conn := range m.connections { - if conn.UserID == userID { - select { - case conn.Send <- msg: - default: - // Channel full, skip this connection - log.Printf("WebSocket: Channel full for %s, skipping broadcast", conn.DeviceName) - } - } - } -} -``` - -**Why**: Current `Broadcast()` sends to ALL users (security issue). User-scoped broadcasts ensure collection updates only go to that user's devices. - ---- - -## Step 2: Update Collections Handler to Use User-Scoped Broadcasts - -### File: `internal/handlers/collections.go` - -**Find all instances of** `h.connManager.Broadcast` **and replace with user-scoped**: - -**Line 333** (AddBooks): -```go -if addedCount > 0 && h.connManager != nil { - h.connManager.BroadcastToUser(userUUID.String(), wsync.BroadcastMessage{ - Type: "collection_updated", - Timestamp: time.Now().Format(time.RFC3339), - Data: map[string]interface{}{ - "collection_id": collectionID.String(), - "action": "books_added", - "book_ids": addedBookIDs, - "count": addedCount, - }, - }) -} -``` - -**Line 401** (RemoveBook): -```go -if h.connManager != nil { - h.connManager.BroadcastToUser(userUUID.String(), wsync.BroadcastMessage{ - Type: "collection_updated", - Timestamp: time.Now().Format(time.RFC3339), - Data: map[string]interface{}{ - "collection_id": collectionID.String(), - "action": "book_removed", - "book_id": bookID.String(), - }, - }) -} -``` - -**Line 831** (BulkRemoveBooks): -```go -if removedCount > 0 && h.connManager != nil { - h.connManager.BroadcastToUser(userUUID.String(), wsync.BroadcastMessage{ - Type: "collection_updated", - Timestamp: time.Now().Format(time.RFC3339), - Data: map[string]interface{}{ - "collection_id": collectionID.String(), - "action": "books_bulk_removed", - "book_ids": removedBookIDs, - "count": removedCount, - }, - }) -} -``` - -**Why**: Ensures collection updates only broadcast to the user who made the change, not all connected users. - ---- - -## Step 3: Add library_id Filter to SQL Query - -### File: `internal/database/queries.sql` - -**Find the `SearchMediaItems` query** (around line 393): - -**Current:** -```sql --- name: SearchMediaItems :many -SELECT mi.*, l.name as library_name, lt.name as library_type_name -FROM media_items mi -JOIN libraries l ON mi.library_id = l.id -JOIN library_types lt ON l.library_type_id = lt.id -LEFT JOIN library_visibility lv ON l.id = lv.library_id AND lv.user_id = sqlc.narg('user_id') -WHERE COALESCE(lv.is_visible, true) = true - AND ( - mi.title ILIKE sqlc.narg('search_pattern') OR - mi.author ILIKE sqlc.narg('search_pattern') OR - mi.series ILIKE sqlc.narg('search_pattern') OR - sqlc.narg('search_pattern') = ANY(mi.tags_search) OR - sqlc.narg('search_pattern') = ANY(mi.contributors_search) - ) -ORDER BY ... -``` - -**Add library_id parameter and filter**: - -```sql --- name: SearchMediaItems :many -SELECT mi.*, l.name as library_name, lt.name as library_type_name -FROM media_items mi -JOIN libraries l ON mi.library_id = l.id -JOIN library_types lt ON l.library_type_id = lt.id -LEFT JOIN library_visibility lv ON l.id = lv.library_id AND lv.user_id = sqlc.narg('user_id') -WHERE COALESCE(lv.is_visible, true) = true - AND ($5::uuid IS NULL OR mi.library_id = $5::uuid) -- library_id filter - AND ( - mi.title ILIKE sqlc.narg('search_pattern') OR - mi.author ILIKE sqlc.narg('search_pattern') OR - mi.series ILIKE sqlc.narg('search_pattern') OR - sqlc.narg('search_pattern') = ANY(mi.tags_search) OR - sqlc.narg('search_pattern') = ANY(mi.contributors_search) - ) -ORDER BY ... -``` - -**Also update `SearchMediaItemsFuzzy`** (around line 418) with the same filter: -```sql -WHERE COALESCE(lv.is_visible, true) = true - AND ($5::uuid IS NULL OR mi.library_id = $5::uuid) -- library_id filter - AND ( - word_similarity(sqlc.narg('search_query'), mi.title) > 0.3 OR - ... - ) -``` - -**Regenerate database code:** -```bash -cd internal/database -sqlc generate -``` - -**Verify** `internal/database/queries.sql.go` now has: -```go -type SearchMediaItemsParams struct { - UserID pgtype.UUID `db:"user_id" json:"user_id"` - SearchPattern pgtype.Text `db:"search_pattern" json:"search_pattern"` - Offset pgtype.Int4 `db:"offset" json:"offset"` - Limit pgtype.Int4 `db:"limit" json:"limit"` - LibraryID pgtype.UUID `db:"library_id" json:"library_id"` // NEW -} -``` - ---- - -## Step 4: Update Search API to Accept library_id - -### File: `internal/handlers/media.go` - -**Find** (around line 1418-1472): - -**Current:** -```go -func (mh *MediaHandler) SearchMediaItems(c echo.Context) error { - query := c.QueryParam("q") - userID := c.Get("user_id").(string) - - if query == "" { - return c.JSON(http.StatusBadRequest, map[string]string{"error": "query parameter 'q' is required"}) - } - - userUUID, err := uuid.Parse(userID) - if err != nil { - return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid user id"}) - } - - limit := int32(50) - offset := int32(0) - - searchPattern := "%" + query + "%" - - partialResults, err := mh.db.SearchMediaItems(c.Request().Context(), database.SearchMediaItemsParams{ - SearchPattern: pgtype.Text{String: searchPattern, Valid: true}, - UserID: pgtype.UUID{Bytes: userUUID, Valid: true}, - Limit: pgtype.Int4{Int32: limit, Valid: true}, - Offset: pgtype.Int4{Int32: offset, Valid: true}, - }) - - if err != nil && err != pgx.ErrNoRows { - return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()}) - } - - if len(partialResults) > 0 { - return c.JSON(http.StatusOK, partialResults) - } - - fuzzyResults, err := mh.db.SearchMediaItemsFuzzy(c.Request().Context(), database.SearchMediaItemsFuzzyParams{ - SearchQuery: pgtype.Text{String: query, Valid: true}, - UserID: pgtype.UUID{Bytes: userUUID, Valid: true}, - Limit: pgtype.Int4{Int32: limit, Valid: true}, - Offset: pgtype.Int4{Int32: offset, Valid: true}, - }) - // ... rest of function -``` - -**Replace with:** -```go -func (mh *MediaHandler) SearchMediaItems(c echo.Context) error { - query := c.QueryParam("q") - userID := c.Get("user_id").(string) - libraryID := c.QueryParam("library_id") // Optional: filter by library - - if query == "" { - return c.JSON(http.StatusBadRequest, map[string]string{"error": "query parameter 'q' is required"}) - } - - userUUID, err := uuid.Parse(userID) - if err != nil { - return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid user id"}) - } - - // Validate library_id if provided - var libUUID pgtype.UUID - if libraryID != "" { - lib, err := uuid.Parse(libraryID) - if err != nil { - return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid library_id"}) - } - libUUID = pgtype.UUID{Bytes: lib, Valid: true} - } - - limit := int32(50) - offset := int32(0) - - searchPattern := "%" + query + "%" - - // Build params - conditionally add library_id filter - partialParams := database.SearchMediaItemsParams{ - SearchPattern: pgtype.Text{String: searchPattern, Valid: true}, - UserID: pgtype.UUID{Bytes: userUUID, Valid: true}, - Limit: pgtype.Int4{Int32: limit, Valid: true}, - Offset: pgtype.Int4{Int32: offset, Valid: true}, - LibraryID: libUUID, // May be invalid (empty) - } - - partialResults, err := mh.db.SearchMediaItems(c.Request().Context(), partialParams) - - if err != nil && err != pgx.ErrNoRows { - return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()}) - } - - if len(partialResults) > 0 { - return c.JSON(http.StatusOK, partialResults) - } - - // Fuzzy search also gets library_id filter - fuzzyParams := database.SearchMediaItemsFuzzyParams{ - SearchQuery: pgtype.Text{String: query, Valid: true}, - UserID: pgtype.UUID{Bytes: userUUID, Valid: true}, - Limit: pgtype.Int4{Int32: limit, Valid: true}, - Offset: pgtype.Int4{Int32: offset, Valid: true}, - LibraryID: libUUID, // May be invalid (empty) - } - - fuzzyResults, err := mh.db.SearchMediaItemsFuzzy(c.Request().Context(), fuzzyParams) - - if err != nil && err != pgx.ErrNoRows { - return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()}) - } - - if len(fuzzyResults) == 0 { - return c.JSON(http.StatusNotFound, map[string]interface{}{ - "error": "no results found", - "query": query, - "results": []interface{}{}, - }) - } - - return c.JSON(http.StatusOK, fuzzyResults) -} -``` - -**Why**: Both partial and fuzzy searches respect library filter for consistent UX. - ---- - -## Step 5: Remove Inline JS from Template - -### File: `templates/collections.templ` - -**Find the inline script block** (lines 255-520) and **delete it**. - -**Add after the `` tag** (after line 116): -```go - - - - - -``` - -**Also update the CollectionDetail function signature** (around line 105): - -**Current:** -```go -func CollectionDetail(user User, collection CollectionData, books []handlers.BookInfo) templ.Component { -``` - -**Replace with:** -```go -func CollectionDetail(user User, collection CollectionData, books []handlers.BookInfo, libraryID string) templ.Component { -``` - ---- - -## Step 6: Add Toggle UI to Add Books Modal - -### File: `templates/collections.templ` - -**Find the Add Books modal content** (around line 228): - -**Current:** -```go -