Feat/adventurer map#117
Conversation
|
Warning Review limit reached
More reviews will be available in 42 minutes and 59 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds an interactive ChangesInteractive Map Dashboard
Sequence Diagram(s)sequenceDiagram
participant Browser
participant DashboardMapPage as page.tsx
participant getMapDashboardData
participant MapDashboardClient
participant useMapMovement
participant MapLocationDialog
participant LeaderboardAPI as GET /api/portal/leaderboard/points
participant Payload
Browser->>DashboardMapPage: GET /dashboard/map
DashboardMapPage->>Payload: getCurrentUser()
alt unauthenticated
DashboardMapPage-->>Browser: redirect /join
else unverified
DashboardMapPage-->>Browser: VerifyAccountNotice
end
DashboardMapPage->>getMapDashboardData: fetch profile/posts/modules/wiki/events/points/streak
getMapDashboardData-->>DashboardMapPage: MapDashboardData
DashboardMapPage-->>Browser: render MapDashboardClient
Browser->>MapDashboardClient: click map location button
MapDashboardClient->>useMapMovement: travelTo(nodeID)
useMapMovement-->>MapDashboardClient: onArrive → setActiveLocation
MapDashboardClient-->>Browser: render MapLocationDialog (LunkerLake)
MapLocationDialog->>LeaderboardAPI: fetch /api/portal/leaderboard/points
LeaderboardAPI->>Payload: auth + query pointEvents + profiles
Payload-->>LeaderboardAPI: events[], profiles[]
LeaderboardAPI-->>MapLocationDialog: { entries, generatedAt }
MapLocationDialog-->>Browser: render ranked leaderboard
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/app/(frontend)/api/portal/leaderboard/points/route.ts (1)
30-41: Remove redundantlimitparameter when usingpagination: false.In Payload CMS v3, setting
pagination: falsereturns all documents regardless of thelimitparameter. Thelimit: 10000is dead code and could mislead future maintainers into thinking there's an upper bound on results.♻️ Proposed fix
const pointEvents = await payload.find({ collection: 'pointEvents', depth: 0, - limit: 10000, overrideAccess: true, pagination: false, where: { status: { equals: 'valid', }, }, })const profiles = await payload.find({ collection: 'profiles', depth: 1, - limit: 10000, overrideAccess: true, pagination: false, where: {Also applies to: 53-58
🤖 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 `@src/app/`(frontend)/api/portal/leaderboard/points/route.ts around lines 30 - 41, Remove the redundant limit parameter from the payload.find() calls where pagination is set to false. In Payload CMS v3, when pagination is false, all documents are returned regardless of the limit value, making the limit parameter unnecessary and potentially misleading. Search for all instances of payload.find() calls in the leaderboard points route that have both pagination: false and a limit parameter (this occurs in the main query around the pointEvents collection and also in the additional location mentioned at lines 53-58), and delete the limit property from each of these calls.
🤖 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 `@docs/feature-spec-index.md`:
- Line 41: In the feature-spec-index.md file, locate the "Interactive map
dashboard" row entry that is currently marked with status "Planned / future" and
move it from its current location to the "Implemented First Slices" section.
Update the status column for this entry from "Planned / future" to "First slice
implemented" to accurately reflect that this PR delivers the first implemented
slice of the feature.
In `@public/assets/map/sprites/characters/ranger.json`:
- Line 18: The source path in the ranger.json manifest file is missing the
generated-poses directory and does not follow the consistent pattern used in
other character manifests. Update the source field value from
assets-src/art-review/ranger-pose-sheet-v1-chroma.png to
assets-src/art-review/generated-poses/ranger-pose-sheet-v1-chroma.png to match
the standard pattern where the generated-poses directory is included in the path
for all character pose sheet sources.
In `@src/app/`(frontend)/dashboard/map/MapDashboardClient.tsx:
- Around line 63-70: The travelToLocation function currently allows travel to
any location that passes the selectedRole check, but it does not respect the
disabled state of locations. Add a check in the travelToLocation function to
return early if location.disabled is true, preventing users from traveling to or
interacting with disabled destinations. Place this check after or alongside the
existing selectedRole validation to ensure disabled locations are not accessible
regardless of the styling applied to them.
In `@src/app/`(frontend)/dashboard/map/mapData.ts:
- Line 82: The leaderboard field is being hard-coded as an empty array on line
82, which causes it to display a false "no standings" state. Instead of using an
empty array, extract the actual leaderboard data from the server payload
response and assign it to the leaderboard field. This will ensure that when data
is fetched from the server, the actual standings are properly displayed rather
than showing an empty state.
In `@tests/e2e/app.spec.ts`:
- Around line 297-323: The test function creates a BrowserContext with
browser.newContext() but if any expect() assertion fails before reaching
context.close(), the context leaks because the cleanup code is never executed.
Wrap all the test operations between creating the context and calling
context.close() in a try block, then move the context.close() call into a
finally block to guarantee the context is always closed regardless of whether
assertions pass or fail.
---
Nitpick comments:
In `@src/app/`(frontend)/api/portal/leaderboard/points/route.ts:
- Around line 30-41: Remove the redundant limit parameter from the
payload.find() calls where pagination is set to false. In Payload CMS v3, when
pagination is false, all documents are returned regardless of the limit value,
making the limit parameter unnecessary and potentially misleading. Search for
all instances of payload.find() calls in the leaderboard points route that have
both pagination: false and a limit parameter (this occurs in the main query
around the pointEvents collection and also in the additional location mentioned
at lines 53-58), and delete the limit property from each of these calls.
🪄 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: 27f0c2f3-daeb-4fe9-90ee-6a550ffd2cae
⛔ Files ignored due to path filters (32)
public/assets/map/characters/alchemist.svgis excluded by!**/*.svgpublic/assets/map/characters/archer.svgis excluded by!**/*.svgpublic/assets/map/characters/cleric.svgis excluded by!**/*.svgpublic/assets/map/characters/druid.svgis excluded by!**/*.svgpublic/assets/map/characters/dwarf.svgis excluded by!**/*.svgpublic/assets/map/characters/healer.svgis excluded by!**/*.svgpublic/assets/map/characters/hunter.svgis excluded by!**/*.svgpublic/assets/map/characters/monk.svgis excluded by!**/*.svgpublic/assets/map/characters/necromancer.svgis excluded by!**/*.svgpublic/assets/map/characters/paladin.svgis excluded by!**/*.svgpublic/assets/map/characters/ranger.svgis excluded by!**/*.svgpublic/assets/map/characters/rogue.svgis excluded by!**/*.svgpublic/assets/map/characters/scribe.svgis excluded by!**/*.svgpublic/assets/map/characters/tavern-keeper.svgis excluded by!**/*.svgpublic/assets/map/characters/warrior.svgis excluded by!**/*.svgpublic/assets/map/characters/wizard.svgis excluded by!**/*.svgpublic/assets/map/sprites/characters/alchemist.pngis excluded by!**/*.pngpublic/assets/map/sprites/characters/archer.pngis excluded by!**/*.pngpublic/assets/map/sprites/characters/cleric.pngis excluded by!**/*.pngpublic/assets/map/sprites/characters/druid.pngis excluded by!**/*.pngpublic/assets/map/sprites/characters/dwarf.pngis excluded by!**/*.pngpublic/assets/map/sprites/characters/healer.pngis excluded by!**/*.pngpublic/assets/map/sprites/characters/hunter.pngis excluded by!**/*.pngpublic/assets/map/sprites/characters/monk.pngis excluded by!**/*.pngpublic/assets/map/sprites/characters/necromancer.pngis excluded by!**/*.pngpublic/assets/map/sprites/characters/paladin.pngis excluded by!**/*.pngpublic/assets/map/sprites/characters/ranger.pngis excluded by!**/*.pngpublic/assets/map/sprites/characters/rogue.pngis excluded by!**/*.pngpublic/assets/map/sprites/characters/scribe.pngis excluded by!**/*.pngpublic/assets/map/sprites/characters/tavern-keeper.pngis excluded by!**/*.pngpublic/assets/map/sprites/characters/warrior.pngis excluded by!**/*.pngpublic/assets/map/sprites/characters/wizard.pngis excluded by!**/*.png
📒 Files selected for processing (37)
docs/feature-spec-index.mddocs/map-dashboard-feature-spec.mdpublic/assets/map/backgrounds/adventure-map-background.webppublic/assets/map/sprites/characters/alchemist.jsonpublic/assets/map/sprites/characters/archer.jsonpublic/assets/map/sprites/characters/cleric.jsonpublic/assets/map/sprites/characters/druid.jsonpublic/assets/map/sprites/characters/dwarf.jsonpublic/assets/map/sprites/characters/healer.jsonpublic/assets/map/sprites/characters/hunter.jsonpublic/assets/map/sprites/characters/monk.jsonpublic/assets/map/sprites/characters/necromancer.jsonpublic/assets/map/sprites/characters/paladin.jsonpublic/assets/map/sprites/characters/ranger.jsonpublic/assets/map/sprites/characters/rogue.jsonpublic/assets/map/sprites/characters/scribe.jsonpublic/assets/map/sprites/characters/tavern-keeper.jsonpublic/assets/map/sprites/characters/warrior.jsonpublic/assets/map/sprites/characters/wizard.jsonsrc/app/(frontend)/api/portal/leaderboard/points/route.tssrc/app/(frontend)/dashboard/map/MapCharacterSelector.tsxsrc/app/(frontend)/dashboard/map/MapDashboardClient.tsxsrc/app/(frontend)/dashboard/map/MapDialog.tsxsrc/app/(frontend)/dashboard/map/MapLocationDialog.tsxsrc/app/(frontend)/dashboard/map/MapSprite.tsxsrc/app/(frontend)/dashboard/map/mapConfig.tssrc/app/(frontend)/dashboard/map/mapData.tssrc/app/(frontend)/dashboard/map/page.tsxsrc/app/(frontend)/dashboard/map/useMapMovement.tssrc/collections/PageCopy.tssrc/endpoints/seed/index.tssrc/endpoints/seed/portal.tssrc/migrations/20260620_120000_dashboard_map_page_copy.tssrc/migrations/index.tssrc/payload-types.tssrc/utilities/pageCopy.tstests/e2e/app.spec.ts
|
Moved Interactive map dashboard into Implemented First Slices and updated status in feature-spec-index.md Fixed ranger manifest source path in ranger.json (line 18) Blocked disabled map destinations in travelToLocation and disabled both map/destination buttons in MapDashboardClient.tsx (line 63) Replaced the hard-coded empty leaderboard with server-loaded point standings in mapData.ts (line 58) Wrapped the map dashboard e2e browser context in try/finally in app.spec.ts (line 297) |
There was a problem hiding this comment.
Pull request overview
Adds an alternate authenticated dashboard experience at /dashboard/map, backed by server-side map data loading and a display-safe points leaderboard API. This fits into the existing Portal primitives by reusing posts/modules/wiki/events/feedback/points rather than introducing new content models.
Changes:
- Introduces
/dashboard/map(server page + client map UI) with character selection, path/node travel, and location dialogs backed by Portal data. - Adds
GET /api/portal/leaderboard/pointsfor authenticated, verified users to fetch a top-10 points leaderboard with visibility filtering. - Extends PageCopy to support a new
dashboardsurface (dashboard-mapkey) with fallback copy, seeds, and a Postgres enum migration; adds Playwright coverage and feature spec docs.
Reviewed changes
Copilot reviewed 36 out of 69 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/app.spec.ts | Adds e2e coverage for map dashboard auth redirect, character selection, travel/dialog open, and leaderboard endpoint response. |
| src/utilities/pageCopy.ts | Adds dashboard-map fallback copy and a getter for map dashboard PageCopy. |
| src/payload-types.ts | Expands PageCopy.surface union to include dashboard. |
| src/migrations/index.ts | Registers a new migration for the PageCopy surface enum update. |
| src/migrations/20260620_120000_dashboard_map_page_copy.ts | Adds Postgres enum value dashboard for PageCopy surface. |
| src/endpoints/seed/portal.ts | Seeds a published dashboard-map PageCopy record (upsert-based portal seeding). |
| src/endpoints/seed/index.ts | Seeds a published dashboard-map PageCopy record (create-based seed flow). |
| src/collections/PageCopy.ts | Maps dashboard-map to /dashboard/map and adds dashboard as an admin surface option. |
| src/app/(frontend)/dashboard/map/useMapMovement.ts | Implements node/path travel animation with reduced-motion snapping. |
| src/app/(frontend)/dashboard/map/page.tsx | Adds the /dashboard/map route with auth + verified-email gating and server data load. |
| src/app/(frontend)/dashboard/map/MapSprite.tsx | Renders the selected character sprite with direction-based frame selection. |
| src/app/(frontend)/dashboard/map/MapLocationDialog.tsx | Adds location dialogs (posts/modules/wiki/events/feedback/points/leaderboard/vibe check). |
| src/app/(frontend)/dashboard/map/MapDialog.tsx | Adds a custom accessible dialog shell with focus management and keyboard handling. |
| src/app/(frontend)/dashboard/map/mapData.ts | Server-side loader for map dashboard data, including role-to-sprite selection and points/leaderboard summaries. |
| src/app/(frontend)/dashboard/map/MapDashboardClient.tsx | Main interactive map UI: background, destination markers/menu, sprite placement, dialogs. |
| src/app/(frontend)/dashboard/map/mapConfig.ts | Defines map nodes/links, destinations, sprite availability lists, and BFS pathfinding. |
| src/app/(frontend)/dashboard/map/MapCharacterSelector.tsx | Role-based character selection dialog with unavailable-art handling and profile links. |
| src/app/(frontend)/api/portal/leaderboard/points/route.ts | Implements authenticated “display-safe” points leaderboard API (top 10, visibility-filtered). |
| public/assets/map/sprites/characters/alchemist.json | Adds sprite-sheet metadata for the alchemist character. |
| public/assets/map/sprites/characters/archer.json | Adds sprite-sheet metadata for the archer character. |
| public/assets/map/sprites/characters/cleric.json | Adds sprite-sheet metadata for the cleric character. |
| public/assets/map/sprites/characters/druid.json | Adds sprite-sheet metadata for the druid character. |
| public/assets/map/sprites/characters/dwarf.json | Adds sprite-sheet metadata for the dwarf character. |
| public/assets/map/sprites/characters/healer.json | Adds sprite-sheet metadata for the healer character. |
| public/assets/map/sprites/characters/hunter.json | Adds sprite-sheet metadata for the hunter character. |
| public/assets/map/sprites/characters/monk.json | Adds sprite-sheet metadata for the monk character. |
| public/assets/map/sprites/characters/necromancer.json | Adds sprite-sheet metadata for the necromancer character. |
| public/assets/map/sprites/characters/paladin.json | Adds sprite-sheet metadata for the paladin character. |
| public/assets/map/sprites/characters/ranger.json | Adds sprite-sheet metadata for the ranger character. |
| public/assets/map/sprites/characters/rogue.json | Adds sprite-sheet metadata for the rogue character. |
| public/assets/map/sprites/characters/scribe.json | Adds sprite-sheet metadata for the scribe character. |
| public/assets/map/sprites/characters/tavern-keeper.json | Adds sprite-sheet metadata for the tavern keeper character. |
| public/assets/map/sprites/characters/warrior.json | Adds sprite-sheet metadata for the warrior character. |
| public/assets/map/sprites/characters/wizard.json | Adds sprite-sheet metadata for the wizard character. |
| public/assets/map/characters/monk.svg | Adds a static portrait asset for monk. |
| public/assets/map/characters/healer.svg | Adds a static portrait asset for healer. |
| public/assets/map/characters/archer.svg | Adds a static portrait asset for archer. |
| docs/map-dashboard-feature-spec.md | Adds/updates the interactive map dashboard feature spec with status and implementation details. |
| docs/feature-spec-index.md | Adds the interactive map dashboard to the feature spec index. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await loginPortalUser(mapPage, email, password) | ||
| await mapPage.goto('/dashboard/map') | ||
| await expect(mapPage.getByRole('heading', { name: /walk the portal roads/i })).toBeVisible() | ||
| await expect(mapPage.getByRole('dialog', { name: /choose your guild form/i })).toBeVisible() | ||
|
|
| document.addEventListener('keydown', onKeyDown) | ||
| window.setTimeout(focusFirst, 0) | ||
|
|
||
| return () => { | ||
| document.removeEventListener('keydown', onKeyDown) | ||
| previousActiveElement?.focus() | ||
| } |
| const getPointSummary = async (user: User) => { | ||
| const payload = await getPayload({ config: configPromise }) | ||
| const result = await payload.find({ | ||
| collection: 'pointEvents', | ||
| depth: 1, | ||
| limit: 1000, | ||
| overrideAccess: false, | ||
| pagination: false, | ||
| sort: '-issuedAt', | ||
| user, | ||
| where: { | ||
| and: [ | ||
| { | ||
| recipient: { | ||
| equals: user.id, | ||
| }, | ||
| }, | ||
| { | ||
| status: { | ||
| equals: 'valid', | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| }) | ||
|
|
||
| return { | ||
| events: result.docs.slice(0, 5), | ||
| total: result.docs.reduce((sum, event) => sum + (event.amount || 0), 0), | ||
| } |
| const [ | ||
| copy, | ||
| profile, | ||
| latestPost, | ||
| prototypeModules, | ||
| recentWikiPages, | ||
| upcomingEvents, | ||
| pointSummary, | ||
| leaderboard, | ||
| dailyEngagementSummary, | ||
| ] = await Promise.all([ | ||
| getDashboardMapPageCopy(), | ||
| getProfileForUser(user.id), | ||
| getLatestPost(user), | ||
| getPrototypeModules(user), | ||
| getRecentWikiPages(user), | ||
| getUpcomingEvents(user), | ||
| getPointSummary(user), | ||
| getPointLeaderboard(user), | ||
| getDailyEngagementSummary(user), | ||
| ]) |
| const aggregatePointTotals = (events: PointEvent[]) => { | ||
| const totals = new Map<string, number>() | ||
|
|
||
| events.forEach((event) => { | ||
| const recipientID = relationID(event.recipient) | ||
| if (!recipientID) return | ||
|
|
||
| totals.set(recipientID, (totals.get(recipientID) || 0) + (event.amount || 0)) | ||
| }) | ||
|
|
||
| return totals | ||
| } | ||
|
|
||
| const relationID = (value: unknown): null | string => { | ||
| if (typeof value === 'number' || typeof value === 'string') return String(value) | ||
| if (value && typeof value === 'object' && 'id' in value) { | ||
| const id = (value as { id?: number | string }).id | ||
| if (typeof id === 'number' || typeof id === 'string') return String(id) | ||
| } | ||
|
|
||
| return null | ||
| } | ||
|
|
||
| const mediaURL = (value: unknown): string | undefined => { | ||
| if (!value || typeof value !== 'object') return undefined | ||
| const url = (value as Media).url | ||
|
|
||
| return typeof url === 'string' ? url : undefined | ||
| } |
Summary
/dashboard/mapas an alternate Portal dashboard view.GET /api/portal/leaderboard/pointsfor a display-safe authenticated point leaderboard.dashboard-mapPageCopy support, fallback copy, seeds, migration, and Payload type update.Summary by CodeRabbit
Release Notes
New Features
/dashboard/mapdashboard featuring character role selection with animated sprites and location-based exploration with contextual dialogsDocumentation