Skip to content

🔄 Sync: [TanStack Query Optimization] Refactor profile update to useMutation#259

Open
ldsgroups225 wants to merge 1 commit intomasterfrom
sync/profile-mutation-10182944810760341213
Open

🔄 Sync: [TanStack Query Optimization] Refactor profile update to useMutation#259
ldsgroups225 wants to merge 1 commit intomasterfrom
sync/profile-mutation-10182944810760341213

Conversation

@ldsgroups225
Copy link
Copy Markdown
Owner

@ldsgroups225 ldsgroups225 commented Apr 6, 2026

What

Refactored the profile update flow in apps/teacher/src/routes/_auth/app/profile.tsx to use TanStack Query's useMutation instead of local isSaving state.

Why

To align with TanStack Query v5 best practices and the application's global mutation tracking patterns. Manual try/catch with local boolean loading states (isSaving) is an anti-pattern when useMutation handles this out of the box with onSuccess/onError callbacks and .isPending states.

Impact on performance

Minimal, but improves code maintainability and sets the stage for future global mutation tracking or optimistic updates.

Measurement / Verification

  • Code review performed
  • Unit tests run with pnpm --filter yeko-teacher test --exclude e2e-tests
  • Typechecks passing with pnpm --filter yeko-teacher typecheck

PR created automatically by Jules for task 10182944810760341213 started by @ldsgroups225

Summary by CodeRabbit

  • Refactor
    • Improved profile update flow and state management.

Migrates manual `isSaving` state and `try/catch` logic to a proper TanStack Query `useMutation` in the profile page, aligning with the project's standard data synchronization practices.
@google-labs-jules
Copy link
Copy Markdown

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 6, 2026

📝 Walkthrough

Walkthrough

The teacher app's profile component was refactored to use React Query's useMutation hook for managing profile updates, replacing manual useState with try/catch/finally error handling. The mutation lifecycle hooks (onSuccess, onError) now handle success notifications, dialog closure, and error logging.

Changes

Cohort / File(s) Summary
Profile Mutation Refactoring
apps/teacher/src/routes/_auth/app/profile.tsx
Replaced manual isSaving state management with React Query useMutation hook. Moved validation and side effects (toast notifications, dialog closure, router invalidation) into mutation lifecycle handlers. Simplified handleUpdateProfile to call mutate() directly.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 From useState to mutation's might,
React Query makes the update flow just right,
No more try-catch scattered 'round,
Hooks and handlers, cleaner ground! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly describes the main change: refactoring profile update to use React Query's useMutation hook, which is the primary focus of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync/profile-mutation-10182944810760341213

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
apps/teacher/src/routes/_auth/app/profile.tsx (1)

90-95: Consider adding Zod schema validation for form input.

