Add piece detail view with local favorites#10
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5025b0dbf0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <Route path="/search" element={<Search />} /> | ||
| <Route path="/artists" element={<ArtistList />} /> | ||
| <Route path="/artists/:artistName" element={<ArtistDetail />} /> | ||
| <Route path="/pieces/:photoId" element={<PieceDetail />} /> |
There was a problem hiding this comment.
This client-only route is not added to internal/api/dashboard.go:isDashboardRoute, which is the server-side allowlist used to fall back to index.html for BrowserRouter paths. In the embedded dashboard, clicking from the gallery works after the SPA is loaded, but reloading/bookmarking or opening /pieces/:photoId in a new tab gets a 404 because the server doesn't recognize the path.
Useful? React with 👍 / 👎.
| return db.DB.Model(&DownloadedPhoto{}). | ||
| Where("id = ?", id). | ||
| Update("favorite", favorite).Error |
There was a problem hiding this comment.
When an adopted database already has one of the supported legacy favorite columns such as favorites or is_favorite, AutoMigrate adds the new favorite column but galleryFavoriteColumn() can still select the existing legacy column for catalog filters/facets while this setter always writes only favorite. In that environment toggling a piece succeeds and detail reads photo.Favorite, but the gallery favorite facet/filter continues to use the old column, so the toggle does not show up there; prefer the new column deterministically or migrate/sync the legacy values before writing only favorite.
Useful? React with 👍 / 👎.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| func galleryCategoryIDFromSourcePage(sourcePage string) string { | ||
| if sourcePage == "" { | ||
| return "unknown" | ||
| } | ||
| parsed, err := url.Parse(sourcePage) | ||
| if err != nil { | ||
| return "unknown" | ||
| } | ||
|
|
||
| parts := strings.Split(strings.Trim(parsed.EscapedPath(), "/"), "/") | ||
| for i := 0; i < len(parts)-1; i++ { | ||
| if strings.EqualFold(parts[i], "category") && parts[i+1] != "" { | ||
| return parts[i+1] | ||
| } | ||
| } | ||
|
|
||
| query := parsed.Query() | ||
| for _, key := range []string{"category", "category_id", "cat"} { | ||
| if value := strings.TrimSpace(query.Get(key)); value != "" { | ||
| return value | ||
| } | ||
| } | ||
|
|
||
| return "unknown" | ||
| } |
There was a problem hiding this comment.
categoryIDFromSourcePage in database.go
galleryCategoryIDFromSourcePage added here is byte-for-byte identical to the unexported categoryIDFromSourcePage at internal/database/database.go:597. Both functions parse the same path/query pattern and return the same values. The two copies can silently drift — for example, a future tweak to query-parameter keys ("cat", "category_id") would need to be applied in both packages, and a miss would cause the facet filter and the detail-page category display to disagree.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/api/gallery.go
Line: 280-304
Comment:
**Duplicate of `categoryIDFromSourcePage` in `database.go`**
`galleryCategoryIDFromSourcePage` added here is byte-for-byte identical to the unexported `categoryIDFromSourcePage` at `internal/database/database.go:597`. Both functions parse the same path/query pattern and return the same values. The two copies can silently drift — for example, a future tweak to query-parameter keys (`"cat"`, `"category_id"`) would need to be applied in both packages, and a miss would cause the facet filter and the detail-page category display to disagree.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
kossoy
deleted the
feat/folio-2-7-piece-detail-image-first--provenance--fa
branch
1 participant
Refs #7
What changed
/pieces/:photoIdimage-first detail route backed by the existing photo detail API.downloaded_photos.favoritethrough the existing favorite endpoints./pieces/:photoIdnow falls back to the embedded dashboard, and gallery favorite facets/filters now prefer the canonicalfavoritecolumn that the toggle writes.Verification
Maestro-Backend: codex (0-end)
Greptile Summary
Adds an image-first
/pieces/:photoIddetail route, migrates favorites from PhotoPrism to localdownloaded_photos.favorite, and surfaces provenance fields (source, provider, category) on photo detail responses. The gallery modal is replaced with navigation to the new route.DownloadedPhoto.Favoriteis added as a first-class GORM column withAutoMigrate;SetPhotoFavoriteand the refactored favorite handlers write and read it directly, removing the PhotoPrism dependency entirely.galleryFavoriteColumnpriority ordering is fixed to prefer the canonical\"favorite\"column over legacy aliases, and the test workaround that manuallyALTER TABLEd the schema is removed.galleryCategoryIDFromSourcePageadded togallery.gois byte-for-byte identical to the pre-existingcategoryIDFromSourcePageindatabase.go— the only code-quality concern in this PR.Confidence Score: 4/5
Safe to merge; all tests pass, the PhotoPrism dependency is cleanly removed, and the new route and DB migration are correctly wired together.
The change is well-scoped, well-tested (full add/get/remove favorite cycle, provenance field assertions, dashboard-route coverage), and passes the product verifier. The one concern is the duplicated category-ID parser introduced in gallery.go — it's harmless today but can silently diverge from the database-layer copy in future edits.
internal/api/gallery.go — galleryCategoryIDFromSourcePage duplicates categoryIDFromSourcePage in database.go.
Important Files Changed
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "Fix piece route fallback and favorite co..." | Re-trigger Greptile