connect profile page to api - #22
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds shared user/profile schemas and backend user read/update endpoints with caching, then wires the mobile app to fetch, display, edit, and navigate between profile-related screens. ChangesBackend user API and mobile profile flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant usersRouter
participant userService
participant Cache
participant DB
Client->>usersRouter: GET /users/:id
usersRouter->>userService: getUserById(id)
userService->>Cache: getOrSet(user:id)
userService->>DB: query public user columns
userService-->>usersRouter: user data
usersRouter-->>Client: { success: true, data }
Client->>usersRouter: PATCH /users/me
usersRouter->>userService: updateProfile(userId, input)
userService->>DB: update users row
userService->>Cache: invalidate(user:id)
userService-->>usersRouter: updated user
usersRouter-->>Client: { success: true, data }
sequenceDiagram
participant User
participant EditProfileScreen
participant updateProfileSchema
participant useUserStore
participant API
User->>EditProfileScreen: edit name/bio, tap Save
EditProfileScreen->>updateProfileSchema: safeParse(input)
alt validation fails
updateProfileSchema-->>EditProfileScreen: issues
EditProfileScreen-->>User: show field errors
else validation succeeds
EditProfileScreen->>useUserStore: updateProfile(data)
useUserStore->>API: PATCH /users/me
API-->>useUserStore: updated user
useUserStore-->>EditProfileScreen: success
EditProfileScreen-->>User: toast + navigate back
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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
🧹 Nitpick comments (7)
apps/mobile/app/edit-profile.tsx (2)
144-146: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer NativeWind's
contentContainerClassNameoverStyleSheet.create.ScrollView in this NativeWind stack supports
contentContainerClassName, sopadding: 16doesn't need aStyleSheet.♻️ Proposed fix
<ScrollView - contentContainerStyle={styles.scroll} + contentContainerClassName="p-4" keyboardDismissMode="on-drag" >-const styles = StyleSheet.create({ - scroll: { - padding: 16, - }, -});As per coding guidelines, "Use NativeWind/Tailwind classes by default; avoid
StyleSheetunless a style cannot be expressed with class names."Also applies to: 209-213
🤖 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/mobile/app/edit-profile.tsx` around lines 144 - 146, The ScrollView in the edit-profile screen is still using a StyleSheet-backed contentContainerStyle for simple padding, which should be expressed with NativeWind instead. Update the ScrollView usage to use contentContainerClassName and move the existing padding from styles.scroll into class names, then remove the now-unused scroll StyleSheet entry and apply the same change to the other ScrollView instance referenced in this component.Source: Coding guidelines
64-69: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueUse NativeWind for the
ScrollViewpadding.
contentContainerStyle={styles.scroll}is unnecessary here;contentContainerClassName="p-4"keeps this screen aligned with the mobile styling guideline.🤖 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/mobile/app/edit-profile.tsx` around lines 64 - 69, The ScrollView on the edit profile screen is still using a style-based content container for padding instead of NativeWind. Update the ScrollView in edit-profile.tsx to use contentContainerClassName for the padding and remove the unnecessary contentContainerStyle reference to styles.scroll so the screen follows the mobile styling pattern consistently.apps/mobile/app/(tabs)/profile.tsx (1)
46-47: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winStore
erroris never surfaced to the user.
useUserStore()exposeserror, but it isn't destructured or rendered anywhere in this screen, so failed profile/posts/stats fetches fail silently with no user feedback. Consider destructuringerrorand rendering a lightweight retry/error banner.🤖 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/mobile/app/`(tabs)/profile.tsx around lines 46 - 47, The profile screen is not surfacing the store’s `error`, so failed fetches can fail silently. Update `profile.tsx` to destructure `error` from `useUserStore()` alongside `profile`, `posts`, `stats`, and the fetch methods, then render a lightweight error/retry banner when `error` is present. Use the existing screen component and fetch callbacks (`fetchProfile`, `fetchUserPosts`, `fetchStats`) so the user can see the failure and retry from the same view.apps/mobile/store/userStore.ts (4)
70-92: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winShared
errorfield is clobbered by concurrent fetches.
fetchProfile,fetchUserPosts, andfetchStatsare all fired concurrently fromprofile.tsxand all write to the sameerrorkey. Whichever call finishes last wins, silently discarding an earlier failure (e.g. a profile-load error gets overwritten by a later, unrelated stats-load success/failure). Consider per-resource error fields (profileError,postsError,statsError) so failures aren't masked.🤖 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/mobile/store/userStore.ts` around lines 70 - 92, The shared error state in userStore is being overwritten by concurrent requests from fetchProfile, fetchUserPosts, and fetchStats. Update the store to use separate error fields such as profileError, postsError, and statsError, and change each fetch method to write only to its own resource-specific error key instead of the common error field. Make sure the existing fetchUserPosts and fetchStats actions, along with fetchProfile, all set and clear their own error state so one request cannot clobber another.
94-101: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
updateProfiledoesn't catch errors like its siblings.Unlike
fetchProfile/fetchUserPosts/fetchStats, this action has no try/catch and doesn't touchisLoading/error. A thrown error here becomes an unhandled promise rejection unless every caller wraps the call in its own try/catch. Worth confirmingedit-profile.tsxhandles this consistently, or align this action with the others for consistency.🤖 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/mobile/store/userStore.ts` around lines 94 - 101, The updateProfile action in userStore should handle failures consistently with fetchProfile, fetchUserPosts, and fetchStats. Update the updateProfile function to wrap the API call and response handling in try/catch, set isLoading/error state as appropriate, and clear/update state on success. Also verify the edit-profile.tsx caller does not rely on unhandled rejections; if needed, align its error handling with the other store actions.
5-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove response/domain types into
@journal/shared.
ApiUser,UserStats, and theUserResponse/UserPostsResponse/UserStatsResponseshapes are redefined here even though@journal/sharedalready hosts related contracts (e.g.UpdateProfileInput, imported on Line 2). These mirror backend response shapes and can silently drift from the actual API contract. Consider exporting these from@journal/shared(ideally as zod schemas) and parsing responses with.parse()/.safeParse()instead of blindascasts on Lines 59, 72, 84.As per coding guidelines,
apps/mobile/**/*.{ts,tsx}: "share schemas and types through@journal/sharedinstead of redefining them per app."🤖 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/mobile/store/userStore.ts` around lines 5 - 35, Move the response/domain contracts used by userStore out of apps/mobile and into `@journal/shared` so ApiUser, UserStats, and the UserResponse/UserPostsResponse/UserStatsResponse shapes have a single source of truth. Update the userStore code to import those shared schemas/types instead of redefining them locally, and use the shared zod validators (e.g. parse/safeParse) when handling api responses in the user-related fetch methods rather than casting with as. Refer to the userStore methods that load profile, posts, and stats so the response validation stays aligned with the backend contract.Source: Coding guidelines
56-68: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winStale
profileisn't cleared whenuserIdchanges.
fetchProfileresetsisLoading/errorbut leaves the previousprofileobject in place until the new fetch resolves. Combined withapps/mobile/app/(tabs)/profile.tsx'sisLoading && !profilegate, if this store is ever populated for one user and then a different user authenticates in the same session (once sign-out insettings.tsxis wired up), the UI will briefly render the previous user's name/bio/avatar because!profileis already false. Consider clearingprofile/posts/statsat the start offetchProfile(or wheneveruserIdchanges) so stale data can't leak across sessions.This is currently latent since sign-out is a no-op in
settings.tsx, but worth verifying it's fixed before that flow ships.🔒 Suggested reset
fetchProfile: async (api, userId) => { - set({ isLoading: true, error: null }); + set({ isLoading: true, error: null, profile: null }); try {🤖 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/mobile/store/userStore.ts` around lines 56 - 68, The `fetchProfile` action in `userStore` leaves the previous `profile` in state while loading a new `userId`, which can leak stale user data into the profile screen. Update `fetchProfile` to clear `profile` and any related state such as `posts` and `stats` when a new fetch starts (or otherwise when `userId` changes), while still setting `isLoading` and `error` appropriately. Make sure the reset happens before the API call so `apps/mobile/app/(tabs)/profile.tsx` cannot render an old profile between sessions.
🤖 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/src/services/userService.ts`:
- Around line 7-13: The getUserById helper is caching and returning the full
users record, which exposes Clerk-synced/private fields through the public GET
/:id path. Update the getUserById function to select a public-safe user
projection before the getOrSet cache stores it, and make sure the returned shape
only includes profile-safe columns while preserving the existing not-found
HttpError behavior.
- Around line 32-48: The updateProfile function currently allows all fields in
UpdateProfileInput to be omitted, which can produce an empty
db.update(users).set(...) payload and fail at the database level. Add a 400
HttpError guard at the start of updateProfile that checks name, bio, and
avatar_url are all undefined and rejects the request before calling db.update,
while keeping the existing update, invalidate, and return flow unchanged.
In `@apps/mobile/app/`(tabs)/profile.tsx:
- Around line 75-112: The profile screen is showing the empty-state too early
because `isLoading` in `profile.tsx` only reflects `fetchProfile`, not the posts
request. Update the store/view state to expose a separate posts-loading flag
from `fetchUserPosts`, and use it in `FlatList`/`ListEmptyComponent` so “No
posts yet” only appears when posts loading has finished and the posts array is
actually empty. Keep the loading gate and empty-state logic tied to the existing
`profile`, `gridData`, and `isLoading`/new posts-loading state in `Profile`.
---
Nitpick comments:
In `@apps/mobile/app/`(tabs)/profile.tsx:
- Around line 46-47: The profile screen is not surfacing the store’s `error`, so
failed fetches can fail silently. Update `profile.tsx` to destructure `error`
from `useUserStore()` alongside `profile`, `posts`, `stats`, and the fetch
methods, then render a lightweight error/retry banner when `error` is present.
Use the existing screen component and fetch callbacks (`fetchProfile`,
`fetchUserPosts`, `fetchStats`) so the user can see the failure and retry from
the same view.
In `@apps/mobile/app/edit-profile.tsx`:
- Around line 144-146: The ScrollView in the edit-profile screen is still using
a StyleSheet-backed contentContainerStyle for simple padding, which should be
expressed with NativeWind instead. Update the ScrollView usage to use
contentContainerClassName and move the existing padding from styles.scroll into
class names, then remove the now-unused scroll StyleSheet entry and apply the
same change to the other ScrollView instance referenced in this component.
- Around line 64-69: The ScrollView on the edit profile screen is still using a
style-based content container for padding instead of NativeWind. Update the
ScrollView in edit-profile.tsx to use contentContainerClassName for the padding
and remove the unnecessary contentContainerStyle reference to styles.scroll so
the screen follows the mobile styling pattern consistently.
In `@apps/mobile/store/userStore.ts`:
- Around line 70-92: The shared error state in userStore is being overwritten by
concurrent requests from fetchProfile, fetchUserPosts, and fetchStats. Update
the store to use separate error fields such as profileError, postsError, and
statsError, and change each fetch method to write only to its own
resource-specific error key instead of the common error field. Make sure the
existing fetchUserPosts and fetchStats actions, along with fetchProfile, all set
and clear their own error state so one request cannot clobber another.
- Around line 94-101: The updateProfile action in userStore should handle
failures consistently with fetchProfile, fetchUserPosts, and fetchStats. Update
the updateProfile function to wrap the API call and response handling in
try/catch, set isLoading/error state as appropriate, and clear/update state on
success. Also verify the edit-profile.tsx caller does not rely on unhandled
rejections; if needed, align its error handling with the other store actions.
- Around line 5-35: Move the response/domain contracts used by userStore out of
apps/mobile and into `@journal/shared` so ApiUser, UserStats, and the
UserResponse/UserPostsResponse/UserStatsResponse shapes have a single source of
truth. Update the userStore code to import those shared schemas/types instead of
redefining them locally, and use the shared zod validators (e.g.
parse/safeParse) when handling api responses in the user-related fetch methods
rather than casting with as. Refer to the userStore methods that load profile,
posts, and stats so the response validation stays aligned with the backend
contract.
- Around line 56-68: The `fetchProfile` action in `userStore` leaves the
previous `profile` in state while loading a new `userId`, which can leak stale
user data into the profile screen. Update `fetchProfile` to clear `profile` and
any related state such as `posts` and `stats` when a new fetch starts (or
otherwise when `userId` changes), while still setting `isLoading` and `error`
appropriately. Make sure the reset happens before the API call so
`apps/mobile/app/(tabs)/profile.tsx` cannot render an old profile between
sessions.
🪄 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 Plus
Run ID: b632e4e0-aa4d-4393-b6b6-309abd7e6dc6
📒 Files selected for processing (13)
apps/api/src/lib/cache.tsapps/api/src/routes/users.tsapps/api/src/services/userService.tsapps/mobile/app/(tabs)/profile.tsxapps/mobile/app/(tabs)/settings.tsxapps/mobile/app/_layout.tsxapps/mobile/app/edit-profile.tsxapps/mobile/app/post/[id].tsxapps/mobile/components/profile/GridImage.tsxapps/mobile/components/profile/ProfileHeader.tsxapps/mobile/components/profile/StatsBanner.tsxapps/mobile/store/userStore.tspackages/shared/src/index.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/mobile/app/(tabs)/profile.tsx (1)
115-130: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueRetry button has no pressed/disabled state.
Multiple rapid taps on "Retry" can fire concurrent
fetchProfile/fetchUserPosts/fetchStatscalls since there's no guard disabling the button while a retry is already in flight. Low priority but worth a quick guard (e.g., disable whileisLoading || isPostsLoading).🤖 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/mobile/app/`(tabs)/profile.tsx around lines 115 - 130, The Retry button in profile.tsx has no in-flight guard, so rapid taps can trigger overlapping loadProfile/fetchProfile/fetchUserPosts/fetchStats requests. Update the Pressable and related state in the Profile screen to disable or ignore presses while loading (for example using isLoading, isPostsLoading, and any stats-loading flag), and reflect the disabled state in the UI so loadProfile cannot be re-entered concurrently.packages/shared/src/index.ts (1)
88-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConfirm intentional distinction between
userIdParamSchemaanduserIdPathParamSchema.Two very similarly named schemas now coexist: the existing
userIdParamSchema(validates{ userId }) and the newly addeduserIdPathParamSchema(validates{ id }, per the change description). The near-identical naming with different field keys is an easy source of import mix-ups (e.g. a route validatingreq.paramswith the wrong schema would silently fail or pass through unexpected shapes). Consider a more distinguishing name (e.g.userIdRouteParamSchemavs. an explicit comment on each) or a shared factoryidParamSchema(key)to reduce duplication.🤖 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 `@packages/shared/src/index.ts` around lines 88 - 93, The issue is that `userIdParamSchema` and `userIdPathParamSchema` are too similar despite validating different object shapes, which makes them easy to mix up at call sites. Update the schema naming in `packages/shared/src/index.ts` to make the distinction explicit, or refactor to a shared helper like an `idParamSchema` factory that takes the key name, and then update any imports/usages to the clearer symbol names.
🤖 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/mobile/app/`(tabs)/profile.tsx:
- Around line 47-71: The retry flow in profile.tsx is clearing already-fetched
data in useUserStore, so a partial failure causes the Profile screen to lose
visible content and show the full loading state again. Update fetchProfile (and
related store actions if needed) so retries only reset the specific slice being
reloaded, or preserve existing profile/posts/stats when data is already present;
keep loadProfile and its callers unchanged except for passing through the
current cache state if needed.
In `@apps/mobile/store/userStore.ts`:
- Around line 41-93: fetchProfile, fetchUserPosts, and fetchStats in userStore
are vulnerable to stale responses overwriting newer user data because they
update the shared store without verifying the active userId. Add a request guard
in these methods, such as a per-request token, sequence id, or AbortController,
and only call set when the response still matches the latest in-flight request
for that userId. Make sure the guard is applied around the async api call and
the schema parse in each of the three methods so older responses are ignored.
---
Nitpick comments:
In `@apps/mobile/app/`(tabs)/profile.tsx:
- Around line 115-130: The Retry button in profile.tsx has no in-flight guard,
so rapid taps can trigger overlapping
loadProfile/fetchProfile/fetchUserPosts/fetchStats requests. Update the
Pressable and related state in the Profile screen to disable or ignore presses
while loading (for example using isLoading, isPostsLoading, and any
stats-loading flag), and reflect the disabled state in the UI so loadProfile
cannot be re-entered concurrently.
In `@packages/shared/src/index.ts`:
- Around line 88-93: The issue is that `userIdParamSchema` and
`userIdPathParamSchema` are too similar despite validating different object
shapes, which makes them easy to mix up at call sites. Update the schema naming
in `packages/shared/src/index.ts` to make the distinction explicit, or refactor
to a shared helper like an `idParamSchema` factory that takes the key name, and
then update any imports/usages to the clearer symbol names.
🪄 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 Plus
Run ID: d4a67bd8-a5db-4bb3-be2b-401286203687
📒 Files selected for processing (5)
apps/api/src/services/userService.tsapps/mobile/app/(tabs)/profile.tsxapps/mobile/app/edit-profile.tsxapps/mobile/store/userStore.tspackages/shared/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/api/src/services/userService.ts
- apps/mobile/app/edit-profile.tsx
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 `@apps/mobile/store/userStore.ts`:
- Around line 115-132: Advance the shared profile request guard in updateProfile
so in-flight fetchProfile calls can’t overwrite the just-saved data. In the
userStore.ts store, update the updateProfile action to bump profileRequestId
before or when applying the PATCH result, using the same request-id mechanism
used by fetchProfile, so any older fetch responses are ignored. Keep the change
localized to updateProfile and the profileRequestId state handling in the
userStore actions.
🪄 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 Plus
Run ID: cd88fe2e-7ade-4536-a779-28aac5012dca
📒 Files selected for processing (5)
apps/api/src/routes/posts.tsapps/api/src/routes/users.tsapps/mobile/app/(tabs)/profile.tsxapps/mobile/store/userStore.tspackages/shared/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/api/src/routes/users.ts
- apps/mobile/app/(tabs)/profile.tsx
1 participant
Summary by CodeRabbit