feat(favorites): server-persist cycle/module favorites with folders and ordering#305
Conversation
…nd ordering Cycle and module favorites lived only in localStorage (per-device, no folders, no ordering). They are now persisted server-side in a favorites tree that can be grouped into folders and reordered. Backend: extend user_favorites with is_folder + sort_order (parent_id already existed); FavoriteService + endpoints to list the tree, favorite a cycle/module, create folders, and move/rename/reorder/delete. Deleting a folder keeps the favorites inside it (they move to the top level). Frontend: a shared workspace-favorites hook + a sidebar favorites tree with create-folder, drag-to-reorder, drag-into/out-of folders, and remove; the cycle/module favorite toggles now write to the server. Removes the two localStorage favorite hooks. Closes Devlaner#205 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughWorkspace favorites now support server-backed cycle and module favorites, folders, nesting, ordering, authenticated CRUD APIs, and a hierarchical sidebar tree. Local-storage cycle/module favorite hooks are replaced with workspace-scoped API state and drag-and-drop controls. ChangesWorkspace Favorites
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Sidebar
participant useWorkspaceFavorites
participant workspaceFavoriteService
participant FavoritesAPI
User->>Sidebar: Drag or toggle a favorite
Sidebar->>useWorkspaceFavorites: Update favorite state
useWorkspaceFavorites->>workspaceFavoriteService: PATCH, POST, or DELETE favorite
workspaceFavoriteService->>FavoritesAPI: Send workspace favorite request
FavoritesAPI-->>workspaceFavoriteService: Return updated result
workspaceFavoriteService-->>useWorkspaceFavorites: Resolve mutation
useWorkspaceFavorites->>workspaceFavoriteService: Reload workspace tree
workspaceFavoriteService-->>Sidebar: Return favorites
Sidebar-->>User: Render updated tree
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/web/src/components/layout/WorkspaceFavoritesTree.tsx (1)
76-172: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffHoist
Rowout of the component body.
Rowis declared insideWorkspaceFavoritesTreeand rendered as a JSX element (<Row … />). React sees a new component type on every parent render, so the entire rows subtree unmounts/remounts each render, which wastes work and can interrupt an in-progress drag. Extract it to a module-level component (passingfavorites/expanded/drag state and callbacks as props) or render it as a plain function call.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/layout/WorkspaceFavoritesTree.tsx` around lines 76 - 172, Move the Row component definition out of WorkspaceFavoritesTree to module scope so its component identity remains stable across parent renders. Pass the required favorites state, expansion state, drag state, and callbacks through props, or invoke the row renderer as a plain function instead of JSX; preserve the existing folder expansion, deletion, navigation, and drag-and-drop behavior.apps/api/migrations/000011_favorites_folders_ordering.up.sql (1)
1-3: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider adding indexes for the new query patterns.
ListByUserAndWorkspacefilters by(user_id, workspace_id)andDeleteOwned's reparent query filters by(user_id, parent_id). The existing unique indexidx_user_fav_entityleads withentity_type, so it can't accelerate these lookups. For auser_favoritestable spanning all users, these are full table scans.📊 Suggested index additions
-- Extend user_favorites for folders and ordering. parent_id already exists. ALTER TABLE user_favorites ADD COLUMN IF NOT EXISTS is_folder BOOLEAN NOT NULL DEFAULT FALSE; ALTER TABLE user_favorites ADD COLUMN IF NOT EXISTS sort_order DOUBLE PRECISION NOT NULL DEFAULT 65535; + +CREATE INDEX IF NOT EXISTS idx_user_fav_user_workspace ON user_favorites (user_id, workspace_id); +CREATE INDEX IF NOT EXISTS idx_user_fav_user_parent ON user_favorites (user_id, parent_id);And in the down migration:
ALTER TABLE user_favorites DROP COLUMN IF EXISTS is_folder; ALTER TABLE user_favorites DROP COLUMN IF EXISTS sort_order; + +DROP INDEX IF EXISTS idx_user_fav_user_workspace; +DROP INDEX IF EXISTS idx_user_fav_user_parent;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/migrations/000011_favorites_folders_ordering.up.sql` around lines 1 - 3, Add indexes in the up migration for the `(user_id, workspace_id)` lookup used by `ListByUserAndWorkspace` and the `(user_id, parent_id)` reparent lookup used by `DeleteOwned`; add matching index drops in the down migration, using the project’s established migration naming conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/internal/handler/favorite_tree.go`:
- Around line 59-67: The CreateFavorite handler must reject empty or
whitespace-only names for both folders and entity favorites before calling the
service or parsing UUIDs. Use strings.TrimSpace on body.Name, return the
existing validation error response for blank names, and add the strings import;
preserve normal folder and favorite creation for non-empty names.
In `@apps/api/internal/service/favorite.go`:
- Around line 36-46: Update FavoriteService.workspace and AddEntity to preserve
infrastructure failures: capture and inspect errors from IsMember and GetByID,
propagate unexpected errors unchanged so they reach the 500 path, and map only
the established not-found or membership sentinel errors to ErrFavoriteForbidden.
Keep the existing forbidden behavior for genuine authorization or missing-entity
cases.
In `@apps/api/internal/store/user_favorite.go`:
- Around line 52-69: Update UserFavoriteStore.AddEntity to handle a concurrent
duplicate-key failure from Create: retry fetching the row by the same user_id,
entity_type, and entity_identifier keys, and return that existing row to
preserve idempotency; return the original error only when the retry cannot find
the conflicting row or encounters another database error.
---
Nitpick comments:
In `@apps/api/migrations/000011_favorites_folders_ordering.up.sql`:
- Around line 1-3: Add indexes in the up migration for the `(user_id,
workspace_id)` lookup used by `ListByUserAndWorkspace` and the `(user_id,
parent_id)` reparent lookup used by `DeleteOwned`; add matching index drops in
the down migration, using the project’s established migration naming
conventions.
In `@apps/web/src/components/layout/WorkspaceFavoritesTree.tsx`:
- Around line 76-172: Move the Row component definition out of
WorkspaceFavoritesTree to module scope so its component identity remains stable
across parent renders. Pass the required favorites state, expansion state, drag
state, and callbacks through props, or invoke the row renderer as a plain
function instead of JSX; preserve the existing folder expansion, deletion,
navigation, and drag-and-drop behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6339baac-be83-4d83-9b81-79af325eb9ee
📒 Files selected for processing (19)
apps/api/internal/handler/favorite.goapps/api/internal/handler/favorite_tree.goapps/api/internal/handler/favorite_tree_test.goapps/api/internal/model/user_favorite.goapps/api/internal/router/router.goapps/api/internal/service/favorite.goapps/api/internal/store/user_favorite.goapps/api/migrations/000011_favorites_folders_ordering.down.sqlapps/api/migrations/000011_favorites_folders_ordering.up.sqlapps/web/src/api/types.tsapps/web/src/components/layout/Sidebar.tsxapps/web/src/components/layout/WorkspaceFavoritesTree.tsxapps/web/src/contexts/FavoritesContext.tsxapps/web/src/hooks/useCycleFavorites.tsapps/web/src/hooks/useModuleFavorites.tsapps/web/src/hooks/useWorkspaceFavorites.tsapps/web/src/pages/CyclesPage.tsxapps/web/src/pages/ModulesPage.tsxapps/web/src/services/workspaceFavoriteService.ts
💤 Files with no reviewable changes (2)
- apps/web/src/hooks/useCycleFavorites.ts
- apps/web/src/hooks/useModuleFavorites.ts
… names Addresses review feedback: - AddEntity is now an atomic OnConflict insert + re-read, so a concurrent favorite of the same entity stays idempotent instead of failing on the unique index. - workspace() and AddEntity surface non-sentinel (infrastructure) errors as 500s instead of masking them as 404 forbidden. - Folder and entity favorite names must be non-empty (400 otherwise), so a blank label can't be persisted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@martian56 this one's green and all of CodeRabbit's notes are resolved. Good to merge when you get a chance? It moves cycle/module favorites off localStorage onto the server with folders + drag ordering - no data migration needed for existing localStorage favorites, they'll just re-star. Happy to walk through anything. |
Feature summary
Cycle and module favorites lived only in
localStorage— per-device, with no folders and no ordering. They are now persisted server-side in a favorites tree that can be grouped into folders and reordered.Linked issues / discussion
Closes #205
User-facing behavior
What changed
API (
apps/api/)000011:is_folder+sort_orderonuser_favorites(parent_idalready existed).model.UserFavorite: folder/ordering fields + cycle/module/folder entity-type constants.store.UserFavoriteStore: list-by-workspace, generic add/remove entity, create folder, owned get/update/delete (deleting a folder re-parents its children to the top level, so nothing is lost).service.FavoriteService: workspace-membership + project-access checks; list, favorite a cycle/module, create folder, rename/move/reorder, delete. A move validates the parent is one of the user's own folders.handler+ router:GET/POST /workspaces/:slug/favorites/,PATCH/DELETE /workspaces/:slug/favorites/:favId/. Existing project/view/page favorite endpoints are untouched.UI (
apps/web/)workspaceFavoriteService+ a shareduseWorkspaceFavoriteshook (state viaFavoritesContext).WorkspaceFavoritesTreein the sidebar: folders, ordering, create-folder, native drag-to-reorder / drag-into-folder, remove.CyclesPage/ModulesPagefavorite toggles now write to the server. The twolocalStoragefavorite hooks are removed.Database
Test plan
go vet ./...and fullgo test ./...green (testcontainers).TestFavorites_CycleFolderOrdering(favorite a cycle, create a folder, move it in + reorder, delete the folder and keep the favorite, then unfavorite) andTestFavorites_NonMemberForbidden.npm run typecheck,npm run lint,npm run buildgreen.Out of scope (follow-ups)
AI assistance
Claude Code— and AI-assisted commits include aCo-Authored-By:trailerChecklist
000011_*.up.sqlAND matching.down.sql--no-verifybypass on the commit (full suite ran on pre-commit)Summary by CodeRabbit