[frontend] change task URLs from /tasks/:id to /task/:slug/:number#38
[frontend] change task URLs from /tasks/:id to /task/:slug/:number#38capcom6 wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughTask retrieval now supports project slug and sequential number identifiers. Frontend routes, navigation, task detail loading, editing, mutations, attachment handling, legacy redirects, and recent-project parsing use the canonical task URL. ChangesCanonical task route migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant TaskAPI
participant TaskService
User->>Frontend: Open /task/{slug}/{number}
Frontend->>TaskAPI: GET /tasks/{slug}/{number}
TaskAPI->>TaskService: GetByProjectAndNumber(slug, number)
TaskService-->>TaskAPI: Task details
TaskAPI-->>Frontend: TaskDetailsResponse
Frontend-->>User: Render task detail
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
🤖 Pull request artifacts
|
|
@coderabbitai full review |
✅ Action performedFull review finished. |
|
@coderabbitai full review |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 35 minutes. |
|
@coderabbitai full review |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 58 minutes. |
38ca146 to
2ce88f3
Compare
2ce88f3 to
63d96f4
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 57 minutes. |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
frontend/src/lib/pages/task-new.svelte (1)
61-61: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider URL-encoding the project slug in the navigate call.
parseProjectSlugFromPathinrecent-projects.svelte.tsusesdecodeURIComponenton the slug, implying slugs may contain encoded characters. UsingencodeURIComponenthere ensures round-trip consistency.♻️ Proposed refactor
- navigate(`/task/${task.project_slug}/${task.number}`); + navigate(`/task/${encodeURIComponent(task.project_slug)}/${task.number}`);🤖 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 `@frontend/src/lib/pages/task-new.svelte` at line 61, URL-encode the project slug before constructing the task route in the navigation call. Update the navigate call in the task creation flow to wrap task.project_slug with encodeURIComponent, preserving compatibility with parseProjectSlugFromPath’s decodeURIComponent handling.
🤖 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 `@internal/server/tasks/handler.go`:
- Around line 189-198: Remove the unreachable empty-slug validation from
getByProjectAndNumber, and after parsing the task number, reject values less
than 1 with a fiber.StatusBadRequest error matching the existing invalid-number
handling.
In `@requests.http`:
- Around line 251-255: Update the “Get task by project slug and number” example
request to use the registered two-segment route `/tasks/{slug}/{number}`; remove
the extra `by-project` segment so it targets `/tasks/my-new-project/1`,
consistent with the handler route and Swagger documentation.
---
Nitpick comments:
In `@frontend/src/lib/pages/task-new.svelte`:
- Line 61: URL-encode the project slug before constructing the task route in the
navigation call. Update the navigate call in the task creation flow to wrap
task.project_slug with encodeURIComponent, preserving compatibility with
parseProjectSlugFromPath’s decodeURIComponent handling.
🪄 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
Run ID: 945a3c63-575b-49b1-9518-0a093d605d2f
📒 Files selected for processing (12)
frontend/src/App.sveltefrontend/src/lib/api/tasks.tsfrontend/src/lib/pages/dashboard-personal.sveltefrontend/src/lib/pages/dashboard-tasks.sveltefrontend/src/lib/pages/project-tasks.sveltefrontend/src/lib/pages/task-detail.sveltefrontend/src/lib/pages/task-edit.sveltefrontend/src/lib/pages/task-new.sveltefrontend/src/lib/stores/recent-projects.svelte.tsinternal/server/docs/docs.gointernal/server/tasks/handler.gorequests.http
3bfe17d to
2bbd06e
Compare
11d870a to
408e38c
Compare
408e38c to
f02a191
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@frontend/src/lib/pages/task-redirect.svelte`:
- Around line 9-23: Update the $effect in task-redirect.svelte to track whether
the effect has been invalidated or the component unmounted, and return a cleanup
function that marks it inactive. Guard both the getTask(id) success handler
before calling navigate and the error handler before assigning error, so stale
requests cannot redirect or update state.
🪄 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
Run ID: 50939855-7111-44fc-84dd-55a5fcb21f02
📒 Files selected for processing (14)
frontend/src/App.sveltefrontend/src/lib/api/tasks.tsfrontend/src/lib/components/TaskTable.sveltefrontend/src/lib/pages/dashboard-personal.sveltefrontend/src/lib/pages/dashboard-tasks.sveltefrontend/src/lib/pages/project-tasks.sveltefrontend/src/lib/pages/task-detail.sveltefrontend/src/lib/pages/task-edit.sveltefrontend/src/lib/pages/task-new.sveltefrontend/src/lib/pages/task-redirect.sveltefrontend/src/lib/stores/recent-projects.svelte.tsinternal/server/docs/docs.gointernal/server/tasks/handler.gorequests.http
🚧 Files skipped from review as they are similar to previous changes (11)
- frontend/src/lib/pages/project-tasks.svelte
- requests.http
- frontend/src/lib/pages/task-new.svelte
- frontend/src/lib/pages/dashboard-tasks.svelte
- frontend/src/lib/pages/dashboard-personal.svelte
- frontend/src/lib/pages/task-edit.svelte
- frontend/src/App.svelte
- internal/server/tasks/handler.go
- frontend/src/lib/stores/recent-projects.svelte.ts
- internal/server/docs/docs.go
- frontend/src/lib/pages/task-detail.svelte
2c9026c to
126053e
Compare
- Add backend endpoint GET /api/v1/tasks/by-project/:slug/:number - Update frontend routes: /task/:slug/:number and /task/:slug/:number/edit - Update all navigation calls to use project_slug + number - Replace onMount with for reactive data fetching on param change - Fix infinite loop in App.svelte by removing sync call from effect body - Use plain variable instead of for assigning mutex - Update parseProjectSlugFromPath to recognize new URL pattern - Add example request to requests.http
e971111 to
c122597
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@frontend/src/lib/api/tasks.ts`:
- Around line 50-52: Update getTaskByProjectAndNumber to request the backend’s
by-project route by inserting the by-project path segment before slug, while
preserving the existing GET method and TaskDetails response handling.
🪄 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: 42c868c9-944f-45f3-859d-6d4f8a1ef4e8
📒 Files selected for processing (14)
frontend/src/App.sveltefrontend/src/lib/api/tasks.tsfrontend/src/lib/components/TaskTable.sveltefrontend/src/lib/pages/dashboard-personal.sveltefrontend/src/lib/pages/dashboard-tasks.sveltefrontend/src/lib/pages/project-tasks.sveltefrontend/src/lib/pages/task-detail.sveltefrontend/src/lib/pages/task-edit.sveltefrontend/src/lib/pages/task-new.sveltefrontend/src/lib/pages/task-redirect.sveltefrontend/src/lib/stores/recent-projects.svelte.tsinternal/server/docs/docs.gointernal/server/tasks/handler.gorequests.http
🚧 Files skipped from review as they are similar to previous changes (12)
- frontend/src/lib/pages/dashboard-tasks.svelte
- frontend/src/lib/components/TaskTable.svelte
- frontend/src/lib/pages/task-new.svelte
- internal/server/docs/docs.go
- frontend/src/lib/pages/project-tasks.svelte
- requests.http
- frontend/src/lib/pages/task-redirect.svelte
- frontend/src/lib/pages/task-edit.svelte
- frontend/src/lib/stores/recent-projects.svelte.ts
- internal/server/tasks/handler.go
- frontend/src/App.svelte
- frontend/src/lib/pages/task-detail.svelte
| export function getTaskByProjectAndNumber(slug: string, number: number): Promise<TaskDetails> { | ||
| return apiRequest<TaskDetails>('GET', `/tasks/${slug}/${number}`) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the backend’s by-project route.
This currently requests /api/v1/tasks/{slug}/{number}, but the new endpoint is /api/v1/tasks/by-project/{slug}/{number}. Canonical task detail loads will therefore return 404.
Proposed fix
export function getTaskByProjectAndNumber(slug: string, number: number): Promise<TaskDetails> {
- return apiRequest<TaskDetails>('GET', `/tasks/${slug}/${number}`)
+ return apiRequest<TaskDetails>(
+ 'GET',
+ `/tasks/by-project/${encodeURIComponent(slug)}/${number}`,
+ )
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function getTaskByProjectAndNumber(slug: string, number: number): Promise<TaskDetails> { | |
| return apiRequest<TaskDetails>('GET', `/tasks/${slug}/${number}`) | |
| } | |
| export function getTaskByProjectAndNumber(slug: string, number: number): Promise<TaskDetails> { | |
| return apiRequest<TaskDetails>( | |
| 'GET', | |
| `/tasks/by-project/${encodeURIComponent(slug)}/${number}`, | |
| ) | |
| } |
🤖 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 `@frontend/src/lib/api/tasks.ts` around lines 50 - 52, Update
getTaskByProjectAndNumber to request the backend’s by-project route by inserting
the by-project path segment before slug, while preserving the existing GET
method and TaskDetails response handling.
Summary by CodeRabbit
/tasks/{slug}/{number}), and updated the app to use canonical/task/{slug}/{number}and/task/{slug}/{number}/editroutes.