show delivery totals and count in delivery order page #15500
Conversation
WalkthroughThis PR adds delivery order totals display functionality. It introduces translation keys for item and quantity totals, implements a new calculation utility for delivery totals using Decimal arithmetic, and renders aggregated totals in a CardFooter component with monetary formatting. Changes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
rithviknishad
added
P1
There was a problem hiding this comment.
Pull request overview
This PR adds a footer section to the delivery order details page that displays aggregate statistics for all deliveries in the order, including total item count, total quantity, and total monetary value.
Changes:
- Implements calculation logic for delivery order totals using Decimal.js for precise arithmetic
- Adds a new CardFooter component displaying three summary metrics with right-aligned formatting
- Introduces new i18n keys "total_items" and "total_quantity" for the summary labels
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/pages/Facility/services/inventory/externalSupply/deliveryOrder/DeliveryOrderShow.tsx | Adds calculateDeliveryTotals function and displays totals in CardFooter with count, quantity, and value |
| public/locale/en.json | Adds translation keys for "total_items" and "total_quantity" |
| if (!supplyDeliveries?.results) { | ||
| return { totalCount: 0, totalQuantity: "0", totalValue: "0" }; | ||
| } | ||
| return calculateDeliveryTotals(supplyDeliveries.results, internal); | ||
| }, [supplyDeliveries?.results, internal]); |
There was a problem hiding this comment.
The useMemo dependency array includes supplyDeliveries?.results which is an array reference. This will cause recalculation on every query refetch even when the actual delivery data hasn't changed. Consider using a more stable dependency like a serialized version of the results or add the results length as a dependency along with checking if the data has actually changed.
| if (!supplyDeliveries?.results) { | |
| return { totalCount: 0, totalQuantity: "0", totalValue: "0" }; | |
| } | |
| return calculateDeliveryTotals(supplyDeliveries.results, internal); | |
| }, [supplyDeliveries?.results, internal]); | |
| const results = supplyDeliveries?.results ?? []; | |
| if (!results.length) { | |
| return { totalCount: 0, totalQuantity: "0", totalValue: "0" }; | |
| } | |
| return calculateDeliveryTotals(results, internal); | |
| }, [JSON.stringify(supplyDeliveries?.results ?? []), internal]); |
| let totalValue = new Decimal(0); | ||
|
|
||
| for (const delivery of deliveries) { | ||
| const qty = new Decimal(delivery.supplied_item_quantity || "0"); |
There was a problem hiding this comment.
The fallback to '0' for supplied_item_quantity may mask data integrity issues. Consider whether a null/undefined quantity should be explicitly handled or logged rather than silently treated as zero, as this could hide problems with incomplete delivery records.
| const qty = new Decimal(delivery.supplied_item_quantity || "0"); | |
| const qtyString = delivery.supplied_item_quantity; | |
| if (qtyString == null) { | |
| // Missing quantity indicates a potentially incomplete delivery record. | |
| // We skip this entry from totals and log for debugging instead of | |
| // silently treating it as zero, which could hide data integrity issues. | |
| // Avoid logging any PHI; inventory identifiers are safe for logs. | |
| // eslint-disable-next-line no-console | |
| console.warn( | |
| "Supply delivery has missing supplied_item_quantity and was excluded from totals.", | |
| ); | |
| continue; | |
| } | |
| const qty = new Decimal(qtyString); |
Deploying care-preview with
|
| Latest commit: |
2da612a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b7ca7227.care-preview-a7w.pages.dev |
| Branch Preview URL: | https://feat-show-delivery-totals-an.care-preview-a7w.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/pages/Facility/services/inventory/externalSupply/deliveryOrder/DeliveryOrderShow.tsx`:
- Around line 77-81: The import order is incorrect: the third‑party symbol
Decimal (from "decimal.js") is placed with utils; move the Decimal import into
the third‑party imports block with the other external libraries (so it appears
before internal utils like round, ShortcutBadge, mutate, query) to follow the
3rd‑party → library → CAREUI → UI → components → hooks → utils → relative
ordering; update the import list by cutting the Decimal import and pasting it
into the top third‑party group so only internal utils remain in the utils
section.
| import { round } from "@/Utils/decimal"; | ||
| import { ShortcutBadge } from "@/Utils/keyboardShortcutComponents"; | ||
| import mutate from "@/Utils/request/mutate"; | ||
| import query from "@/Utils/request/query"; | ||
| import Decimal from "decimal.js"; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Import order: Move Decimal to the 3rd-party imports section.
The Decimal import from decimal.js is a third-party library and should be placed with other 3rd-party imports at the top of the file, not after utility imports. As per coding guidelines, the import order should be: 3rd-party → library → CAREUI → UI → components → hooks → utils → relative.
🔧 Suggested fix
Move line 81 to after the other third-party imports (after line 14):
import { Trans, useTranslation } from "react-i18next";
import { toast } from "sonner";
+import Decimal from "decimal.js";
import CareIcon from "@/CAREUI/icons/CareIcon";And remove it from its current location:
import { round } from "@/Utils/decimal";
import { ShortcutBadge } from "@/Utils/keyboardShortcutComponents";
import mutate from "@/Utils/request/mutate";
import query from "@/Utils/request/query";
-import Decimal from "decimal.js";📝 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 { round } from "@/Utils/decimal"; | |
| import { ShortcutBadge } from "@/Utils/keyboardShortcutComponents"; | |
| import mutate from "@/Utils/request/mutate"; | |
| import query from "@/Utils/request/query"; | |
| import Decimal from "decimal.js"; | |
| import { round } from "@/Utils/decimal"; | |
| import { ShortcutBadge } from "@/Utils/keyboardShortcutComponents"; | |
| import mutate from "@/Utils/request/mutate"; | |
| import query from "@/Utils/request/query"; |
🤖 Prompt for AI Agents
In
`@src/pages/Facility/services/inventory/externalSupply/deliveryOrder/DeliveryOrderShow.tsx`
around lines 77 - 81, The import order is incorrect: the third‑party symbol
Decimal (from "decimal.js") is placed with utils; move the Decimal import into
the third‑party imports block with the other external libraries (so it appears
before internal utils like round, ShortcutBadge, mutate, query) to follow the
3rd‑party → library → CAREUI → UI → components → hooks → utils → relative
ordering; update the import list by cutting the Decimal import and pasting it
into the top third‑party group so only internal utils remain in the utils
section.
🎭 Playwright Test ResultsStatus: ❌ Failed
📊 Detailed results are available in the playwright-final-report artifact. Run: #5563 |
| totalQuantity = totalQuantity.plus(qty); | ||
|
|
||
| const priceComponents: MonetaryComponent[] = internal | ||
| ? delivery.supplied_inventory_item?.product?.charge_item_definition |
There was a problem hiding this comment.
Also need to account for category level monetary components.
There was a problem hiding this comment.
ideally when category is auto selected from batch, it should also apply tax. fixing that should automatically solve this. will handle separatley
|
This won't work, the calculation for total is incorrect |
Summary by CodeRabbit