-
Notifications
You must be signed in to change notification settings - Fork 1k
Add toggleable sort by name and show created by options in PrintChargeItems #15306
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…eItems - Add 'Sort by Name' toggle to optionally sort charge items alphabetically within categories - Add 'Show Created By' toggle to optionally display the user who created each charge item - Both options are disabled by default - Dynamic column span adjustments based on visibility of Created By column
WalkthroughIntroduces two toggleable features to the Print Charge Items view: sorting charge items by name and displaying created_by information. Updates include localization strings, new UI toggle controls in the component, and extension of ChargeItemRead type with created_by and updated_by fields. Changes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds two new toggleable options to the Print Charge Items view for billing accounts: sorting charge items alphabetically by name within categories, and displaying the creator's name for each charge item.
Changes:
- Added "Sort by Name" toggle to sort charge items alphabetically within each category
- Added "Show Created By" toggle to display a column showing who created each charge item
- Updated type definitions to include
created_byandupdated_byfields
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/types/billing/chargeItem/chargeItem.ts | Added created_by and updated_by fields to ChargeItemRead interface |
| src/pages/Facility/billing/account/components/PrintChargeItems.tsx | Implemented toggle switches for sorting and column visibility, with conditional rendering logic |
| public/locale/en.json | Added "show_created_by" locale string and updated "sort_by_name" text |
| const baseItems: ChargeItemRead[] = | ||
| groups[categoryTitle] ?? []; | ||
| const items = sortByName | ||
| ? baseItems.sort((a, b) => |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sort() method mutates the original array. This means baseItems (which comes from groups[categoryTitle]) will be sorted in place, affecting the original data structure. Use [...baseItems].sort() or baseItems.slice().sort() to create a copy before sorting.
| ? baseItems.sort((a, b) => | |
| ? [...baseItems].sort((a, b) => |
| "sort_by_latest_payment": "Newest Payment First", | ||
| "sort_by_locations": "Number of Locations", | ||
| "sort_by_name": "Name", | ||
| "sort_by_name": "Sort by name", |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change modifies an existing locale key value from 'Name' to 'Sort by name'. This is a breaking change that affects all existing usages of this key throughout the application. Consider using a new key like 'sort_items_by_name' for this specific feature instead of modifying the existing shared key.
| "sort_by_name": "Sort by name", | |
| "sort_by_name": "Name", | |
| "sort_items_by_name": "Sort by name", |
Deploying care-preview with
|
| Latest commit: |
641a795
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4b34098d.care-preview-a7w.pages.dev |
| Branch Preview URL: | https://issues-print-charge-items-en.care-preview-a7w.pages.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pages/Facility/billing/account/components/PrintChargeItems.tsx (2)
454-465: IncorrectcolSpanvalue causes table misalignment.The table has 7 columns without
showCreatedByand 8 columns with it. The category header row should span all columns except the amount column, requiringcolSpanof 6 or 7 respectively.🐛 Proposed fix
<TableCell - colSpan={showCreatedBy ? 6 : 5} + colSpan={showCreatedBy ? 7 : 6} className="capitalize" >
538-553: SamecolSpanissue in grand total row.The grand total row has the same off-by-one error as the category header row.
🐛 Proposed fix
<TableCell - colSpan={showCreatedBy ? 6 : 5} + colSpan={showCreatedBy ? 7 : 6} className="text-right pr-2" >
🤖 Fix all issues with AI agents
In `@public/locale/en.json`:
- Line 5303: The locale entry with key "sort_by_name" uses sentence case; update
its value to title case by changing "Sort by name" to "Sort by Name" so it
matches other option labels (edit the "sort_by_name" value in the en.json
locale).
In `@src/pages/Facility/billing/account/components/PrintChargeItems.tsx`:
- Around line 504-511: The cell rendering for created_by's first name (inside
the showCreatedBy conditional and the TableCell) lacks a fallback; update the
expression that reads chargeItem.created_by?.first_name to provide a "-"
fallback when the value is empty or falsy (e.g., use a nullish-coalescing or
ternary: chargeItem.created_by?.first_name ?? "-" or a conditional that renders
"-" when it's blank) so it matches the other table cells' presentation.
- Around line 413-419: The current code mutates the grouped array because
baseItems references groups[categoryTitle] and Array.prototype.sort() sorts
in-place; to fix this, ensure you create a shallow copy before sorting (e.g.,
use [...baseItems] or baseItems.slice()) so that when sortByName is true you
assign items to a sorted copy rather than mutating groups[categoryTitle], and
when sortByName is false items remains the original grouped array; update the
expression that sets items (which currently uses baseItems.sort(...)) to sort a
copy instead.
In `@src/types/billing/chargeItem/chargeItem.ts`:
- Around line 100-101: Update the ChargeItem type definition so its audit user
fields are nullable: change the properties created_by and updated_by on the
ChargeItem interface/type to accept UserReadMinimal | null (instead of required
UserReadMinimal). Locate the declarations for created_by and updated_by in
chargeItem.ts and modify their types to match other audit-tracked types (e.g.,
formSubmission.resourceRequest patterns) so the UI optional chaining
(created_by?.first_name) is type-safe.
| "sort_by_latest_payment": "Newest Payment First", | ||
| "sort_by_locations": "Number of Locations", | ||
| "sort_by_name": "Name", | ||
| "sort_by_name": "Sort by name", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use title case for consistency (“Sort by Name”).
The option label is title case elsewhere and in the PR objective.
✏️ Suggested fix
- "sort_by_name": "Sort by name",
+ "sort_by_name": "Sort by Name",📝 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.
| "sort_by_name": "Sort by name", | |
| "sort_by_name": "Sort by Name", |
🤖 Prompt for AI Agents
In `@public/locale/en.json` at line 5303, The locale entry with key "sort_by_name"
uses sentence case; update its value to title case by changing "Sort by name" to
"Sort by Name" so it matches other option labels (edit the "sort_by_name" value
in the en.json locale).
| const baseItems: ChargeItemRead[] = | ||
| groups[categoryTitle] ?? []; | ||
| const items = sortByName | ||
| ? baseItems.sort((a, b) => | ||
| a.title.localeCompare(b.title), | ||
| ) | ||
| : baseItems; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid mutating the original array during sorting.
Array.prototype.sort() mutates the array in place. Since baseItems references groups[categoryTitle], this mutates the grouped data derived from chargeItems.results. When toggling sortByName off, items will remain sorted instead of reverting to their original order.
🐛 Proposed fix using a non-mutating sort
const baseItems: ChargeItemRead[] =
groups[categoryTitle] ?? [];
const items = sortByName
- ? baseItems.sort((a, b) =>
+ ? [...baseItems].sort((a, b) =>
a.title.localeCompare(b.title),
)
: baseItems;🤖 Prompt for AI Agents
In `@src/pages/Facility/billing/account/components/PrintChargeItems.tsx` around
lines 413 - 419, The current code mutates the grouped array because baseItems
references groups[categoryTitle] and Array.prototype.sort() sorts in-place; to
fix this, ensure you create a shallow copy before sorting (e.g., use
[...baseItems] or baseItems.slice()) so that when sortByName is true you assign
items to a sorted copy rather than mutating groups[categoryTitle], and when
sortByName is false items remains the original grouped array; update the
expression that sets items (which currently uses baseItems.sort(...)) to sort a
copy instead.
| {showCreatedBy && ( | ||
| <TableCell className="w-16"> | ||
| { | ||
| chargeItem.created_by | ||
| ?.first_name | ||
| } | ||
| </TableCell> | ||
| )} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add fallback for empty first_name for consistency.
Other cells in this table use "-" as a fallback for missing values (e.g., invoice number). Consider adding the same fallback here for visual consistency.
💡 Suggested improvement
{showCreatedBy && (
<TableCell className="w-16">
- {
- chargeItem.created_by
- ?.first_name
- }
+ {chargeItem.created_by?.first_name || "-"}
</TableCell>
)}📝 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.
| {showCreatedBy && ( | |
| <TableCell className="w-16"> | |
| { | |
| chargeItem.created_by | |
| ?.first_name | |
| } | |
| </TableCell> | |
| )} | |
| {showCreatedBy && ( | |
| <TableCell className="w-16"> | |
| {chargeItem.created_by?.first_name || "-"} | |
| </TableCell> | |
| )} |
🤖 Prompt for AI Agents
In `@src/pages/Facility/billing/account/components/PrintChargeItems.tsx` around
lines 504 - 511, The cell rendering for created_by's first name (inside the
showCreatedBy conditional and the TableCell) lacks a fallback; update the
expression that reads chargeItem.created_by?.first_name to provide a "-"
fallback when the value is empty or falsy (e.g., use a nullish-coalescing or
ternary: chargeItem.created_by?.first_name ?? "-" or a conditional that renders
"-" when it's blank) so it matches the other table cells' presentation.
| created_by: UserReadMinimal; | ||
| updated_by: UserReadMinimal; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Find usages of created_by in the codebase
rg -n "created_by" --type=ts --type=tsx -g '!*.d.ts' | head -40Repository: ohcnetwork/care_fe
Length of output: 89
🏁 Script executed:
# Check the full context of the ChargeItemRead interface
cat -n src/types/billing/chargeItem/chargeItem.ts | sed -n '95,110p'Repository: ohcnetwork/care_fe
Length of output: 629
🏁 Script executed:
# Look for the API serializer/response definition to understand backend contract
fd -e py -e json "chargeitem\|charge_item" --type f | head -20Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Search for any API tests or serializers that show what fields are returned
rg -n "created_by" --type=py 2>/dev/null | head -20Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Fix the rg command - use correct file type format
rg -n "created_by" --type ts -g '!*.d.ts' -A2 -B2 | head -80Repository: ohcnetwork/care_fe
Length of output: 4603
🏁 Script executed:
# Look for any type definitions that might show optional nullability handling
rg -n "created_by\?" --type ts | head -20Repository: ohcnetwork/care_fe
Length of output: 2190
🏁 Script executed:
# Check the PR or commit message for context on why these fields are required
git log --all --oneline --grep="created_by\|charge.*item" 2>/dev/null | head -20Repository: ohcnetwork/care_fe
Length of output: 1572
🏁 Script executed:
# Look at imports and check if UserReadMinimal can be null
cat -n src/types/billing/chargeItem/chargeItem.ts | head -20Repository: ohcnetwork/care_fe
Length of output: 862
🏁 Script executed:
# Look for the specific commit that added these fields to chargeItem
git log -p --all -- src/types/billing/chargeItem/chargeItem.ts 2>/dev/null | head -100Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Check the Print Charge Items UI component to see how created_by is accessed
rg -n "PrintChargeItem\|created_by" src/components --type ts -A3 -B1 | head -60Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Verify the pattern - check if other similar read interfaces use nullable versions
rg -n "created_by.*null|updated_by.*null" src/types --type tsRepository: ohcnetwork/care_fe
Length of output: 732
🏁 Script executed:
# Find the Print Charge Items component directly
fd -type f "print.*charge\|charge.*print" src/components -iname "*.tsx"Repository: ohcnetwork/care_fe
Length of output: 232
🏁 Script executed:
# Look for any API response handling for charge items
rg -n "ChargeItemRead" src --type ts -B2 -A2 | head -80Repository: ohcnetwork/care_fe
Length of output: 6122
🏁 Script executed:
# Check for any null handling patterns in existing UI that uses created_by
rg -n "created_by\?" src/components -A1 | head -40Repository: ohcnetwork/care_fe
Length of output: 2795
🏁 Script executed:
# Find the Print Charge Items component
find src/components -iname "*print*charge*" -o -iname "*charge*print*"Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Search for PrintChargeItem or similar
rg -n "PrintChargeItem\|print.*charge\|Charge.*Item.*Print" src/components --type tsRepository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Search for components that import or use ChargeItemRead
rg -n "ChargeItemRead" src/components --type ts -B2 -A2Repository: ohcnetwork/care_fe
Length of output: 6375
🏁 Script executed:
# Look for any component that displays charge item information or created_by
rg -n "charge.*item\|ChargeItem" src/components -i --type ts | grep -i "created\|print" | head -20Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Check the PR files that were modified - look at git status or recent changes
git diff --name-only 2>/dev/null | head -20Repository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Search broadly for created_by in src to understand the broader pattern
rg "created_by\?" src/types --type ts | wc -lRepository: ohcnetwork/care_fe
Length of output: 63
🏁 Script executed:
# Look for any commit or file that mentions print and charge item
git log --all --oneline --grep="print.*charge\|charge.*print" -i 2>/dev/null | head -5Repository: ohcnetwork/care_fe
Length of output: 376
🏁 Script executed:
# Search for any display of created_by in charge item context
rg -n "created_by" src/components/Billing -A2 -B2 --type tsRepository: ohcnetwork/care_fe
Length of output: 44
🏁 Script executed:
# Check if there's a specific print or display component in Billing
find src/components/Billing -name "*.tsx" -o -name "*.ts"Repository: ohcnetwork/care_fe
Length of output: 442
🏁 Script executed:
# Search for PrintChargeItems component directly
rg -n "PrintChargeItems" src --type ts -A3 -B1Repository: ohcnetwork/care_fe
Length of output: 1509
🏁 Script executed:
# Look in src/pages for charge item print related components
find src/pages -name "*[Cc]harge*" -o -name "*[Pp]rint*" | grep -i chargeRepository: ohcnetwork/care_fe
Length of output: 1344
🏁 Script executed:
# Search for any file with Print in its name
find src -name "*Print*" -type fRepository: ohcnetwork/care_fe
Length of output: 1253
🏁 Script executed:
# Examine the PrintChargeItems component
cat -n src/pages/Facility/billing/account/components/PrintChargeItems.tsx | head -150Repository: ohcnetwork/care_fe
Length of output: 6143
🏁 Script executed:
# Check specifically for created_by usage in PrintChargeItems
rg -n "created_by" src/pages/Facility/billing/account/components/PrintChargeItems.tsx -A3 -B3Repository: ohcnetwork/care_fe
Length of output: 1257
Make created_by and updated_by optional in the type definition.
The UI code in PrintChargeItems.tsx (line 507-508) already uses optional chaining (created_by?.first_name), indicating the developer anticipated these fields could be null. However, the type definition marks them as required. This mismatch creates unnecessary risk.
Similar audit-tracked fields across the codebase use nullable versions:
formSubmission.ts:created_by: UserReadMinimal | nullresourceRequest.ts:created_by: UserReadMinimal | null
Update to:
created_by: UserReadMinimal | null;
updated_by: UserReadMinimal | null;This aligns the type with the UI's defensive implementation and accommodates edge cases where the creating/updating user may no longer exist or data predates audit tracking.
🤖 Prompt for AI Agents
In `@src/types/billing/chargeItem/chargeItem.ts` around lines 100 - 101, Update
the ChargeItem type definition so its audit user fields are nullable: change the
properties created_by and updated_by on the ChargeItem interface/type to accept
UserReadMinimal | null (instead of required UserReadMinimal). Locate the
declarations for created_by and updated_by in chargeItem.ts and modify their
types to match other audit-tracked types (e.g., formSubmission.resourceRequest
patterns) so the UI optional chaining (created_by?.first_name) is type-safe.
🎭 Playwright Test ResultsStatus: ❌ Failed
📊 Detailed results are available in the playwright-final-report artifact. Run: #5127 |
|
@rithviknishad 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! 🙌 |
Summary
Closes #15305
This PR adds two new toggleable options to the Print Charge Items view for billing accounts:
Changes
Sort by Name - When enabled, charge items within each category are sorted alphabetically by title. Disabled by default.
Show Created By - When enabled, displays a "Created By" column showing the first name of the user who created each charge item. Disabled by default.
Implementation Details
Screenshots
Please test locally to verify the toggle behavior
@ohcnetwork/care-fe-code-reviewers
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.