fix: suid screen getting stuck #4229
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
WalkthroughThe pull request modifies the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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 (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
apps/web/app/s/[surveyId]/components/SurveyLinkUsed.tsx (1)
Line range hint
1-38: Good architectural solution for handling survey link states.The transition to client-side rendering with proper error state handling is a solid architectural choice for this use case. It ensures that users get immediate feedback when attempting to access an already submitted survey, rather than facing an infinite loading state.
A few key improvements in this implementation:
- Reactive error handling through client-side rendering
- Clear user feedback with visual indicators
- Flexible message customization with fallbacks
This should effectively resolve the issue reported in #4224.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
apps/web/app/s/[surveyId]/components/SurveyLinkUsed.tsx(2 hunks)
🔇 Additional comments (2)
apps/web/app/s/[surveyId]/components/SurveyLinkUsed.tsx (2)
Line range hint 17-38: LGTM! Verify translation strings.
The error state UI implementation looks good with proper fallback messages and clear visual feedback. This should effectively communicate to users when they try to access an already submitted survey.
Let's verify the translation keys exist:
✅ Verification successful
Translation keys are properly defined and consistent across languages ✅
The translation keys survey_already_answered_heading and survey_already_answered_subheading are correctly defined in all language files (en-US, de-DE, pt-BR) with appropriate translations. The implementation will work as expected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the translation keys in the messages files
rg "survey_already_answered_(heading|subheading)" --type json
Length of output: 1447
1-4: LGTM! Verify client-side rendering impact.
The transition to client-side rendering with "use client" and useTranslations is appropriate for fixing the infinite loading issue, as it ensures the component can handle the error state reactively.
Let's verify no other components depend on this being server-rendered:
✅ Verification successful
Client-side rendering transition is safe
The component is only used conditionally within LinkSurvey.tsx to display a message when a single-use survey has already been submitted. Converting it to a client component is appropriate and safe as:
- It's used as a leaf component for displaying a static message
- The parent component already handles the state check (
hasFinishedSingleUseResponse) - No server-side dependencies are affected by this change
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for imports of SurveyLinkUsed to verify usage
rg -l "SurveyLinkUsed" --type tsx
Length of output: 61
Script:
#!/bin/bash
# Search for imports of SurveyLinkUsed using ripgrep without file type filter
rg -l "SurveyLinkUsed"
Length of output: 130
Script:
#!/bin/bash
# Check how SurveyLinkUsed is used in LinkSurvey.tsx
rg "SurveyLinkUsed" apps/web/app/s/[surveyId]/components/LinkSurvey.tsx -B 2 -A 2
Length of output: 541
| export const SurveyLinkUsed = async ({ singleUseMessage }: SurveyLinkUsedProps) => { | ||
| const t = await getTranslations(); | ||
| const t = useTranslations(); | ||
| const defaultHeading = t("s.survey_already_answered_heading"); |
There was a problem hiding this comment.
Remove async keyword from component signature.
The component is using the useTranslations hook which is synchronous, but the component is incorrectly marked as async. React hooks cannot be used in async components, and this could lead to unexpected behavior.
Apply this fix:
- export const SurveyLinkUsed = async ({ singleUseMessage }: SurveyLinkUsedProps) => {
+ export const SurveyLinkUsed = ({ singleUseMessage }: SurveyLinkUsedProps) => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const SurveyLinkUsed = async ({ singleUseMessage }: SurveyLinkUsedProps) => { | |
| const t = await getTranslations(); | |
| const t = useTranslations(); | |
| const defaultHeading = t("s.survey_already_answered_heading"); | |
| export const SurveyLinkUsed = ({ singleUseMessage }: SurveyLinkUsedProps) => { | |
| const t = useTranslations(); | |
| const defaultHeading = t("s.survey_already_answered_heading"); |
What does this PR do?
Fixes #4224
How should this be tested?
Test suid link page for survey which is already taken
Checklist
Required
pnpm buildconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
New Features
SurveyLinkUsedcomponent for enhanced client-side rendering.Bug Fixes
Refactor