Merged
Conversation
📝 WalkthroughWalkthroughThis PR removes ID-immutability validation from the global record update handler in main.go, adds a "My Stuff" navigation link to the authenticated user dropdown in the layout with adjusted styling, and introduces a new "My Stuff" page featuring a tabbed interface for browsing user-owned CPS and character records from PocketBase. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new "My Stuff" page for users to manage their created CPs and characters, updates the navigation layout, and modifies backend hooks. Feedback includes a security recommendation to use the pb.filter() helper to prevent potential injection, a suggestion to restore the validateIdImmutable hook in the backend to maintain record integrity, and a best-practice recommendation to move authentication guards to SvelteKit load functions to improve the user experience.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@website/src/routes/mystuff/`+page.svelte:
- Around line 29-33: Guard against stale responses by tracking each load with a
request id and only updating state if the response corresponds to the most
recent load: add a component-scoped counter (e.g., currentLoadId) and, inside
the function before calling pb.collection(...).getFullList, capture const
myLoadId = ++currentLoadId and const myMode = mode; after the await (and in the
catch), only assign items, clear items on error, or set isLoading = false if
myLoadId === currentLoadId and mode === myMode; otherwise ignore the result so a
slower previous response cannot overwrite items or isLoading for a different
tab. Ensure references to items, mode, and isLoading are guarded by that check.
- Around line 48-58: The tab buttons in +page.svelte use role="tab" but don't
expose selection to AT; update both button elements (the ones using class "tab
{mode === 'cps' ? 'tab-active' : ''}" and "tab {mode === 'characters' ?
'tab-active' : ''}") to include aria-selected attributes such that
aria-selected="true" when mode === 'cps' (for the My CPs button) or mode ===
'characters' (for the My Characters button) and aria-selected="false" otherwise;
keep the existing onclick handlers (onclick={() => mode = 'cps'} / onclick={()
=> mode = 'characters'}) and class logic unchanged so visual and programmatic
state remain synchronized.
- Line 82: The Edit anchor in +page.svelte uses a dynamic href
"/edit/{mode}/{item.id}" which points to non-existent routes (/edit/cps/[id] and
/edit/characters/[id]) and will 404; either implement route handlers for those
paths (create +page.svelte or route files for /edit/cps/[id] and
/edit/characters/[id] to accept mode and id and render the edit forms) or change
the anchor href in +page.svelte (the <a href="/edit/{mode}/{item.id}"> link) to
an existing route pattern used by the app (e.g., the current item edit route) so
clicks resolve to a real page.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 16217073-6b77-4e6e-b06d-4c5eaebcb6bd
📒 Files selected for processing (3)
main.gowebsite/src/routes/+layout.sveltewebsite/src/routes/mystuff/+page.svelte
💤 Files with no reviewable changes (1)
- main.go
Comment on lines
+29
to
+33
| items = await pb.collection(collection).getFullList(options); | ||
| } catch (err) { | ||
| console.error("Fetch error:", err); | ||
| } finally { | ||
| isLoading = false; |
There was a problem hiding this comment.
Guard against stale responses when switching tabs.
Line 39 can start overlapping requests; a slower previous response can still assign items after mode changed, rendering CPs under the Characters tab or vice versa. Errors also keep the previous items visible.
🐛 Proposed fix to ignore stale loads
let mode = $state<ViewMode>('cps');
let items = $state<any[]>([]);
let isLoading = $state(true);
+ let fetchVersion = 0;
async function fetchMyData(currentMode: ViewMode) {
+ const version = ++fetchVersion;
+
// 安全守卫：如果没有登录，直接跳走
if (!pb.authStore.isValid || !pb.authStore.record) {
- goto('/login');
+ isLoading = false;
+ await goto('/login');
return;
}
isLoading = true;
const userId = pb.authStore.record.id;
@@
- items = await pb.collection(collection).getFullList(options);
+ const nextItems = await pb.collection(collection).getFullList(options);
+ if (version === fetchVersion) {
+ items = nextItems;
+ }
} catch (err) {
console.error("Fetch error:", err);
+ if (version === fetchVersion) {
+ items = [];
+ }
} finally {
- isLoading = false;
+ if (version === fetchVersion) {
+ isLoading = false;
+ }
}
}Also applies to: 37-40
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@website/src/routes/mystuff/`+page.svelte around lines 29 - 33, Guard against
stale responses by tracking each load with a request id and only updating state
if the response corresponds to the most recent load: add a component-scoped
counter (e.g., currentLoadId) and, inside the function before calling
pb.collection(...).getFullList, capture const myLoadId = ++currentLoadId and
const myMode = mode; after the await (and in the catch), only assign items,
clear items on error, or set isLoading = false if myLoadId === currentLoadId and
mode === myMode; otherwise ignore the result so a slower previous response
cannot overwrite items or isLoading for a different tab. Ensure references to
items, mode, and isLoading are guarded by that check.
Comment on lines
+48
to
+58
| <div role="tablist" class="tabs tabs-boxed mt-6 w-fit"> | ||
| <button | ||
| role="tab" | ||
| class="tab {mode === 'cps' ? 'tab-active' : ''}" | ||
| onclick={() => mode = 'cps'} | ||
| >My CPs</button> | ||
| <button | ||
| role="tab" | ||
| class="tab {mode === 'characters' ? 'tab-active' : ''}" | ||
| onclick={() => mode = 'characters'} | ||
| >My Characters</button> |
There was a problem hiding this comment.
Expose the active tab state to assistive technology.
These buttons declare role="tab", but the selected state is only represented by CSS. Add aria-selected so screen readers can identify the active tab.
♿ Proposed tab accessibility fix
<button
+ type="button"
role="tab"
+ aria-selected={mode === 'cps'}
class="tab {mode === 'cps' ? 'tab-active' : ''}"
onclick={() => mode = 'cps'}
>My CPs</button>
<button
+ type="button"
role="tab"
+ aria-selected={mode === 'characters'}
class="tab {mode === 'characters' ? 'tab-active' : ''}"
onclick={() => mode = 'characters'}
>My Characters</button>📝 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.
Suggested change
| <div role="tablist" class="tabs tabs-boxed mt-6 w-fit"> | |
| <button | |
| role="tab" | |
| class="tab {mode === 'cps' ? 'tab-active' : ''}" | |
| onclick={() => mode = 'cps'} | |
| >My CPs</button> | |
| <button | |
| role="tab" | |
| class="tab {mode === 'characters' ? 'tab-active' : ''}" | |
| onclick={() => mode = 'characters'} | |
| >My Characters</button> | |
| <div role="tablist" class="tabs tabs-boxed mt-6 w-fit"> | |
| <button | |
| type="button" | |
| role="tab" | |
| aria-selected={mode === 'cps'} | |
| class="tab {mode === 'cps' ? 'tab-active' : ''}" | |
| onclick={() => mode = 'cps'} | |
| >My CPs</button> | |
| <button | |
| type="button" | |
| role="tab" | |
| aria-selected={mode === 'characters'} | |
| class="tab {mode === 'characters' ? 'tab-active' : ''}" | |
| onclick={() => mode = 'characters'} | |
| >My Characters</button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@website/src/routes/mystuff/`+page.svelte around lines 48 - 58, The tab
buttons in +page.svelte use role="tab" but don't expose selection to AT; update
both button elements (the ones using class "tab {mode === 'cps' ? 'tab-active' :
''}" and "tab {mode === 'characters' ? 'tab-active' : ''}") to include
aria-selected attributes such that aria-selected="true" when mode === 'cps' (for
the My CPs button) or mode === 'characters' (for the My Characters button) and
aria-selected="false" otherwise; keep the existing onclick handlers (onclick={()
=> mode = 'cps'} / onclick={() => mode = 'characters'}) and class logic
unchanged so visual and programmatic state remain synchronized.
| <div class="card-body p-5"> | ||
| <h2 class="card-title">{item.name}</h2> | ||
| <div class="card-actions justify-end mt-4"> | ||
| <a href="/edit/{mode}/{item.id}" class="btn btn-sm btn-ghost">Edit</a> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify that SvelteKit route files exist for the edit URLs generated by My Stuff.
# Expected: route files/directories matching /edit/cps/[id] and /edit/characters/[id], or update the hrefs to the actual edit route shape.
printf 'Generated edit hrefs in Svelte files:\n'
rg -nP 'href="/edit/\{mode\}/\{item\.id\}"' -C2 --glob '*.svelte' || true
printf '\nRelevant SvelteKit route files:\n'
fd -p '\+page\.svelte$' website/src/routes \
| sort \
| rg '/edit/|/cps/|/characters/' || trueRepository: pdnode-team/cp
Length of output: 790
🏁 Script executed:
#!/bin/bash
# Check if /edit directory exists and list all route files
printf 'Checking for /edit directory:\n'
ls -la website/src/routes/ | grep edit || echo "No /edit directory found"
printf '\nAll +page.svelte files in website/src/routes:\n'
find website/src/routes -name '+page.svelte' -type f | sort
printf '\nChecking for any edit route patterns:\n'
find website/src/routes -type d -name edit || echo "No /edit directory found"Repository: pdnode-team/cp
Length of output: 555
🏁 Script executed:
#!/bin/bash
# Search for any edit route references or edit functionality
printf 'Searching for edit-related functionality:\n'
rg -i 'edit.*route|route.*edit' website/src/routes --type svelte -l || echo "No edit route references found"
printf '\nSearching for +layout or server files that might handle edit:\n'
find website/src/routes/edit website/src/routes/cps website/src/routes/characters -name '+layout*' -o -name '+server*' 2>/dev/null || echo "No edit layout/server files found"
printf '\nChecking mystuff page for edit functionality context:\n'
grep -n -i 'edit' website/src/routes/mystuff/+page.svelteRepository: pdnode-team/cp
Length of output: 432
Update the Edit link destination or create the edit routes.
The edit routes at /edit/cps/[id] and /edit/characters/[id] do not exist in the codebase. The Edit button on line 82 will generate 404 errors. Either create these routes to handle edit functionality or change the href to an existing route.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@website/src/routes/mystuff/`+page.svelte at line 82, The Edit anchor in
+page.svelte uses a dynamic href "/edit/{mode}/{item.id}" which points to
non-existent routes (/edit/cps/[id] and /edit/characters/[id]) and will 404;
either implement route handlers for those paths (create +page.svelte or route
files for /edit/cps/[id] and /edit/characters/[id] to accept mode and id and
render the edit forms) or change the anchor href in +page.svelte (the <a
href="/edit/{mode}/{item.id}"> link) to an existing route pattern used by the
app (e.g., the current item edit route) so clicks resolve to a real page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Style