-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🐛 Fix issue in credentials modal to properly update value #1866
🐛 Fix issue in credentials modal to properly update value #1866
Conversation
@basuabhirup is attempting to deploy a commit to the Typebot Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes involve updating the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apps/builder/src/features/forge/components/credentials/UpdateForgedCredentialsModalContent.tsx (1)
65-67
: Consider handling potential race conditions.The addition of
refetchCredentials()
afteronUpdate()
correctly addresses the stale data issue. However, ifonUpdate()
triggers a re-render, there might be a race condition between the re-render and the refetch.Consider wrapping both calls in a single effect:
onSuccess: () => { - onUpdate(); - refetchCredentials(); + Promise.all([ + onUpdate(), + refetchCredentials() + ]).catch(console.error); },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- apps/builder/src/features/forge/components/credentials/UpdateForgedCredentialsModalContent.tsx (2 hunks)
🔇 Additional comments (2)
apps/builder/src/features/forge/components/credentials/UpdateForgedCredentialsModalContent.tsx (2)
Line range hint
38-47
: LGTM! Good addition of refetch functionality.The addition of
refetch
from the query hook is a good practice that will help maintain data freshness.
Line range hint
38-67
: Verify the fix resolves the stale data issue.The implementation looks correct, but let's verify that the modal now shows updated values when reopened.
✅ Verification successful
Let me gather more information about the similar modal components to ensure they handle updates correctly.
The fix correctly handles stale data and is consistent with other similar implementations
The implementation in
UpdateForgedCredentialsModalContent.tsx
is correct and follows the same pattern used in other credential modal components in the codebase. All three components (SmtpUpdateModalContent
,UpdateStripeCredentialsModalContent
, andUpdateForgedCredentialsModalContent
) properly:
- Initialize state from
existingCredentials
usinguseEffect
- Reset state only when
existingCredentials
changes and local state is not yet set- Properly handle updates with mutation callbacks
- Refresh data after updates using appropriate refetch/callback mechanisms
The fix will resolve the stale data issue as intended.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any other components that might be caching or storing credential data # which could interfere with this fix echo "Searching for other credential-related state management..." rg -l "credentials" --type ts --type tsx | xargs rg "useState.*credentials|useQuery.*credentials" -A 2 echo "Searching for other modal components that might need similar fixes..." rg -l "Modal.*Content" --type tsx | xargs rg "getCredentials.useQuery" -A 2Length of output: 1647
Script:
#!/bin/bash # Check the implementation of similar modal components for potential stale data issues echo "Checking SmtpUpdateModalContent implementation..." cat "apps/builder/src/features/blocks/integrations/sendEmail/components/SmtpUpdateModalContent.tsx" echo -e "\nChecking UpdateStripeCredentialsModalContent implementation..." cat "apps/builder/src/features/blocks/inputs/payment/components/UpdateStripeCredentialsModalContent.tsx"Length of output: 10703
Fixes #1856
fix-typebot-issue-1856.mp4