Add silent for useQuery in useQuestionnaireOptions #9973
Conversation
WalkthroughThe pull request introduces modifications to the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Finishing Touches
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 (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/hooks/useQuestionnaireOptions.ts (2)
Line range hint
16-28: Consider documenting error handling behavior.Since we're explicitly silencing errors, it would be helpful to document why this is necessary and how errors are handled in this case.
Add a comment explaining the silent flag:
const { data } = useQuery({ queryKey: ["questionnaire", slug] as const, queryFn: query(questionnaireApi.list, { queryParams: { tag_slug: slug, }, + // Suppress error notifications as 404s are expected when no questionnaires exist for a tag silent: true, }), });
Line range hint
16-28: Consider handling loading and error states explicitly.The current implementation only uses the
dataproperty fromuseQuery. Consider destructuringisLoadinganderrorstates for better user experience.Here's a suggested improvement:
- const { data } = useQuery({ + const { data, isLoading, error } = useQuery({ queryKey: ["questionnaire", slug] as const, queryFn: query(questionnaireApi.list, { queryParams: { tag_slug: slug, }, silent: true, }), }); + if (isLoading) { + return DEFAULT_OPTIONS; + } + + // Still return default options if there's an error + if (error) { + console.debug('Failed to fetch questionnaire options:', error); + return DEFAULT_OPTIONS; + } const questionnaireOptions =
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/hooks/useQuestionnaireOptions.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: OSSAR-Scan
- GitHub Check: cypress-run (1)
🔇 Additional comments (1)
src/hooks/useQuestionnaireOptions.ts (1)
25-25: Verify error handling with silent option.The addition of
silent: truelooks good as it will suppress unnecessary error notifications. However, let's verify the error handling implementation.Let's check how errors are handled across the codebase:
✅ Verification successful
Error handling implementation is appropriate with silent option
The
silent: trueoption is well-implemented as the hook:
- Has fallback to DEFAULT_OPTIONS if the API call fails
- Gracefully handles undefined data with nullish coalescing
- Doesn't need error toasts as default options ensure UI functionality
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check error handling patterns in query utility and related files # Check query utility implementation echo "Checking query utility implementation:" rg -A 5 "function query" "src/Utils/request" # Check for error boundary usage with useQuery echo "Checking error boundary usage:" rg -A 5 "ErrorBoundary.*useQuery" "src" # Check for similar silent patterns echo "Checking similar silent patterns:" rg "silent:\s*true.*useQuery" "src"Length of output: 748
Script:
#!/bin/bash # Check the complete implementation of useQuestionnaireOptions echo "Checking useQuestionnaireOptions implementation:" rg -l "useQuestionnaireOptions" src | xargs cat # Check callApi error handling echo -e "\nChecking callApi error handling:" rg -A 10 "function callApi" src # Look for similar hooks pattern echo -e "\nChecking similar hooks patterns:" rg -l "useQuery.*options" src | xargs catLength of output: 16369
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Utils/request/types.ts (1)
52-52: LGTM! Consider adding JSDoc documentation.The type enhancement for the
silentoption is well-designed, allowing for both static boolean values and dynamic functions. This flexibility enables better control over error handling based on response characteristics.Consider adding JSDoc documentation to explain the usage of the function variant:
/** * Controls whether errors should be silently handled. * @param {boolean} silent - Directly controls silent behavior * @param {Function} silent - Determines silent behavior based on the response * @example * // Silence specific status codes * silent: (res) => res.status === 404 */ silent?: boolean | ((response: Response) => boolean);src/Utils/request/query.ts (1)
35-39: LGTM! Implementation handles both silent variants correctly.The error handling logic properly implements the enhanced silent option, safely handling both boolean and function variants with appropriate type checking.
Consider extracting the silent logic into a helper function for better readability:
+const getSilentValue = ( + silent: boolean | ((response: Response) => boolean) | undefined, + response: Response +): boolean => { + return typeof silent === "function" ? silent(response) : (silent ?? false); +}; if (!res.ok) { - const isSilent = - typeof options?.silent === "function" - ? options.silent(res) - : (options?.silent ?? false); + const isSilent = getSilentValue(options?.silent, res); throw new HTTPError({ message: "Request Failed", status: res.status, silent: isSilent, cause: data as unknown as Record<string, unknown>, }); }Also applies to: 43-43
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Utils/request/query.ts(1 hunks)src/Utils/request/types.ts(1 hunks)src/hooks/useQuestionnaireOptions.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/hooks/useQuestionnaireOptions.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
|
@Jacobjeevan Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit