feat(achievements): show achievement rewards as XP - #6646
Conversation
Follows daily-api retiring achievement points: achievements now grant XP into the same balance quests feed, so the level shown in Game Center and on the Highest level leaderboard already accounts for them. - read Achievement.xp and AchievementSyncResult.xpGained/totalXp - every "pts" / "points" label becomes XP, including the sync modal counter and the achievements snapshot card - drop the Most achievement points leaderboard, now a strict subset of Highest level - the sync analytics event keeps points_gained alongside xp_gained so existing dashboards keep reporting Requires the API change to be deployed first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…ed board Review follow-ups on the API rename: - AchievementSyncResult.totalXp is now totalAchievementXp, matching daily-api. The same collision existed client side, so useProfileAchievements and the Game Center summary rename too — all three count achievement XP only, while questDashboard.level.totalXp is the whole balance. - /users/mostAchievementPoints permanently redirects to /users/highestLevel instead of 404ing, since that board is the superset this PR retires it for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rebelchris
left a comment
rebelchris
left a comment
There was a problem hiding this comment.
Re-checked against the current head (84d44bf, after the merge from main). This looks clean: the points-to-XP rename is applied consistently across the GraphQL layer, the leaderboard config, the widgets, and the tests, and the retired mostAchievementPoints slug now issues a permanent redirect to /users/highestLevel rather than 404ing.
Deploy-ordering gate (main actionable item)
packages/shared/src/graphql/user/achievements.ts now selects xp on Achievement and xpGained / totalAchievementXp on the sync result. Those fields only exist in dailydotdev/daily-api#4265, which is still open and unmerged as of this review. If this PR ships first, ACHIEVEMENTS_QUERY and SYNC_ACHIEVEMENTS_MUTATION will fail schema validation on the live API. The highestLevel leaderboard has a isHighestLevelSchemaMissing fallback in pages/users/[id].tsx, but the achievements queries have no equivalent. Suggest holding this until #4265 is merged and deployed, or noting the dependency in the PR description so it is not merged out of order.
Minor notes
AchievementSyncModal.tsxstill emitspoints_gainedalongsidexp_gained(set to the same value, for legacy dashboards). Worth adding a removal date or a tracking ticket in the comment so it does not linger indefinitely.- No leftover references to
mostAchievementPoints,totalPoints, orpointsGainedremain at head, apart from the intentionalRETIRED_ACHIEVEMENT_POINTS_SLUGconstant used for the redirect.
This review was generated by Smith, daily.dev's AI assistant, on behalf of the requester.
…sModal typecheck_strict_changed failed on CI: touching this file for the XP copy pulled it into the changed set, and the strict gate then surfaced two violations that predate this PR. - handleClose took an optional event but onRequestClose requires one; both call sites always pass an event, so drop the optional marker rather than widen the contract - ProfilePicture requires a user and useAuthContext().user is optional, so only render the avatar when there is one — the name beside it already falls back to "You" Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nto feat/achievement-xp-migration
1 participant
Why
Follows dailydotdev/daily-api#4265, which retires achievement points: achievements now grant XP into the same balance quests feed, so the level in Game Center and on the Highest level leaderboard already accounts for them. This side catches the UI up so users see one currency instead of two.
What
Achievement.xpandAchievementSyncResult.xpGained/totalAchievementXp/users, now a strict subset of "Highest level";/users/mostAchievementPointspermanently redirects there rather than 404ing on old bookmarkstotalAchievementXpthroughout (useProfileAchievements, Game Center summary), so they don't read asquestDashboard.level.totalXp, which is the user's whole balancepoints_gainedalongsidexp_gainedso existing dashboards keep reportingGame Center needed no change: its "Total XP" reads
questDashboard.level.totalXp, which starts including achievements the moment the API ships.Deploy order
Gate this on dailydotdev/daily-api#4265 being deployed, not merely merged.
xp,xpGainedandtotalAchievementXponly exist there. If this ships first the whole achievements surface fails rather than degrading — profile, Game Center, every modal, sync and the snapshot card.Follow-ups
points_gainedis kept in the sync analytics event purely for dashboard continuity and has no removal date — worth a ticket alongside the API's deprecated-alias cleanup so it doesn't live forever.Test plan
sortAchievements,AchievementCard,AchievementTrackerButton,AchievementPickerModal,AchievementsWidget,gameCenter,UsersLeaderboardStaticProps,GameCenterStaticPropstypecheckclean for every touched file (remaining errors are pre-existing onmaininuseUserExperienceForm.spec.tsxandtransaction.spec.tsx)🤖 Generated with Claude Code
Preview domain
https://feat-achievement-xp-migration.preview.app.daily.dev