The current validation only checks if newName is non-empty. Per coding guidelines, form inputs in routes should be validated using Zod schemas from @repo/data-ops/zod-schema/*.

Additionally, newName should be passed to mutate() so the mutation has access to the value being saved:

💡 Suggested improvement
+ import { z } from 'zod'
+ 
+ const profileUpdateSchema = z.object({
+   name: z.string().min(1).max(100),
+ })

  const handleUpdateProfile = () => {
-   if (!newName.trim())
-     return
-
-   updateProfileMutation.mutate()
+   const result = profileUpdateSchema.safeParse({ name: newName.trim() })
+   if (!result.success) {
+     toast.error(LL.profile.invalidName?.() ?? 'Invalid name')
+     return
+   }
+   updateProfileMutation.mutate({ name: result.data.name })
  }

And update the mutation to accept variables:

- const updateProfileMutation = useMutation({
-   mutationFn: async () => {
+ const updateProfileMutation = useMutation({
+   mutationFn: async (variables: { name: string }) => {
      // TODO: Call actual profile update API with variables.name
      return Promise.resolve()
    },

As per coding guidelines: "Validate all form inputs and server function inputs using Zod schemas from @repo/data-ops/zod-schema/*"

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/teacher/src/routes/_auth/app/profile.tsx` around lines 90 - 95, The
handler handleUpdateProfile currently only checks newName.trim() and calls
updateProfileMutation.mutate() with no data; add Zod validation using the
appropriate schema from `@repo/data-ops/zod-schema/`* (e.g., a profile/updateName
schema) to validate newName before mutating, and pass the validated value into
updateProfileMutation.mutate({ name: validatedName }); also update the mutation
resolver (the function referenced by updateProfileMutation) to accept and
validate the incoming variables with the same Zod schema so the server-side
receives typed/validated input.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/teacher/src/routes/_auth/app/profile.tsx`:
- Around line 72-86: The current updateProfileMutation's mutationFn is a
placeholder that doesn't persist the newName state and mutate() is called
without arguments; replace the stub in updateProfileMutation (mutationFn) to
call the real server-side profile update function (e.g., updateProfile or an
equivalent data-ops function) and accept typed input (e.g., { name: string }),
then call mutate({ name: newName }) from the UI instead of mutate() so newName
is passed through; only show LL.profile.updated(), close the editor
(setIsEditProfileOpen(false)) and call router.invalidate() after the server call
succeeds; also ensure you validate the current session/auth before invoking
updateProfile (check session or a validateSession helper) and surface server
errors in onError (toast.error and console.error) so persistence is ensured
before signaling success.

---

Nitpick comments:
In `@apps/teacher/src/routes/_auth/app/profile.tsx`:
- Around line 90-95: The handler handleUpdateProfile currently only checks
newName.trim() and calls updateProfileMutation.mutate() with no data; add Zod
validation using the appropriate schema from `@repo/data-ops/zod-schema/`* (e.g.,
a profile/updateName schema) to validate newName before mutating, and pass the
validated value into updateProfileMutation.mutate({ name: validatedName }); also
update the mutation resolver (the function referenced by updateProfileMutation)
to accept and validate the incoming variables with the same Zod schema so the
server-side receives typed/validated input.
🪄 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: b582774b-271b-44fb-a4b5-bc1d9be1d078

📥 Commits

Reviewing files that changed from the base of the PR and between 7e37ef8 and 268f53e.

📒 Files selected for processing (1)
  • apps/teacher/src/routes/_auth/app/profile.tsx

Comment on lines +72 to +86
const updateProfileMutation = useMutation({
mutationFn: async () => {
// simulate save
return Promise.resolve()
},
onSuccess: () => {
toast.success(LL.profile.updated())
setIsEditProfileOpen(false)
router.invalidate() // Refresh data
}
catch (error) {
},
onError: (error) => {
console.error('Failed to update profile', error)
toast.error(LL.profile.updateError())
}
finally {
setIsSaving(false)
}
},
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there's an existing profile update API/server function in the codebase
echo "=== Searching for profile update functions ==="
rg -n -C3 'updateProfile|update.*profile|profile.*update' --type ts --type tsx -g '!*.test.*' -g '!*.spec.*' || echo "No matches found"

echo ""
echo "=== Searching for user update mutations ==="
rg -n -C3 'updateUser|user.*update' --type ts --type tsx -g '!*.test.*' -g '!*.spec.*' || echo "No matches found"

echo ""
echo "=== Checking auth client methods ==="
rg -n 'authClient\.' apps/teacher/src/ | head -20

Repository: ldsgroups225/Yeko

Length of output: 571


🏁 Script executed:

#!/bin/bash
# Read the complete profile.tsx file to understand the full context
echo "=== Reading profile.tsx file ==="
wc -l apps/teacher/src/routes/_auth/app/profile.tsx

echo ""
echo "=== Content of profile.tsx ==="
cat -n apps/teacher/src/routes/_auth/app/profile.tsx

echo ""
echo "=== Searching for profile/user update API routes ==="
find apps/teacher/src/routes -name "*.ts" -o -name "*.tsx" | xargs grep -l "profile\|user" 2>/dev/null | head -10

Repository: ldsgroups225/Yeko

Length of output: 17057


🏁 Script executed:

#!/bin/bash
# Search for any server-side mutations or server actions related to profile/user updates
echo "=== Searching for server functions in data-ops ==="
find . -path "*/data-ops/*" -name "*.ts" | xargs grep -l "profile\|updateUser\|updateProfile" 2>/dev/null | head -10

echo ""
echo "=== Searching for any POST/PUT endpoints in routes ==="
rg -n "POST|PUT|PATCH" apps/teacher/src/routes/ --type ts 2>/dev/null | grep -i "profile\|user" | head -10

echo ""
echo "=== Checking if there's a useUpdateProfile or similar hook ==="
rg -n "useUpdateProfile|useUpdateUser\|useMutation" apps/teacher/src/ | grep -i profile

Repository: ldsgroups225/Yeko

Length of output: 447


🏁 Script executed:

#!/bin/bash
# Check if this is part of a larger refactor or intentional scaffolding
echo "=== Looking for TODO or FIXME comments related to profile ==="
rg -n "TODO|FIXME|WIP" apps/teacher/src/routes/_auth/app/profile.tsx

echo ""
echo "=== Checking git status or recent changes (if available) ==="
git log --oneline apps/teacher/src/routes/_auth/app/profile.tsx 2>/dev/null | head -5 || echo "Git history not available"

echo ""
echo "=== Checking for env or config related to profile API ==="
rg -n "profile.*api\|profile.*endpoint" apps/teacher/src/ --type ts --type tsx 2>/dev/null | head -10

Repository: ldsgroups225/Yeko

Length of output: 307


🏁 Script executed:

#!/bin/bash
# Check the write.ts files in data-ops to see available functions
echo "=== Checking data-ops write functions ==="
cat -n ./packages/data-ops/src/queries/school-admin/users/write.ts | head -50

echo ""
echo "=== Searching for any update or write functions exported from data-ops ==="
rg -n "export.*function|export const" ./packages/data-ops/src/queries/ | grep -i "user\|profile" | head -20

echo ""
echo "=== Check what's available in data-ops index or main exports ==="
find ./packages/data-ops/src -name "index.ts" -o -name "index.js" | xargs cat 2>/dev/null | head -30

Repository: ldsgroups225/Yeko

Length of output: 6387


Mutation is a placeholder that doesn't persist newName.

The mutationFn resolves immediately without calling an actual API or server function to update the profile. The newName value from state is never passed to the mutation—mutate() is called without arguments on line 94. Consequently:

  • router.invalidate() will re-fetch unchanged session data
  • The success toast misleads the user into thinking their name was saved
  • No user profile update function exists in data-ops yet

To complete this implementation:

  1. Create or locate the profile update server function
  2. Pass newName to the mutation: mutate({ name: newName }) with proper types
  3. Ensure the mutation actually persists changes before showing the success toast
  4. Verify the session is validated before executing the mutation (per auth requirements)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/teacher/src/routes/_auth/app/profile.tsx` around lines 72 - 86, The
current updateProfileMutation's mutationFn is a placeholder that doesn't persist
the newName state and mutate() is called without arguments; replace the stub in
updateProfileMutation (mutationFn) to call the real server-side profile update
function (e.g., updateProfile or an equivalent data-ops function) and accept
typed input (e.g., { name: string }), then call mutate({ name: newName }) from
the UI instead of mutate() so newName is passed through; only show
LL.profile.updated(), close the editor (setIsEditProfileOpen(false)) and call
router.invalidate() after the server call succeeds; also ensure you validate the
current session/auth before invoking updateProfile (check session or a
validateSession helper) and surface server errors in onError (toast.error and
console.error) so persistence is ensured before signaling success.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant