Add base for fhir forms #9262
Add base for fhir forms #9262
Conversation
|
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughThis pull request introduces comprehensive changes to the Care application's architecture, focusing on enhancing routing, component structure, and user interface design. The modifications span multiple domains, including questionnaire management, scheduling, patient details, and facility interactions. Key improvements include the introduction of new routing mechanisms using raviger, implementation of modular UI components, and the addition of structured questionnaire and scheduling functionalities. Changes
Possibly Related PRs
Suggested Labels
Suggested Reviewers
Poem
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. |
CARE
|
||||||||||||||||||||||||||||
| Project |
CARE
|
| Branch Review |
form-field-v1
|
| Run status |
|
| Run duration | 05m 01s |
| Commit |
|
| Committer | Bodhish Thomas |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
135
|
| View all changes introduced in this branch ↗︎ | |
Deploying care-fe with
|
| Latest commit: |
ea19afa
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1a4fe324.care-fe.pages.dev |
| Branch Preview URL: | https://form-field-v1.care-fe.pages.dev |
|
👋 Hi, @bodhish, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
github-actions
bot
added
Deploy-Failed
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 27
🧹 Outside diff range and nitpick comments (32)
src/components/ui/select.tsx (4)
1-4: Insert missing semicolons in import statementsThe static analysis tool indicates that semicolons are missing at the end of the import statements on lines 1-4. Please add semicolons to comply with the project's code style guidelines.
Apply this diff to add the missing semicolons:
import * as React from "react" +; import * as SelectPrimitive from "@radix-ui/react-select" +; import { cn } from "@/lib/utils" +; import { CheckIcon, ChevronDownIcon, ChevronUpIcon } from "@radix-ui/react-icons" +;🧰 Tools
🪛 eslint
[error] 1-1: Replace
*·as·React·from·"react"with{⏎··CheckIcon,⏎··ChevronDownIcon,⏎··ChevronUpIcon,⏎}·from·"@radix-ui/react-icons";(prettier/prettier)
[error] 2-2: Insert
;(prettier/prettier)
[error] 3-3: Replace
{·cn·}·from·"@/lib/utils"with*·as·React·from·"react";⏎(prettier/prettier)
[error] 4-4: Replace
CheckIcon,·ChevronDownIcon,·ChevronUpIcon·}·from·"@radix-ui/react-icons"withcn·}·from·"@/lib/utils";(prettier/prettier)
6-10: Add missing semicolons at the end of statementsSeveral lines are missing semicolons, which is causing errors in the static analysis tool (Prettier). Please add semicolons at the end of the statements on the following lines to adhere to the coding standards:
Lines: 6, 8, 10, 20, 29, 30, 40, 46, 47, 57, 63, 65, 78, 88, 96, 97, 108, 109, 119, 130, 131, 142, 143, 156
Example diff for lines 6 and 8:
const Select = SelectPrimitive.Root +; const SelectGroup = SelectPrimitive.Group +;Please apply similar changes to the other lines.
Also applies to: 20-20, 29-30, 40-40, 46-47, 57-57, 63-63, 65-65, 78-78, 88-88, 96-97, 108-109, 119-119, 130-131, 142-143, 156-156
🧰 Tools
🪛 eslint
[error] 6-6: Insert
;(prettier/prettier)
[error] 8-8: Insert
;(prettier/prettier)
[error] 10-10: Insert
;(prettier/prettier)
18-21: Consistent use of thecnutility function for className propEnsure that the
cnutility function is consistently used for theclassNameprop in all components. In some cases, additional class names might be concatenated improperly or without consideration for conditional classes.Review the
classNameconcatenations in the following components to ensure consistency and correctness:
SelectTrigger(lines 18-21)SelectScrollUpButton(lines 39-41)SelectScrollDownButton(lines 55-58)SelectContent(lines 74-79)SelectViewportwithinSelectContent(lines 85-90)SelectItem(lines 117-120)SelectSeparator(lines 138-141)Also applies to: 39-41, 55-58, 74-79, 85-90, 117-120, 138-141
🧰 Tools
🪛 eslint
[error] 20-20: Insert
,(prettier/prettier)
26-28: Accessibility considerations for iconsWhen using icons within interactive components, ensure that they are accessible to screen readers. Consider adding
aria-hidden="true"to decorative icons to prevent them from being announced by screen readers.Apply this diff to add
aria-hiddenattributes:<SelectPrimitive.Icon asChild> - <ChevronDownIcon className="h-4 w-4 opacity-50" /> + <ChevronDownIcon className="h-4 w-4 opacity-50" aria-hidden="true" /> </SelectPrimitive.Icon>Repeat similar changes for other icons in the components.
Also applies to: 44-45, 61-62, 124-126
src/components/Questionnaire/show.tsx (3)
10-11: Potential TypeScript type annotation improvementIn the import statement on lines 10-11, consider importing types using the
import typesyntax for improved type-only imports, which can enhance build performance and clarity.Apply this diff:
-import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs"; +import type { TabsTriggerProps } from "@/components/ui/tabs"; +import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs";Adjust the imports accordingly if you're only using types from the module.
21-22: Consistency in interface and function definitionsEnsure that interfaces and function props are consistently defined and include necessary type annotations. The
QuestionItemcomponent's props could be more explicitly typed, and default values can be assigned using default parameters or by setting defaultProps.Review and update the
QuestionItemcomponent definition:interface QuestionItemProps { question: Question; - depth?: number; + depth: number; } - function QuestionItem({ question, depth = 0 }: QuestionItemProps) { + function QuestionItem({ question, depth }: QuestionItemProps) { + depth = depth || 0;Consider setting a default value for
depthif it's optional.Also applies to: 26-32, 55-61
162-168: Simplify conditional className assignmentIn lines 162-168, the conditional class assignment within the
classNameprop can be simplified for better readability.Apply this diff:
<Badge className={ - questionnaire.status === "active" - ? "bg-green-100 text-green-800" - : "" - } + cn({ + "bg-green-100 text-green-800": questionnaire.status === "active", + }) } >Ensure that the
cnutility supports conditional class handling.src/components/Questionnaire/QuestionTypes/QuestionInput.tsx (1)
276-279: Simplify conditional rendering for hidden questionsIn lines 276-279, the logic for hiding disabled questions can be simplified for clarity.
Apply this diff:
-if (!isEnabled && question.disabled_display === "hidden") { +if (!isEnabled && question.disabled_display !== "visible") { return null; }This change assumes that any value other than
"visible"should result in the question being hidden.src/components/Questionnaire/QuestionnaireForm.tsx (1)
331-360: Enhance accessibility of the submit button during loadingWhen the submit button is processing (lines 331-360), ensure that accessibility considerations are met. For example, provide an
aria-busyattribute or usedisabledto inform assistive technologies.Apply this diff:
<Button type="button" onClick={handleSubmit} disabled={isProcessing || hasErrors} + aria-busy={isProcessing} className="relative" >src/types/questionnaire/form.ts (2)
3-6: Consider adding validation constraints for the value fieldThe
ResponseValuetype allows for multiple primitive types without any validation constraints. This could lead to inconsistent data handling downstream.Consider adding more specific types or validation:
export type ResponseValue = { - value?: string | number | boolean; + value?: { + string?: string; + number?: number; + boolean?: boolean; + }; code?: Code; };This would ensure that the type of value is explicitly known when accessing it.
8-16: Add validation for taken_at format and consider making question_id optionalA few suggestions for the interface:
- The
taken_atfield should probably enforce ISO date string format- Since this appears to be a FHIR-based implementation,
question_idmight be redundant withlink_idand could be optionalConsider these modifications:
export interface QuestionnaireResponse { - question_id: string; + question_id?: string; link_id: string; values: ResponseValue[]; note?: string; - taken_at?: string; + taken_at?: `${number}-${number}-${number}T${number}:${number}:${number}Z`; body_site?: Code; method?: Code; }src/types/questionnaire/questionnaire.ts (1)
15-26: Add FHIR-specific fields to QuestionnaireDetailThe interface seems to be missing some standard FHIR Questionnaire fields.
Consider adding these FHIR-specific fields:
export interface QuestionnaireDetail { id: string; slug: string; version?: string; code?: Code; questions: Question[]; type: QuestionnaireType; title: string; description?: string; status: QuestionStatus; subject_type: SubjectType; + publisher?: string; + lastUpdated?: string; + language?: string; + url?: string; + meta?: { + versionId: string; + lastUpdated: string; + }; }src/components/Questionnaire/QuestionTypes/MedicationQuestion.tsx (1)
6-11: Add TypeScript documentation for the props interfaceConsider adding JSDoc comments to document the purpose and requirements of each prop, especially since this is part of a FHIR implementation.
+/** + * Props for the MedicationQuestion component + * @param question - FHIR Question object containing the medication question details + * @param questionnaireResponse - Current state of the questionnaire response + * @param updateQuestionnaireResponseCB - Callback to update the questionnaire response + * @param disabled - Optional flag to disable the input + */ interface MedicationQuestionProps { question: Question; questionnaireResponse: QuestionnaireResponse; updateQuestionnaireResponseCB: (response: QuestionnaireResponse) => void; disabled?: boolean; }src/components/ui/textarea.tsx (1)
5-6: Enhance TextareaProps interface with additional functionalityConsider adding commonly needed textarea features:
- Resize control
- Character limit
- Error state
export interface TextareaProps - extends React.TextareaHTMLAttributes<HTMLTextAreaElement> {} + extends React.TextareaHTMLAttributes<HTMLTextAreaElement> { + /** Control textarea resizing behavior */ + resize?: 'none' | 'vertical' | 'horizontal' | 'both'; + /** Maximum character limit */ + maxLength?: number; + /** Show character count */ + showCount?: boolean; + /** Error state */ + error?: boolean; + }src/components/ui/input.tsx (1)
Line range hint
1-22: Add support for input addons and improve accessibilityConsider adding support for prefix/suffix elements and improving accessibility.
const Input = React.forwardRef<HTMLInputElement, InputProps>( - ({ className, type, ...props }, ref) => { + ({ className, type, error, prefix, suffix, ...props }, ref) => { return ( + <div className="relative flex items-center"> + {prefix && ( + <div className="absolute left-3 flex items-center pointer-events-none"> + {prefix} + </div> + )} <input type={type} className={cn( // ... existing classes ..., + prefix && "pl-10", + suffix && "pr-10", className )} ref={ref} + aria-invalid={error} {...props} /> + {suffix && ( + <div className="absolute right-3 flex items-center pointer-events-none"> + {suffix} + </div> + )} + </div> ); }, );src/types/questionnaire/batch.ts (1)
38-43: Consider adding validation error typesThe
ValidationErrorResponseinterface could benefit from being more specific about what types of validation errors can occur.Consider creating a discriminated union type for different validation error scenarios:
type ValidationErrorType = | { type: 'QUESTION_VALIDATION'; error: QuestionValidationError } | { type: 'DETAILED_VALIDATION'; error: DetailedValidationError }; export interface ValidationErrorResponse { data: { errors: ValidationErrorType[]; }; status_code: 400 | 404; }.cursorrules (1)
36-37: Consider adding more context to the new guidelinesWhile the additions are clear, they could benefit from more specific guidance.
Consider expanding these guidelines with more details:
- Use raviger for routing. - Use care icons for icons. + Use raviger for routing (see examples in /Routers/AppRouter.tsx). + Use care icons for icons (import from @/components/icons).src/components/ui/badge.tsx (2)
6-24: Consider extracting color tokens for better theme managementWhile the implementation is solid, the color values are hardcoded in the variants. This could make theme customization more difficult.
Consider extracting these values to a theme configuration:
const colors = { badge: { default: { light: 'bg-gray-900 text-gray-50', dark: 'dark:bg-gray-50 dark:text-gray-900' }, // ... other variants } } as const; const badgeVariants = cva( "inline-flex items-center rounded-md border px-2.5 py-0.5 text-xs font-semibold transition-colors focus:outline-none focus:ring-2 focus:ring-offset-2", { variants: { variant: { default: `border-transparent ${colors.badge.default.light} shadow hover:bg-gray-900/80 ${colors.badge.default.dark} dark:hover:bg-gray-50/80`, // ... other variants } } } );🧰 Tools
🪛 eslint
[error] 23-23: Insert
,(prettier/prettier)
[error] 24-24: Insert
;(prettier/prettier)
🪛 GitHub Check: lint
[failure] 23-23:
Insert,
[failure] 24-24:
Insert;
30-34: Consider adding aria-label for accessibilityThe badge component could benefit from better accessibility support.
Consider adding an aria-label prop:
- function Badge({ className, variant, ...props }: BadgeProps) { + function Badge({ + className, + variant, + 'aria-label': ariaLabel, + ...props + }: BadgeProps & { 'aria-label'?: string }) { return ( <div className={cn(badgeVariants({ variant }), className)} + aria-label={ariaLabel} {...props} /> ); }🧰 Tools
🪛 eslint
[error] 33-33: Insert
;(prettier/prettier)
🪛 GitHub Check: lint
[failure] 33-33:
Insert;src/types/questionnaire/question.ts (1)
3-16: Consider adding validation types for question responsesThe
QuestionTypecovers basic data types but could benefit from additional validation-related types. For example:
regexfor string validation patternsrangefor numeric boundsmulti-choicefor multiple selection supportsrc/components/Questionnaire/QuestionTypes/NotesInput.tsx (1)
21-23: Consider persisting notes visibility stateThe show/hide state resets on component re-render. Consider persisting this preference:
- In URL parameters
- In local storage
- In parent component state
src/components/Questionnaire/QuestionRenderer.tsx (2)
30-37: Add mount check to scroll behaviorThe scroll effect could trigger unnecessarily on component mount if
activeGroupIdis set.Add a mount check:
+ const isFirstMount = useRef(true); useEffect(() => { + if (isFirstMount.current) { + isFirstMount.current = false; + return; + } if (activeGroupId && questionRefs.current[activeGroupId]) { questionRefs.current[activeGroupId]?.scrollIntoView({ behavior: "smooth", block: "start", }); } }, [activeGroupId]);
39-50: Simplify response update logicThe response update logic can be simplified using array methods.
Consider this more concise approach:
const handleResponseChange = (updatedResponse: QuestionnaireResponse) => { - const newResponses = [...responses]; - const index = newResponses.findIndex( - (r) => r.question_id === updatedResponse.question_id, - ); - if (index !== -1) { - newResponses[index] = updatedResponse; - } else { - newResponses.push(updatedResponse); - } + const newResponses = responses.filter( + (r) => r.question_id !== updatedResponse.question_id + ); + newResponses.push(updatedResponse); onResponseChange(newResponses); };src/components/Patient/EncounterQuestionnaire.tsx (1)
11-16: Consider adding JSDoc documentation for the Props interfaceThe Props interface would benefit from JSDoc comments explaining the purpose of each prop, especially
questionnaireSlugwhich is optional.+/** + * Props for the EncounterQuestionnaire component + * @param facilityId - ID of the healthcare facility + * @param patientId - ID of the patient + * @param consultationId - ID of the consultation/encounter + * @param questionnaireSlug - Optional slug to identify specific questionnaire type + */ interface Props { facilityId: string; patientId: string; consultationId: string; questionnaireSlug?: string; }src/components/Questionnaire/QuestionTypes/QuestionGroup.tsx (1)
68-79: Optimize recursive rendering performanceThe recursive rendering of nested questions could benefit from performance optimizations.
Consider these optimizations:
- Add
useMemofor the mapped questions- Pass the depth parameter to child components
- {question.questions?.map((subQuestion) => ( + {useMemo(() => question.questions?.map((subQuestion) => ( <QuestionGroup key={subQuestion.id} question={subQuestion} questionnaireResponses={questionnaireResponses} updateQuestionnaireResponseCB={updateQuestionnaireResponseCB} errors={errors} clearError={clearError} disabled={disabled} activeGroupId={activeGroupId} + _depth={_depth + 1} /> - ))} + )), [question.questions, questionnaireResponses, errors, disabled, activeGroupId])}src/components/Questionnaire/QuestionnaireSearch.tsx (1)
67-72: Enhance accessibility and search performanceConsider the following improvements:
- Add proper ARIA attributes for accessibility
- Implement debouncing for search input to optimize performance
<input className="flex h-11 w-full rounded-md bg-transparent py-3 text-sm outline-none placeholder:text-muted-foreground disabled:cursor-not-allowed disabled:opacity-50" placeholder="Search questionnaires..." + aria-label="Search questionnaires" + role="searchbox" value={search} - onChange={(e) => setSearch(e.target.value)} + onChange={debounce((e) => setSearch(e.target.value), 300)} />src/components/Questionnaire/index.tsx (1)
70-78: Improve status badge implementationConsider using semantic color variables and adding a proper aria-label for accessibility.
<Badge className={ questionnaire.status === "active" - ? "bg-green-100 text-green-800" + ? "bg-success-100 text-success-800" : "" } + aria-label={`Status: ${questionnaire.status}`} > {questionnaire.status} </Badge>src/Routers/AppRouter.tsx (1)
71-73: Consider adding route documentationWhile the routes are properly implemented, adding JSDoc comments describing the purpose and expected parameters of these routes would improve maintainability.
+ /** + * Routes for managing questionnaires + * /questionnaire - Lists all questionnaires + * /questionnaire/:id - Shows details of a specific questionnaire + */ // Questionnaire Routes "/questionnaire": () => <QuestionnaireList />, "/questionnaire/:id": ({ id }) => <QuestionnaireShow id={id} />,src/Routers/routes/ConsultationRoutes.tsx (1)
163-179: Consider enhancing type safety and maintainabilityWhile the routes are implemented correctly, consider these improvements:
- Add type safety for the slug parameter
- Extract route paths to constants for better maintainability
+const QUESTIONNAIRE_ROUTES = { + BASE: "/facility/:facilityId/patient/:patientId/consultation/:consultationId/questionnaire", + WITH_SLUG: "/facility/:facilityId/patient/:patientId/consultation/:consultationId/questionnaire/:slug", +} as const; + +type QuestionnaireSlug = string & { __brand: 'QuestionnaireSlug' }; + - "/facility/:facilityId/patient/:patientId/consultation/:consultationId/questionnaire": + [QUESTIONNAIRE_ROUTES.BASE]: ({ facilityId, patientId, consultationId }) => ( <EncounterQuestionnaire facilityId={facilityId} patientId={patientId} consultationId={consultationId} /> ), - "/facility/:facilityId/patient/:patientId/consultation/:consultationId/questionnaire/:slug": + [QUESTIONNAIRE_ROUTES.WITH_SLUG]: ({ facilityId, patientId, consultationId, slug }) => ( <EncounterQuestionnaire facilityId={facilityId} patientId={patientId} consultationId={consultationId} - questionnaireSlug={slug} + questionnaireSlug={slug as QuestionnaireSlug} /> ),src/Utils/request/api.tsx (3)
93-100: Remove unused importThe
QuestionnaireResponsetype is imported but never used in this file.-import type { - QuestionnaireDetail, - QuestionnaireResponse, -} from "@/types/questionnaire/questionnaire"; +import type { QuestionnaireDetail } from "@/types/questionnaire/questionnaire";🧰 Tools
🪛 eslint
[error] 99-99: 'QuestionnaireResponse' is defined but never used. Allowed unused vars must match /^_/u.
(@typescript-eslint/no-unused-vars)
🪛 GitHub Check: lint
[failure] 99-99:
'QuestionnaireResponse' is defined but never used. Allowed unused vars must match /^_/u
1456-1466: Consider extending response types for FHIR complianceThe response structure could be extended to better align with FHIR Questionnaire Response format:
- Add
authoredtimestamp- Add
authorreference- Consider adding
item.repeatfor repeating questions
1470-1477: Add retry mechanism for batch requestsThe batch request endpoint should handle partial failures gracefully.
Consider:
- Adding retry logic for failed items in the batch
- Implementing a dead letter queue for failed requests
- Adding batch size limits to prevent timeouts
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (32)
.cursorrules(1 hunks)package.json(1 hunks)src/Routers/AppRouter.tsx(2 hunks)src/Routers/routes/ConsultationRoutes.tsx(2 hunks)src/Utils/request/api.tsx(2 hunks)src/Utils/request/request.ts(1 hunks)src/components/Patient/EncounterQuestionnaire.tsx(1 hunks)src/components/Questionnaire/QuestionRenderer.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/ChoiceQuestion.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/MedicationQuestion.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/NotesInput.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/QuestionGroup.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/QuestionInput.tsx(1 hunks)src/components/Questionnaire/QuestionnaireForm.tsx(1 hunks)src/components/Questionnaire/QuestionnaireSearch.tsx(1 hunks)src/components/Questionnaire/index.tsx(1 hunks)src/components/Questionnaire/show.tsx(1 hunks)src/components/ui/badge.tsx(1 hunks)src/components/ui/card.tsx(2 hunks)src/components/ui/command.tsx(10 hunks)src/components/ui/dialog.tsx(9 hunks)src/components/ui/input.tsx(1 hunks)src/components/ui/popover.tsx(2 hunks)src/components/ui/select.tsx(1 hunks)src/components/ui/tabs.tsx(1 hunks)src/components/ui/textarea.tsx(1 hunks)src/types/questionnaire/batch.ts(1 hunks)src/types/questionnaire/code.ts(1 hunks)src/types/questionnaire/form.ts(1 hunks)src/types/questionnaire/question.ts(1 hunks)src/types/questionnaire/questionnaire.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/types/questionnaire/code.ts
🧰 Additional context used
🪛 eslint
src/components/ui/badge.tsx
[error] 1-2: Replace import·*·as·React·from·"react"⏎import·{·cva,·type·VariantProps·}·from·"class-variance-authority" with import·{·type·VariantProps,·cva·}·from·"class-variance-authority";⏎import·*·as·React·from·"react";
(prettier/prettier)
[error] 4-4: Insert ;
(prettier/prettier)
[error] 23-23: Insert ,
(prettier/prettier)
[error] 24-24: Insert ;
(prettier/prettier)
[error] 33-33: Insert ;
(prettier/prettier)
[error] 36-36: Insert ;
(prettier/prettier)
src/components/Questionnaire/show.tsx
[error] 102-102: 'handleFormSubmit' is assigned a value but never used. Allowed unused vars must match /^_/u.
(@typescript-eslint/no-unused-vars)
[error] 102-102: 'values' is defined but never used. Allowed unused args must match /^_/u.
(@typescript-eslint/no-unused-vars)
src/components/ui/command.tsx
[error] 1-1: Replace *·as·React·from·"react" with {·type·DialogProps·}·from·"@radix-ui/react-dialog";
(prettier/prettier)
[error] 2-2: Replace type·DialogProps·}·from·"@radix-ui/react-dialog" with MagnifyingGlassIcon·}·from·"@radix-ui/react-icons";
(prettier/prettier)
[error] 3-3: Insert ;⏎import·*·as·React·from·"react";⏎
(prettier/prettier)
[error] 4-4: Insert ;⏎
(prettier/prettier)
[error] 5-6: Replace ⏎import·{·MagnifyingGlassIcon·}·from·"@radix-ui/react-icons" with ;
(prettier/prettier)
[error] 16-16: Insert ,
(prettier/prettier)
[error] 20-20: Insert ;
(prettier/prettier)
[error] 21-21: Insert ;
(prettier/prettier)
[error] 32-32: Insert ;
(prettier/prettier)
[error] 33-33: Insert ;
(prettier/prettier)
[error] 45-45: Insert ,
(prettier/prettier)
[error] 50-50: Insert ;
(prettier/prettier)
[error] 52-52: Insert ;
(prettier/prettier)
[error] 63-63: Insert ;
(prettier/prettier)
[error] 65-65: Insert ;
(prettier/prettier)
[error] 76-76: Insert ;
(prettier/prettier)
[error] 78-78: Insert ;
(prettier/prettier)
[error] 88-88: Insert ,
(prettier/prettier)
[error] 92-92: Insert ;
(prettier/prettier)
[error] 94-94: Insert ;
(prettier/prettier)
[error] 105-105: Insert ;
(prettier/prettier)
[error] 106-106: Insert ;
(prettier/prettier)
[error] 116-116: Insert ,
(prettier/prettier)
[error] 120-120: Insert ;
(prettier/prettier)
[error] 122-122: Insert ;
(prettier/prettier)
[error] 132-132: Insert ,
(prettier/prettier)
[error] 136-136: Insert ;
(prettier/prettier)
[error] 137-137: Insert ;
(prettier/prettier)
[error] 138-138: Insert ;
(prettier/prettier)
[error] 150-150: Insert ;
(prettier/prettier)
src/components/ui/select.tsx
[error] 1-1: Replace *·as·React·from·"react" with {⏎··CheckIcon,⏎··ChevronDownIcon,⏎··ChevronUpIcon,⏎}·from·"@radix-ui/react-icons";
(prettier/prettier)
[error] 2-2: Insert ;
(prettier/prettier)
[error] 3-3: Replace {·cn·}·from·"@/lib/utils" with *·as·React·from·"react";⏎
(prettier/prettier)
[error] 4-4: Replace CheckIcon,·ChevronDownIcon,·ChevronUpIcon·}·from·"@radix-ui/react-icons" with cn·}·from·"@/lib/utils";
(prettier/prettier)
[error] 6-6: Insert ;
(prettier/prettier)
[error] 8-8: Insert ;
(prettier/prettier)
[error] 10-10: Insert ;
(prettier/prettier)
[error] 20-20: Insert ,
(prettier/prettier)
[error] 29-29: Insert ;
(prettier/prettier)
[error] 30-30: Insert ;
(prettier/prettier)
[error] 40-40: Insert ,
(prettier/prettier)
[error] 46-46: Insert ;
(prettier/prettier)
[error] 47-47: Insert ;
(prettier/prettier)
[error] 57-57: Insert ,
(prettier/prettier)
[error] 63-63: Insert ;
(prettier/prettier)
[error] 65-65: Insert ;
(prettier/prettier)
[error] 78-78: Insert ,
(prettier/prettier)
[error] 88-88: Insert ,
(prettier/prettier)
[error] 96-96: Insert ;
(prettier/prettier)
[error] 97-97: Insert ;
(prettier/prettier)
[error] 108-108: Insert ;
(prettier/prettier)
[error] 109-109: Insert ;
(prettier/prettier)
[error] 119-119: Insert ,
(prettier/prettier)
[error] 130-130: Insert ;
(prettier/prettier)
[error] 131-131: Insert ;
(prettier/prettier)
[error] 142-142: Insert ;
(prettier/prettier)
[error] 143-143: Insert ;
(prettier/prettier)
[error] 156-156: Insert ;
(prettier/prettier)
src/Utils/request/api.tsx
[error] 99-99: 'QuestionnaireResponse' is defined but never used. Allowed unused vars must match /^_/u.
(@typescript-eslint/no-unused-vars)
src/components/ui/dialog.tsx
[error] 1-2: Replace ⏎ with ;
(prettier/prettier)
[error] 3-3: Delete import·*·as·React·from·"react"
(prettier/prettier)
[error] 4-4: Insert ;
(prettier/prettier)
[error] 5-5: Replace cn·}·from·"@/lib/utils" with Cross2Icon·}·from·"@radix-ui/react-icons";
(prettier/prettier)
[error] 6-6: Replace {·Cross2Icon·}·from·"@radix-ui/react-icons" with *·as·React·from·"react";⏎⏎import·{·cn·}·from·"@/lib/utils";
(prettier/prettier)
[error] 8-8: Insert ;
(prettier/prettier)
[error] 10-10: Insert ;
(prettier/prettier)
[error] 12-12: Insert ;
(prettier/prettier)
[error] 14-14: Insert ;
(prettier/prettier)
[error] 24-24: Insert ,
(prettier/prettier)
[error] 28-28: Insert ;
(prettier/prettier)
[error] 29-29: Insert ;
(prettier/prettier)
[error] 41-41: Insert ,
(prettier/prettier)
[error] 52-52: Insert ;
(prettier/prettier)
[error] 53-53: Insert ;
(prettier/prettier)
[error] 62-62: Insert ,
(prettier/prettier)
[error] 66-66: Insert ;
(prettier/prettier)
[error] 67-67: Insert ;
(prettier/prettier)
[error] 76-76: Insert ,
(prettier/prettier)
[error] 80-80: Insert ;
(prettier/prettier)
[error] 81-81: Insert ;
(prettier/prettier)
[error] 91-91: Insert ,
(prettier/prettier)
[error] 95-95: Insert ;
(prettier/prettier)
[error] 96-96: Insert ;
(prettier/prettier)
[error] 107-107: Insert ;
(prettier/prettier)
[error] 108-108: Insert ;
(prettier/prettier)
[error] 121-121: Insert ;
(prettier/prettier)
src/components/ui/popover.tsx
[error] 1-2: Replace import·*·as·React·from·"react"⏎import·*·as·PopoverPrimitive·from·"@radix-ui/react-popover" with import·*·as·PopoverPrimitive·from·"@radix-ui/react-popover";⏎import·*·as·React·from·"react";
(prettier/prettier)
[error] 4-4: Insert ;
(prettier/prettier)
[error] 6-6: Insert ;
(prettier/prettier)
[error] 8-8: Insert ;
(prettier/prettier)
[error] 10-10: Insert ;
(prettier/prettier)
[error] 23-23: Insert ,
(prettier/prettier)
[error] 28-28: Insert ;
(prettier/prettier)
[error] 29-29: Insert ;
(prettier/prettier)
[error] 31-31: Insert ;
(prettier/prettier)
🪛 GitHub Check: lint
src/components/ui/badge.tsx
[failure] 1-1:
Replace import·*·as·React·from·"react"⏎import·{·cva,·type·VariantProps·}·from·"class-variance-authority" with import·{·type·VariantProps,·cva·}·from·"class-variance-authority";⏎import·*·as·React·from·"react";
[failure] 4-4:
Insert ;
[failure] 23-23:
Insert ,
[failure] 24-24:
Insert ;
[failure] 33-33:
Insert ;
[failure] 36-36:
Insert ;
src/components/Questionnaire/show.tsx
[failure] 102-102:
'handleFormSubmit' is assigned a value but never used. Allowed unused vars must match /^_/u
[failure] 102-102:
'values' is defined but never used. Allowed unused args must match /^_/u
src/components/ui/command.tsx
[failure] 1-1:
Replace *·as·React·from·"react" with {·type·DialogProps·}·from·"@radix-ui/react-dialog";
src/Utils/request/api.tsx
[failure] 99-99:
'QuestionnaireResponse' is defined but never used. Allowed unused vars must match /^_/u
🪛 Biome (1.9.4)
src/components/Questionnaire/show.tsx
[error] 117-119: This code is unreachable
(lint/correctness/noUnreachable)
src/components/Questionnaire/QuestionRenderer.tsx
[error] 57-57: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
src/components/Questionnaire/QuestionnaireForm.tsx
[error] 10-10: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (18)
src/components/ui/select.tsx (2)
145-156: Exported components might be missing from index file
Ensure that all newly created components are exported from the index file if they are intended for use in other parts of the application. This facilitates easier imports and maintains consistency.
Check whether these components are exported in a central index file:
SelectSelectGroupSelectValueSelectTriggerSelectContentSelectLabelSelectItemSelectSeparatorSelectScrollUpButtonSelectScrollDownButton
If they are meant to be used outside this file, consider exporting them from src/components/ui/index.tsx or the relevant index file.
🧰 Tools
🪛 eslint
[error] 156-156: Insert ;
(prettier/prettier)
1-4: Correct import order and resolve conflicting static analysis hints
The static analysis hints suggest replacing the import statements, but the provided suggestions seem incorrect and conflict with the actual code. Ensure that the imports are correctly ordered and that all necessary modules are imported without conflicts.
Run the following script to check for import statement correctness:
✅ Verification successful
Import statements are correctly structured and all imports are used
The verification shows that:
- All imported components from
@radix-ui/react-selectare used throughout the select component implementation - The
cnutility from@/lib/utilsis used for className composition - All icons (
CheckIcon,ChevronDownIcon,ChevronUpIcon) are used in their respective components - The imports follow a logical order: React core, external components, utilities, and icons
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that import statements are correctly structured and no duplicate imports exist.
# Look for duplicate imports
rg 'import.*from' src/components/ui/select.tsx | sort | uniq -d
# Check for unused imports
eslint src/components/ui/select.tsx --rule 'no-unused-vars:error'
Length of output: 228
Script:
#!/bin/bash
# Let's verify the imports usage in the file
rg -A 10 'import.*from' src/components/ui/select.tsx
# Check if these imported components are actually used in the file
rg 'CheckIcon|ChevronDownIcon|ChevronUpIcon|SelectPrimitive|cn' src/components/ui/select.tsx
Length of output: 3354
🧰 Tools
🪛 eslint
[error] 1-1: Replace *·as·React·from·"react" with {⏎··CheckIcon,⏎··ChevronDownIcon,⏎··ChevronUpIcon,⏎}·from·"@radix-ui/react-icons";
(prettier/prettier)
[error] 2-2: Insert ;
(prettier/prettier)
[error] 3-3: Replace {·cn·}·from·"@/lib/utils" with *·as·React·from·"react";⏎
(prettier/prettier)
[error] 4-4: Replace CheckIcon,·ChevronDownIcon,·ChevronUpIcon·}·from·"@radix-ui/react-icons" with cn·}·from·"@/lib/utils";
(prettier/prettier)
src/components/Questionnaire/show.tsx (1)
117-119: Unreachable code detected
The commented-out code within the handleFormSubmit function makes the subsequent lines unreachable. Since the function is not in use, removing it, as suggested earlier, will resolve this issue.
🧰 Tools
🪛 Biome (1.9.4)
[error] 117-119: This code is unreachable
(lint/correctness/noUnreachable)
src/components/Questionnaire/QuestionTypes/QuestionInput.tsx (1)
127-131: Avoid redundant computation of isQuestionEnabled
The isQuestionEnabled function is called multiple times and recalculated within the renderSingleInput function. Consider computing it once and passing it as a parameter or using useMemo for optimization.
[performance]
Apply this diff:
const isEnabled = isQuestionEnabled();
const renderSingleInput = (responseValue: ResponseValue, index: number) => {
- const isEnabled = isQuestionEnabled();
const commonProps = {
disabled: !isEnabled || disabled,
"aria-hidden": !isEnabled,src/components/Questionnaire/QuestionTypes/MedicationQuestion.tsx (1)
1-4: Consider adding error boundary and loading states
Since this component will likely interact with medication data APIs, it should handle loading and error states appropriately.
src/types/questionnaire/question.ts (1)
40-66: Verify FHIR compliance and add documentation
The Question interface appears to follow FHIR Questionnaire structure but needs verification and documentation:
- Are all required FHIR fields included?
- Do optional fields align with FHIR specs?
- Missing JSDoc comments explaining field purposes
Consider splitting this into smaller interfaces for better maintainability:
interface QuestionBase {
id: string;
link_id: string;
text: string;
type: QuestionType;
}
interface QuestionValidation {
required?: boolean;
max_length?: number;
answer_constraint?: string;
}
interface QuestionBehavior {
disabled_display?: "hidden" | "protected";
repeats?: boolean;
read_only?: boolean;
}
export interface Question extends QuestionBase, QuestionValidation, QuestionBehavior {
// ... remaining fields
}src/components/Questionnaire/QuestionTypes/ChoiceQuestion.tsx (1)
39-59: 🛠️ Refactor suggestion
Enhance accessibility and user experience
The Select component needs accessibility improvements:
- Missing aria-label
- No error state handling
- No loading state
<Select
value={currentValue}
onValueChange={handleValueChange}
disabled={disabled}
+ aria-label={question.text}
>
<SelectTrigger className="w-full">
- <SelectValue placeholder="Select an option" />
+ <SelectValue placeholder={`Select ${question.text.toLowerCase()}`} />
</SelectTrigger>
- <SelectContent>
+ <SelectContent className="max-h-[300px]">
{options.map((option: AnswerOption) => (
<SelectItem
key={option.value.toString()}
value={option.value.toString()}
+ aria-label={option.value}
>
{option.value}
</SelectItem>
))}
</SelectContent>
</Select>Likely invalid or redundant comment.
src/components/ui/tabs.tsx (1)
1-53: LGTM! Well-structured tabs implementation
Great implementation using Radix UI primitives with:
- Built-in accessibility features
- Proper TypeScript types
- Comprehensive styling including focus states
- Clean component composition
src/components/Questionnaire/QuestionnaireSearch.tsx (2)
16-19: LGTM! Well-defined props interface
The interface is properly typed with a clear callback signature and optional disabled flag.
74-97: LGTM! Clean implementation of results rendering
The code handles both empty and populated states appropriately, with clear visual feedback for users.
src/components/Questionnaire/index.tsx (1)
12-21: LGTM! Clean implementation of data fetching and loading state
The component properly handles loading states and data fetching with clear separation of concerns.
src/components/ui/command.tsx (1)
Line range hint 1-150: Address formatting inconsistencies
The file has multiple formatting inconsistencies flagged by the linter. Consider running prettier to automatically fix these issues.
🧰 Tools
🪛 eslint
[error] 16-16: Insert ,
(prettier/prettier)
[error] 20-20: Insert ;
(prettier/prettier)
[error] 21-21: Insert ;
(prettier/prettier)
[error] 32-32: Insert ;
(prettier/prettier)
[error] 33-33: Insert ;
(prettier/prettier)
src/Routers/AppRouter.tsx (1)
31-32: LGTM: Import statements are properly organized
The new imports for questionnaire components follow the existing import pattern and use appropriate relative paths.
src/Routers/routes/ConsultationRoutes.tsx (2)
14-14: LGTM: Import statement is properly placed
The import for EncounterQuestionnaire follows the existing pattern for patient-related components.
163-179: Verify error handling for invalid slugs
Ensure that the EncounterQuestionnaire component properly handles cases where an invalid slug is provided.
✅ Verification successful
Error handling for invalid slugs is properly implemented
The codebase shows comprehensive error handling for invalid questionnaire slugs:
- The QuestionnaireForm component handles loading errors through the
questionnaireErrorstate - Invalid submissions are caught and processed through
handleSubmissionError - Form-level and question-specific validation errors are properly managed
- The component supports both optional and required slug scenarios
- Error states are properly propagated to the UI through the error state management
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for error handling in EncounterQuestionnaire component
ast-grep --pattern 'function EncounterQuestionnaire($_) {
$$$
try {
$$$
} catch {
$$$
}
$$$
}'
Length of output: 114
Script:
#!/bin/bash
# Let's first locate the EncounterQuestionnaire component file
fd "EncounterQuestionnaire" --type f
# Then let's examine its content for error handling patterns
rg "catch|error|invalid|throw" -A 5 -B 5 $(fd "EncounterQuestionnaire" --type f)
# Also search for any validation or error state handling related to questionnaire slugs
rg "questionnaireSlug|slug" -A 5 -B 5 $(fd "EncounterQuestionnaire" --type f)
Length of output: 1442
Script:
#!/bin/bash
# Let's examine the QuestionnaireForm component for slug validation
fd "QuestionnaireForm" --type f
# Check error handling in QuestionnaireForm related to slugs
rg "questionnaireSlug|slug|error|invalid" -A 5 -B 5 $(fd "QuestionnaireForm" --type f)
Length of output: 4442
src/components/ui/popover.tsx (1)
Line range hint 1-31: LGTM!
The changes are formatting-only and don't affect the component's functionality.
🧰 Tools
🪛 eslint
[error] 1-2: Replace import·*·as·React·from·"react"⏎import·*·as·PopoverPrimitive·from·"@radix-ui/react-popover" with import·*·as·PopoverPrimitive·from·"@radix-ui/react-popover";⏎import·*·as·React·from·"react";
(prettier/prettier)
[error] 4-4: Insert ;
(prettier/prettier)
[error] 6-6: Insert ;
(prettier/prettier)
[error] 8-8: Insert ;
(prettier/prettier)
[error] 10-10: Insert ;
(prettier/prettier)
src/components/ui/dialog.tsx (2)
Line range hint 3-121: LGTM!
The remaining changes are formatting-only and don't affect the component's functionality.
🧰 Tools
🪛 eslint
[error] 1-2: Replace ⏎ with ;
(prettier/prettier)
[error] 3-3: Delete import·*·as·React·from·"react"
(prettier/prettier)
[error] 4-4: Insert ;
(prettier/prettier)
[error] 5-5: Replace cn·}·from·"@/lib/utils" with Cross2Icon·}·from·"@radix-ui/react-icons";
(prettier/prettier)
[error] 6-6: Replace {·Cross2Icon·}·from·"@radix-ui/react-icons" with *·as·React·from·"react";⏎⏎import·{·cn·}·from·"@/lib/utils";
(prettier/prettier)
[error] 8-8: Insert ;
(prettier/prettier)
[error] 10-10: Insert ;
(prettier/prettier)
[error] 12-12: Insert ;
(prettier/prettier)
[error] 14-14: Insert ;
(prettier/prettier)
1-1: Verify client-side only usage
The "use client" directive indicates this component should only be used on the client side.
✅ Verification successful
Dialog component's client-side usage is correctly enforced
The Dialog component from src/components/ui/dialog.tsx is correctly marked with "use client" directive as it uses client-side interactivity features. The component is being used appropriately through the DialogModal wrapper component which is built on top of @headlessui/react Dialog primitives, which are inherently client-side components. No server components were found importing or using this Dialog component directly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if Dialog component is imported in any server components
rg -g '*.{ts,tsx}' '"use server".*import.*Dialog|import.*Dialog.*"use server"'
Length of output: 80
Script:
#!/bin/bash
# Let's try a different approach to check Dialog usage
# First, find all server components
rg -l "use server" --type ts --type tsx
# Then find all files that import Dialog
rg -l "import.*Dialog.*from.*dialog'" --type ts --type tsx
# Also check for any direct usage of Dialog in server components
rg -C 2 "<Dialog" $(rg -l "use server" --type ts --type tsx)
Length of output: 18713
Script:
#!/bin/bash
# Let's check if any of these files using Dialog are server components
rg -l "use server" src/components/Facility/StaffCountCard.tsx src/components/Facility/PatientNoteCard.tsx src/components/Facility/LocationManagement.tsx src/components/Facility/FacilityStaffList.tsx src/components/Facility/DischargeSummaryModal.tsx src/components/Facility/DischargeModal.tsx src/components/Facility/ConsultationCard.tsx src/components/Facility/BedTypeCard.tsx src/components/Facility/FacilityCard.tsx src/components/Assets/AssetServiceEditModal.tsx src/components/Facility/MinQuantityRequiredModal.tsx src/components/Assets/AssetImportModal.tsx
# Also check if DialogModal is a client component
rg "use client" -A 5 -B 5 $(rg -l "DialogModal")
Length of output: 2206
Script:
#!/bin/bash
# Let's check the DialogModal component implementation
rg -l "DialogModal" src/components/Common/
# Then check its content
cat $(rg -l "DialogModal" src/components/Common/Dialog.tsx)
Length of output: 2925
🧰 Tools
🪛 eslint
[error] 1-2: Replace ⏎ with ;
(prettier/prettier)
| const handleFormSubmit = async (values: QuestionnaireResponse[]) => { | ||
| try { | ||
| // await request(routes.questionnaire.submit, { | ||
| // pathParams: { id }, | ||
| // body: { | ||
| // encounter: "some-encounter-id", | ||
| // responses: values.map((v) => ({ | ||
| // question_id: v.id, | ||
| // value: v.value, | ||
| // note: v.note, | ||
| // bodysite: v.bodysite, | ||
| // method: v.method, | ||
| // })), | ||
| // }, | ||
| // }); | ||
| } catch (error) { | ||
| console.error("Failed to submit form:", error); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Remove unused function handleFormSubmit
The handleFormSubmit function is defined but never used, leading to unused variable warnings from static analysis tools.
Apply this diff to remove the unused function:
- const handleFormSubmit = async (values: QuestionnaireResponse[]) => {
- try {
- // Commented-out code
- } catch (error) {
- console.error("Failed to submit form:", error);
- }
- };Alternatively, if this function is intended for future use, consider adding a // TODO comment to indicate that it is a placeholder.
🧰 Tools
🪛 Biome (1.9.4)
[error] 117-119: This code is unreachable
(lint/correctness/noUnreachable)
🪛 eslint
[error] 102-102: 'handleFormSubmit' is assigned a value but never used. Allowed unused vars must match /^_/u.
(@typescript-eslint/no-unused-vars)
[error] 102-102: 'values' is defined but never used. Allowed unused args must match /^_/u.
(@typescript-eslint/no-unused-vars)
🪛 GitHub Check: lint
[failure] 102-102:
'handleFormSubmit' is assigned a value but never used. Allowed unused vars must match /^_/u
[failure] 102-102:
'values' is defined but never used. Allowed unused args must match /^_/u
|
|
||
| import Loading from "@/components/Common/Loading"; | ||
|
|
||
| import { Error, Success } from "@/Utils/Notifications"; |
There was a problem hiding this comment.
Rename Error and Success to avoid shadowing global variables
The variables Error and Success on line 10 shadow the global Error object, which can lead to confusion and unexpected behavior.
Apply this diff to rename the variables:
-import { Error, Success } from "@/Utils/Notifications";
+import { Error as NotifyError, Success as NotifySuccess } from "@/Utils/Notifications";Remember to update all references to these variables within the file accordingly.
📝 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.
| import { Error, Success } from "@/Utils/Notifications"; | |
| import { Error as NotifyError, Success as NotifySuccess } from "@/Utils/Notifications"; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
| export type SubjectType = "patient"; | ||
|
|
||
| export type QuestionStatus = "active" | "retired" | "draft"; | ||
|
|
||
| export type QuestionnaireType = | ||
| | "custom" | ||
| | "allergy_intolerance" | ||
| | "medication_request" | ||
| | "medication_statement" | ||
| | "immunization"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance type safety with FHIR-compliant enums
The current string literal types could be more robust and FHIR-compliant.
Consider using enums for better type safety and FHIR compliance:
-export type SubjectType = "patient";
+export enum SubjectType {
+ Patient = "Patient",
+ Group = "Group",
+ Device = "Device"
+}
-export type QuestionStatus = "active" | "retired" | "draft";
+export enum QuestionStatus {
+ Draft = "draft",
+ Active = "active",
+ Retired = "retired",
+ Published = "published"
+}
-export type QuestionnaireType =
- | "custom"
- | "allergy_intolerance"
- | "medication_request"
- | "medication_statement"
- | "immunization";
+export enum QuestionnaireType {
+ Custom = "custom",
+ AllergyIntolerance = "allergy_intolerance",
+ MedicationRequest = "medication_request",
+ MedicationStatement = "medication_statement",
+ Immunization = "immunization"
+}📝 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 type SubjectType = "patient"; | |
| export type QuestionStatus = "active" | "retired" | "draft"; | |
| export type QuestionnaireType = | |
| | "custom" | |
| | "allergy_intolerance" | |
| | "medication_request" | |
| | "medication_statement" | |
| | "immunization"; | |
| export enum SubjectType { | |
| Patient = "Patient", | |
| Group = "Group", | |
| Device = "Device" | |
| } | |
| export enum QuestionStatus { | |
| Draft = "draft", | |
| Active = "active", | |
| Retired = "retired", | |
| Published = "published" | |
| } | |
| export enum QuestionnaireType { | |
| Custom = "custom", | |
| AllergyIntolerance = "allergy_intolerance", | |
| MedicationRequest = "medication_request", | |
| MedicationStatement = "medication_statement", | |
| Immunization = "immunization" | |
| } |
| interface AllergyQuestionProps { | ||
| question: Question; | ||
| questionnaireResponse: QuestionnaireResponse; | ||
| updateQuestionnaireResponseCB: (response: QuestionnaireResponse) => void; | ||
| disabled?: boolean; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation prop and error handling to interface
The props interface could be enhanced to handle validation and errors.
Consider adding these props:
interface AllergyQuestionProps {
question: Question;
questionnaireResponse: QuestionnaireResponse;
updateQuestionnaireResponseCB: (response: QuestionnaireResponse) => void;
disabled?: boolean;
+ validation?: {
+ required?: boolean;
+ minAnswers?: number;
+ maxAnswers?: number;
+ };
+ error?: string;
+ onError?: (error: string) => void;
}📝 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.
| interface AllergyQuestionProps { | |
| question: Question; | |
| questionnaireResponse: QuestionnaireResponse; | |
| updateQuestionnaireResponseCB: (response: QuestionnaireResponse) => void; | |
| disabled?: boolean; | |
| } | |
| interface AllergyQuestionProps { | |
| question: Question; | |
| questionnaireResponse: QuestionnaireResponse; | |
| updateQuestionnaireResponseCB: (response: QuestionnaireResponse) => void; | |
| disabled?: boolean; | |
| validation?: { | |
| required?: boolean; | |
| minAnswers?: number; | |
| maxAnswers?: number; | |
| }; | |
| error?: string; | |
| onError?: (error: string) => void; | |
| } |
| export function AllergyQuestion({ question }: AllergyQuestionProps) { | ||
| return ( | ||
| <div className="space-y-4"> | ||
| <Label>{question.text}</Label> | ||
| <div className="rounded-lg border border-dashed border-gray-300 bg-gray-50 p-4 text-center text-sm text-gray-500"> | ||
| Allergy component to be implemented | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Implement proper FHIR AllergyIntolerance structure
The current implementation is a placeholder and needs to be updated to handle FHIR AllergyIntolerance resources properly.
The component should:
- Handle FHIR AllergyIntolerance fields (type, category, criticality, etc.)
- Implement proper error handling
- Support multiple allergies
- Include proper validation
Would you like me to provide a complete implementation that follows FHIR specifications?
Also, consider adding proper aria labels for accessibility:
- <div className="space-y-4">
+ <div className="space-y-4" role="group" aria-labelledby={`allergy-question-${question.id}`}>
- <Label>{question.text}</Label>
+ <Label id={`allergy-question-${question.id}`}>{question.text}</Label>Committable suggestion skipped: line range outside the PR's diff.
| <Label className="text-lg font-semibold text-green-600"> | ||
| {question.text} | ||
| </Label> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance accessibility for question group labels
The Label component should be associated with its form controls using aria-labelledby.
- <Label className="text-lg font-semibold text-green-600">
+ <Label
+ id={`group-label-${question.id}`}
+ className="text-lg font-semibold text-green-600"
+ >
{question.text}
</Label>Then update the questions container:
- <div className="space-y-2">
+ <div
+ className="space-y-2"
+ role="group"
+ aria-labelledby={`group-label-${question.id}`}
+ >Committable suggestion skipped: line range outside the PR's diff.
| const { | ||
| data: questionnaireList, | ||
| loading: listLoading, | ||
| error: _listError, | ||
| } = useQuery(routes.questionnaire.list); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider enhancing error handling
The error from useQuery (_listError) is currently ignored. Consider displaying an error state to improve user experience when the API request fails.
- error: _listError,
+ error: listError,
+ ...
+ if (listError) {
+ return (
+ <div className="text-red-500">
+ Failed to load questionnaires. Please try again.
+ </div>
+ );
+ }Committable suggestion skipped: line range outside the PR's diff.
| <div className="overflow-hidden rounded-lg bg-white shadow"> | ||
| <table className="min-w-full divide-y divide-gray-200"> | ||
| <thead className="bg-gray-50"> | ||
| <tr> | ||
| <th className="px-6 py-3 text-left text-xs font-medium uppercase tracking-wider text-gray-500"> | ||
| Title | ||
| </th> | ||
| <th className="px-6 py-3 text-left text-xs font-medium uppercase tracking-wider text-gray-500"> | ||
| Description | ||
| </th> | ||
| <th className="px-6 py-3 text-left text-xs font-medium uppercase tracking-wider text-gray-500"> | ||
| Status | ||
| </th> | ||
| <th className="px-6 py-3 text-left text-xs font-medium uppercase tracking-wider text-gray-500"> | ||
| Version | ||
| </th> | ||
| </tr> | ||
| </thead> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance table accessibility
The table header cells should include the scope attribute for better accessibility. Also, consider adding a caption for screen readers.
<table className="min-w-full divide-y divide-gray-200">
+ <caption className="sr-only">List of questionnaires</caption>
<thead className="bg-gray-50">
<tr>
- <th className="px-6 py-3 text-left text-xs font-medium uppercase tracking-wider text-gray-500">
+ <th scope="col" className="px-6 py-3 text-left text-xs font-medium uppercase tracking-wider text-gray-500">
Title
</th>Committable suggestion skipped: line range outside the PR's diff.
| @@ -60,11 +60,13 @@ | |||
| "@pnotify/mobile": "^5.2.0", | |||
| "@radix-ui/react-dialog": "^1.1.2", | |||
| "@radix-ui/react-dropdown-menu": "^2.1.2", | |||
| "@radix-ui/react-icons": "^1.3.2", | |||
There was a problem hiding this comment.
💡 Codebase verification
Warning: Cannot remove @radix-ui/react-icons as it's actively used
The package is currently imported in multiple UI components:
- src/components/ui/select.tsx (CheckIcon, ChevronDownIcon, ChevronUpIcon)
- src/components/ui/dialog.tsx (Cross2Icon)
- src/components/ui/command.tsx (MagnifyingGlassIcon)
- src/components/ui/toast.tsx (Cross2Icon)
- src/components/ui/dropdown-menu.tsx
Removing this package will break these components. Either keep the dependency or replace these icon imports with alternatives before removal.
🔗 Analysis chain
Verify icon usage after removing @radix-ui/react-icons
The removal of @radix-ui/react-icons might impact existing icon usage in the codebase. Let's verify there are no remaining dependencies on this package.
Also applies to: 67-67, 69-69
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining usage of @radix-ui/react-icons
rg "@radix-ui/react-icons" --type ts --type tsx
Length of output: 75
Script:
#!/bin/bash
# Search for any remaining usage of @radix-ui/react-icons with correct file extensions
rg "@radix-ui/react-icons" -t js -t jsx -t ts
Length of output: 73
Script:
#!/bin/bash
# Search for any remaining usage of @radix-ui/react-icons without file type restrictions
rg "@radix-ui/react-icons"
Length of output: 1567
src/Utils/request/api.tsx
Outdated
| // Questionnaire Routes | ||
| questionnaire: { | ||
| list: { | ||
| path: "/api/v1/questionnaire/", | ||
| method: "GET", | ||
| TRes: Type<PaginatedResponse<QuestionnaireDetail>>(), | ||
| }, | ||
|
|
||
| detail: { | ||
| path: "/api/v1/questionnaire/{id}/", | ||
| method: "GET", | ||
| TRes: Type<QuestionnaireDetail>(), | ||
| }, | ||
|
|
||
| create: { | ||
| path: "/api/v1/questionnaire/", | ||
| method: "POST", | ||
| TRes: Type<QuestionnaireDetail>(), | ||
| TBody: Type<Partial<QuestionnaireDetail>>(), | ||
| }, | ||
|
|
||
| update: { | ||
| path: "/api/v1/questionnaire/{id}/", | ||
| method: "PUT", | ||
| TRes: Type<QuestionnaireDetail>(), | ||
| TBody: Type<QuestionnaireDetail>(), | ||
| }, | ||
|
|
||
| partialUpdate: { | ||
| path: "/api/v1/questionnaire/{id}/", | ||
| method: "PATCH", | ||
| TRes: Type<QuestionnaireDetail>(), | ||
| TBody: Type<Partial<QuestionnaireDetail>>(), | ||
| }, | ||
|
|
||
| delete: { | ||
| path: "/api/v1/questionnaire/{id}/", | ||
| method: "DELETE", | ||
| TRes: Type<Record<string, never>>(), | ||
| }, | ||
|
|
||
| submit: { | ||
| path: "/api/v1/questionnaire/{id}/submit/", | ||
| method: "POST", | ||
| TRes: Type<Record<string, never>>(), | ||
| TBody: Type<{ | ||
| resource_id: string; | ||
| encounter: string; | ||
| responses: Array<{ | ||
| question_id: string; | ||
| value: string | number | boolean; | ||
| note?: string; | ||
| bodysite?: string; | ||
| method?: string; | ||
| }>; | ||
| }>(), | ||
| }, | ||
| }, |
There was a problem hiding this comment.
💡 Codebase verification
Based on the searches performed, I can now generate the final response since we've thoroughly checked for:
- Existing validation code for questionnaire responses
- Validation schemas in the questionnaire types directory
- Response handling in components
The lack of results from these searches indicates there's no existing validation for questionnaire responses, confirming the review comment's concern.
Add validation for questionnaire response values
The questionnaire submit endpoint accepts complex response data without any validation. Add runtime validation to ensure:
- Required fields are present
- Response values match their expected types (string/number/boolean)
- Optional fields (note, bodysite, method) are properly typed when present
Example locations that need validation:
src/types/questionnaire/form.ts: Add Zod/Yup schema for response validationsrc/components/Questionnaire/QuestionnaireForm.tsx: Implement validation before submission
🔗 Analysis chain
Consider adding validation for questionnaire responses
The submit endpoint accepts complex response data but doesn't specify validation requirements in the type definition.
Consider adding:
- Response validation using Zod/Yup schemas
- Type guards for response values since they can be string/number/boolean
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if validation types/schemas exist for questionnaire responses
rg -g '*.{ts,tsx}' -A 5 'questionnaire.*validation|questionnaire.*schema|questionnaire.*validate'
Length of output: 99
Script:
#!/bin/bash
# Let's check for any validation related code in the codebase, particularly around questionnaires
rg -g '*.{ts,tsx}' -B 2 -A 5 'interface.*Questionnaire|type.*Questionnaire'
# Also search for any validation utilities or schemas in the codebase
rg -g '*.{ts,tsx}' -B 2 -A 5 'zod|yup|validate|schema'
# Let's also check how questionnaire responses are handled
ast-grep --pattern 'function $FUNC($$$) {
$$$
questionnaire$$$submit($$$)
$$$
}'
Length of output: 66387
Script:
#!/bin/bash
# Let's check for any validation related code specifically for questionnaire responses
rg -g '*.{ts,tsx}' -B 2 -A 5 'QuestionnaireResponse.*validate|QuestionnaireResponse.*schema'
# Let's also check the types/questionnaire directory for any validation related code
rg -g '*.{ts,tsx}' -B 2 -A 5 'validate|schema' src/types/questionnaire/
# Let's look at how questionnaire responses are handled in the form component
ast-grep --pattern 'function $FUNC($$$) {
$$$
responses: QuestionnaireResponse$$$
$$$
}'
Length of output: 265
|
👋 Hi, @bodhish, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (12)
src/components/Common/SearchByMultipleFields.tsx (3)
161-164: Consider adding additional type guard for more robust event handlingWhile the current implementation works, it could be made more robust by handling edge cases where event.target exists but value is undefined.
Consider this safer implementation:
const handleSearchChange = useCallback((event: EventType) => { - const value = "target" in event ? event.target.value : event.value; + const value = "target" in event && event.target?.value !== undefined + ? event.target.value + : event.value; setSearchValue(value); }, []);
170-175: Consider adding validation before triggering search for phone numbersWhen handling phone number inputs, it would be beneficial to validate the input before triggering the search to prevent unnecessary API calls with invalid numbers.
Consider adding validation in the useEffect that triggers the search:
useEffect(() => { + if (selectedOption.type === 'phone' && error) { + return; + } const timeout = setTimeout( () => selectedOption.value !== searchValue && onSearch(selectedOption.key, searchValue), 1000, ); return () => clearTimeout(timeout); - }, [searchValue]); + }, [searchValue, selectedOption.type, error]);
Line range hint
52-175: Consider architectural improvements for better maintainabilityWhile the current implementation is solid, consider these architectural improvements:
- Extract the debounced search logic into a custom hook:
function useDebounceSearch(callback: (key: string, value: string) => void, delay: number) { return useCallback((key: string, value: string) => { const handler = setTimeout(() => callback(key, value), delay); return () => clearTimeout(handler); }, [callback, delay]); }
- Consider extracting keyboard event handling into a custom hook to improve readability and reusability.
src/components/Questionnaire/show.tsx (2)
25-60: Consider adding memoization for better performance with deep nesting.The recursive implementation of
QuestionItemis well-structured, but it might benefit from memoization when dealing with deeply nested question groups.Consider wrapping the component with
React.memo:-function QuestionItem({ +const QuestionItem = React.memo(function QuestionItem({ question, depth = 0, -}: { +}: { question: Question; depth?: number; -}) { +}) { // ... component implementation ... -} +});
120-127: Enhance tab accessibility with ARIA labels.While the tab implementation is clean, it could benefit from additional accessibility attributes.
Consider adding ARIA labels:
<Tabs value={activeTab} onValueChange={(value: string) => setActiveTab(value as TabValue)} + aria-label="Questionnaire views" > - <TabsList className="mb-4"> + <TabsList className="mb-4" aria-label="View options"> <TabsTrigger value="details">Details</TabsTrigger> <TabsTrigger value="preview">Preview Form</TabsTrigger> </TabsList>src/types/questionnaire/question.ts (3)
3-16: Consider adding missing FHIR questionnaire item typesThe
QuestionTypeunion includes most common types but is missing some standard FHIR questionnaire item types that might be needed in the future:
attachment- For file uploadsquantity- For measurements with unitsreference- For references to other FHIR resourcesexport type QuestionType = | "group" | "display" | "boolean" | "decimal" | "integer" | "date" | "dateTime" | "time" | "string" | "text" | "url" | "choice" | "structured" + | "attachment" + | "quantity" + | "reference";
18-20: Add JSDoc comments explaining the structured question typesThe
StructuredQuestionTypeseems to handle specific FHIR resource types, but its purpose and usage aren't immediately clear. Consider adding documentation.+/** + * Defines types of structured data entry that map to specific FHIR resources. + * Used when the question requires capturing complex data structures rather than simple values. + */ export type StructuredQuestionType = | "allergy_intolerance" | "medication_request";
22-39: Excellent type-safe implementation of enabling conditionsThe implementation effectively ensures type safety for different operators and their corresponding value types. Consider these minor improvements:
+/** + * Type-safe definitions for numeric comparison operations + * @property operator - The comparison operator + * @property answer - The numeric value to compare against + */ type EnableWhenNumeric = { operator: "greater" | "less" | "greater_or_equals" | "less_or_equals"; - answer: number; + answer: number | null; }; +/** + * Type-safe definitions for boolean operations + * @property operator - The boolean operator + * @property answer - The boolean value to compare against + */ type EnableWhenBoolean = { operator: "exists" | "equals" | "not_equals"; - answer: boolean; + answer: boolean | null; }; +/** + * Type-safe definitions for string operations + * @property operator - The string comparison operator + * @property answer - The string value to compare against + */ type EnableWhenString = { operator: "equals" | "not_equals"; - answer: string; + answer: string | null; };src/components/Questionnaire/QuestionnaireForm.tsx (2)
280-281: Avoid unnecessary wrapping of componentsThe lines 280-281 return a
<div>that wraps a single child without adding any additional properties or styles. This extra<div>might introduce unwanted DOM nodes.Apply this diff to remove the unnecessary
<div>:- </div>If the wrapping is necessary in specific cases, consider adding comments to clarify its purpose.
283-286: Simplify conditional rendering for component visibilityThe check for
isEnabledandquestion.disabled_displaycan be simplified to improve readability.Apply this diff to simplify the condition:
- if (!isEnabled && question.disabled_display === "hidden") { - return null; - } + if (!isEnabled) { + if (question.disabled_display === "hidden") return null; + }This makes the conditional logic clearer and easier to maintain.
src/components/Questionnaire/QuestionTypes/QuestionInput.tsx (2)
47-93: Optimize theisQuestionEnabledfunctionThe
isQuestionEnabledfunction is recreated on every render. Consider usinguseMemoto memoize the result for performance improvement. Additionally, ensure that it handles all possible operator cases or provides a default behavior.Apply this diff to memoize the function:
+ import { useMemo } from "react"; - const isQuestionEnabled = () => { + const isQuestionEnabled = useMemo(() => { // function body - }; + }, [questionnaireResponses, question]);Make sure to include
questionnaireResponsesandquestionin the dependency array.
178-185: Avoid unnecessary nesting in theChoiceQuestioncomponentThe
ChoiceQuestioncomponent wraps the updated values within another object, which might not be necessary. Review if this structure can be simplified to prevent redundant code.Review the data structure being passed to
ChoiceQuestionand adjust accordingly to simplify the code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
src/components/Common/SearchByMultipleFields.tsx(2 hunks)src/components/Questionnaire/QuestionTypes/QuestionInput.tsx(1 hunks)src/components/Questionnaire/QuestionnaireForm.tsx(1 hunks)src/components/Questionnaire/show.tsx(1 hunks)src/types/questionnaire/question.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Questionnaire/QuestionnaireForm.tsx
[error] 10-10: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (8)
src/components/Common/SearchByMultipleFields.tsx (1)
52-52: LGTM: Well-defined event type union
The EventType definition properly handles both React input events and custom value objects, making the component more flexible.
src/components/Questionnaire/show.tsx (2)
1-24: LGTM! Well-organized imports and type definitions.
The imports are logically grouped, and the TypeScript types are clearly defined. The use of a string literal type for tabs is a good practice for type safety.
194-197:
Remove hardcoded IDs in preview form.
The resourceId and encounterId are hardcoded with placeholder values. This could cause issues in production.
Let's verify the usage of these IDs:
Consider one of these approaches:
- resourceId={"some_patient_id"}
- encounterId={"some_encounter_id"}
+ resourceId={currentPatientId} // Pass from props or context
+ encounterId={currentEncounterId} // Pass from props or context✅ Verification successful
Hardcoded IDs in preview form should be replaced with actual values
The verification confirms that resourceId and encounterId are properly used in other components like EncounterQuestionnaire.tsx where they are passed meaningful values (patientId and consultationId). The hardcoded values are only present in the preview form in show.tsx. Consider:
- Replace hardcoded values in
src/components/Questionnaire/show.tsxwith actual patient and encounter IDs from the context or props - Follow the pattern used in
src/components/Patient/EncounterQuestionnaire.tsxwhere real IDs are passed
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for patterns of resourceId and encounterId usage
rg -A 5 "resourceId|encounterId" --type typescript
Length of output: 87
Script:
#!/bin/bash
# Search for patterns of resourceId and encounterId usage with broader file extensions
rg -A 5 "resourceId|encounterId" --type-add 'ts:*.{ts,tsx}' --type ts
Length of output: 2794
Script:
#!/bin/bash
# Find all TypeScript/React files that might contain these patterns
fd -e ts -e tsx | xargs grep -l "resourceId\|encounterId"
Length of output: 198
Script:
#!/bin/bash
# Use ast-grep to find props or interface definitions containing these fields
ast-grep --pattern 'interface $_ {
$$$
resourceId: $_
$$$
}'
ast-grep --pattern 'interface $_ {
$$$
encounterId: $_
$$$
}'
Length of output: 1232
src/types/questionnaire/question.ts (1)
46-72: 🛠️ Refactor suggestion
Improve type safety and documentation for Question interface
The interface is comprehensive but could benefit from better type constraints and documentation.
+/**
+ * Represents a questionnaire item following FHIR specifications with custom extensions
+ */
export interface Question {
id: string;
link_id: string;
code?: Code;
text: string;
description?: string;
type: QuestionType;
structured_type?: StructuredQuestionType;
required?: boolean;
collect_time?: boolean;
collect_performer?: boolean;
collect_body_site?: boolean;
collect_method?: boolean;
enable_when?: EnableWhen[];
- enable_behavior?: "all" | "any";
+ enable_behavior?: EnableWhen[] extends never[] ? never : "all" | "any";
disabled_display?: "hidden" | "protected";
repeats?: boolean;
read_only?: boolean;
- max_length?: number;
+ max_length?: type extends "string" | "text" | "url" ? number : never;
answer_constraint?: string;
- answer_option?: AnswerOption[];
+ answer_option?: AnswerOption<typeof type>[];
answer_value_set?: string;
answer_unit?: Code;
is_observation?: boolean;
questions?: Question[];
formula?: string;
}The changes above:
- Add interface documentation
- Make
enable_behaviorrequired whenenable_whenis present - Restrict
max_lengthto text-based questions only - Ensure answer options match the question type
Let's verify the usage of these types in the codebase:
src/components/Questionnaire/QuestionnaireForm.tsx (2)
10-10: Rename Error and Success to avoid shadowing global identifiers
The imports Error and Success on line 10 shadow the global Error object in JavaScript, which can lead to unexpected behaviors and make debugging more difficult. It's advisable to rename these imports to avoid this issue.
Apply this diff to rename the variables:
- import { Error, Success } from "@/Utils/Notifications";
+ import { Error as NotifyError, Success as NotifySuccess } from "@/Utils/Notifications";Remember to update all references to Error and Success within this file accordingly.
🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
94-95:
Add a return statement after rendering the error alert
After rendering the error alert, the function should return to prevent further execution. Currently, the code may continue executing, which might lead to unintended behavior.
Apply this diff to add a return statement:
</Alert>
+ );
+ return;
}Likely invalid or redundant comment.
src/components/Questionnaire/QuestionTypes/QuestionInput.tsx (2)
56-57:
Check for dependentValue existence before accessing value
On line 56, the code checks if (!dependentValue?.value), but if dependentValue is undefined, accessing .value could lead to an error. Update the condition to ensure safety.
Apply this diff to fix the condition:
- if (!dependentValue?.value) return false;
+ if (!dependentValue || dependentValue.value === undefined) return false;Likely invalid or redundant comment.
140-151: Handle potential null values in removeButton rendering
The removeButton variable is conditionally rendered, but if questionnaireResponse.values is not defined, it could cause an error. Ensure that questionnaireResponse.values is always an array before accessing its properties.
Run the following script to check all instances of questionnaireResponse.values:
Consider initializing values with an empty array if it's undefined.
src/types/questionnaire/question.ts
Outdated
| export interface AnswerOption { | ||
| value: string; | ||
| initialSelected?: boolean; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance type safety for answer options
The AnswerOption interface could be more type-safe by ensuring the value matches the question type it's used with.
+/**
+ * Represents a possible answer option for choice questions
+ * @property value - The option value, should match the question's type
+ * @property initialSelected - Whether this option should be pre-selected
+ */
-export interface AnswerOption {
+export interface AnswerOption<T extends QuestionType = QuestionType> {
- value: string;
+ value: T extends "boolean" ? boolean :
+ T extends "decimal" | "integer" ? number :
+ T extends "date" | "dateTime" ? string :
+ string;
initialSelected?: boolean;
}Committable suggestion skipped: line range outside the PR's diff.
| console.log(questionnaireError); | ||
| return ( |
There was a problem hiding this comment.
Avoid using console.log in production code
The console.log(questionnaireError); statement on line 86 can expose sensitive information and clutter the console in a production environment. It's better to handle errors gracefully or use a logging library that can be configured for different environments.
Apply this diff to remove the console statement:
- console.log(questionnaireError);If logging is necessary for debugging, consider using a logging utility that respects environment settings.
📝 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.
| console.log(questionnaireError); | |
| return ( | |
| return ( |
| const initializeResponses = ( | ||
| questions: Question[], | ||
| ): QuestionnaireResponse[] => { | ||
| const responses: QuestionnaireResponse[] = []; | ||
|
|
||
| const processQuestion = (q: Question) => { | ||
| if (q.type === "group" && q.questions) { | ||
| q.questions.forEach(processQuestion); | ||
| } else { | ||
| responses.push({ | ||
| question_id: q.id, | ||
| link_id: q.link_id, | ||
| values: [], | ||
| }); | ||
| } | ||
| }; | ||
|
|
||
| questions.forEach(processQuestion); | ||
| return responses; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move function initializeResponses outside of the component
The initializeResponses function is defined inside the QuestionnaireForm component, causing it to be recreated on every render, which is inefficient. Consider moving it outside the component or wrapping it with useCallback if it depends on props or state.
Apply this diff to move the function outside:
- const initializeResponses = (
- questions: Question[],
- ): QuestionnaireResponse[] => {
- // function body
- };Place it above the component definition to make it a standalone function.
Committable suggestion skipped: line range outside the PR's diff.
| const handleSubmit = async () => { | ||
| if (hasErrors) return; | ||
|
|
||
| const response = await submitBatch({ | ||
| body: { | ||
| requests: questionnaireForms.map((form) => ({ | ||
| url: `/api/v1/questionnaire/${form.questionnaire.slug}/submit/`, | ||
| method: "POST", | ||
| body: { | ||
| resource_id: resourceId, | ||
| encounter: encounterId, | ||
| results: form.responses | ||
| .filter((response) => response.values.length > 0) | ||
| .map((response) => ({ | ||
| question_id: response.question_id, | ||
| values: response.values.map((value) => ({ | ||
| ...(value.code | ||
| ? { code: value.code } | ||
| : { value: String(value.value || "") }), | ||
| })), | ||
| note: response.note, | ||
| body_site: response.body_site, | ||
| method: response.method, | ||
| })), | ||
| }, | ||
| })), | ||
| }, | ||
| }); | ||
|
|
||
| if (!response.data) { | ||
| if (response.error) { | ||
| handleSubmissionError( | ||
| response.error.results as ValidationErrorResponse[], | ||
| ); | ||
| Error({ msg: "Failed to submit questionnaire" }); | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Based on the gathered information, I can now generate the final response:
Add comprehensive error handling for submitBatch response
The current error handling only checks for response.data and response.error, but the type definitions show that the response should contain a results array of type BatchResponseResult. Add proper error handling:
- Handle case when response is undefined
- Check for
resultsarray in the response - Provide appropriate user feedback for each error scenario
if (!response) {
Error({ msg: "Failed to submit questionnaire: Network error" });
return;
}
if (!response.data?.results) {
Error({ msg: "Failed to submit questionnaire: Invalid response format" });
return;
}
if (response.error) {
handleSubmissionError(response.error.results as ValidationErrorResponse[]);
Error({ msg: "Failed to submit questionnaire" });
return;
}🔗 Analysis chain
Handle potential errors from the submitBatch call
The handleSubmit function assumes that response.data or response.error will always be present, but there might be cases where the response is undefined or has unexpected properties. Add error handling to manage such cases and provide feedback to the user.
Run the following script to check the usage of submitBatch:
Ensure that the code appropriately handles all possible outcomes from the submitBatch function.
Let me gather more information about the submitBatch implementation and error handling patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all possible response scenarios from submitBatch are handled.
# Search for submitBatch usage
rg "const response = await submitBatch\(" -A10
# Check for handling of undefined or unexpected responses
Length of output: 1046
Script:
#!/bin/bash
# Search for submitBatch implementation
ast-grep --pattern 'const submitBatch = $_'
# Search for Error component/function usage pattern
rg "Error\(\{" -A2
# Search for any type definitions related to submitBatch response
rg "type.*Response.*=" -A5
Length of output: 26357
| return null; | ||
| } | ||
|
|
||
| const error = errors.find((e) => e.question_id === question.id)?.error; |
There was a problem hiding this comment.
Avoid potential undefined error when accessing errors.find
When accessing errors.find, ensure that errors is defined and is an array. If errors can be undefined, it may cause a runtime error.
Apply this diff to add a default value:
- const error = errors.find((e) => e.question_id === question.id)?.error;
+ const error = errors?.find((e) => e.question_id === question.id)?.error || "";📝 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.
| const error = errors.find((e) => e.question_id === question.id)?.error; | |
| const error = errors?.find((e) => e.question_id === question.id)?.error || ""; |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (2)
src/components/ui/autocomplete.tsx (2)
48-49: Handle Display of Selected Value More RobustlyWhen displaying the selected value, the current implementation searches for the value in the
optionsarray:{value ? options.find((option) => option === value) : placeholder}If
optionsis an array of objects or if the value might not be found, this could result inundefinedbeing displayed.Consider directly displaying the
valueor handling the case where the value is not in the options:-{value ? options.find((option) => option === value) : placeholder} +{value || placeholder}Or, if
optionsmight contain different data structures, ensure that the display logic accounts for that.
58-75: Optimize Rendering by Adding Keys toCommandGroupEach
CommandItemhas akeyprop, which is good. However, wrapping them in aCommandGroupwithout akeymay cause React to throw a key warning if there are multiple groups rendered conditionally.Add a
keytoCommandGroupif you are rendering multiple groups dynamically.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
package.json(2 hunks)src/Utils/request/api.tsx(2 hunks)src/Utils/request/types.ts(2 hunks)src/Utils/request/useQuery.ts(1 hunks)src/components/Questionnaire/QuestionTypes/MedicationQuestion.tsx(1 hunks)src/components/ui/autocomplete.tsx(1 hunks)src/components/ui/select.tsx(1 hunks)src/types/emr/medication-request.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🔇 Additional comments (9)
src/Utils/request/api.tsx (2)
1424-1480: Add validation for questionnaire response values
As previously noted, the questionnaire.submit endpoint accepts complex response data without validation. To ensure data integrity, please add runtime validation for:
- Presence of required fields
- Correct data types for response values (
string,number,boolean) - Proper typing of optional fields (
note,bodysite,method)
1424-1480: New questionnaire routes are well-defined
The added API routes for questionnaires are consistent with the existing API structure and enhance the application's capabilities.
src/Utils/request/types.ts (1)
11-13: Approve updates to QueryRoute interface
The inclusion of the generic parameter TBody and the expanded method property to support multiple HTTP methods enhance flexibility and align with the new API requirements.
Also applies to: 22-22
src/Utils/request/useQuery.ts (1)
17-18: Approve update to useQuery function signature
The addition of the TBody generic parameter to the useQuery function aligns with the updated QueryRoute interface, ensuring type consistency across the application.
src/components/Questionnaire/QuestionTypes/MedicationQuestion.tsx (2)
1-31: LGTM! Well-structured imports and interface definition.
The imports are properly organized and the MedicationQuestionProps interface is well-defined with appropriate types for FHIR questionnaire handling.
33-38:
Implement missing props and FHIR response handling
The component is not utilizing all the defined props:
questionnaireResponseandupdateQuestionnaireResponseCBare defined but not useddisabledprop is not implemented- No FHIR response handling is implemented
Apply this diff to implement the missing functionality:
-export function MedicationQuestion({ question }: MedicationQuestionProps) {
+export function MedicationQuestion({
+ question,
+ questionnaireResponse,
+ updateQuestionnaireResponseCB,
+ disabled
+}: MedicationQuestionProps) {
const medicationSearch = useQuery(routes.valueset.expand, {
pathParams: { system: "system-medication" },
body: { count: 10 },
prefetch: false,
});
+
+ const handleMedicationSelect = (code: string) => {
+ updateQuestionnaireResponseCB({
+ ...questionnaireResponse,
+ item: [
+ ...(questionnaireResponse.item || []),
+ {
+ linkId: question.linkId,
+ answer: [{
+ valueCoding: {
+ system: "system-medication",
+ code
+ }
+ }]
+ }
+ ]
+ });
+ };src/components/ui/select.tsx (3)
17-35: Implementation of SelectTrigger Component is Correct
The SelectTrigger component correctly uses React.forwardRef and properly forwards props and refs to the underlying SelectPrimitive.Trigger. The class names are appropriately managed using the cn utility function, ensuring that custom styles can be added via the className prop.
72-102: Verify Positioning and Styling of SelectContent
The SelectContent component uses the position prop set to "popper", and includes complex styling and animations. Please verify that:
- The positioning works correctly across different screen sizes and orientations.
- The animations transition smoothly without causing jank or layout shifts.
- The component behaves correctly in various browsers and supports accessibility requirements.
116-136: Ensure Accessibility Compliance for SelectItem Components
The SelectItem component should meet accessibility standards to provide a good user experience for all users. Please verify that:
- ARIA attributes are correctly applied.
- Keyboard navigation (e.g., using arrow keys, Enter, and Escape) functions as expected.
- Screen readers correctly announce the items and their selected states.
Consider using accessibility testing tools to confirm compliance.
src/types/emr/medication-request.ts
Outdated
|
|
||
| type MedicationRequestPrioritie = "routine" | "urgent" | "asap" | "stat"; | ||
|
|
||
| type MedicationRequestCategorie = |
There was a problem hiding this comment.
Typo in type name 'MedicationRequestCategorie'
The type name MedicationRequestCategorie seems misspelled. It should be MedicationRequestCategory.
Apply this diff to correct the typo:
-type MedicationRequestCategorie =
+type MedicationRequestCategory =Ensure to update all references to MedicationRequestCategorie accordingly.
📝 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.
| type MedicationRequestCategorie = | |
| type MedicationRequestCategory = |
src/types/emr/medication-request.ts
Outdated
| | "filler_order" | ||
| | "instance_order"; | ||
|
|
||
| type MedicationRequestPrioritie = "routine" | "urgent" | "asap" | "stat"; |
There was a problem hiding this comment.
Typo in type name 'MedicationRequestPrioritie'
The type name MedicationRequestPrioritie seems misspelled. It should be MedicationRequestPriority.
Apply this diff to correct the typo:
-type MedicationRequestPrioritie = "routine" | "urgent" | "asap" | "stat";
+type MedicationRequestPriority = "routine" | "urgent" | "asap" | "stat";Ensure to update all references to MedicationRequestPrioritie accordingly.
📝 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.
| type MedicationRequestPrioritie = "routine" | "urgent" | "asap" | "stat"; | |
| type MedicationRequestPriority = "routine" | "urgent" | "asap" | "stat"; |
| return ( | ||
| <div className="space-y-4"> | ||
| <Label>{question.text}</Label> | ||
| <div className="rounded-lg border border-dashed border-gray-300 bg-gray-50 p-4 text-center text-sm text-gray-500"> | ||
| <Popover> | ||
| <PopoverTrigger asChild> | ||
| <Button | ||
| variant="outline" | ||
| role="combobox" | ||
| className="w-full justify-between" | ||
| > | ||
| Search | ||
| <CaretSortIcon className="ml-2 size-4 shrink-0 opacity-50" /> | ||
| </Button> | ||
| </PopoverTrigger> | ||
| <PopoverContent className="w-full p-0"> | ||
| <Command filter={() => 1}> | ||
| <CommandInput | ||
| placeholder="Search option..." | ||
| className="my-1" | ||
| onValueChange={(search) => | ||
| medicationSearch.refetch({ body: { search } }) | ||
| } | ||
| /> | ||
| <CommandList> | ||
| <CommandEmpty> | ||
| {medicationSearch.loading ? <Loading /> : "No options found"} | ||
| </CommandEmpty> | ||
| <CommandGroup> | ||
| {medicationSearch.data?.results.map((option) => ( | ||
| <CommandItem | ||
| key={option.code} | ||
| value={option.code} | ||
| // onSelect={(currentValue) => { | ||
| // onChange(currentValue === value ? "" : currentValue); | ||
| // }} | ||
| > | ||
| {/* <CheckIcon | ||
| className={cn( | ||
| "mr-2 h-4 w-4", | ||
| value === option ? "opacity-100" : "opacity-0", | ||
| )} | ||
| /> */} | ||
| <span>{option.display}</span> | ||
| </CommandItem> | ||
| ))} | ||
| </CommandGroup> | ||
| </CommandList> | ||
| </Command> | ||
| </PopoverContent> | ||
| </Popover> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Multiple UI and functionality improvements needed
Several issues need to be addressed in the UI implementation:
- The Command filter function
() => 1bypasses filtering - Selection handling code is commented out
- Missing proper accessibility attributes
- No error handling for the API call
- Loading state could be improved
Apply these improvements:
<Button
variant="outline"
role="combobox"
+ aria-label={`Select medication for ${question.text}`}
+ disabled={disabled}
className="w-full justify-between"
>
- Search
+ {medicationSearch.data?.selected?.display || "Search medications"}
<CaretSortIcon className="ml-2 size-4 shrink-0 opacity-50" />
</Button>
-<Command filter={() => 1}>
+<Command>
<CommandInput
placeholder="Search option..."
className="my-1"
+ disabled={disabled}
onValueChange={(search) =>
- medicationSearch.refetch({ body: { search } })
+ search.length >= 2 && medicationSearch.refetch({ body: { search } })
}
/>
<CommandList>
<CommandEmpty>
- {medicationSearch.loading ? <Loading /> : "No options found"}
+ {medicationSearch.loading ? (
+ <Loading />
+ ) : medicationSearch.error ? (
+ <div className="text-red-500">Error loading medications</div>
+ ) : (
+ "No options found"
+ )}
</CommandEmpty>
<CommandGroup>
{medicationSearch.data?.results.map((option) => (
<CommandItem
key={option.code}
value={option.code}
- // onSelect={(currentValue) => {
- // onChange(currentValue === value ? "" : currentValue);
- // }}
+ onSelect={handleMedicationSelect}
>
- {/* <CheckIcon
- className={cn(
- "mr-2 h-4 w-4",
- value === option ? "opacity-100" : "opacity-0",
- )}
- /> */}
+ <CheckIcon
+ className={cn(
+ "mr-2 h-4 w-4",
+ questionnaireResponse.item?.some(
+ item =>
+ item.linkId === question.linkId &&
+ item.answer?.[0]?.valueCoding?.code === option.code
+ )
+ ? "opacity-100"
+ : "opacity-0"
+ )}
+ />
<span>{option.display}</span>
</CommandItem>
))}Consider implementing debouncing for the search input to prevent excessive API calls.
Committable suggestion skipped: line range outside the PR's diff.
src/components/ui/autocomplete.tsx
Outdated
| interface AutocompleteProps { | ||
| options: string[]; | ||
| value: string; | ||
| onChange: (value: string) => void; | ||
| onSearch: (value: string) => void; | ||
| placeholder?: string; | ||
| noOptionsMessage?: string; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Unused onSearch Prop in AutocompleteProps
The onSearch prop is defined in the AutocompleteProps interface but is not utilized within the component. This may lead to confusion or missed functionality.
Apply this diff to implement the onSearch callback within the component:
interface AutocompleteProps {
options: string[];
value: string;
onChange: (value: string) => void;
- onSearch: (value: string) => void;
+ onSearch?: (value: string) => void;
placeholder?: string;
noOptionsMessage?: string;
}Adjust the component to use the onSearch prop:
export default function Autocomplete({
options,
value,
onChange,
+ onSearch,
placeholder = "Select...",
noOptionsMessage = "No options found",
}: AutocompleteProps) {Add the onValueChange handler to CommandInput to handle search input changes:
<CommandInput
placeholder="Search option..."
+ onValueChange={(value) => {
+ onSearch?.(value);
+ }}
/>If the search functionality is not required, consider removing the onSearch prop from the interface.
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (12)
src/components/ui/table.tsx (4)
5-18: Consider adding ARIA attributes for better accessibilityThe Table component implementation looks good, following React best practices with proper TypeScript types and overflow handling.
Consider adding these ARIA attributes for better accessibility:
<table ref={ref} className={cn("w-full caption-bottom text-sm", className)} + role="grid" + aria-label="Data table" {...props} />
19-53: Consider adding type safety for className propThe table section components are well-implemented with proper TypeScript types and consistent styling patterns.
Consider adding type safety for className prop using a union type of allowed Tailwind classes:
type TableSectionProps = React.HTMLAttributes<HTMLTableSectionElement> & { className?: `${string}border-${string}` | `${string}bg-${string}` | string; };
54-98: Consider memoization for optimal performanceThe row and cell components are well-implemented with proper state handling and styling. However, in tables with many rows, performance could be optimized.
Consider memoizing rows to prevent unnecessary re-renders:
const MemoizedTableRow = React.memo(TableRow); const MemoizedTableCell = React.memo(TableCell); // Export the memoized versions export { MemoizedTableRow as TableRow, MemoizedTableCell as TableCell };
111-120: Consider creating a barrel file for UI componentsThe exports are well-organized. However, as this seems to be part of a UI component library, consider improving the module organization.
Consider creating a barrel file (index.ts) in the ui directory to centralize component exports:
// src/components/ui/index.ts export * from './table'; export * from './badge'; // If these exist export * from './select'; // If these existThis would allow consumers to import multiple UI components in a cleaner way:
import { Table, Badge, Select } from '@/components/ui';src/components/Questionnaire/structured/handlers.ts (1)
31-40: Automate property mapping from camelCase to snake_caseThe mapping of properties from the data object to the request body involves manually converting camelCase to snake_case for each field. This approach can be error-prone and hard to maintain as the number of fields increases. Consider using a utility function to automate the conversion, ensuring consistency and reducing the likelihood of typos.
src/types/questionnaire/allergyIntolerance.ts (1)
15-24: Consider using a utility to map properties from camelCase to snake_caseThe
AllergyIntoleranceRequestinterface redefines the properties fromAllergyIntolerancewith snake_case keys. This leads to code duplication and potential maintenance issues. Consider using a utility type or a transformation function to map the properties from camelCase to snake_case automatically, ensuring consistency and reducing redundancy.src/components/Questionnaire/structured/types.ts (1)
15-18: Ensure consistency in defining request types for structured question typesIn
StructuredRequestMap, theallergy_intolerancekey maps toAllergyIntoleranceRequest, which is appropriate. However,medication_requestmaps toMedicationRequest, which may not represent the request type ifMedicationRequestis intended as a data type. Consider defining a separate request type formedication_requestto maintain consistency and clarity.src/types/emr/medicationRequest.ts (1)
78-91: Consider flattening the nested structureThe deep nesting in
DosageInstructioncould make it difficult to work with and type-check.Consider:
- Breaking down complex nested structures into smaller, more manageable interfaces
- Using composition over deep nesting
- Adding helper functions for common operations on these nested structures
src/components/Questionnaire/QuestionTypes/QuestionInput.tsx (1)
47-93: Extract complex condition logic into a utility functionThe
isQuestionEnabledfunction contains complex conditional logic that could be moved to a separate utility.Consider moving this logic to a separate utility file:
// utils/questionConditions.ts export function evaluateQuestionCondition( enableWhen: EnableWhen, responses: QuestionnaireResponse[] ): boolean { // ... condition logic }src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx (2)
118-118: Remove console.log statementRemove debugging console.log statement before production deployment.
- console.log(allergies);
62-66: Optimize allergy search implementationConsider implementing debouncing for the allergy search to prevent excessive API calls.
+ import { useDebounce } from '@/hooks/useDebounce'; const allergySearch = useQuery(routes.valueset.expand, { pathParams: { system: "system-allergy-code" }, body: { count: 10 }, prefetch: false, });src/components/Questionnaire/QuestionnaireForm.tsx (1)
126-159: Simplify error handling logicConsider extracting error handling logic into separate utility functions for better maintainability.
+ const handleQuestionError = (form: QuestionnaireFormState, error: QuestionValidationError) => { + form.errors.push({ + question_id: error.question_id, + error: error.error ?? error.msg, + } as QuestionValidationError); + }; + const handleFormLevelError = (form: QuestionnaireFormState, error: DetailedValidationError, errorMessages: string[]) => { + const fieldName = error.loc[0]; + errorMessages.push(`Error in ${form.questionnaire.title}: ${fieldName} - ${error.msg}`); + }; const handleSubmissionError = (results: ValidationErrorResponse[]) => { const updatedForms = [...questionnaireForms]; const errorMessages: string[] = []; results.forEach((result, index) => { const form = updatedForms[index]; result.data.errors.forEach((error) => { if ("question_id" in error) { + handleQuestionError(form, error); } else if ("loc" in error) { + handleFormLevelError(form, error, errorMessages); } else { errorMessages.push(`Error in ${form.questionnaire.title}`); } }); }); setQuestionnaireForms(updatedForms); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (11)
src/Utils/request/api.tsx(2 hunks)src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/ChoiceQuestion.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/QuestionInput.tsx(1 hunks)src/components/Questionnaire/QuestionnaireForm.tsx(1 hunks)src/components/Questionnaire/structured/handlers.ts(1 hunks)src/components/Questionnaire/structured/types.ts(1 hunks)src/components/ui/table.tsx(1 hunks)src/types/emr/medicationRequest.ts(1 hunks)src/types/questionnaire/allergyIntolerance.ts(1 hunks)src/types/questionnaire/form.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Questionnaire/QuestionTypes/ChoiceQuestion.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Questionnaire/QuestionnaireForm.tsx
[error] 10-10: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (12)
src/components/ui/table.tsx (2)
99-110: LGTM! Well-implemented caption component
The TableCaption component follows the established patterns and includes proper styling for both light and dark modes.
1-4: Verify the cn utility implementation
The code relies on the cn utility from @/lib/utils. Let's ensure it's properly implemented for class name merging.
✅ Verification successful
The cn utility implementation is properly verified
The cn utility in src/lib/utils.ts is correctly implemented using the clsx package and tailwind-merge. The implementation:
import { clsx, type ClassValue } from "clsx"
import { twMerge } from "tailwind-merge"
export function cn(...inputs: ClassValue[]) {
return twMerge(clsx(inputs))
}combines clsx for conditional class merging with twMerge for Tailwind-specific class deduplication, which is the recommended pattern for React components using Tailwind CSS.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the implementation of cn utility
rg -A 5 "export.*cn.*=" src/lib/utils.ts
Length of output: 42
Script:
#!/bin/bash
# Let's try finding the cn utility with different approaches
# First, let's find any file that might contain the cn utility
fd utils
# Then, let's search for cn function definition in any typescript/javascript file
ast-grep --pattern 'export function cn'
# Also search for const cn definition
ast-grep --pattern 'export const cn'
# Finally, let's try a broader search for cn using ripgrep
rg "cn.*=" -A 5
Length of output: 3803
Script:
#!/bin/bash
# Let's try more specific searches to locate the cn utility
# Search in lib/utils.ts with more context
rg -A 10 "cn" src/lib/utils.ts
# Look for clsx or classnames imports which are commonly used for cn implementation
rg "import.*\b(clsx|classnames)\b"
# Search for any file that might contain className merging utility
rg -A 10 "className.*merge|clsx|classnames"
Length of output: 25600
src/components/Questionnaire/structured/handlers.ts (1)
57-63: Function getStructuredRequests is correctly implemented
The getStructuredRequests function correctly leverages the handlers to generate structured requests based on the provided type, data, and context.
src/Utils/request/api.tsx (2)
1465-1479: Add validation for questionnaire response values
The questionnaire.submit endpoint accepts complex response data without any validation specified in the type definitions. Consider adding runtime validation to ensure that required fields are present and that response values match their expected types.
1423-1480: Questionnaire routes are correctly defined
The new questionnaire routes are appropriately added and follow the existing API structure.
src/types/questionnaire/form.ts (1)
22-31: Consider making critical fields required
The interface has several optional fields that could be required based on business logic. For example, structured_type being null could lead to runtime issues.
Let's verify the usage of these optional fields:
Consider:
- Making
structured_typenon-nullable if it's required for certain operations - Adding JSDoc comments to document when optional fields are expected to be present
src/components/Questionnaire/QuestionTypes/QuestionInput.tsx (1)
292-292: Avoid potential undefined error when accessing errors.find
When accessing errors.find, ensure that errors is defined and is an array.
src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx (3)
69-83: Implement complete FHIR AllergyIntolerance structure
The current implementation needs to be updated to fully comply with FHIR AllergyIntolerance resource specifications.
Consider implementing:
- FHIR AllergyIntolerance fields (type, category, criticality, etc.)
- Proper error handling
- Multiple allergies support
- Proper validation
Would you like me to provide a complete FHIR-compliant implementation?
43-48: 🛠️ Refactor suggestion
Add validation and error handling props to interface
The interface should include validation and error handling capabilities for a more robust implementation.
interface AllergyQuestionProps {
question: Question;
questionnaireResponse: QuestionnaireResponse;
updateQuestionnaireResponseCB: (response: QuestionnaireResponse) => void;
disabled?: boolean;
+ validation?: {
+ required?: boolean;
+ minAnswers?: number;
+ maxAnswers?: number;
+ };
+ error?: string;
+ onError?: (error: string) => void;
}121-123: 🛠️ Refactor suggestion
Enhance accessibility with ARIA labels
Add proper ARIA attributes to improve accessibility for screen readers.
- <div className="space-y-4">
+ <div className="space-y-4" role="region" aria-labelledby={`allergy-section-${question.id}`}>
- <Label>{question.text}</Label>
+ <Label id={`allergy-section-${question.id}`}>{question.text}</Label>
- <div className="rounded-lg border p-4">
+ <div className="rounded-lg border p-4" role="table" aria-label="Allergy Information">Likely invalid or redundant comment.
src/components/Questionnaire/QuestionnaireForm.tsx (2)
10-10:
Rename Error and Success to avoid shadowing global variables
The variables Error and Success shadow the global Error object.
- import { Error, Success } from "@/Utils/Notifications";
+ import { Error as NotifyError, Success as NotifySuccess } from "@/Utils/Notifications";🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
223-235:
Enhance submission error handling
Add comprehensive error handling for the submission response.
const response = await submitBatch({
body: { requests },
});
+ if (!response) {
+ Error({ msg: "Failed to submit questionnaire: Network error" });
+ return;
+ }
+ if (!response.data?.results) {
+ Error({ msg: "Failed to submit questionnaire: Invalid response format" });
+ return;
+ }
if (!response.data) {
if (response.error) {
handleSubmissionError(
response.error.results as ValidationErrorResponse[],
);
Error({ msg: "Failed to submit questionnaire" });
}
return;
}| getRequests: (_medications, _context) => { | ||
| // Implement medication request handling | ||
| return []; | ||
| }, |
There was a problem hiding this comment.
Implement the medication_request handler
The getRequests method for medication_request currently returns an empty array, indicating that the handler is not yet implemented. This will result in no requests being generated for medication requests, which may lead to missing functionality.
Would you like assistance in implementing the medication_request handler?
| allergyIntolerance: { | ||
| create: { | ||
| method: "POST", | ||
| path: "/api/v1/patient/:patientId/allergy_intolerance/", |
There was a problem hiding this comment.
Fix inconsistent placeholder syntax in route path
In the route definition for patient.allergyIntolerance.create, the path uses :patientId as a placeholder. This is inconsistent with the rest of the codebase, which uses {patientId} for placeholders. Replace :patientId with {patientId} to maintain consistency.
Apply this diff to fix the issue:
- path: "/api/v1/patient/:patientId/allergy_intolerance/",
+ path: "/api/v1/patient/{patientId}/allergy_intolerance/",📝 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.
| path: "/api/v1/patient/:patientId/allergy_intolerance/", | |
| path: "/api/v1/patient/{patientId}/allergy_intolerance/", |
| export interface AllergyIntoleranceRequest { | ||
| clinical_status: string; | ||
| verification_status: string; | ||
| category: string; | ||
| criticality: string; | ||
| code: Code; | ||
| last_occurrence?: string; | ||
| note?: string; | ||
| encounter: string; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Correct the comment or structure to reflect extension of the base type
The comment suggests that AllergyIntoleranceRequest extends the base type AllergyIntolerance. However, it currently does not extend it, and the properties are redefined with different naming conventions. Consider extending the base type and handling the required fields and naming conventions appropriately.
src/types/questionnaire/form.ts
Outdated
| export type ResponseValue = { | ||
| type: | ||
| | "string" | ||
| | "number" | ||
| | "boolean" | ||
| | "allergy_intolerance" | ||
| | "medication_request"; | ||
| value?: | ||
| | string | ||
| | number | ||
| | boolean | ||
| | AllergyIntolerance[] | ||
| | MedicationRequest[]; | ||
| code?: Code; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add type validation for ResponseValue
The ResponseValue type allows for potential type mismatches between the type discriminator and the actual value. For example, a value of type "string" could contain a number.
Consider adding type guards or using a more strictly typed union:
export type ResponseValue = {
- type: "string" | "number" | "boolean" | "allergy_intolerance" | "medication_request";
- value?: string | number | boolean | AllergyIntolerance[] | MedicationRequest[];
+ type: "string";
+ value?: string;
+} | {
+ type: "number";
+ value?: number;
+} | {
+ type: "boolean";
+ value?: boolean;
+} | {
+ type: "allergy_intolerance";
+ value?: AllergyIntolerance[];
+} | {
+ type: "medication_request";
+ value?: MedicationRequest[];
} & {
code?: Code;
};📝 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 type ResponseValue = { | |
| type: | |
| | "string" | |
| | "number" | |
| | "boolean" | |
| | "allergy_intolerance" | |
| | "medication_request"; | |
| value?: | |
| | string | |
| | number | |
| | boolean | |
| | AllergyIntolerance[] | |
| | MedicationRequest[]; | |
| code?: Code; | |
| }; | |
| export type ResponseValue = { | |
| type: "string"; | |
| value?: string; | |
| } | { | |
| type: "number"; | |
| value?: number; | |
| } | { | |
| type: "boolean"; | |
| value?: boolean; | |
| } | { | |
| type: "allergy_intolerance"; | |
| value?: AllergyIntolerance[]; | |
| } | { | |
| type: "medication_request"; | |
| value?: MedicationRequest[]; | |
| } & { | |
| code?: Code; | |
| }; |
src/types/emr/medicationRequest.ts
Outdated
| readonly id: string; | ||
| status?: MedicationRequestStatus; | ||
| status_reason?: MedicationRequestStatusReason; | ||
| status_changed?: string | null; // DateTime |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use a proper DateTime type
Using string type for DateTime fields can lead to inconsistencies and validation issues.
Consider:
- Using a specific DateTime type or
- Adding runtime validation for the DateTime string format
- status_changed?: string | null; // DateTime
+ status_changed?: Date | null;Committable suggestion skipped: line range outside the PR's diff.
| step={question.type === "decimal" ? "0.01" : "1"} | ||
| {...commonProps} | ||
| /> | ||
| {removeButton} | ||
| </div> | ||
| ); | ||
| case "choice": | ||
| return ( | ||
| <div className="flex gap-2"> | ||
| <div className="flex-1"> | ||
| <ChoiceQuestion | ||
| question={question} | ||
| questionnaireResponse={{ | ||
| ...questionnaireResponse, | ||
| values: [responseValue], | ||
| }} | ||
| updateQuestionnaireResponseCB={(response) => { | ||
| const updatedValues = [...questionnaireResponse.values]; | ||
| updatedValues[index] = response.values[0]; | ||
| handleValueChange({ | ||
| ...questionnaireResponse, | ||
| values: updatedValues, | ||
| }); | ||
| }} | ||
| disabled={!isEnabled || disabled} | ||
| /> | ||
| </div> | ||
| {removeButton} | ||
| </div> | ||
| ); | ||
| case "text": | ||
| return ( | ||
| <div className="flex gap-2"> | ||
| <Textarea | ||
| value={responseValue.value?.toString() || ""} | ||
| onChange={(e) => { | ||
| const updatedValues = [...questionnaireResponse.values]; | ||
| updatedValues[index] = { | ||
| type: "string", | ||
| value: e.target.value, | ||
| }; | ||
| handleValueChange({ | ||
| ...questionnaireResponse, | ||
| values: updatedValues, | ||
| }); | ||
| }} | ||
| className="min-h-[100px]" | ||
| {...commonProps} | ||
| /> | ||
| {removeButton} | ||
| </div> | ||
| ); | ||
| case "display": | ||
| return null; | ||
| case "structured": | ||
| switch (question.structured_type) { | ||
| case "medication_request": | ||
| return ( | ||
| <MedicationQuestion | ||
| question={question} | ||
| questionnaireResponse={questionnaireResponse} | ||
| updateQuestionnaireResponseCB={updateQuestionnaireResponseCB} | ||
| disabled={!isEnabled} | ||
| /> | ||
| ); | ||
| case "allergy_intolerance": | ||
| return ( | ||
| <AllergyQuestion | ||
| question={question} | ||
| questionnaireResponse={questionnaireResponse} | ||
| updateQuestionnaireResponseCB={updateQuestionnaireResponseCB} | ||
| disabled={!isEnabled} | ||
| /> | ||
| ); | ||
| } | ||
| return null; | ||
| default: | ||
| return ( | ||
| <div className="flex gap-2"> | ||
| <Input | ||
| type="text" | ||
| value={responseValue.value?.toString() || ""} | ||
| onChange={(e) => { | ||
| const updatedValues = [...questionnaireResponse.values]; | ||
| updatedValues[index] = { | ||
| type: "string", | ||
| value: e.target.value, | ||
| }; | ||
| handleValueChange({ | ||
| ...questionnaireResponse, | ||
| values: updatedValues, | ||
| }); | ||
| }} | ||
| {...commonProps} | ||
| /> | ||
| {removeButton} | ||
| </div> | ||
| ); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Break down the large switch statement into smaller components
The renderSingleInput function is too large and handles too many responsibilities.
Consider breaking it down into separate components for each question type:
const QuestionTypeComponents = {
decimal: NumberInput,
integer: NumberInput,
choice: ChoiceInput,
text: TextInput,
// ... other types
} as const;
function renderSingleInput(responseValue: ResponseValue, index: number) {
const Component = QuestionTypeComponents[question.type];
return Component ? (
<Component
value={responseValue}
index={index}
onChange={handleValueChange}
{...commonProps}
/>
) : null;
}| interface BatchRequest { | ||
| url: string; | ||
| method: string; | ||
| body: Record<string, any>; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety of BatchRequest interface
Replace Record<string, any> with a more specific type definition.
interface BatchRequest {
url: string;
method: string;
- body: Record<string, any>;
+ body: {
+ resource_id?: string;
+ encounter?: string;
+ results?: Array<{
+ question_id: string;
+ values: Array<{
+ code?: { code: string; display: string; system: string };
+ value?: string;
+ }>;
+ note?: string;
+ body_site?: string;
+ method?: string;
+ }>;
+ };
}📝 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.
| interface BatchRequest { | |
| url: string; | |
| method: string; | |
| body: Record<string, any>; | |
| } | |
| interface BatchRequest { | |
| url: string; | |
| method: string; | |
| body: { | |
| resource_id?: string; | |
| encounter?: string; | |
| results?: Array<{ | |
| question_id: string; | |
| values: Array<{ | |
| code?: { code: string; display: string; system: string }; | |
| value?: string; | |
| }>; | |
| note?: string; | |
| body_site?: string; | |
| method?: string; | |
| }>; | |
| }; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
src/components/Questionnaire/structured/handlers.ts (2)
60-66: Add error handling for unknown question types.The function could throw runtime errors if an unknown question type is provided.
Consider adding type checking:
export function getStructuredRequests<T extends StructuredQuestionType>( type: T, data: DataTypeFor<T>[], context: StructuredHandlerContext, ) { + if (!(type in handlers)) { + throw new Error(`No handler implemented for question type: ${type}`); + } return handlers[type].getRequests(data, context); }
1-66: Consider adding JSDoc documentation for better maintainability.The code implements a well-structured handler pattern, but adding documentation would help future maintainers understand:
- The purpose and responsibility of each handler
- Expected data formats and validation rules
- How to implement new handlers
Example documentation:
/** * Context for structured handlers containing reference IDs */ interface StructuredHandlerContext { resourceId: string; encounterId: string; } /** * Generic handler for processing structured questionnaire data * @template T - The type of structured question being handled */ type StructuredHandler<T extends StructuredQuestionType> = { /** * Generates API requests for the given data * @param data - Array of structured data to process * @param context - Context containing reference IDs * @returns Array of API requests to be executed */ getRequests: ( data: DataTypeFor<T>[], context: StructuredHandlerContext, ) => Array<{...}>; };src/types/emr/medicationRequest.ts (1)
3-69: Add JSDoc comments to explain the FHIR constantsThe constants are well-structured, but adding JSDoc comments would improve code maintainability by documenting the meaning and usage of each status, intent, priority, and category.
+/** + * Status values for a medication request as per FHIR specification. + * @see http://hl7.org/fhir/ValueSet/medicationrequest-status + */ export const MEDICATION_REQUEST_STATUS = [ "active", "on-hold", // ... ] as const;src/components/Questionnaire/QuestionTypes/MedicationQuestion.tsx (1)
76-132: Extract default medication values to constantsThe default values in
handleAddMedicationshould be extracted to constants for better maintainability.+const DEFAULT_MEDICATION = { + status: "active" as const, + intent: "plan" as const, + category: "inpatient" as const, + priority: "urgent" as const, + do_not_perform: false, + medication: { + display: "Paracetamol-containing product", + system: "http://snomed.info/sct", + code: "90332006", + }, + dosage_instruction: [], +} as const; const handleAddMedication = () => { const newMedications: MedicationRequest[] = [ ...medications, - { - status: "active", - intent: "plan", - // ... - }, + { + ...DEFAULT_MEDICATION, + authored_on: new Date().toISOString(), + }, ]; // ... };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/components/Questionnaire/QuestionTypes/MedicationQuestion.tsx(1 hunks)src/components/Questionnaire/structured/handlers.ts(1 hunks)src/types/emr/medicationRequest.ts(1 hunks)
🔇 Additional comments (7)
src/components/Questionnaire/structured/handlers.ts (3)
1-22: LGTM! Well-structured type definitions.
The type definitions provide strong type safety and are well-organized. The generic StructuredHandler<T> type is particularly well-designed, allowing for type-safe request generation based on the question type.
27-48: LGTM! Well-implemented allergy handler with proper defaults.
The allergy_intolerance handler is well-implemented with:
- Proper handling of optional fields using nullish coalescing
- Clear type safety with RequestTypeFor
- Good default values for required fields
49-58: 🛠️ Refactor suggestion
Enhance the medication request handler with validation and defaults.
The current implementation lacks validation and default values that could prevent potential runtime issues.
Consider implementing similar safeguards as the allergy handler:
medication_request: {
- getRequests: (medications, { encounterId }) => {
- return medications.map((medication) => ({
- url: `/api/v1/consultation/${encounterId}/medication/request/`,
- method: "POST",
- body: medication,
- }));
+ getRequests: (medications, { encounterId }) => {
+ return medications.map((medication) => {
+ const body: RequestTypeFor<"medication_request"> = {
+ status: medication.status ?? "active",
+ intent: medication.intent ?? "order",
+ medication: medication.medication,
+ dosageInstruction: medication.dosageInstruction,
+ encounter: encounterId,
+ };
+
+ return {
+ url: `/api/v1/consultation/${encounterId}/medication/request/`,
+ method: "POST",
+ body,
+ };
+ });
},
},src/types/emr/medicationRequest.ts (3)
70-72: LGTM!
The TimingUnit and DoseType types are well-defined using string literals.
74-100: LGTM!
The interfaces are well-structured and properly follow FHIR resource definitions.
117-131: Use proper DateTime type for temporal fields
The status_changed and authored_on fields are using string type for DateTime values, which could lead to validation issues.
src/components/Questionnaire/QuestionTypes/MedicationQuestion.tsx (1)
51-68: LGTM!
The component setup and state management are well-structured.
| const medicationSearch = useQuery(routes.valueset.expand, { | ||
| pathParams: { system: "system-medication" }, | ||
| body: { count: 10 }, | ||
| prefetch: false, | ||
| }); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve search functionality
- The Command filter
() => 1bypasses client-side filtering. - Consider adding debouncing to prevent excessive API calls.
-<Command filter={() => 1}>
+<Command>
<CommandInput
placeholder="Search allergies..."
className="my-1"
- onValueChange={(search) =>
- medicationSearch.refetch({ body: { search } })
- }
+ onValueChange={debounce((search) => {
+ if (search.length >= 2) {
+ medicationSearch.refetch({ body: { search } });
+ }
+ }, 300)}
/>Also applies to: 168-175
| > | ||
| {category} | ||
| </SelectItem> | ||
| ))} | ||
| </SelectContent> | ||
| </Select> | ||
| </TableCell> | ||
| <TableCell className="min-w-[150px]"> | ||
| <Select | ||
| value={medication.priority} | ||
| onValueChange={(value) => | ||
| handleUpdateMedication(index, { | ||
| priority: value as MedicationRequestPriority, | ||
| }) | ||
| } | ||
| disabled={disabled} | ||
| > | ||
| <SelectTrigger className="w-full"> | ||
| <SelectValue placeholder="Priority" /> | ||
| </SelectTrigger> | ||
| <SelectContent> | ||
| {MEDICATION_REQUEST_PRIORITY.map((priority) => ( | ||
| <SelectItem | ||
| key={priority} | ||
| value={priority} | ||
| className="capitalize" | ||
| > | ||
| {priority} | ||
| </SelectItem> | ||
| ))} | ||
| </SelectContent> | ||
| </Select> | ||
| </TableCell> | ||
| <TableCell className="min-w-[150px]"> | ||
| <Select | ||
| value={medication.status} | ||
| onValueChange={(value) => | ||
| handleUpdateMedication(index, { | ||
| status: value as MedicationRequestStatus, | ||
| }) | ||
| } | ||
| disabled={disabled} | ||
| > | ||
| <SelectTrigger className="w-full"> | ||
| <SelectValue placeholder="Status" /> | ||
| </SelectTrigger> | ||
| <SelectContent> | ||
| {MEDICATION_REQUEST_STATUS.map((status) => ( | ||
| <SelectItem | ||
| key={status} | ||
| value={status} | ||
| className="capitalize" | ||
| > | ||
| {status} | ||
| </SelectItem> | ||
| ))} | ||
| </SelectContent> | ||
| </Select> | ||
| </TableCell> | ||
| <TableCell className="min-w-[200px]"> | ||
| <input | ||
| type="text" | ||
| className="w-full rounded-md border p-2" | ||
| placeholder="Note" | ||
| value={medication.note || ""} | ||
| onChange={(e) => | ||
| handleUpdateMedication(index, { note: e.target.value }) | ||
| } | ||
| disabled={disabled} | ||
| /> | ||
| </TableCell> | ||
| <TableCell className="min-w-[50px]"> | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| className="h-8 w-8" | ||
| onClick={() => handleRemoveMedication(index)} | ||
| disabled={disabled} | ||
| > | ||
| <TrashIcon className="h-4 w-4" /> | ||
| </Button> | ||
| </TableCell> | ||
| </TableRow> | ||
| ))} | ||
| </TableBody> | ||
| </Table> | ||
| </div> | ||
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| className="mt-4" | ||
| onClick={handleAddMedication} | ||
| disabled={disabled} | ||
| > | ||
| <PlusIcon className="mr-2 h-4 w-4" /> | ||
| Add Medication | ||
| </Button> | ||
| </div> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve accessibility and internationalization
Several improvements are needed:
- Add aria-label to input fields
- Internationalize placeholder texts
- Add proper error handling for the API call
<input
type="number"
className="w-full rounded-md border p-2"
- placeholder="Dosage"
+ placeholder={t('medication.dosage.placeholder')}
+ aria-label={t('medication.dosage.label')}
value={medication.dosage_instruction[0]?.dose_and_rate?.[0]?.dose_quantity?.value || ""}
disabled={disabled}
/>
<CommandEmpty>
- {medicationSearch.loading ? "Loading..." : "No medications found"}
+ {medicationSearch.loading ? (
+ <Loading />
+ ) : medicationSearch.error ? (
+ <div role="alert" className="text-red-500">
+ {t('medication.search.error')}
+ </div>
+ ) : (
+ t('medication.search.no_results')
+ )}
</CommandEmpty>Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Outside diff range and nitpick comments (22)
src/components/Facility/ConsultationDetails/ConsultationNursingTab.tsx (3)
142-149: LGTM! Consider extracting the type definition.The type safety improvements are well-implemented, using TypeScript's type system effectively to ensure only valid nursing procedures are included.
Consider extracting the type definition to make it reusable:
+// At the top of the file with other types +type NursingCareProcedure = (typeof NURSING_CARE_PROCEDURES)[number]; + const NursingPlot = ({ consultationId }: ConsultationTabProps) => { // ... - // First, let's get the type from NURSING_CARE_PROCEDURES - type NursingCareProcedure = (typeof NURSING_CARE_PROCEDURES)[number]; - // Then update the fieldsToDisplay filtering with type assertion fieldsToDisplay = new Set( Array.from(fieldsToDisplay).filter((field): field is NursingCareProcedure =>
Line range hint
89-99: Enhance error handling for API calls.Both
fetchDailyRoundsandgetDatafunctions could benefit from proper error handling to improve user experience when API calls fail.Example implementation:
const fetchDailyRounds = async (currentPage: number, consultationId: string) => { + try { const { res, data } = await request(routes.dailyRoundsAnalyse, { body: { page: currentPage, fields: NursingPlotFields }, pathParams: { consultationId }, }); if (res?.ok && data) { setResults(data.results as { [date: string]: NursingPlotRes }); setTotalCount(data.count); } + } catch (error) { + console.error('Failed to fetch daily rounds:', error); + // Consider showing an error message to the user + } };Also applies to: 191-201
Line range hint
15-63: Improve type safety for choice mappings.The
REVERSE_CHOICESobject could benefit from proper TypeScript types to ensure type safety when accessing values.Example implementation:
type AppetiteChoice = 'INCREASED' | 'SATISFACTORY' | 'REDUCED' | 'NO_TASTE_FOR_FOOD' | 'CANNOT_BE_ASSESSED'; type BladderDrainageChoice = 'NORMAL' | 'CONDOM_CATHETER' | /* ... */; interface ReverseChoices { appetite: Record<number, AppetiteChoice>; bladder_drainage: Record<number, BladderDrainageChoice>; // ... other choice mappings } const REVERSE_CHOICES: ReverseChoices = { // ... existing mappings } as const;src/components/Patient/PatientRegister.tsx (3)
Line range hint
292-397: Consider extracting form validation logic to a separate utility.The form validation logic is complex and handles multiple concerns. Consider extracting it to a separate utility function for better maintainability and reusability.
+// src/utils/validation/patientFormValidation.ts +export const validatePatientForm = (form: any, insuranceDetails: any[]) => { + const errors: Partial<Record<keyof any, FieldError>> = {}; + + // Extract validation logic here... + + return errors; +}; -const validateForm = (form: any) => { +const validateForm = (form: any) => { + return validatePatientForm(form, insuranceDetails); - const errors: Partial<Record<keyof any, FieldError>> = {}; - // ... rest of the validation logic -};
Line range hint
1182-1209: Enhance error handling for insurance details.The insurance details section could benefit from more specific error messages and validation feedback.
<InsuranceDetailsBuilder name="insurance_details" value={insuranceDetails} onChange={({ value }) => setInsuranceDetails(value)} error={insuranceDetailsError} + onError={(error) => { + setInsuranceDetailsError({ + ...error, + message: `Invalid insurance details: ${error.message}` + }); + }} gridView />
Line range hint
1214-1238: Extract user types to constants for better maintainability.Consider extracting user types to constants and simplifying the authorization logic.
+const FACILITY_ADMIN_TYPES = ['DistrictAdmin', 'StateAdmin'] as const; +type FacilityAdminType = typeof FACILITY_ADMIN_TYPES[number]; export function patientRegisterAuth( authUser: UserModel, facilityObject: FacilityModel | undefined, facilityId: string, ) { - const showAllFacilityUsers = ["DistrictAdmin", "StateAdmin"]; + const isHomeFacilityUser = !FACILITY_ADMIN_TYPES.includes(authUser.user_type as FacilityAdminType) && + authUser.home_facility_object?.id === facilityId; + + const isAuthorizedAdmin = ( + (authUser.user_type === "DistrictAdmin" && authUser.district === facilityObject?.district) || + (authUser.user_type === "StateAdmin" && authUser.state === facilityObject?.state) + ); + return isHomeFacilityUser || isAuthorizedAdmin; - if ( - !showAllFacilityUsers.includes(authUser.user_type) && - authUser.home_facility_object?.id === facilityId - ) { - return true; - } - if ( - authUser.user_type === "DistrictAdmin" && - authUser.district === facilityObject?.district - ) { - return true; - } - if ( - authUser.user_type === "StateAdmin" && - authUser.state === facilityObject?.state - ) { - return true; - } - - return false; }src/PluginRegistry.ts (2)
66-74: Prevent loading duplicate scriptsThe
loadScriptfunction does not check if the script has already been loaded, which could result in duplicate script executions. Consider adding a check to prevent loading the same script multiple times.Apply this diff to add a duplicate script check:
private loadScript(url: string): Promise<void> { return new Promise((resolve, reject) => { + if (document.querySelector(`script[src="${url}"]`)) { + return resolve(); + } const script = document.createElement("script"); script.src = url; script.onload = () => resolve(); script.onerror = () => reject(new Error(`Failed to load script: ${url}`)); document.head.appendChild(script); }); }
Line range hint
87-112: Remove unused functionisStaticallyImportedByEntryThe function
isStaticallyImportedByEntryis defined but never used within this file. Unused code can cause confusion and should be removed to keep the codebase clean.Apply this diff to remove the unused function:
-// Recursive function to check if the module is statically imported by an entry point -function isStaticallyImportedByEntry( - getModuleInfo: (moduleId: string) => any, - moduleId: string, - visited = new Set(), -) { - if (visited.has(moduleId)) return false; - visited.add(moduleId); - - const modInfo = getModuleInfo(moduleId); - if (!modInfo) return false; - - // Check if the module is an entry point - if (modInfo.isEntry) { - return true; - } - - // Check all static importers - for (const importerId of modInfo.importers) { - if (isStaticallyImportedByEntry(getModuleInfo, importerId, visited)) { - return true; - } - } - - return false; -}vite.config.mts (1)
118-121: Clarify double quoting inremoteUrlThe
remoteUrlvariable includes double quotes within the string, which might be unnecessary and could lead to confusion. Ensure that the URL string is correctly formatted.Review the
remoteUrlassignment:- const remoteUrl = `"https://${org}.github.io/${repo}/assets/remoteEntry.js"`; + const remoteUrl = `https://${org}.github.io/${repo}/assets/remoteEntry.js`;And adjust the
externalproperty accordingly:external: `Promise.resolve("${remoteUrl}")`,src/types/plugConfig.ts (1)
3-3: Avoid usingany; preferunknownfor better type safetyUsing
anybypasses type checking and can lead to runtime errors. Consider usingunknownto maintain type safety while accommodating various types.Apply this diff to improve type safety:
slug: string; - meta: Record<string, any>; + meta: Record<string, unknown>;tsconfig.json (1)
25-25: Consider adding types path to baseUrl.The typeRoots configuration is good, but for better module resolution, consider adding the remote-modules.d.ts path to the paths configuration as well.
"paths": { "@/*": ["./src/*"], "@core/*": ["src/*"], - "@careConfig": ["./care.config.ts"] + "@careConfig": ["./care.config.ts"], + "@types/*": ["./src/remote-modules.d.ts"] }scripts/install-platform-deps.ts (1)
7-16: Consider externalizing dependency configuration.The platform-specific dependencies are hard-coded. Consider moving them to a configuration file for better maintainability.
+// platform-deps.config.ts +export const platformDependencies = { darwin: { arm64: ["@rollup/rollup-darwin-arm64"], x64: ["@rollup/rollup-darwin-x64"], }, linux: { arm64: ["@rollup/rollup-linux-arm64-gnu"], x64: ["@rollup/rollup-linux-x64-gnu"], }, };netlify.toml (1)
Line range hint
19-27: Consider enabling strict CSP.The Content-Security-Policy header is in report-only mode. Consider enabling strict CSP after analyzing the reports.
Also, the nonce value should be dynamically generated per request, not hard-coded.
src/components/ErrorPages/DefaultErrorPage.tsx (1)
22-33: Consider distinct images for different error typesBoth error types currently use the same image (
/images/404.svg). Consider using distinct images to help users better understand different error scenarios.const errorContent = { PAGE_NOT_FOUND: { image: "/images/404.svg", title: t("page_not_found"), message: t("404_message"), }, PAGE_LOAD_ERROR: { - image: "/images/404.svg", + image: "/images/error.svg", title: t("page_load_error"), message: t("could_not_load_page"), }, };src/PluginEngine.tsx (1)
11-11: Remove or implement synchronous import commentThe comment indicates a synchronous import but there's no implementation following it. Either implement the synchronous import or remove the comment to avoid confusion.
src/pages/Apps/PlugConfigList.tsx (2)
46-58: Add empty state handling for the configurations listThe table should display a message when no configurations are available.
<TableBody> + {(!data?.configs || data.configs.length === 0) && ( + <TableRow> + <TableCell colSpan={2} className="text-center text-gray-500"> + No configurations found. Click 'Add New Config' to create one. + </TableCell> + </TableRow> + )} {data?.configs?.map((config: PlugConfig) => (
50-55: Add confirmation tooltip for the edit actionThe edit button's purpose isn't immediately clear from just the pen icon. Consider adding a tooltip.
<Button variant="ghost" + title="Edit configuration" onClick={() => navigate(`/apps/plug-configs/${config.slug}`)} > <CareIcon icon="l-pen" />src/components/ui/alert-dialog.tsx (3)
29-45: Add ARIA labels for better accessibilityThe AlertDialogContent should include ARIA labels to improve accessibility.
const AlertDialogContent = React.forwardRef< React.ElementRef<typeof AlertDialogPrimitive.Content>, React.ComponentPropsWithoutRef<typeof AlertDialogPrimitive.Content> >(({ className, ...props }, ref) => ( <AlertDialogPortal> <AlertDialogOverlay /> <AlertDialogPrimitive.Content ref={ref} + aria-labelledby="alert-dialog-title" + aria-describedby="alert-dialog-description" className={cn( "fixed left-[50%] top-[50%] z-50 grid w-full max-w-lg translate-x-[-50%] translate-y-[-50%] gap-4 border border-gray-200 bg-white p-6 shadow-lg duration-200 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[state=closed]:slide-out-to-left-1/2 data-[state=closed]:slide-out-to-top-[48%] data-[state=open]:slide-in-from-left-1/2 data-[state=open]:slide-in-from-top-[48%] dark:border-gray-800 dark:bg-gray-950 sm:rounded-lg", className, )} {...props} /> </AlertDialogPortal> ));
75-85: Add id props for ARIA associationThe Title and Description components should accept and forward id props for ARIA association.
const AlertDialogTitle = React.forwardRef< React.ElementRef<typeof AlertDialogPrimitive.Title>, React.ComponentPropsWithoutRef<typeof AlertDialogPrimitive.Title> >(({ className, ...props }, ref) => ( <AlertDialogPrimitive.Title ref={ref} + id="alert-dialog-title" className={cn("text-lg font-semibold", className)} {...props} /> )); const AlertDialogDescription = React.forwardRef< React.ElementRef<typeof AlertDialogPrimitive.Description>, React.ComponentPropsWithoutRef<typeof AlertDialogPrimitive.Description> >(({ className, ...props }, ref) => ( <AlertDialogPrimitive.Description ref={ref} + id="alert-dialog-description" className={cn("text-sm text-gray-500 dark:text-gray-400", className)} {...props} /> ));Also applies to: 87-98
1-7: Add JSDoc documentation for complex componentsConsider adding comprehensive JSDoc documentation for the components to improve developer experience.
+/** + * A set of components for building accessible alert dialogs using Radix UI primitives. + * + * @example + * ```tsx + * <AlertDialog> + * <AlertDialogTrigger>Open</AlertDialogTrigger> + * <AlertDialogContent> + * <AlertDialogHeader> + * <AlertDialogTitle>Are you sure?</AlertDialogTitle> + * <AlertDialogDescription>This action cannot be undone.</AlertDialogDescription> + * </AlertDialogHeader> + * <AlertDialogFooter> + * <AlertDialogCancel>Cancel</AlertDialogCancel> + * <AlertDialogAction>Continue</AlertDialogAction> + * </AlertDialogFooter> + * </AlertDialogContent> + * </AlertDialog> + * ``` + */ import * as AlertDialogPrimitive from "@radix-ui/react-alert-dialog";src/pages/Apps/PlugConfigEdit.tsx (1)
140-150: Add JSON validation feedback in real-timeThe meta field accepts JSON input but doesn't validate it until submission.
Consider adding real-time validation:
<div> <label className="mb-1 block text-sm font-medium">Meta (JSON)</label> <Textarea value={config.meta} onChange={(e) => { const meta = e.target.value; + try { + if (meta) { + JSON.parse(meta); + } + setConfig((prev) => ({ ...prev, meta })); + } catch (error) { + // Invalid JSON, but still update the field to allow editing + setConfig((prev) => ({ ...prev, meta })); + } }} rows={10} + className={`${ + meta && (() => { try { JSON.parse(meta); return false; } catch { return true; } })() + ? "border-red-500" + : "" + }`} /> + {meta && (() => { try { JSON.parse(meta); return false; } catch { return true; } })() && ( + <p className="text-sm text-red-500">Invalid JSON format</p> + )} </div>src/Utils/request/api.tsx (1)
1410-1422: Remove commented codeThe commented out
listendpoint should be removed since it's not being used. Keep the codebase clean by removing unused code.valueset: { - // list: { - // path: "/api/v1/valueset/", - // method: "GET", - // TRes: Type<PaginatedResponse<ValueSet>>(), - // }, expand: { path: "/api/v1/valueset/{system}/expand/", method: "POST", TBody: Type<{ search: string; count: number }>(), TRes: Type<{ results: Code[] }>(), }, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (25)
.env(1 hunks).gitignore(1 hunks)netlify.toml(1 hunks)package.json(4 hunks)public/locale/en.json(3 hunks)scripts/install-platform-deps.ts(1 hunks)scripts/setup-care-apps.js(0 hunks)scripts/setup-care-apps.ts(1 hunks)src/PluginEngine.tsx(2 hunks)src/PluginRegistry.ts(1 hunks)src/Routers/AppRouter.tsx(5 hunks)src/Utils/AuthorizeFor.tsx(2 hunks)src/Utils/request/api.tsx(2 hunks)src/Utils/request/request.ts(1 hunks)src/components/ErrorPages/DefaultErrorPage.tsx(1 hunks)src/components/Facility/ConsultationDetails/ConsultationNursingTab.tsx(1 hunks)src/components/Facility/ConsultationDetails/index.tsx(2 hunks)src/components/Patient/PatientRegister.tsx(3 hunks)src/components/ui/alert-dialog.tsx(1 hunks)src/pages/Apps/PlugConfigEdit.tsx(1 hunks)src/pages/Apps/PlugConfigList.tsx(1 hunks)src/pluginTypes.ts(0 hunks)src/types/plugConfig.ts(1 hunks)tsconfig.json(2 hunks)vite.config.mts(4 hunks)
💤 Files with no reviewable changes (2)
- src/pluginTypes.ts
- scripts/setup-care-apps.js
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- src/Utils/request/request.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Routers/AppRouter.tsx
🧰 Additional context used
📓 Learnings (1)
src/components/Facility/ConsultationDetails/ConsultationNursingTab.tsx (1)
Learnt from: sainak
PR: ohcnetwork/care_fe#9079
File: src/components/Facility/ConsultationDetails/ConsultationNursingTab.tsx:93-117
Timestamp: 2024-11-13T11:33:22.403Z
Learning: In `src/components/Facility/ConsultationDetails/ConsultationNursingTab.tsx`, the parent component manages the loading state, so child components like `NursingPlot` should not implement their own loading indicators.
🔇 Additional comments (19)
src/components/Facility/ConsultationDetails/ConsultationNursingTab.tsx (1)
Line range hint 211-213: Move loading state to parent component.
Based on previous learnings from PR #9079, the parent component should manage loading states. Consider lifting the loading state to ConsultationNursingTab.
src/components/Patient/PatientRegister.tsx (2)
87-91: LGTM! Import changes are well-organized.
The new imports for ErrorPage and dialog components are properly structured using relative paths.
854-854: LGTM! Error handling update is consistent.
The replacement of Error404 with ErrorPage aligns with the standardized error handling approach mentioned in the summary.
.env (2)
10-10: Verify that REACT_CARE_API_URL is correct and accessible
Ensure that the updated API URL https://care-api.do.ohc.network is correct and that the application can reach it successfully.
Run the following script to verify the API URL:
✅ Verification successful
API endpoint is accessible and responding correctly
The API endpoint https://care-api.do.ohc.network is responding with HTTP 200 OK status, confirming that it's accessible and functioning as expected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the API URL returns a successful response
# Test: Send a HEAD request to the API URL. Expect: HTTP 200 OK
curl -I https://care-api.do.ohc.network | head -n 1
Length of output: 461
17-17: Confirm configuration of REACT_ENABLED_APPS
Verify that REACT_ENABLED_APPS is correctly set to "ohcnetwork/care_livekit_fe@main" and that the referenced app is available and compatible.
Run the following script to check the availability of the remote application:
✅ Verification successful
Remote application is accessible and correctly configured
The remote application ohcnetwork/care_livekit_fe@main is accessible at the expected URL, confirmed by the successful HTTP 200 response from the remoteEntry.js endpoint. The REACT_ENABLED_APPS configuration appears to be valid and properly set up.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the remote application's remoteEntry.js file is accessible
# Parse the organization, repository, and branch
org="ohcnetwork"
repo="care_livekit_fe"
branch="main"
# Construct the remote URL
remote_url="https://${org}.github.io/${repo}/assets/remoteEntry.js"
# Test: Send a HEAD request to the remote URL. Expect: HTTP 200 OK
curl -I "${remote_url}" | head -n 1
Length of output: 551
tsconfig.json (1)
3-4: Verify browser compatibility with ES2022.
The target has been set to ES2022. While this is good for modern features, ensure your target browsers support ES2022 features.
✅ Verification successful
ES2022 target is appropriate for the project's browser support
The browserslist configuration and development setup indicate good compatibility with ES2022:
- Production targets modern browsers (
>0.2%,not dead) - Development targets latest versions of major browsers
- Project uses Vite with SWC for modern build tooling
- No explicit polyfills or legacy browser support requirements found
The ES2022 target in tsconfig.json aligns well with the project's browser compatibility requirements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check browserslist configuration
fd -t f -e json package.json -x cat {} | jq -r '.browserslist // empty'
Length of output: 263
Script:
#!/bin/bash
# Check package.json for any transpilation tools and their configurations
fd -t f -e json package.json -x cat {} | jq -r '.dependencies, .devDependencies'
# Look for babel configuration files
fd -t f -g '.babelrc*' -g 'babel.config.*'
# Check for any polyfill imports or core-js usage in the codebase
rg -l "core-js|@babel/polyfill|regenerator-runtime"
Length of output: 3823
netlify.toml (1)
7-7: Verify impact of removing legacy-peer-deps flag.
The change from --legacy-peer-deps to --no-optional might affect dependency resolution. Ensure all required dependencies install correctly.
✅ Verification successful
Removing --legacy-peer-deps flag is safe in this context
The change from --legacy-peer-deps to --no-optional is safe because:
- The package.json shows only two optional dependencies (
@rollup/rollup-linux-arm64-gnuand@rollup/rollup-linux-x64-gnu) which are platform-specific build tools - There are no peer dependency conflicts visible in the dependencies
- The project uses a modern Node.js version (>=22.11.0) and npm version (10.9.0) which have improved dependency resolution
- The core dependencies are explicitly specified with their versions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for peer dependency warnings in package.json
fd -t f -e json package.json -x cat {} | jq -r '.peerDependencies // empty'
Length of output: 79
Script:
#!/bin/bash
# Let's check the package.json content to understand dependencies
fd -t f -e json package.json -x cat {}
# Also check for any npm-related scripts or configurations
fd -t f -e json package.json -x jq '.scripts'
Length of output: 7415
src/Utils/AuthorizeFor.tsx (1)
3-3: Verify error page behavior consistency
The change from Error404 to ErrorPage aligns with the centralized error handling approach. However, we should ensure the user experience remains consistent.
Also applies to: 50-50
✅ Verification successful
Error page behavior is consistent and improved
The new DefaultErrorPage component maintains consistent behavior and actually improves the error handling by:
- Supporting multiple error types (404 and page load errors) through a single component
- Using proper translations for all text content
- Maintaining the same visual elements (404.svg image)
- Including a consistent "Return to Care" navigation link
- Properly cleaning up notifications on mount
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if Error404 is still referenced elsewhere in the codebase
rg "Error404" --type tsx --type ts
# Check for other potential error page implementations
fd -e tsx -e ts -x grep -l "ErrorPage" {}
Length of output: 377
Script:
#!/bin/bash
# Check Error404 references with ripgrep's default file type detection
rg "Error404"
# Look at the implementation of DefaultErrorPage to verify its behavior
ast-grep --pattern 'const DefaultErrorPage = $_' -l typescript,typescriptreact
# Check the actual content of DefaultErrorPage
cat src/components/ErrorPages/DefaultErrorPage.tsx
Length of output: 1703
src/components/ErrorPages/DefaultErrorPage.tsx (1)
7-11: LGTM: Well-defined types for error handling
The introduction of ErrorType and ErrorPageProps provides good type safety and extensibility for error scenarios.
src/PluginEngine.tsx (1)
Line range hint 1-1: PR Requirements Check
- The PR description is marked as TBD. Please update it to describe the changes and their purpose.
- As per the checklist, ensure documentation at docs.ohc.network is updated to reflect these error handling changes.
- Good job maintaining UI text in i18n files across the error pages.
package.json (3)
62-62: Warning: Cannot remove @radix-ui/react-icons as it's actively used
160-163: Verify platform-specific dependencies
The addition of optional dependencies for Linux suggests platform-specific build requirements.
✅ Verification successful
Let me check the install-platform-deps.ts file to better understand how these dependencies are being used.
Platform-specific dependencies are correctly configured
The optional dependencies are properly managed through the install-platform-deps.ts script, which installs the appropriate Rollup binaries based on the host platform and architecture. The script handles both Linux (arm64/x64) and Darwin (arm64/x64) platforms, making these dependencies essential for the build process on Linux systems.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any platform-specific code that might require these dependencies
rg -g "!package*.json" "rollup.*linux"
Length of output: 2666
Script:
#!/bin/bash
# Check the content of install-platform-deps.ts to understand the dependency usage
cat scripts/install-platform-deps.ts
Length of output: 835
36-38: Verify the setup script execution order
The build script now runs setup before other commands. Ensure this doesn't break existing build processes.
✅ Verification successful
Let me check the content of these scripts to understand their dependencies and execution order.
Let me check the git history to see if this is a new change in the build order and what was the previous setup.
After analyzing the scripts' content, I can now provide a final response:
Build script order change is safe
The setup script (setup-care-apps.ts) generates a plugin map based on environment variables, which is required for the subsequent build steps. Running it before build:meta, supported-browsers, and build:react is the correct order as:
- It creates the necessary
pluginMap.tsfile that other build steps might depend on - It doesn't have any dependencies on the other build steps
- The platform dependencies script runs separately during
postinstall, handling only runtime dependencies
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if setup script exists and is executable
test -f scripts/setup-care-apps.ts && echo "Setup script exists" || echo "Setup script missing"
test -f scripts/install-platform-deps.ts && echo "Platform deps script exists" || echo "Platform deps script missing"
Length of output: 193
Script:
#!/bin/bash
# Check the content of both scripts
echo "=== Content of setup-care-apps.ts ==="
cat scripts/setup-care-apps.ts
echo -e "\n=== Content of install-platform-deps.ts ==="
cat scripts/install-platform-deps.ts
Length of output: 2867
Script:
#!/bin/bash
# Check recent changes to package.json
git log -p -5 -- package.json | grep -A 5 '"build":'
Length of output: 0
src/components/Facility/ConsultationDetails/index.tsx (1)
9-9: Verify consistent error handling across the application
The change from Error404 to ErrorPage should be consistent with other error handling implementations.
Also applies to: 189-189
✅ Verification successful
Error handling is consistently using DefaultErrorPage component
The search results show that Error404 is not used anywhere in the codebase, and ErrorPage (imported from @/components/ErrorPages/DefaultErrorPage) is consistently used across the application for error handling. The change in the ConsultationDetails component aligns with the existing error handling pattern used in other components like AppRouter, AuthorizeFor, and PatientRegister.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other usages of Error404 that might need updating
rg "Error404" --type ts --type tsx
# Check for existing usage patterns of ErrorPage
rg "ErrorPage" --type ts --type tsx
Length of output: 126
Script:
#!/bin/bash
# Search for Error404 occurrences without file type restrictions
rg "Error404"
# Search for ErrorPage occurrences without file type restrictions
rg "ErrorPage"
# Find all TypeScript and TypeScript React files that might contain error-related components
fd "\.(ts|tsx)$" | grep -i "error"
Length of output: 1737
src/Utils/request/api.tsx (4)
93-99: LGTM: Clean imports for new functionality
The new imports are well-organized and properly typed, supporting the questionnaire and plug config features.
1424-1481: Add validation for questionnaire response values
The questionnaire submit endpoint accepts complex response data without any validation. Add runtime validation to ensure:
- Required fields are present
- Response values match their expected types (string/number/boolean)
- Optional fields (note, bodysite, method) are properly typed when present
1483-1490: LGTM: Well-structured batch request endpoint
The batch request endpoint is properly implemented with appropriate typing for both request and response.
1492-1499: Fix inconsistent placeholder syntax in route path
In the route definition for patient.allergyIntolerance.create, the path uses :patientId as a placeholder. This is inconsistent with the rest of the codebase, which uses {patientId} for placeholders. Replace :patientId with {patientId} to maintain consistency.
public/locale/en.json (1)
325-325: LGTM: Clear and consistent localization strings
The new localization strings are well-written, user-friendly, and follow the existing pattern in the file. They provide clear feedback for settings and error states.
Also applies to: 534-534, 1024-1024
| await this.loadScript(config.url); | ||
| const pluginModule = (window as any)[config.name]; | ||
| if (!pluginModule) { | ||
| throw new Error( | ||
| `Plugin ${config.name} not found after loading script`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Review security implications of dynamically loading external scripts
Loading scripts from external URLs (config.url) and executing them can introduce security vulnerabilities such as XSS attacks. Ensure that all plugin URLs are from trusted sources and consider implementing security measures like Subresource Integrity (SRI) or Content Security Policy (CSP) to mitigate risks.
| (window as any).React = React; | ||
| (window as any).ReactDOM = ReactDOM; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid assigning React and ReactDOM to the global window object
Assigning React and ReactDOM to the global window object can lead to conflicts and unintended side effects, especially in a complex application with multiple scripts. Consider passing these libraries directly to your plugins or utilizing a module bundler that can handle dependencies more elegantly.
Apply this diff to remove the global assignments:
- (window as any).React = React;
- (window as any).ReactDOM = ReactDOM;Consider refactoring plugin loading to provide React and ReactDOM without polluting the global namespace.
Committable suggestion skipped: line range outside the PR's diff.
vite.config.mts
Outdated
| federation({ | ||
| name: "core", | ||
| remotes: { | ||
| care_livekit_fe: { | ||
| external: `Promise.resolve("http://localhost:5173/assets/remoteEntry.js")`, | ||
| externalType: "promise", | ||
| from: "vite", | ||
| }, | ||
| }, | ||
| shared: ["react", "react-dom"], | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use getRemotes function to configure remotes dynamically
The getRemotes function is defined but not utilized. Instead of hardcoding the remotes, consider using getRemotes(env.REACT_ENABLED_APPS) to dynamically configure them based on environment variables.
Apply this diff to modify the federation plugin configuration:
federation({
name: "core",
- remotes: {
- care_livekit_fe: {
- external: `Promise.resolve("http://localhost:5173/assets/remoteEntry.js")`,
- externalType: "promise",
- from: "vite",
- },
- },
+ remotes: getRemotes(env.REACT_ENABLED_APPS),
shared: ["react", "react-dom"],
}),Committable suggestion skipped: line range outside the PR's diff.
scripts/install-platform-deps.ts
Outdated
| execSync(`npm install --no-save ${platformDeps.join(" ")}`, { | ||
| stdio: "inherit", | ||
| }); |
There was a problem hiding this comment.
Security: Avoid using execSync with user input.
While there's no direct user input here, using execSync can be dangerous. Consider using spawn or exec with proper argument escaping.
- execSync(`npm install --no-save ${platformDeps.join(" ")}`, {
+ const { spawn } = require('child_process');
+ const npmInstall = spawn('npm', ['install', '--no-save', ...platformDeps], {
stdio: "inherit",
- });
+ });
+ npmInstall.on('error', (error) => {
+ throw error;
+ });Committable suggestion skipped: line range outside the PR's diff.
| fs.writeFileSync(pluginMapPath, pluginMapContent); | ||
| console.log("Generated pluginMap.ts"); |
There was a problem hiding this comment.
Add error handling for file operations
File system operations should be wrapped in try-catch blocks to handle potential errors.
-fs.writeFileSync(pluginMapPath, pluginMapContent);
-console.log("Generated pluginMap.ts");
+try {
+ fs.writeFileSync(pluginMapPath, pluginMapContent);
+ console.log("Generated pluginMap.ts");
+} catch (error) {
+ console.error("Failed to write pluginMap.ts:", error);
+ process.exit(1);
+}📝 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.
| fs.writeFileSync(pluginMapPath, pluginMapContent); | |
| console.log("Generated pluginMap.ts"); | |
| try { | |
| fs.writeFileSync(pluginMapPath, pluginMapContent); | |
| console.log("Generated pluginMap.ts"); | |
| } catch (error) { | |
| console.error("Failed to write pluginMap.ts:", error); | |
| process.exit(1); | |
| } |
| // Generate pluginMap.ts | ||
| const pluginMapPath = path.join(__dirname, "..", "src", "pluginMap.ts"); | ||
| const pluginMapContent = `// Use type assertion for the static import | ||
| ${plugins | ||
| .map( | ||
| (plugin) => | ||
| `// @ts-expect-error Remote module will be available at runtime\nimport ${plugin.camelCaseName}Manifest from "${plugin.repo}/manifest";`, | ||
| ) | ||
| .join("\n")} | ||
|
|
||
| import type { PluginManifest } from "./pluginTypes"; | ||
|
|
||
| const pluginMap: PluginManifest[] = [ | ||
| ${plugins.map((plugin) => `${plugin.camelCaseName}Manifest as PluginManifest`).join(",\n ")} | ||
| ]; | ||
|
|
||
| export { pluginMap }; | ||
| `; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding type validation for generated content
The generated TypeScript content should be validated to ensure type safety.
+// Add type validation helper
+function validatePluginManifest(manifest: unknown): manifest is PluginManifest {
+ // Add runtime type checks based on PluginManifest interface
+ return true; // Implement actual validation
+}
+
const pluginMapContent = `// Use type assertion for the static import
${plugins
.map(
(plugin) =>
`// @ts-expect-error Remote module will be available at runtime\nimport ${plugin.camelCaseName}Manifest from "${plugin.repo}/manifest";`,
)
.join("\n")}
import type { PluginManifest } from "./pluginTypes";
+// Add runtime type validation
const pluginMap: PluginManifest[] = [
${plugins.map((plugin) => `
+ // Validate at runtime
+ validatePluginManifest(${plugin.camelCaseName}Manifest)
+ ? (${plugin.camelCaseName}Manifest as PluginManifest)
+ : (() => { throw new Error(\`Invalid manifest for ${plugin.repo}\`) })()
`).join(",\n ")}
];Committable suggestion skipped: line range outside the PR's diff.
| // Function to read enabled apps from env | ||
| function readAppsConfig(): Plugin[] { | ||
| const appsConfig = process.env.REACT_ENABLED_APPS | ||
| ? process.env.REACT_ENABLED_APPS.split(",").map((app) => { | ||
| const [package_] = app.split("@"); | ||
| const [, repo] = package_.split("/"); | ||
| return { | ||
| repo, | ||
| // Convert repo name to camelCase for import | ||
| camelCaseName: repo | ||
| .replace(/[-_]/g, "") | ||
| .replace(/\b\w/g, (char, index) => | ||
| index === 0 ? char.toLowerCase() : char.toUpperCase(), | ||
| ), | ||
| }; | ||
| }) | ||
| : []; | ||
| console.log("Found plugins: ", appsConfig); | ||
| return appsConfig; | ||
| } |
There was a problem hiding this comment.
Add input validation for environment variables
The function assumes environment variables are properly formatted but doesn't validate the input structure.
function readAppsConfig(): Plugin[] {
+ if (!process.env.REACT_ENABLED_APPS) {
+ console.warn('REACT_ENABLED_APPS is not defined');
+ return [];
+ }
+
+ const apps = process.env.REACT_ENABLED_APPS.split(',');
+
+ return apps.reduce<Plugin[]>((acc, app) => {
+ const parts = app.trim().split('@');
+ if (parts.length !== 2) {
+ console.warn(`Invalid app format: ${app}`);
+ return acc;
+ }
+
+ const [package_] = parts;
+ const repoParts = package_.split('/');
+ if (repoParts.length !== 2) {
+ console.warn(`Invalid package format: ${package_}`);
+ return acc;
+ }
+
+ const [, repo] = repoParts;
+ return [...acc, {
+ repo,
+ camelCaseName: repo
+ .replace(/[-_]/g, '')
+ .replace(/\b\w/g, (char, index) =>
+ index === 0 ? char.toLowerCase() : char.toUpperCase()
+ ),
+ }];
+ }, []);
}📝 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.
| // Function to read enabled apps from env | |
| function readAppsConfig(): Plugin[] { | |
| const appsConfig = process.env.REACT_ENABLED_APPS | |
| ? process.env.REACT_ENABLED_APPS.split(",").map((app) => { | |
| const [package_] = app.split("@"); | |
| const [, repo] = package_.split("/"); | |
| return { | |
| repo, | |
| // Convert repo name to camelCase for import | |
| camelCaseName: repo | |
| .replace(/[-_]/g, "") | |
| .replace(/\b\w/g, (char, index) => | |
| index === 0 ? char.toLowerCase() : char.toUpperCase(), | |
| ), | |
| }; | |
| }) | |
| : []; | |
| console.log("Found plugins: ", appsConfig); | |
| return appsConfig; | |
| } | |
| // Function to read enabled apps from env | |
| function readAppsConfig(): Plugin[] { | |
| if (!process.env.REACT_ENABLED_APPS) { | |
| console.warn('REACT_ENABLED_APPS is not defined'); | |
| return []; | |
| } | |
| const apps = process.env.REACT_ENABLED_APPS.split(','); | |
| return apps.reduce<Plugin[]>((acc, app) => { | |
| const parts = app.trim().split('@'); | |
| if (parts.length !== 2) { | |
| console.warn(`Invalid app format: ${app}`); | |
| return acc; | |
| } | |
| const [package_] = parts; | |
| const repoParts = package_.split('/'); | |
| if (repoParts.length !== 2) { | |
| console.warn(`Invalid package format: ${package_}`); | |
| return acc; | |
| } | |
| const [, repo] = repoParts; | |
| return [...acc, { | |
| repo, | |
| camelCaseName: repo | |
| .replace(/[-_]/g, '') | |
| .replace(/\b\w/g, (char, index) => | |
| index === 0 ? char.toLowerCase() : char.toUpperCase() | |
| ), | |
| }]; | |
| }, []); | |
| } |
| const handleDelete = async () => { | ||
| try { | ||
| await request(routes.plugConfig.deletePlugConfig, { | ||
| pathParams: { slug }, | ||
| }); | ||
| navigate("/apps"); | ||
| } catch (error) { | ||
| console.error("Error deleting config:", error); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling for delete operation
The delete operation lacks proper error feedback to the user.
Add user feedback:
const handleDelete = async () => {
try {
await request(routes.plugConfig.deletePlugConfig, {
pathParams: { slug },
});
navigate("/apps");
} catch (error) {
- console.error("Error deleting config:", error);
+ alert("Failed to delete configuration");
+ console.error("Error deleting config:", error);
}
};📝 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.
| const handleDelete = async () => { | |
| try { | |
| await request(routes.plugConfig.deletePlugConfig, { | |
| pathParams: { slug }, | |
| }); | |
| navigate("/apps"); | |
| } catch (error) { | |
| console.error("Error deleting config:", error); | |
| } | |
| }; | |
| const handleDelete = async () => { | |
| try { | |
| await request(routes.plugConfig.deletePlugConfig, { | |
| pathParams: { slug }, | |
| }); | |
| navigate("/apps"); | |
| } catch (error) { | |
| alert("Failed to delete configuration"); | |
| console.error("Error deleting config:", error); | |
| } | |
| }; |
| const handleSubmit = async (e: React.FormEvent) => { | ||
| e.preventDefault(); | ||
| // Meta is parsed to a JSON object to clear whitespaces when saving | ||
| const meta = JSON.parse(config.meta); | ||
| const configPayload = { ...config, meta }; | ||
| try { | ||
| if (isNew) { | ||
| await request(routes.plugConfig.createPlugConfig, { | ||
| body: configPayload, | ||
| }); | ||
| } else { | ||
| await request(routes.plugConfig.updatePlugConfig, { | ||
| pathParams: { slug }, | ||
| body: configPayload, | ||
| }); | ||
| } | ||
| navigate("/apps"); | ||
| } catch (error) { | ||
| console.error("Error saving config:", error); | ||
| } |
There was a problem hiding this comment.
Add error handling and user feedback for JSON parsing
The JSON parsing could fail if invalid JSON is entered, but there's no user feedback mechanism.
Consider adding try-catch and validation:
const handleSubmit = async (e: React.FormEvent) => {
e.preventDefault();
- // Meta is parsed to a JSON object to clear whitespaces when saving
- const meta = JSON.parse(config.meta);
- const configPayload = { ...config, meta };
try {
+ // Validate JSON format before saving
+ const meta = JSON.parse(config.meta);
+ const configPayload = { ...config, meta };
if (isNew) {
await request(routes.plugConfig.createPlugConfig, {
body: configPayload,
});
} else {
await request(routes.plugConfig.updatePlugConfig, {
pathParams: { slug },
body: configPayload,
});
}
navigate("/apps");
} catch (error) {
- console.error("Error saving config:", error);
+ if (error instanceof SyntaxError) {
+ // Handle JSON parsing error
+ alert("Invalid JSON format in meta field");
+ } else {
+ // Handle API error
+ alert("Error saving configuration");
+ console.error("Error saving config:", error);
+ }
}
};📝 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.
| const handleSubmit = async (e: React.FormEvent) => { | |
| e.preventDefault(); | |
| // Meta is parsed to a JSON object to clear whitespaces when saving | |
| const meta = JSON.parse(config.meta); | |
| const configPayload = { ...config, meta }; | |
| try { | |
| if (isNew) { | |
| await request(routes.plugConfig.createPlugConfig, { | |
| body: configPayload, | |
| }); | |
| } else { | |
| await request(routes.plugConfig.updatePlugConfig, { | |
| pathParams: { slug }, | |
| body: configPayload, | |
| }); | |
| } | |
| navigate("/apps"); | |
| } catch (error) { | |
| console.error("Error saving config:", error); | |
| } | |
| const handleSubmit = async (e: React.FormEvent) => { | |
| e.preventDefault(); | |
| try { | |
| // Validate JSON format before saving | |
| const meta = JSON.parse(config.meta); | |
| const configPayload = { ...config, meta }; | |
| if (isNew) { | |
| await request(routes.plugConfig.createPlugConfig, { | |
| body: configPayload, | |
| }); | |
| } else { | |
| await request(routes.plugConfig.updatePlugConfig, { | |
| pathParams: { slug }, | |
| body: configPayload, | |
| }); | |
| } | |
| navigate("/apps"); | |
| } catch (error) { | |
| if (error instanceof SyntaxError) { | |
| // Handle JSON parsing error | |
| alert("Invalid JSON format in meta field"); | |
| } else { | |
| // Handle API error | |
| alert("Error saving configuration"); | |
| console.error("Error saving config:", error); | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
vite.config.mts (2)
87-112: Consider enhancing error handling for edge casesThe recursive implementation looks good, but consider adding error handling for:
- Invalid module IDs
- Maximum recursion depth
- Malformed module info
function isStaticallyImportedByEntry( getModuleInfo: (moduleId: string) => any, moduleId: string, visited = new Set(), + depth = 0, ) { + const MAX_DEPTH = 100; + if (depth > MAX_DEPTH) { + console.warn(`Maximum recursion depth reached for module: ${moduleId}`); + return false; + } + if (typeof moduleId !== 'string') { + console.warn('Invalid module ID provided'); + return false; + } if (visited.has(moduleId)) return false; visited.add(moduleId); const modInfo = getModuleInfo(moduleId); if (!modInfo) return false; if (modInfo.isEntry) return true; for (const importerId of modInfo.importers) { - if (isStaticallyImportedByEntry(getModuleInfo, importerId, visited)) { + if (isStaticallyImportedByEntry(getModuleInfo, importerId, visited, depth + 1)) { return true; } } return false; }
305-309: Consider strengthening CSP directivesThe CSP configuration could be enhanced for better security:
- Add
frame-ancestorsdirective to prevent clickjacking- Consider using
strict-dynamicwith nonces- Evaluate if all domains in
connect-srcare necessary"Content-Security-Policy-Report-Only": `default-src 'self';\ script-src 'self' blob: 'nonce-f51b9742' https://plausible.10bedicu.in;\ style-src 'self' 'unsafe-inline';\ connect-src 'self' https://plausible.10bedicu.in;\ img-src 'self' https://cdn.ohc.network ${cdnUrls};\ - object-src 'self' ${cdnUrls};`, + object-src 'self' ${cdnUrls};\ + frame-ancestors 'none';\ + base-uri 'self';`,src/components/Questionnaire/QuestionTypes/MedicationQuestion.tsx (2)
51-56: Add JSDoc documentation and improve type safetyConsider adding JSDoc documentation to describe the component's purpose and props. Also, consider making the
questionprop more specific to medication questions.+/** + * A form component for handling FHIR medication requests in questionnaires. + * Allows users to add, remove, and modify medication requests with dosage, + * intent, category, priority, and status. + */ interface MedicationQuestionProps { + /** The medication-specific question configuration */ question: Question; + /** The current questionnaire response containing medication requests */ questionnaireResponse: QuestionnaireResponse; + /** Callback to update the questionnaire response */ updateQuestionnaireResponseCB: (response: QuestionnaireResponse) => void; + /** Whether the form is disabled */ disabled?: boolean; }
58-359: Consider architectural improvements for better maintainabilityThe component handles a lot of complexity. Consider these architectural improvements:
- Split the medication row into a separate component for better maintainability
- Add a custom hook for medication state management
- Implement proper form validation using a form library like react-hook-form
- Add unit tests to ensure FHIR compliance
Would you like me to help create a GitHub issue to track these improvements?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/components/Questionnaire/QuestionTypes/MedicationQuestion.tsx(1 hunks)src/types/emr/medicationRequest.ts(1 hunks)vite.config.mts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/types/emr/medicationRequest.ts
🔇 Additional comments (4)
vite.config.mts (2)
114-165: Well-implemented module federation configuration
The implementation successfully addresses the previous review comment by utilizing the getRemotes function. The function:
- Handles both GitHub Pages and local development URLs
- Includes clear documentation
- Implements proper error handling and logging
Also applies to: 187-198
288-290: Verify browser compatibility and build performance impacts
The ES2022 target and commented-out dependency optimization may have implications:
- ES2022 features might not work in older browsers
- Disabling dependency optimization could affect build performance
Also applies to: 292-298
✅ Verification successful
Browser compatibility is well-configured, dependency optimization can be safely disabled
Based on the findings:
- The project has explicit browser targets in package.json (
>0.2%, not dead, not op_mini all), which ensures broad modern browser compatibility - The ES2022 target is safe as it's supported by all modern browsers that meet the browserslist criteria
- Dependency optimization is optional in Vite and can be safely disabled when not needed for performance optimization
The commented-out optimizeDeps configuration won't impact build performance significantly as Vite's default behavior is already optimized.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check browser compatibility for ES2022
npx browserslist "supports es2022"
# Check if any dependencies require optimization
fd -e json$ -x jq -r '.dependencies | keys[]' {} \; | sort -u > /tmp/deps.txt
rg -l "import.*from.*node_modules" | while read -r file; do
echo "File with direct node_modules imports: $file"
done
Length of output: 251
Script:
#!/bin/bash
# Let's try a different approach to verify the implications
# 1. Check for browser/environment targets in configuration
rg -A 5 "target.*[\"']es"
# 2. Check for optimizeDeps usage and configurations
rg -A 5 "optimizeDeps"
# 3. Look for any performance-related comments or configurations
rg -i "(performance|optimization|speed)" .
# 4. Check for browser compatibility configurations
fd "browserslist|.browserslistrc"
cat .browserslistrc 2>/dev/null || true
Length of output: 40375
Script:
#!/bin/bash
# Let's check for browser compatibility configurations in package.json and any other relevant files
rg -A 5 "browserslist|targets" package.json
# Check for any Vite-specific browser compatibility configurations
rg -A 5 "browser|target" vite.config.mts
# Look for any dependency optimization related comments or configurations
rg -B 2 -A 2 "optimizeDeps|dependencies|optimization" vite.config.mts
# Check if there are any performance-related comments
rg -B 2 "//.*performance|/\*.*performance" vite.config.mts
Length of output: 1432
src/components/Questionnaire/QuestionTypes/MedicationQuestion.tsx (2)
70-74: 🛠️ Refactor suggestion
Improve medication search implementation
The current implementation needs several improvements:
Previous review comments already suggested improvements for the search functionality. The suggestions remain valid:
- Add debouncing to prevent excessive API calls
- Add proper error handling
- Remove the filter bypass
Please refer to the previous review comments for the implementation details.
173-177: 🛠️ Refactor suggestion
Improve loading and error states
The loading and error states need better handling and accessibility.
<CommandEmpty>
- {medicationSearch.loading
- ? "Loading..."
- : "No medications found"}
+ {medicationSearch.loading ? (
+ <div role="status" aria-live="polite">
+ {t('medication.search.loading')}
+ </div>
+ ) : medicationSearch.error ? (
+ <div role="alert" className="text-red-500">
+ {t('medication.search.error')}
+ </div>
+ ) : (
+ <div role="status" aria-live="polite">
+ {t('medication.search.no_results')}
+ </div>
+ )}
</CommandEmpty>Likely invalid or redundant comment.
| <div className="space-y-4"> | ||
| <Label>{question.text}</Label> | ||
| <div className="rounded-lg border p-4"> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add proper accessibility attributes and internationalization
The form elements need proper accessibility attributes and internationalized text.
<div className="space-y-4">
- <Label>{question.text}</Label>
+ <Label htmlFor={`medication-${question.linkId}`}>{question.text}</Label>
- <div className="rounded-lg border p-4">
+ <div
+ className="rounded-lg border p-4"
+ role="region"
+ aria-labelledby={`medication-${question.linkId}`}
+ >📝 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.
| <div className="space-y-4"> | |
| <Label>{question.text}</Label> | |
| <div className="rounded-lg border p-4"> | |
| <div className="space-y-4"> | |
| <Label htmlFor={`medication-${question.linkId}`}>{question.text}</Label> | |
| <div | |
| className="rounded-lg border p-4" | |
| role="region" | |
| aria-labelledby={`medication-${question.linkId}`} | |
| > |
| <input | ||
| type="number" | ||
| className="w-full rounded-md border p-2" | ||
| placeholder="Dosage" | ||
| value={ | ||
| medication.dosage_instruction[0]?.dose_and_rate?.[0] | ||
| ?.dose_quantity?.value || "" | ||
| } | ||
| disabled={disabled} | ||
| /> | ||
| </TableCell> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add form validation and improve input accessibility
The dosage input needs proper validation and accessibility improvements.
<input
type="number"
+ id={`dosage-${index}`}
+ name={`dosage-${index}`}
+ aria-label={t('medication.dosage.label')}
className="w-full rounded-md border p-2"
- placeholder="Dosage"
+ placeholder={t('medication.dosage.placeholder')}
+ min="0"
+ step="0.1"
value={
medication.dosage_instruction[0]?.dose_and_rate?.[0]
?.dose_quantity?.value || ""
}
disabled={disabled}
+ onChange={(e) => {
+ const value = parseFloat(e.target.value);
+ if (!isNaN(value) && value >= 0) {
+ handleUpdateMedication(index, {
+ dosage_instruction: [{
+ dose_and_rate: [{
+ dose_quantity: {
+ value: value,
+ unit: 'mg', // Consider making this configurable
+ },
+ }],
+ }],
+ });
+ }
+ }}
/>Committable suggestion skipped: line range outside the PR's diff.
| const handleAddMedication = () => { | ||
| const newMedications: MedicationRequest[] = [ | ||
| ...medications, | ||
| { | ||
| status: "active", | ||
| intent: "plan", | ||
| category: "inpatient", | ||
| priority: "urgent", | ||
| do_not_perform: false, | ||
| medication: undefined, | ||
| authored_on: new Date().toISOString(), | ||
| dosage_instruction: [], | ||
| }, | ||
| ]; | ||
| setMedications(newMedications); | ||
| updateQuestionnaireResponseCB({ | ||
| ...questionnaireResponse, | ||
| values: [ | ||
| { | ||
| type: "medication_request", | ||
| value: newMedications, | ||
| }, | ||
| ], | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve medication request initialization and validation
The default values for new medications should be centralized and validated against FHIR specifications.
+const DEFAULT_MEDICATION_REQUEST: Omit<MedicationRequest, 'medication'> = {
+ status: 'active',
+ intent: 'plan',
+ category: 'inpatient',
+ priority: 'urgent',
+ do_not_perform: false,
+ authored_on: new Date().toISOString(),
+ dosage_instruction: [],
+};
const handleAddMedication = () => {
const newMedications: MedicationRequest[] = [
...medications,
{
- status: "active",
- intent: "plan",
- category: "inpatient",
- priority: "urgent",
- do_not_perform: false,
- medication: undefined,
- authored_on: new Date().toISOString(),
- dosage_instruction: [],
+ ...DEFAULT_MEDICATION_REQUEST,
+ medication: undefined,
},
];📝 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.
| const handleAddMedication = () => { | |
| const newMedications: MedicationRequest[] = [ | |
| ...medications, | |
| { | |
| status: "active", | |
| intent: "plan", | |
| category: "inpatient", | |
| priority: "urgent", | |
| do_not_perform: false, | |
| medication: undefined, | |
| authored_on: new Date().toISOString(), | |
| dosage_instruction: [], | |
| }, | |
| ]; | |
| setMedications(newMedications); | |
| updateQuestionnaireResponseCB({ | |
| ...questionnaireResponse, | |
| values: [ | |
| { | |
| type: "medication_request", | |
| value: newMedications, | |
| }, | |
| ], | |
| }); | |
| }; | |
| const DEFAULT_MEDICATION_REQUEST: Omit<MedicationRequest, 'medication'> = { | |
| status: 'active', | |
| intent: 'plan', | |
| category: 'inpatient', | |
| priority: 'urgent', | |
| do_not_perform: false, | |
| authored_on: new Date().toISOString(), | |
| dosage_instruction: [], | |
| }; | |
| const handleAddMedication = () => { | |
| const newMedications: MedicationRequest[] = [ | |
| ...medications, | |
| { | |
| ...DEFAULT_MEDICATION_REQUEST, | |
| medication: undefined, | |
| }, | |
| ]; | |
| setMedications(newMedications); | |
| updateQuestionnaireResponseCB({ | |
| ...questionnaireResponse, | |
| values: [ | |
| { | |
| type: "medication_request", | |
| value: newMedications, | |
| }, | |
| ], | |
| }); | |
| }; |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (5)
src/types/questionnaire/code.ts (1)
1-6: Add JSDoc documentation for the Code interface.Consider adding comprehensive documentation to explain the purpose and usage of this interface, as it appears to be a core type for FHIR implementation.
+/** + * Represents a FHIR coding element with system, code, and optional display text. + * @see https://www.hl7.org/fhir/datatypes.html#Coding + */ export interface Code { + /** URI of the coding system */ system: string; + /** Symbol in syntax defined by the system */ code: string; + /** Representation defined by the system */ display?: string; }src/components/Questionnaire/ValueSetSelect.tsx (2)
59-61: Implement search debouncing.The current implementation triggers an API call on every keystroke. Consider adding debouncing to optimize performance and reduce unnecessary API calls.
+import { useDebouncedCallback } from 'use-debounce'; +const debouncedSearch = useDebouncedCallback( + (search: string) => searchQuery.refetch({ body: { search } }), + 300 +); onValueChange={(search) => - searchQuery.refetch({ body: { search } }) + debouncedSearch(search) }
68-82: Add pagination support.The current implementation only fetches 10 results. Consider implementing pagination or infinite scroll for better user experience with large datasets.
Consider:
- Adding a "Load more" button
- Implementing infinite scroll
- Tracking total count and current page
- Showing loading state for subsequent pages
src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx (1)
30-35: Add validation props to interfaceConsider enhancing the interface with validation-related props to improve form handling.
interface AllergyQuestionProps { question: Question; questionnaireResponse: QuestionnaireResponse; updateQuestionnaireResponseCB: (response: QuestionnaireResponse) => void; disabled?: boolean; + validation?: { + required?: boolean; + minAllergies?: number; + maxAllergies?: number; + }; + error?: string; + onError?: (error: string) => void; }src/components/Questionnaire/QuestionTypes/MedicationQuestion.tsx (1)
154-257: Enhance FHIR compliance for select optionsConsider using proper FHIR value set bindings for select options.
The current implementation uses simple string arrays for options. Consider implementing proper FHIR value set bindings using the ValueSet resource. This would provide better interoperability and standardization.
Would you like me to provide an example implementation using FHIR ValueSet resources?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/MedicationQuestion.tsx(1 hunks)src/components/Questionnaire/ValueSetSelect.tsx(1 hunks)src/types/questionnaire/code.ts(1 hunks)
🔇 Additional comments (4)
src/types/questionnaire/code.ts (1)
8-20: Verify FHIR compliance of system URIs.
The current system identifiers appear to be custom strings rather than standard FHIR URIs. Consider using standard FHIR terminology system URIs where applicable (e.g., SNOMED CT, LOINC, etc.).
For example:
- For medications: "http://www.nlm.nih.gov/research/umls/rxnorm"
- For conditions: "http://snomed.info/sct"
- For observations: "http://loinc.org"
src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx (1)
102-104: 🛠️ Refactor suggestion
Improve accessibility attributes
Add proper accessibility attributes to form elements.
-<div className="space-y-4">
+<div className="space-y-4" role="group" aria-labelledby={`allergy-question-${question.linkId}`}>
- <Label>{question.text}</Label>
+ <Label id={`allergy-question-${question.linkId}`}>{question.text}</Label>Likely invalid or redundant comment.
src/components/Questionnaire/QuestionTypes/MedicationQuestion.tsx (2)
143-153: 🛠️ Refactor suggestion
Improve dosage input handling
Add proper validation and unit handling for dosage input.
<input
type="number"
+ min="0"
+ step="0.1"
className="w-full rounded-md border p-2"
- placeholder="Dosage"
+ placeholder="Dosage (mg)"
value={
medication.dosage_instruction[0]?.dose_and_rate?.[0]
?.dose_quantity?.value || ""
}
+ onChange={(e) => {
+ const value = parseFloat(e.target.value);
+ if (!isNaN(value) && value >= 0) {
+ handleUpdateMedication(index, {
+ dosage_instruction: [{
+ dose_and_rate: [{
+ dose_quantity: {
+ value: value,
+ unit: 'mg',
+ system: 'http://unitsofmeasure.org',
+ code: 'mg'
+ }
+ }]
+ }]
+ });
+ }
+ }}
disabled={disabled}
+ aria-label="Medication dosage in milligrams"
/>Likely invalid or redundant comment.
57-81: 🛠️ Refactor suggestion
Extract default medication request values
Move default values to a constant and add validation.
+const DEFAULT_MEDICATION_REQUEST: Omit<MedicationRequest, 'medication'> = {
+ status: 'active',
+ intent: 'plan',
+ category: 'inpatient',
+ priority: 'urgent',
+ do_not_perform: false,
+ authored_on: new Date().toISOString(),
+ dosage_instruction: [],
+};
const handleAddMedication = () => {
+ if (medications.length >= 10) {
+ onError?.('Maximum number of medications reached');
+ return;
+ }
const newMedications: MedicationRequest[] = [
...medications,
{
- status: "active",
- intent: "plan",
- category: "inpatient",
- priority: "urgent",
- do_not_perform: false,
- medication: undefined,
- authored_on: new Date().toISOString(),
- dosage_instruction: [],
+ ...DEFAULT_MEDICATION_REQUEST,
+ medication: undefined,
},
];Likely invalid or redundant comment.
| <Button | ||
| variant="outline" | ||
| role="combobox" | ||
| className="w-[300px] justify-between truncate" | ||
| > | ||
| {value?.display || placeholder} | ||
| </Button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance accessibility attributes.
The combobox implementation is missing some important ARIA attributes for better accessibility.
<Button
variant="outline"
role="combobox"
+ aria-expanded={open}
+ aria-controls="valueset-search"
+ aria-label={`Select ${placeholder}`}
className="w-[300px] justify-between truncate"
>📝 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.
| <Button | |
| variant="outline" | |
| role="combobox" | |
| className="w-[300px] justify-between truncate" | |
| > | |
| {value?.display || placeholder} | |
| </Button> | |
| <Button | |
| variant="outline" | |
| role="combobox" | |
| aria-expanded={open} | |
| aria-controls="valueset-search" | |
| aria-label={`Select ${placeholder}`} | |
| className="w-[300px] justify-between truncate" | |
| > | |
| {value?.display || placeholder} | |
| </Button> |
| const searchQuery = useQuery(routes.valueset.expand, { | ||
| pathParams: { system }, | ||
| body: { count: 10 }, | ||
| prefetch: false, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and loading states.
The API call lacks error handling and might benefit from a more robust implementation.
const searchQuery = useQuery(routes.valueset.expand, {
pathParams: { system },
body: { count: 10 },
prefetch: false,
+ onError: (error) => {
+ // Handle error appropriately
+ console.error('Failed to fetch valueset:', error);
+ },
});Consider implementing:
- Error state display in the UI
- Retry mechanism for failed requests
- Proper error boundaries
Committable suggestion skipped: line range outside the PR's diff.
| }); | ||
| }; | ||
|
|
||
| console.log(allergies); |
There was a problem hiding this comment.
Remove console.log statement
Remove debugging console.log statement before production deployment.
- console.log(allergies);| <input | ||
| type="date" | ||
| className="w-full rounded-md border p-2" | ||
| value={allergy.lastOccurrence || ""} | ||
| onChange={(e) => | ||
| updateAllergy(index, { lastOccurrence: e.target.value }) | ||
| } | ||
| disabled={disabled} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation for date field
Add proper validation and formatting for the date input.
<input
type="date"
className="w-full rounded-md border p-2"
+ max={new Date().toISOString().split('T')[0]}
value={allergy.lastOccurrence || ""}
- onChange={(e) =>
- updateAllergy(index, { lastOccurrence: e.target.value })
- }
+ onChange={(e) => {
+ const date = new Date(e.target.value);
+ if (!isNaN(date.getTime())) {
+ updateAllergy(index, { lastOccurrence: e.target.value });
+ }
+ }}
disabled={disabled}
+ aria-label="Last occurrence date"
/>📝 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.
| <input | |
| type="date" | |
| className="w-full rounded-md border p-2" | |
| value={allergy.lastOccurrence || ""} | |
| onChange={(e) => | |
| updateAllergy(index, { lastOccurrence: e.target.value }) | |
| } | |
| disabled={disabled} | |
| /> | |
| <input | |
| type="date" | |
| className="w-full rounded-md border p-2" | |
| max={new Date().toISOString().split('T')[0]} | |
| value={allergy.lastOccurrence || ""} | |
| onChange={(e) => { | |
| const date = new Date(e.target.value); | |
| if (!isNaN(date.getTime())) { | |
| updateAllergy(index, { lastOccurrence: e.target.value }); | |
| } | |
| }} | |
| disabled={disabled} | |
| aria-label="Last occurrence date" | |
| /> |
| const [allergies, setAllergies] = useState<AllergyIntolerance[]>(() => { | ||
| return ( | ||
| (questionnaireResponse.values?.[0]?.value as AllergyIntolerance[]) || [] | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for state initialization
Consider adding error handling for invalid questionnaire response values.
const [allergies, setAllergies] = useState<AllergyIntolerance[]>(() => {
+ try {
return (
(questionnaireResponse.values?.[0]?.value as AllergyIntolerance[]) || []
);
+ } catch (error) {
+ console.error('Error initializing allergies:', error);
+ onError?.('Failed to initialize allergies');
+ return [];
+ }
});📝 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.
| const [allergies, setAllergies] = useState<AllergyIntolerance[]>(() => { | |
| return ( | |
| (questionnaireResponse.values?.[0]?.value as AllergyIntolerance[]) || [] | |
| ); | |
| }); | |
| const [allergies, setAllergies] = useState<AllergyIntolerance[]>(() => { | |
| try { | |
| return ( | |
| (questionnaireResponse.values?.[0]?.value as AllergyIntolerance[]) || [] | |
| ); | |
| } catch (error) { | |
| console.error('Error initializing allergies:', error); | |
| onError?.('Failed to initialize allergies'); | |
| return []; | |
| } | |
| }); |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
src/components/Questionnaire/ValueSetSelect.tsx (1)
22-29: Add JSDoc documentation for the component and its props.The component would benefit from proper documentation explaining its purpose and usage.
+/** + * A reusable select component for FHIR ValueSet codes. + * @component + */ interface Props { + /** The ValueSet system identifier */ system: ValueSetSystem; + /** Currently selected code value */ value?: Code | null; + /** Callback fired when a new value is selected */ onSelect: (value: Code) => void; + /** Custom placeholder text */ placeholder?: string; + /** Custom message shown when no results are found */ noResultsMessage?: string; + /** Whether the select is disabled */ disabled?: boolean; }src/components/Questionnaire/QuestionTypes/MedicationQuestion.tsx (2)
119-134: Improve table accessibility and internationalization.Table headers should be internationalized and include proper accessibility attributes.
<TableRow> - <TableHead className="w-[200px]">Medication</TableHead> + <TableHead className="w-[200px]" scope="col">{t('medication.table.medication')}</TableHead> - <TableHead className="w-[150px]">Route</TableHead> + <TableHead className="w-[150px]" scope="col">{t('medication.table.route')}</TableHead> // Apply similar changes to other TableHead components </TableRow>
356-365: Enhance button interactions and internationalization.The buttons need internationalization and better user interaction patterns.
+import { useConfirm } from '@/hooks/useConfirm'; export function MedicationQuestion() { + const { confirm } = useConfirm(); + + const handleRemoveMedicationWithConfirm = async (index: number) => { + if (await confirm({ + title: t('medication.remove.confirm.title'), + message: t('medication.remove.confirm.message'), + })) { + handleRemoveMedication(index); + } + }; return ( <Button variant="outline" size="sm" className="mt-4" onClick={handleAddMedication} disabled={disabled} > <PlusIcon className="mr-2 h-4 w-4" /> - Add Medication + {t('medication.action.add')} </Button> ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/components/Questionnaire/QuestionTypes/MedicationQuestion.tsx(1 hunks)src/components/Questionnaire/ValueSetSelect.tsx(1 hunks)
🔇 Additional comments (5)
src/components/Questionnaire/ValueSetSelect.tsx (2)
50-56: 🛠️ Refactor suggestion
Enhance accessibility attributes.
The combobox implementation is missing important ARIA attributes.
<Button
variant="outline"
role="combobox"
+ aria-expanded={open}
+ aria-controls="valueset-search"
+ aria-label={`Select ${placeholder}`}
className="w-full justify-between truncate"
>
{value?.display || placeholder}
</Button>41-45: 🛠️ Refactor suggestion
Add error handling for API calls.
The API call lacks error handling and loading states.
const searchQuery = useQuery(routes.valueset.expand, {
pathParams: { system },
body: { count: 10 },
prefetch: false,
+ onError: (error) => {
+ console.error('Failed to fetch valueset:', error);
+ },
});src/components/Questionnaire/QuestionTypes/MedicationQuestion.tsx (3)
58-82: 🛠️ Refactor suggestion
Improve medication request initialization and validation.
The default values for new medications should be centralized and validated.
+const DEFAULT_MEDICATION_REQUEST: Omit<MedicationRequest, 'medication'> = {
+ status: 'active',
+ intent: 'plan',
+ category: 'inpatient',
+ priority: 'urgent',
+ do_not_perform: false,
+ authored_on: new Date().toISOString(),
+ dosage_instruction: [],
+};
const handleAddMedication = () => {
const newMedications: MedicationRequest[] = [
...medications,
{
- status: "active",
- intent: "plan",
- category: "inpatient",
- priority: "urgent",
- do_not_perform: false,
- medication: undefined,
- authored_on: new Date().toISOString(),
- dosage_instruction: [],
+ ...DEFAULT_MEDICATION_REQUEST,
+ medication: undefined,
},
];113-115: 🛠️ Refactor suggestion
Add proper accessibility attributes and internationalization.
The form elements need proper accessibility attributes.
<div className="space-y-4">
- <Label>{question.text}</Label>
+ <Label htmlFor={`medication-${question.linkId}`}>{question.text}</Label>
- <div className="rounded-lg border p-4">
+ <div
+ className="rounded-lg border p-4"
+ role="region"
+ aria-labelledby={`medication-${question.linkId}`}
+ >194-204: 🛠️ Refactor suggestion
Improve dosage input validation and unit handling.
The dosage input lacks proper validation and unit specification.
<Input
type="number"
+ min="0"
+ step="0.1"
placeholder="Dosage"
value={
medication.dosage_instruction[0]?.dose_and_rate?.[0]
?.dose_quantity?.value || ""
}
+ onChange={(e) => {
+ const value = parseFloat(e.target.value);
+ if (!isNaN(value) && value >= 0) {
+ handleUpdateMedication(index, {
+ dosage_instruction: [{
+ ...medication.dosage_instruction[0],
+ dose_and_rate: [{
+ dose_quantity: {
+ value,
+ unit: 'mg', // Consider making this configurable
+ },
+ }],
+ }],
+ });
+ }
+ }}
disabled={disabled}
/>Likely invalid or redundant comment.
| <Command filter={() => 1}> | ||
| <CommandInput | ||
| placeholder={placeholder} | ||
| className="outline-none border-none ring-0 shadow-none" | ||
| onValueChange={(search) => | ||
| searchQuery.refetch({ body: { search } }) | ||
| } | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize search implementation.
The current implementation has two potential performance issues:
- The Command filter is bypassed with
filter={() => 1} - Search is triggered on every keystroke without debouncing
+import { useDebouncedCallback } from 'use-debounce';
+
export default function ValueSetSelect({
// ...props
}) {
+ const debouncedSearch = useDebouncedCallback(
+ (search: string) => {
+ if (search.length >= 2) {
+ searchQuery.refetch({ body: { search } });
+ }
+ },
+ 300
+ );
+
return (
- <Command filter={() => 1}>
+ <Command>
<CommandInput
placeholder={placeholder}
className="outline-none border-none ring-0 shadow-none"
- onValueChange={(search) =>
- searchQuery.refetch({ body: { search } })
- }
+ onValueChange={debouncedSearch}
/>Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
src/types/questionnaire/form.ts (1)
24-33: Add JSDoc documentation for QuestionnaireResponseThe interface should be documented to clarify the purpose of each field and their relationship to FHIR QuestionnaireResponse resource.
+/** + * Represents a FHIR QuestionnaireResponse item + * @see https://www.hl7.org/fhir/questionnaireresponse.html + */ export interface QuestionnaireResponse { + /** Unique identifier for the question */ question_id: string; + /** Type of structured data, if applicable */ structured_type: StructuredQuestionType | null; + /** Corresponds to QuestionnaireItem.linkId */ link_id: string; + /** The response values for this item */ values: ResponseValue[]; + /** Additional notes about the response */ note?: string; + /** When this response was recorded */ taken_at?: string; + /** Body site for the response, if applicable */ body_site?: Code; + /** Method used for the response, if applicable */ method?: Code; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
src/types/questionnaire/code.ts(1 hunks)src/types/questionnaire/form.ts(1 hunks)src/types/questionnaire/quantity.ts(1 hunks)src/types/questionnaire/question.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/types/questionnaire/code.ts
🔇 Additional comments (3)
src/types/questionnaire/form.ts (1)
7-22: Add type validation for ResponseValue
The current type structure allows potential type mismatches between the discriminator and value.
src/types/questionnaire/question.ts (2)
19-21: Verify FHIR compliance for structured types
Ensure that the structured question types align with FHIR resource types.
✅ Verification successful
Let me search for FHIR resource type definitions to verify compliance.
Let me check the actual implementations and interfaces to verify FHIR compliance.
Structured types follow FHIR R4 resource definitions
The implemented types align with FHIR R4 specifications:
AllergyIntoleranceincludes standard FHIR fields: clinicalStatus, verificationStatus, category, criticalityMedicationRequestfollows FHIR structure with proper status codes, intent, category, timing, and dosage instructions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of structured types
rg -g '*.{ts,tsx}' -A 5 'structured_type.*allergy_intolerance|medication_request'
Length of output: 4772
Script:
#!/bin/bash
# Search for FHIR type definitions and interfaces
rg -g '*.{ts,tsx}' 'interface.*(?:AllergyIntolerance|MedicationRequest)' -A 5
# Also check for any FHIR-related types or constants
rg -g '*.{ts,tsx}' 'FHIR.*Resource.*Type' -A 5
Length of output: 1455
Script:
#!/bin/bash
# Check the complete interface definitions
cat src/types/questionnaire/allergyIntolerance.ts
cat src/types/emr/medicationRequest.ts
# Look for any FHIR-related imports or type definitions
rg -g '*.{ts,tsx}' 'from.*fhir|import.*fhir' -A 2
Length of output: 3545
3-17: Consider adding validation for QuestionType combinations
Some question types may have incompatible combinations with other fields. Consider adding type guards or validation.
src/types/questionnaire/quantity.ts
Outdated
| export interface Quantity { | ||
| value: number; | ||
| unit: string; | ||
| system?: string; | ||
| code?: string; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add JSDoc documentation and value constraints
The Quantity interface should include documentation and validation constraints to ensure FHIR compliance:
+/**
+ * Represents a FHIR Quantity datatype
+ * @see https://www.hl7.org/fhir/datatypes.html#Quantity
+ */
export interface Quantity {
- value: number;
+ // Ensure value is a valid decimal number
+ value: number & { __brand: 'ValidDecimal' };
unit: string;
+ // URI that identifies the system
system?: string;
+ // System-specific code for the unit
code?: string;
}Consider adding a type guard or validation function:
function isValidDecimal(value: number): value is number & { __brand: 'ValidDecimal' } {
return Number.isFinite(value) && !Number.isNaN(value);
}
src/types/questionnaire/question.ts
Outdated
| export interface Question { | ||
| id: string; | ||
| link_id: string; | ||
| code?: Code; | ||
| text: string; | ||
| description?: string; | ||
| type: QuestionType; | ||
| structured_type?: StructuredQuestionType; | ||
| required?: boolean; | ||
| collect_time?: boolean; | ||
| collect_performer?: boolean; | ||
| collect_body_site?: boolean; | ||
| collect_method?: boolean; | ||
| enable_when?: EnableWhen[]; | ||
| enable_behavior?: "all" | "any"; | ||
| disabled_display?: "hidden" | "protected"; | ||
| repeats?: boolean; | ||
| read_only?: boolean; | ||
| max_length?: number; | ||
| answer_constraint?: string; | ||
| answer_option?: AnswerOption[]; | ||
| answer_value_set?: string; | ||
| answer_unit?: Code; | ||
| is_observation?: boolean; | ||
| unit?: Code; | ||
| questions?: Question[]; | ||
| formula?: string; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add JSDoc documentation and field validations
The Question interface needs documentation and additional validations for certain fields.
+/**
+ * Represents a FHIR Questionnaire item
+ * @see https://www.hl7.org/fhir/questionnaire.html#QuestionnaireItem
+ */
export interface Question {
id: string;
link_id: string;
code?: Code;
text: string;
description?: string;
type: QuestionType;
structured_type?: StructuredQuestionType;
required?: boolean;
collect_time?: boolean;
collect_performer?: boolean;
collect_body_site?: boolean;
collect_method?: boolean;
enable_when?: EnableWhen[];
- enable_behavior?: "all" | "any";
+ // Ensure enable_behavior is set when enable_when has multiple conditions
+ enable_behavior: EnableWhen extends Array<infer T> ?
+ T[] extends [any, any, ...any[]] ? "all" | "any" : undefined :
+ undefined;
disabled_display?: "hidden" | "protected";
repeats?: boolean;
read_only?: boolean;
- max_length?: number;
+ // Ensure max_length is positive when specified
+ max_length?: number & { __brand: 'PositiveInteger' };
answer_constraint?: string;
answer_option?: AnswerOption[];
answer_value_set?: string;
answer_unit?: Code;
is_observation?: boolean;
unit?: Code;
questions?: Question[];
formula?: string;
}
+/**
+ * Type guard for positive integers
+ */
+function isPositiveInteger(value: number): value is number & { __brand: 'PositiveInteger' } {
+ return Number.isInteger(value) && value > 0;
+}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (3)
src/types/emr/allergyIntolerance.ts (1)
1-1: Consider restructuring the import pathThe Code type is imported from the questionnaire module, which suggests a potential architectural concern. Consider moving shared types to a common types directory.
-import { Code } from "../questionnaire/code"; +import { Code } from "../common/code";src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx (2)
106-118: Add loading and error states to the tableThe table should handle loading and error states gracefully.
+ const [isLoading, setIsLoading] = useState(false); + const [error, setError] = useState<string | null>(null); return ( <div className="space-y-4"> <Label>{question.text}</Label> <div className="rounded-lg border p-4"> <div className="overflow-x-auto"> + {isLoading && <div>Loading allergies...</div>} + {error && <div className="text-red-500">{error}</div>} + {!isLoading && !error && ( <Table>
123-130: Add error handling to ValueSetSelectThe ValueSetSelect component should handle loading and error states for the allergy code lookup.
<ValueSetSelect system="system-allergy-code" value={allergy.code} onSelect={(allergy) => updateAllergy(index, { code: allergy }) } disabled={disabled} + onError={(error) => setError(error)} + loading={isLoading} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx(1 hunks)src/components/Questionnaire/structured/types.ts(1 hunks)src/types/emr/allergyIntolerance.ts(1 hunks)src/types/questionnaire/form.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/types/questionnaire/form.ts
- src/components/Questionnaire/structured/types.ts
🔇 Additional comments (2)
src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx (2)
43-47:
Add type guard for questionnaire response values
Add type validation to ensure the response values are of the correct type.
const [allergies, setAllergies] = useState<AllergyIntolerance[]>(() => {
+ const responseValue = questionnaireResponse.values?.[0]?.value;
+ if (responseValue && !Array.isArray(responseValue)) {
+ console.error('Invalid questionnaire response value type');
+ return [];
+ }
return (
- (questionnaireResponse.values?.[0]?.value as AllergyIntolerance[]) || []
+ (responseValue as AllergyIntolerance[]) || []
);
});Likely invalid or redundant comment.
102-104: 🛠️ Refactor suggestion
Add proper accessibility attributes
Enhance accessibility by adding proper ARIA attributes and roles.
-<div className="space-y-4">
+<div className="space-y-4" role="region" aria-labelledby="allergy-section">
- <Label>{question.text}</Label>
+ <Label id="allergy-section">{question.text}</Label>
- <div className="rounded-lg border p-4">
+ <div className="rounded-lg border p-4" role="form" aria-label="Allergy form">Likely invalid or redundant comment.
src/types/emr/allergyIntolerance.ts
Outdated
| export interface AllergyIntolerance { | ||
| code: Code; | ||
| clinicalStatus?: string; | ||
| verificationStatus?: string; | ||
| category?: string; | ||
| criticality?: string; | ||
| lastOccurrence?: string; | ||
| note?: string; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance type safety and FHIR compliance
The interface could be improved in several ways:
- Add FHIR-required fields like
typeandpatient - Use enums for constrained string fields
- Add JSDoc documentation
Consider this enhancement:
/**
* Represents an allergy or intolerance - a propensity, or a potential risk to an individual,
* to have an adverse reaction to a substance.
* @see https://www.hl7.org/fhir/allergyintolerance.html
*/
export enum ClinicalStatus {
Active = "active",
Inactive = "inactive",
Resolved = "resolved"
}
export enum VerificationStatus {
Unconfirmed = "unconfirmed",
Confirmed = "confirmed",
Refuted = "refuted",
EnteredInError = "entered-in-error"
}
export enum AllergyCategory {
Food = "food",
Medication = "medication",
Environment = "environment",
Biologic = "biologic"
}
export interface AllergyIntolerance {
/** Identification code for the allergy or intolerance */
code: Code;
/** The clinical status of the allergy or intolerance */
clinicalStatus?: ClinicalStatus;
/** Assertion about certainty associated with the propensity */
verificationStatus?: VerificationStatus;
/** Category of the identified substance */
category?: AllergyCategory;
/** Estimate of the potential clinical harm */
criticality?: string;
/** Date of last known occurrence of a reaction */
lastOccurrence?: string;
/** Additional text note about the allergy */
note?: string;
/** The type of the allergy - allergy | intolerance */
type: "allergy" | "intolerance";
/** Who the sensitivity is for */
patient: string;
}| export interface AllergyIntoleranceRequest { | ||
| clinical_status: string; | ||
| verification_status: string; | ||
| category: string; | ||
| criticality: string; | ||
| code: Code; | ||
| last_occurrence?: string; | ||
| note?: string; | ||
| encounter: string; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type extension and consistency
The request interface duplicates fields from the base type and uses inconsistent naming conventions. Consider:
- Using proper type extension
- Maintaining consistent naming
- Adding field validation
Consider this refactoring:
/**
* Request type for creating or updating an allergy intolerance record.
* Enforces required fields and API-specific naming conventions.
*/
export interface AllergyIntoleranceRequest {
/** Base allergy data */
allergy: Omit<AllergyIntolerance, 'clinicalStatus' | 'verificationStatus' | 'category' | 'criticality'>;
/** The clinical status in API format */
clinical_status: ClinicalStatus;
/** The verification status in API format */
verification_status: VerificationStatus;
/** The category in API format */
category: AllergyCategory;
/** The criticality in API format */
criticality: string;
/** Reference to the encounter where this was recorded */
encounter: string;
}
src/types/emr/allergyIntolerance.ts
Outdated
| import { Code } from "../questionnaire/code"; | ||
|
|
||
| // Base type for allergy data | ||
| export interface AllergyIntolerance { | ||
| code: Code; | ||
| clinicalStatus?: string; | ||
| verificationStatus?: string; | ||
| category?: string; | ||
| criticality?: string; | ||
| lastOccurrence?: string; | ||
| note?: string; | ||
| } | ||
|
|
||
| // Type for API request, extends base type with required fields | ||
| export interface AllergyIntoleranceRequest { | ||
| clinical_status: string; | ||
| verification_status: string; | ||
| category: string; | ||
| criticality: string; | ||
| code: Code; | ||
| last_occurrence?: string; | ||
| note?: string; | ||
| encounter: string; | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
FHIR compliance issues detected in type definitions
- Both
allergyIntolerance.tsandmedicationRequest.tsshow inconsistent property naming conventions:AllergyIntoleranceRequestuses snake_case (clinical_status,verification_status)MedicationRequestrelated interfaces mix camelCase and snake_case (statusReasonvsdose_range,period_unit)
- Several properties deviate from FHIR R4 specifications:
- Missing standard FHIR metadata fields (e.g.,
id,meta) - Inconsistent nullable handling (mix of optional fields and explicit null unions)
- Non-standard field names (e.g.,
status_changedinstead ofstatusReason)
- Missing standard FHIR metadata fields (e.g.,
Recommend:
- Standardize on camelCase for all properties to match FHIR conventions
- Add missing FHIR resource metadata fields
- Align nullable fields handling consistently
- Review against official FHIR R4 resource definitions
🔗 Analysis chain
Verify FHIR compliance across the codebase
Let's ensure consistent FHIR implementation across related files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other FHIR-related type definitions that might need similar improvements
# Look for other FHIR-related type definitions
echo "Searching for other FHIR-related type definitions..."
rg -t typescript "interface.*\{" src/types/emr/
# Check for consistent enum usage
echo "Checking for existing enum definitions..."
rg -t typescript "enum.*\{" src/types/emr/
# Look for FHIR resource references
echo "Checking for FHIR resource references..."
rg -t typescript "(@see|@link).*hl7\.org/fhir" src/
Length of output: 552
Script:
#!/bin/bash
# Let's try without typescript file type specification since it's not recognized
# Search for interface definitions in EMR types
echo "Searching for FHIR-related interfaces..."
rg "interface.*\{" src/types/emr/
# Search for enum definitions
echo "Searching for enum definitions..."
rg "enum.*\{" src/types/emr/
# Look for FHIR references in comments or documentation
echo "Searching for FHIR references..."
rg "hl7\.org/fhir" src/
# Search for other potential FHIR-related files
echo "Searching for FHIR-related files..."
fd . src/types/emr/
Length of output: 1135
Script:
#!/bin/bash
# Let's examine the content of medicationRequest.ts to check its FHIR compliance
echo "Content of medicationRequest.ts:"
cat src/types/emr/medicationRequest.ts
# Let's also check if there are any snake_case vs camelCase inconsistencies
echo -e "\nChecking for snake_case properties:"
rg ":\s*\{[^}]*_" src/types/emr/
Length of output: 3082
| const updateAllergy = ( | ||
| index: number, | ||
| updates: Partial<AllergyIntolerance>, | ||
| ) => { | ||
| const newAllergies = allergies.map((allergy, i) => | ||
| i === index ? { ...allergy, ...updates } : allergy, | ||
| ); | ||
| setAllergies(newAllergies); | ||
| updateQuestionnaireResponseCB({ | ||
| ...questionnaireResponse, | ||
| values: [ | ||
| { | ||
| type: "allergy_intolerance", | ||
| value: newAllergies, | ||
| }, | ||
| ], | ||
| }); | ||
| }; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize state updates with proper validation
Add validation and optimize performance for allergy updates.
const updateAllergy = (
index: number,
updates: Partial<AllergyIntolerance>,
) => {
+ // Validate updates
+ if (!isValidAllergyUpdate(updates)) {
+ console.error('Invalid allergy update:', updates);
+ return;
+ }
+
const newAllergies = allergies.map((allergy, i) =>
i === index ? { ...allergy, ...updates } : allergy,
);
setAllergies(newAllergies);
+
+ // Debounce updates to prevent rapid re-renders
+ debounce(() => {
updateQuestionnaireResponseCB({
...questionnaireResponse,
values: [
{
type: "allergy_intolerance",
value: newAllergies,
},
],
});
+ }, 300)();
};
+
+function isValidAllergyUpdate(updates: Partial<AllergyIntolerance>): boolean {
+ // Add validation logic here
+ return true;
+}Committable suggestion skipped: line range outside the PR's diff.
| import { AllergyIntolerance } from "@/types/emr/allergyIntolerance"; | ||
| import { QuestionnaireResponse } from "@/types/questionnaire/form"; | ||
| import { Question } from "@/types/questionnaire/question"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Import FHIR R4 types for better type safety
Consider using official FHIR R4 TypeScript definitions instead of custom types for better interoperability and type safety.
-import { AllergyIntolerance } from "@/types/emr/allergyIntolerance";
-import { QuestionnaireResponse } from "@/types/questionnaire/form";
-import { Question } from "@/types/questionnaire/question";
+import { AllergyIntolerance, QuestionnaireResponse, QuestionnaireItem } from "fhir/r4";Committable suggestion skipped: line range outside the PR's diff.
| const handleAddAllergy = () => { | ||
| const newAllergies = [ | ||
| ...allergies, | ||
| { code: { code: "", display: "", system: "" } }, | ||
| ]; | ||
| setAllergies(newAllergies); | ||
| updateQuestionnaireResponseCB({ | ||
| ...questionnaireResponse, | ||
| values: [ | ||
| { | ||
| type: "allergy_intolerance", | ||
| value: newAllergies, | ||
| }, | ||
| ], | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Validate new allergy entries
The new allergy initialization should include required FHIR fields and proper validation.
const handleAddAllergy = () => {
const newAllergies = [
...allergies,
- { code: { code: "", display: "", system: "" } },
+ {
+ resourceType: "AllergyIntolerance",
+ code: { code: "", display: "", system: "" },
+ clinicalStatus: {
+ coding: [{
+ system: "http://terminology.hl7.org/CodeSystem/allergyintolerance-clinical",
+ code: "active"
+ }]
+ },
+ verificationStatus: {
+ coding: [{
+ system: "http://terminology.hl7.org/CodeSystem/allergyintolerance-verification",
+ code: "unconfirmed"
+ }]
+ },
+ type: "allergy",
+ category: ["medication"],
+ criticality: "low",
+ patient: { reference: "Patient/" + patientId } // Add patientId to props
+ },
];
setAllergies(newAllergies);
+
+ // Debounce updates to prevent rapid re-renders
+ debounce(() => {
updateQuestionnaireResponseCB({
...questionnaireResponse,
values: [
{
type: "allergy_intolerance",
value: newAllergies,
},
],
});
+ }, 300)();
};Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (3)
src/components/Questionnaire/QuestionTypes/ConditionQuestion.tsx (1)
155-155: Avoid using array index as key in list renderingUsing the array index as a key in list rendering can cause issues when items are added or removed, potentially leading to inconsistent component states. Consider using a unique identifier for each condition.
If
condition.code.codeis unique and always defined, use it as the key:- <TableRow key={index}> + <TableRow key={condition.code.code || index}>If
condition.code.codemight not be unique or defined, consider generating a unique ID when adding conditions.src/types/questionnaire/question.ts (1)
48-75: Add JSDoc documentation and runtime validations.The interface would benefit from documentation and runtime validations for certain fields.
+/** + * Represents a FHIR Questionnaire item + * @see https://www.hl7.org/fhir/questionnaire.html#QuestionnaireItem + */ export interface Question { id: string; link_id: string; code?: Code; text: string; description?: string; type: QuestionType; structured_type?: StructuredQuestionType; required?: boolean; collect_time?: boolean; collect_performer?: boolean; collect_body_site?: boolean; collect_method?: boolean; enable_when?: EnableWhen[]; - enable_behavior?: "all" | "any"; + // Ensure enable_behavior is set when enable_when has multiple conditions + enable_behavior: EnableWhen extends Array<infer T> ? + T[] extends [any, any, ...any[]] ? "all" | "any" : undefined : + undefined; disabled_display?: "hidden" | "protected"; repeats?: boolean; read_only?: boolean; - max_length?: number; + // Ensure max_length is positive when specified + max_length?: number & { __brand: 'PositiveInteger' }; answer_constraint?: string; answer_option?: AnswerOption[]; answer_value_set?: string; answer_unit?: Code; is_observation?: boolean; unit?: Code; questions?: Question[]; formula?: string; }src/components/Questionnaire/QuestionTypes/QuestionInput.tsx (1)
48-94: Simplify the isQuestionEnabled function.The function is complex and could be simplified using array methods.
const isQuestionEnabled = () => { if (!question.enable_when?.length) return true; - const checkCondition = (enableWhen: EnableWhen) => { + const checkCondition = ({ question: questionId, operator, answer }: EnableWhen) => { const dependentValue = questionnaireResponses.find( - (v) => v.link_id === enableWhen.question, + (v) => v.link_id === questionId, )?.values[0]?.value; - // Early return if no dependent value exists if (!dependentValue) return false; - switch (enableWhen.operator) { - case "exists": - return dependentValue !== undefined && dependentValue !== null; - case "equals": - return dependentValue === enableWhen.answer; - case "not_equals": - return dependentValue !== enableWhen.answer; - case "greater": - return typeof dependentValue === "number" && dependentValue > enableWhen.answer; - case "less": - return typeof dependentValue === "number" && dependentValue < enableWhen.answer; - case "greater_or_equals": - return typeof dependentValue === "number" && dependentValue >= enableWhen.answer; - case "less_or_equals": - return typeof dependentValue === "number" && dependentValue <= enableWhen.answer; - default: - return true; - } + const operatorMap = { + exists: () => true, + equals: () => dependentValue === answer, + not_equals: () => dependentValue !== answer, + greater: () => typeof dependentValue === "number" && dependentValue > answer, + less: () => typeof dependentValue === "number" && dependentValue < answer, + greater_or_equals: () => typeof dependentValue === "number" && dependentValue >= answer, + less_or_equals: () => typeof dependentValue === "number" && dependentValue <= answer, + }; + + return operatorMap[operator]?.() ?? true; }; return question.enable_behavior === "any" ? question.enable_when.some(checkCondition) : question.enable_when.every(checkCondition); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
.env(1 hunks)src/components/Questionnaire/QuestionTypes/ConditionQuestion.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/QuestionInput.tsx(1 hunks)src/components/Questionnaire/QuestionnaireForm.tsx(1 hunks)src/components/Questionnaire/structured/handlers.ts(1 hunks)src/components/Questionnaire/structured/types.ts(1 hunks)src/types/questionnaire/condition.ts(1 hunks)src/types/questionnaire/form.ts(1 hunks)src/types/questionnaire/question.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .env
- src/components/Questionnaire/structured/types.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Questionnaire/QuestionnaireForm.tsx
[error] 10-10: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (12)
src/components/Questionnaire/QuestionnaireForm.tsx (5)
10-10: Rename Error and Success to avoid shadowing global variables
The variables Error and Success on line 10 shadow the global Error object, which can lead to confusion and unexpected behavior.
Apply this diff to rename the variables:
- import { Error, Success } from "@/Utils/Notifications";
+ import { Error as NotifyError, Success as NotifySuccess } from "@/Utils/Notifications";Remember to update all references to these variables within the file accordingly.
🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
93-93: Avoid using console.log in production code
The console.log(questionnaireError); statement on line 93 can expose sensitive information and clutter the console in a production environment. It's better to handle errors gracefully or use a logging library that can be configured for different environments.
Apply this diff to remove the console statement:
- console.log(questionnaireError);If logging is necessary for debugging, consider using a logging utility that respects environment settings.
104-124: Move initializeResponses function outside of the component
The initializeResponses function is defined inside the QuestionnaireForm component, causing it to be recreated on every render, which is inefficient. Consider moving it outside the component to improve performance.
Apply this diff to move the function outside:
+ function initializeResponses(
+ questions: Question[],
+ ): QuestionnaireResponse[] {
+ // function body
+ }
export function QuestionnaireForm({
questionnaireSlug,
resourceId,
encounterId,
onSubmit,
onCancel,
}: QuestionnaireFormProps) {
- const initializeResponses = (
- questions: Question[],
- ): QuestionnaireResponse[] => {
- // function body
- };Place it above the component definition to make it a standalone function.
33-36: Improve type safety of BatchRequest interface
Replace Record<string, any> with a more specific type definition to enhance type safety and reduce potential runtime errors.
Consider defining a specific interface for body, reflecting the structure expected by the API.
interface BatchRequest {
url: string;
method: string;
- body: Record<string, any>;
+ body: {
+ resource_id?: string;
+ encounter?: string;
+ results?: Array<{
+ question_id: string;
+ values: Array<{
+ code?: { code: string; display: string; system: string };
+ value?: string;
+ }>;
+ note?: string;
+ body_site?: string;
+ method?: string;
+ }>;
+ };
}223-236:
Add null check for response to prevent potential runtime errors
Currently, the code assumes response is defined, but in case of a network error or unexpected issue, response might be undefined, leading to a runtime error when accessing response.data. Add a null check for response before accessing its properties.
Update the condition to:
- if (!response.data) {
+ if (!response || !response.data) {Likely invalid or redundant comment.
src/types/questionnaire/form.ts (1)
8-25: Add type validation for ResponseValue
The ResponseValue type allows for potential type mismatches between the type discriminator and the actual value. For example, a value of type "string" could contain a number. Consider defining ResponseValue as a discriminated union to enforce strict type safety.
Proposed change:
export type ResponseValue =
| {
type: "string";
value?: string;
code?: Code;
quantity?: Quantity;
}
| {
type: "number";
value?: number;
code?: Code;
quantity?: Quantity;
}
| {
type: "boolean";
value?: boolean;
code?: Code;
quantity?: Quantity;
}
| {
type: "allergy_intolerance";
value?: AllergyIntolerance[];
code?: Code;
quantity?: Quantity;
}
| {
type: "medication_request";
value?: MedicationRequest[];
code?: Code;
quantity?: Quantity;
}
| {
type: "condition";
value?: Condition[];
code?: Code;
quantity?: Quantity;
};src/types/questionnaire/condition.ts (1)
1-44: LGTM!
The type definitions are accurate and conform to best practices.
src/types/questionnaire/question.ts (3)
3-22: LGTM! Well-structured type definitions.
The type definitions are clear, comprehensive, and align well with FHIR resource types.
24-41: Type-safe implementation of operator-answer combinations.
The implementation correctly uses discriminated unions to enforce type safety for different operators and their corresponding answer types.
43-46: 🛠️ Refactor suggestion
Enhance type safety for answer options.
The current implementation allows any string value regardless of the question type. Consider making the interface generic to ensure type safety.
-export interface AnswerOption {
+export interface AnswerOption<T extends QuestionType = QuestionType> {
- value: string;
+ value: T extends "boolean" ? boolean :
+ T extends "decimal" | "integer" ? number :
+ T extends "date" | "dateTime" ? string :
+ string;
initialSelected?: boolean;
}Likely invalid or redundant comment.
src/components/Questionnaire/QuestionTypes/QuestionInput.tsx (2)
302-302:
Fix potential undefined error when accessing errors.
The current implementation assumes errors is always defined and is an array.
-const error = errors.find((e) => e.question_id === question.id)?.error;
+const error = errors?.find((e) => e.question_id === question.id)?.error || "";Likely invalid or redundant comment.
152-269: 🛠️ Refactor suggestion
Break down the large switch statement into smaller components.
The renderSingleInput function is too complex and handles too many responsibilities. Consider breaking it down into separate components.
+const QuestionTypeComponents = {
+ decimal: NumberInput,
+ integer: NumberInput,
+ choice: ChoiceInput,
+ text: TextInput,
+ structured: StructuredInput,
+} as const;
+
function renderSingleInput(responseValue: ResponseValue, index: number) {
+ const Component = QuestionTypeComponents[question.type];
+ return Component ? (
+ <Component
+ value={responseValue}
+ index={index}
+ onChange={handleValueChange}
+ {...commonProps}
+ />
+ ) : null;
}Likely invalid or redundant comment.
|
|
||
| const conditionSearch = useQuery(routes.valueset.expand, { | ||
| // TODO: change type in system-condition-code after backend is updated | ||
| pathParams: { system: "system-codition-code" }, |
There was a problem hiding this comment.
Fix typo in 'system-condition-code' path parameter
There's a typo in the system path parameter. 'codition' should be 'condition'.
Apply this diff to fix the typo:
- pathParams: { system: "system-codition-code" },
+ pathParams: { system: "system-condition-code" },📝 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.
| pathParams: { system: "system-codition-code" }, | |
| pathParams: { system: "system-condition-code" }, |
| const handleRemoveCondition = (index: number) => { | ||
| const newConditions = conditions.filter((_, i) => i !== index); | ||
| setConditions(newConditions); | ||
| updateQuestionnaireResponseCB({ | ||
| ...questionnaireResponse, | ||
| values: [ | ||
| { | ||
| type: "condition", | ||
| value: newConditions, | ||
| }, | ||
| ], | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Ensure principal diagnosis is updated when a condition is removed
When a condition is removed, if it was selected as the principal diagnosis, the principalDiagnosis state should be updated to prevent referencing a non-existent condition.
Consider adding a check in handleRemoveCondition:
const newConditions = conditions.filter((_, i) => i !== index);
+ const removedCondition = conditions[index];
+ if (principalDiagnosis === removedCondition.code.code) {
+ setPrincipalDiagnosis("");
+ updateQuestionnaireResponseCB({
+ ...questionnaireResponse,
+ values: [
+ {
+ type: "condition",
+ value: newConditions,
+ },
+ {
+ type: "string",
+ value: "",
+ },
+ ],
+ });
+ } else {
+ updateQuestionnaireResponseCB({
+ ...questionnaireResponse,
+ values: [
+ {
+ type: "condition",
+ value: newConditions,
+ },
+ {
+ type: "string",
+ value: principalDiagnosis,
+ },
+ ],
+ });
+ }
setConditions(newConditions);📝 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.
| const handleRemoveCondition = (index: number) => { | |
| const newConditions = conditions.filter((_, i) => i !== index); | |
| setConditions(newConditions); | |
| updateQuestionnaireResponseCB({ | |
| ...questionnaireResponse, | |
| values: [ | |
| { | |
| type: "condition", | |
| value: newConditions, | |
| }, | |
| ], | |
| }); | |
| }; | |
| const handleRemoveCondition = (index: number) => { | |
| const newConditions = conditions.filter((_, i) => i !== index); | |
| const removedCondition = conditions[index]; | |
| if (principalDiagnosis === removedCondition.code.code) { | |
| setPrincipalDiagnosis(""); | |
| updateQuestionnaireResponseCB({ | |
| ...questionnaireResponse, | |
| values: [ | |
| { | |
| type: "condition", | |
| value: newConditions, | |
| }, | |
| { | |
| type: "string", | |
| value: "", | |
| }, | |
| ], | |
| }); | |
| } else { | |
| updateQuestionnaireResponseCB({ | |
| ...questionnaireResponse, | |
| values: [ | |
| { | |
| type: "condition", | |
| value: newConditions, | |
| }, | |
| { | |
| type: "string", | |
| value: principalDiagnosis, | |
| }, | |
| ], | |
| }); | |
| } | |
| setConditions(newConditions); | |
| }; |
| <CommandInput | ||
| placeholder="Search conditions..." | ||
| className="my-1" | ||
| onValueChange={(search) => | ||
| conditionSearch.refetch({ body: { search } }) | ||
| } | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Debounce the search input to optimize API calls
Currently, the onValueChange handler triggers conditionSearch.refetch on every input change, which can result in excessive API calls as the user types. Implement debouncing to optimize performance and reduce unnecessary network requests.
Consider using a debouncing utility like lodash.debounce or implementing your own debounce function.
Example:
+ import { useCallback } from "react";
+ import debounce from "lodash.debounce";
+ const debouncedRefetch = useCallback(
+ debounce((search) => conditionSearch.refetch({ body: { search } }), 300),
+ [conditionSearch],
+ );
<CommandInput
placeholder="Search conditions..."
className="my-1"
onValueChange={(search) =>
- conditionSearch.refetch({ body: { search } })
+ debouncedRefetch(search)
}
/>Committable suggestion skipped: line range outside the PR's diff.
| medication_request: { | ||
| getRequests: (medications, { encounterId }) => { | ||
| return medications.map((medication) => ({ | ||
| url: `/api/v1/consultation/${encounterId}/medication/request/`, | ||
| method: "POST", | ||
| body: medication, | ||
| })); | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add field validation and default values to medication_request handler.
Unlike the allergy_intolerance handler, the medication_request handler doesn't validate required fields or provide default values. This could lead to invalid requests.
medication_request: {
getRequests: (medications, { encounterId }) => {
return medications.map((medication) => ({
url: `/api/v1/consultation/${encounterId}/medication/request/`,
method: "POST",
- body: medication,
+ body: {
+ status: medication.status ?? "active",
+ intent: medication.intent ?? "order",
+ category: medication.category ?? "community",
+ medication: medication.medication,
+ dosage_text: medication.dosageText,
+ encounter: encounterId,
+ },
}));
},
},📝 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.
| medication_request: { | |
| getRequests: (medications, { encounterId }) => { | |
| return medications.map((medication) => ({ | |
| url: `/api/v1/consultation/${encounterId}/medication/request/`, | |
| method: "POST", | |
| body: medication, | |
| })); | |
| }, | |
| }, | |
| medication_request: { | |
| getRequests: (medications, { encounterId }) => { | |
| return medications.map((medication) => ({ | |
| url: `/api/v1/consultation/${encounterId}/medication/request/`, | |
| method: "POST", | |
| body: { | |
| status: medication.status ?? "active", | |
| intent: medication.intent ?? "order", | |
| category: medication.category ?? "community", | |
| medication: medication.medication, | |
| dosage_text: medication.dosageText, | |
| encounter: encounterId, | |
| }, | |
| })); | |
| }, | |
| }, |
github-actions
bot
added
merge conflict
|
👋 Hi, @bodhish, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
src/types/questionnaire/form.ts (1)
27-36: Consider type refinements and add documentationA few suggestions to improve the interface:
- Consider using a more specific type for
taken_atinstead of string (e.g.,Dateor ISO date string type)- Add JSDoc documentation to explain the purpose of each field
Example implementation:
/** * Represents a response to a questionnaire item */ export interface QuestionnaireResponse { /** Unique identifier for the question */ question_id: string; /** Type of structured data expected in the response */ structured_type: StructuredQuestionType | null; /** Hierarchical link identifier for the question */ link_id: string; /** Array of response values */ values: ResponseValue[]; /** Optional additional notes */ note?: string; /** ISO 8601 timestamp when the response was recorded */ taken_at?: string; /** Code indicating the body site relevant to the response */ body_site?: Code; /** Code indicating the method used to obtain the response */ method?: Code; }package.json (1)
148-148: Consider consolidating TypeScript execution toolsBoth
ts-nodeandtsxare included, buttsxis a more modern and efficient alternative tots-node. Consider removingts-nodeto avoid redundancy.Also applies to: 155-155
src/components/Questionnaire/QuestionnaireForm.tsx (1)
244-281: Consider extracting the navigation panel into a separate componentThe navigation panel code between lines 244-281 is extensive and involves complex UI logic. Extracting it into a separate component will improve code readability, enhance maintainability, and promote reusability across the application.
You can create a new component, for example,
QuestionnaireNavigation, and move the related code into it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
package.json(4 hunks)public/locale/en.json(5 hunks)src/components/Patient/PatientRegister.tsx(2 hunks)src/components/Questionnaire/QuestionnaireForm.tsx(1 hunks)src/types/questionnaire/form.ts(1 hunks)src/types/questionnaire/quantity.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/types/questionnaire/quantity.ts
- public/locale/en.json
- src/components/Patient/PatientRegister.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Questionnaire/QuestionnaireForm.tsx
[error] 10-10: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (8)
src/types/questionnaire/form.ts (2)
1-6: LGTM! Imports are well-structured
The imports are properly organized and each import has a clear purpose in the type definitions.
8-25:
Improve type safety for ResponseValue
The current type definition allows for potential type mismatches between the type discriminator and the actual value. This could lead to runtime errors.
Consider using a discriminated union type as previously suggested to enforce type safety:
-export type ResponseValue = {
- type:
- | "string"
- | "number"
- | "boolean"
- | "allergy_intolerance"
- | "medication_request"
- | "condition";
- value?:
- | string
- | number
- | boolean
- | AllergyIntolerance[]
- | MedicationRequest[]
- | Condition[];
- value_code?: Code;
- value_quantity?: Quantity;
-};
+export type ResponseValue = (
+ | {
+ type: "string";
+ value?: string;
+ }
+ | {
+ type: "number";
+ value?: number;
+ }
+ | {
+ type: "boolean";
+ value?: boolean;
+ }
+ | {
+ type: "allergy_intolerance";
+ value?: AllergyIntolerance[];
+ }
+ | {
+ type: "medication_request";
+ value?: MedicationRequest[];
+ }
+ | {
+ type: "condition";
+ value?: Condition[];
+ }
+) & {
+ value_code?: Code;
+ value_quantity?: Quantity;
+};package.json (2)
36-38: LGTM: Improved build process with TypeScript support
The build sequence and setup scripts have been enhanced with proper TypeScript support using tsx. This is a good improvement as it provides better type safety during the setup process.
158-161: Verify Rollup version compatibility
The addition of Linux-specific Rollup dependencies is good for cross-platform support. However, we should ensure these versions are compatible with the project's Rollup configuration.
✅ Verification successful
Based on the search results, I can see that:
- The main Rollup version used in the project is 4.24.0
- The Linux-specific dependencies being added (@rollup/rollup-linux-arm64-gnu and @rollup/rollup-linux-x64-gnu) are at version 4.13.0
- The package.json shows these are added as optional dependencies
Since the Linux-specific dependencies (4.13.0) are older than the main Rollup version (4.24.0), this could potentially cause compatibility issues. Let me verify the compatibility requirements.
Let me check the Rollup plugin dependencies to verify version compatibility.
Based on the gathered information, I can now provide a final response:
The Linux-specific Rollup dependencies are compatible
The version mismatch between the Linux-specific Rollup dependencies (4.13.0) and the main Rollup version (4.24.0) is not a concern because:
- These are optional dependencies specifically for Linux platform support
- The version range 4.13.x is compatible with 4.24.x as they are both within the same major version
- All Rollup plugins in the project support Rollup v4.x.x as shown by their peer dependencies (e.g.,
"rollup": "^2.78.0||^3.0.0||^4.0.0")
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the main Rollup version is specified in any config files
rg -A 1 "rollup" --type json
Length of output: 50162
Script:
#!/bin/bash
# Check Rollup's package.json for peer dependency requirements of platform-specific packages
rg -A 5 "peerDependencies" package-lock.json
Length of output: 42304
Script:
#!/bin/bash
# Check Rollup plugin dependencies and their version requirements
rg -A 2 '"rollup":\s*"[^"]+' package-lock.json
Length of output: 804
src/components/Questionnaire/QuestionnaireForm.tsx (4)
10-10: 🛠️ Refactor suggestion
Rename Error and Success to avoid shadowing global variables
The imports Error and Success on line 10 shadow the global Error and Success constructors, which can lead to confusion and potential bugs.
Apply this diff to rename the imports:
- import { Error, Success } from "@/Utils/Notifications";
+ import { ErrorNotification, SuccessNotification } from "@/Utils/Notifications";Remember to update all references to Error and Success within the file accordingly.
🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
93-93: 🛠️ Refactor suggestion
Remove console.log statements from production code
The console.log(questionnaireError); statement on line 93 can expose internal errors and clutter the console in a production environment. Consider removing it or using proper logging mechanisms.
Apply this diff to remove the console statement:
- console.log(questionnaireError);223-235: 🛠️ Refactor suggestion
Add comprehensive error handling for submitBatch response
The current error handling in handleSubmit only checks for response.data and response.error, but there might be cases where the response itself is undefined or has unexpected properties. Please ensure that all possible response scenarios are handled to prevent runtime errors and provide better user feedback.
You can modify the error handling as follows:
const response = await submitBatch({
body: { requests },
});
+ if (!response) {
+ ErrorNotification({ msg: "Failed to submit questionnaire: Network error" });
+ return;
+ }
- if (!response.data) {
+ if (response.error) {
handleSubmissionError(
response.error.results as ValidationErrorResponse[],
);
ErrorNotification({ msg: "Failed to submit questionnaire" });
return;
}
+ if (!response.data) {
+ ErrorNotification({ msg: "Failed to submit questionnaire: Invalid response format" });
+ return;
+ }
SuccessNotification({ msg: "Questionnaire submitted successfully" });
onSubmit?.();33-37: 🛠️ Refactor suggestion
Improve type safety of BatchRequest interface
The body property of the BatchRequest interface is currently typed as Record<string, any>, which reduces type safety. Defining a specific type for body will help catch potential errors at compile time.
Consider updating the BatchRequest interface as follows:
interface BatchRequest {
url: string;
method: string;
- body: Record<string, any>;
+ body: {
+ resource_id?: string;
+ encounter?: string;
+ results?: Array<{
+ question_id: string;
+ values: Array<{
+ value_code?: { code: string; display: string; system: string };
+ value?: string;
+ }>;
+ note?: string;
+ body_site?: string;
+ method?: string;
+ }>;
+ };
}| @@ -56,15 +53,19 @@ | |||
| "@googlemaps/typescript-guards": "^2.0.3", | |||
| "@headlessui/react": "^2.2.0", | |||
| "@hello-pangea/dnd": "^17.0.0", | |||
| "@originjs/vite-plugin-federation": "^1.3.6", | |||
There was a problem hiding this comment.
💡 Codebase verification
Module federation plugin added but not configured
The @originjs/vite-plugin-federation package has been added as a dependency, but there's no configuration for module federation in the codebase. No vite config files were found, and there are no federation-related configurations or exposed/remote module setups. This suggests the package was added but not yet implemented.
- Missing
vite.config.ts/jswith module federation setup - No exposed or remote module configurations found
- No federation-related imports or usage in the codebase
🔗 Analysis chain
Verify module federation configuration
The addition of @originjs/vite-plugin-federation suggests implementation of micro-frontend architecture. This is a significant change that requires proper configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for module federation configuration
rg -A 10 "federation" --type ts --type js
# Check for exposed modules configuration
rg -A 10 "exposes:|remotes:" --type ts --type js
Length of output: 92
Script:
#!/bin/bash
# Check for vite config files that might contain federation setup
fd -e ts -e js -e mjs "vite.config"
# Check package.json for any federation-related dependencies or scripts
cat package.json
# Look for any federation-related imports or configurations in the codebase
rg -g "!package.json" "federation" --type ts --type js --type json
Length of output: 6807
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/components/Users/UserHome.tsx (1)
90-90: Consider adding error context for better UX.While using ErrorPage is good for standardization, we could improve the user experience by providing context about why they're seeing the error.
Consider passing an error message prop:
- return <ErrorPage />; + return <ErrorPage message={t('invalid_user_tab', { tab })} />;src/Utils/request/types.ts (1)
11-13: Consider consolidating QueryRoute and MutationRoute interfacesThe expanded
QueryRouteinterface now significantly overlaps withMutationRoute, as both support the same HTTP methods and body types. This could lead to confusion about when to use each type.Consider either:
- Maintaining clear separation:
export interface QueryRoute<TData, TBody = unknown> extends RouteBase<TData> { method?: "GET"; // Keep QueryRoute for GET operations only TBody?: TBody; }
- Or consolidating into a single type with a discriminator:
export interface ApiRoute<TData, TBody = unknown> extends RouteBase<TData> { method: "GET" | "POST" | "PUT" | "PATCH" | "DELETE"; TBody?: TBody; isQuery?: boolean; // discriminator to differentiate query vs mutation behavior }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
package.json(4 hunks)src/Utils/request/types.ts(2 hunks)src/Utils/request/useQuery.ts(1 hunks)src/components/Facility/ConsultationDetails/index.tsx(2 hunks)src/components/Patient/PatientRegister.tsx(2 hunks)src/components/Users/UserHome.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/Facility/ConsultationDetails/index.tsx
- src/Utils/request/useQuery.ts
- src/components/Patient/PatientRegister.tsx
🔇 Additional comments (7)
src/components/Users/UserHome.tsx (1)
8-8: LGTM! Error handling standardization looks good.
The change from Error404 to ErrorPage aligns well with the standardization of error handling across the application.
Also applies to: 90-90
package.json (5)
36-38: LGTM! Build process improvements
The changes improve the build process by:
- Ensuring setup runs before build
- Using
tsxfor better TypeScript support - Adding platform-specific dependency handling
59-59: LGTM! UI component additions
The addition of Radix UI components (@radix-ui/react-alert-dialog, @radix-ui/react-select, @radix-ui/react-tabs) aligns with the PR's objective of adding FHIR forms support.
Also applies to: 66-66, 68-68
56-56: Module federation plugin needs configuration
150-150: LGTM! TypeScript tooling additions
The addition of ts-node and tsx aligns with the improved TypeScript support in the build scripts.
Also applies to: 157-157
160-163: LGTM! Platform-specific dependencies properly configured
The addition of Linux-specific Rollup dependencies as optional dependencies is correctly configured, ensuring proper builds on Linux platforms while not affecting other platforms.
src/Utils/request/types.ts (1)
22-22: Verify type inference in consuming code
The addition of TBody to the Route type union might require updates in places where Route is used. Ensure proper type inference and explicit type parameters where needed.
✅ Verification successful
Type changes are properly handled across the codebase
The codebase shows consistent and correct usage of the Route type with proper type parameters:
QueryRouteandMutationRouteboth correctly implement the newTBodyparameter- All usages in components and utilities properly specify both type parameters:
request.tsusesRoute<TData, TBody>useQuery.tsusesQueryRoute<TData, TBody>useMutation.tsusesMutationRoute<TData, TBody>- Components like
Export.tsxandBoard.tsxuse the type with appropriate parameters
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find usages of Route type to verify proper type parameters
ast-grep --pattern 'Route<$_>'
# Find potential type errors in consuming code
ast-grep --pattern 'Route<$_, $_>'
Length of output: 69
Script:
#!/bin/bash
# Search for any usage of Route type in TypeScript files
rg "Route[<\s]" --type typescript -A 2
# Search for type definitions or interfaces using Route
ast-grep --pattern 'type $_ = Route'
ast-grep --pattern 'interface $_ extends Route'
# Search for variable or parameter declarations using Route type
ast-grep --pattern ': Route'
Length of output: 195
Script:
#!/bin/bash
# Search for any usage of Route type in ts files
rg "Route[<\s]" -t ts -A 2
# Search for files containing Route type
fd -e ts -x cat {} | rg "Route[<\s]" -A 2
# Look for type definitions in the same file
cat src/Utils/request/types.ts
Length of output: 8229
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| phoneNumber: `+91${phone}`, | ||
| createdAt: new Date().toISOString(), | ||
| }; | ||
| localStorage.setItem(CarePatientTokenKey, JSON.stringify(tokenData)); |
Check failure
Code scanning / CodeQL
Clear text storage of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 4 days ago
To fix the problem, we need to encrypt the sensitive information before storing it in local storage. We can use the crypto-js library to handle encryption and decryption. The steps to fix the issue are as follows:
- Install the
crypto-jslibrary. - Import the necessary functions from
crypto-js. - Create utility functions for encrypting and decrypting data.
- Encrypt the token data before storing it in local storage.
- Decrypt the token data when retrieving it from local storage.
| @@ -1,2 +1,3 @@ | ||
| import careConfig from "@careConfig"; | ||
| import CryptoJS from "crypto-js"; | ||
| import { useMutation } from "@tanstack/react-query"; | ||
| @@ -39,2 +40,13 @@ | ||
|
|
||
| const secretKey = "your-secret-key"; // Replace with a secure key | ||
|
|
||
| const encryptData = (data: string) => { | ||
| return CryptoJS.AES.encrypt(data, secretKey).toString(); | ||
| }; | ||
|
|
||
| const decryptData = (ciphertext: string) => { | ||
| const bytes = CryptoJS.AES.decrypt(ciphertext, secretKey); | ||
| return bytes.toString(CryptoJS.enc.Utf8); | ||
| }; | ||
|
|
||
| interface LoginFormData { | ||
| @@ -162,3 +174,4 @@ | ||
| }; | ||
| localStorage.setItem(CarePatientTokenKey, JSON.stringify(tokenData)); | ||
| const encryptedTokenData = encryptData(JSON.stringify(tokenData)); | ||
| localStorage.setItem(CarePatientTokenKey, encryptedTokenData); | ||
| Notification.Success({ msg: t("verify_otp_success_login") }); |
| @@ -130,3 +130,4 @@ | ||
| "xlsx": "https://cdn.sheetjs.com/xlsx-0.20.3/xlsx-0.20.3.tgz", | ||
| "zod": "^3.23.8" | ||
| "zod": "^3.23.8", | ||
| "crypto-js": "^4.2.0" | ||
| }, |
| Package | Version | Security advisories |
| crypto-js (npm) | 4.2.0 | None |
| phoneNumber: phoneNumber, | ||
| createdAt: new Date().toISOString(), | ||
| }; | ||
| localStorage.setItem(CarePatientTokenKey, JSON.stringify(tokenData)); |
Check failure
Code scanning / CodeQL
Clear text storage of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 4 days ago
To fix the problem, we need to ensure that the phone number is encrypted before being stored in localStorage. We can use the crypto module from Node.js to encrypt the phone number. The encryption key should be securely managed and not hardcoded in the code.
- Import the
cryptomodule. - Create an encryption function that uses a secure algorithm to encrypt the phone number.
- Encrypt the phone number before storing it in
localStorage.
| @@ -8,2 +8,3 @@ | ||
| import { z } from "zod"; | ||
| import crypto from "crypto"; | ||
|
|
||
| @@ -41,2 +42,7 @@ | ||
|
|
||
| const encrypt = (text: string, key: string) => { | ||
| const cipher = crypto.createCipher('aes-256-ctr', key); | ||
| return cipher.update(text, 'utf8', 'hex') + cipher.final('hex'); | ||
| }; | ||
|
|
||
| const FormSchema = z.object({ | ||
| @@ -159,3 +165,3 @@ | ||
| token: response.access, | ||
| phoneNumber: phoneNumber, | ||
| phoneNumber: encrypt(phoneNumber, 'your-encryption-key'), | ||
| createdAt: new Date().toISOString(), |
…nd updating the structure for improved readability.
…search and pagination
…UserForm component
|
@bodhish 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
TBD
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
New Features
EncounterQuestionnaire,QuestionRenderer, and various question types (e.g.,BooleanQuestion,TextQuestion,NumberQuestion).AppointmentCreatePage,AppointmentsPage, andAppointmentTokenPage.Calendarcomponent for date selection and aDatePickerfor date input.FacilitiesPage,FacilityDetailsPage, and various appointment-related routes.SymptomsListandDiagnosisListcomponents to display patient symptoms and diagnoses.ResourceRequestscomponent for managing resource requests related to patients.Callout,ColoredIndicator, andDrawercomponents for improved UI interactions.UserAvailabilityTabfor managing user availability in a calendar format.ScheduleTemplateFormandScheduleTemplatesListfor managing schedule templates.Toastercomponent for improved notification handling.Logincomponent to support staff and patient login modes with improved state management and validation.AllergyListandDiagnosisListcomponents for displaying patient allergies and diagnoses.ScheduleExceptionFormandScheduleExceptionsListfor managing schedule exceptions.AppointmentSuccessandOTPcomponents for appointment confirmation and OTP verification.ColoredIndicatorandCalloutcomponents for enhanced UI feedback.PatientRegistrationandPatientSelectcomponents for improved patient management functionality.FacilityHomeandFacilityDetailsPageto streamline facility management processes.Bug Fixes
Documentation
Chores