chore: remove obsolete collection planning documents
Remove completed implementation plans that have been superseded: - COLLECTION_LIBRARY_FILTERING_PLAN.md (library filtering feature completed) - IMPLEMENTATION_COLLECTION_FIX.md (collection detail fix implemented) These plans were for features that have already been implemented in recent commits. Keeping only current/future planning docs.
This commit is contained in:
@@ -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
|
||||
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user