Skip to content

[ENG-887] Fix internal and external delivery batch failure and refine supply delivery limit UI #16661

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.

Sign up for GitHub

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

Open
NikhilA8606 wants to merge 19 commits into
base: develop
Choose a base branch
from
Open

[ENG-887] Fix internal and external delivery batch failure and refine supply delivery limit UI #16661

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
8af2fcc
ENG-887 Fix Internal delivery batch failing
NikhilA8606 Aug 10, 2026
6b395a5
Added a callout to prevent users from adding more deliveries than the…
NikhilA8606 Aug 11, 2026
5914505
used result.length instead of count
NikhilA8606 Aug 11, 2026
88ca1a9
add suggested changes
NikhilA8606 Aug 12, 2026
79db63c
cleanups
NikhilA8606 Aug 13, 2026
d5a0115
cleanups
NikhilA8606 Aug 13, 2026
e0520a1
query cleanup
NikhilA8606 Aug 13, 2026
90f9e34
revert missed callout
NikhilA8606 Aug 13, 2026
9432850
add prevention check in receive and update stock too
NikhilA8606 Aug 13, 2026
2692ceb
added disable before it is used
NikhilA8606 Aug 13, 2026
77d07d2
Merge branch 'develop' into ENG-887-fix-internal-delivery-batch-failing
NikhilA8606 Aug 14, 2026
0ba9bea
all supplydelverires should be completed or abandoned
NikhilA8606 Aug 17, 2026
5392e1a
cleanups
NikhilA8606 Aug 17, 2026
a1b51bc
added ai suggestions
NikhilA8606 Aug 17, 2026
66c7c29
test bug fix
NikhilA8606 Aug 17, 2026
78188be
updated the test
NikhilA8606 Aug 17, 2026
9445a5c
added callout for load items too
NikhilA8606 Aug 18, 2026
e09e007
minor cleanups
NikhilA8606 Aug 18, 2026
dcc43a2
resolved merge conflicts
NikhilA8606 Aug 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .example.env
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,5 +194,9 @@ REACT_DECIMAL_ROUNDING_METHOD=ROUND_HALF_UP
# Maximum number of forms that can be favorited in the forms dialog (default: 5)
REACT_MAX_FORM_DIALOG_FAVORITES=5

# Maximum number of datapoints allowed in a single upsert request (default: 100)
# Must not exceed the backend limit
REACT_MAX_DATAPOINTS_PER_UPSERT=100

# Default tab for medication selector. Valid values: product, valueset
REACT_MEDICATION_VALUE_SET_SELECT_DEFAULT_TAB=product
11 changes: 9 additions & 2 deletions care.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ const careConfig = {
: undefined),

defaultDischargeDisposition: env.REACT_DEFAULT_DISCHARGE_DISPOSITION as
| EncounterDischargeDisposition
| undefined,
EncounterDischargeDisposition | undefined,

mapFallbackUrlTemplate:
env.REACT_MAPS_FALLBACK_URL_TEMPLATE ||
Expand Down Expand Up @@ -407,6 +406,14 @@ const careConfig = {
maxFormDialogFavorites: env.REACT_MAX_FORM_DIALOG_FAVORITES
? parseInt(env.REACT_MAX_FORM_DIALOG_FAVORITES, 10)
: 5,

/**
* Maximum number of datapoints allowed in a single upsert request.
* This should be set with whatever backend sets.
*/
maxDatapointsPerUpsert: env.REACT_MAX_DATAPOINTS_PER_UPSERT
Comment thread
rithviknishad marked this conversation as resolved.
? parseInt(env.REACT_MAX_DATAPOINTS_PER_UPSERT, 10)
Comment thread
NikhilA8606 marked this conversation as resolved.
: 100,
Comment thread
rithviknishad marked this conversation as resolved.
Comment thread
rithviknishad marked this conversation as resolved.
Comment thread
NikhilA8606 marked this conversation as resolved.
} as const;

export default careConfig;
4 changes: 3 additions & 1 deletion public/locale/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3058,6 +3058,7 @@
"issuer_type": "Issuer Type",
"item": "Item",
"item_condition": "Item Condition?",
"item_limit_reached": "Item limit reached",
Comment thread
NikhilA8606 marked this conversation as resolved.
Comment thread
NikhilA8606 marked this conversation as resolved.
"item_location": "Item Location",
"item_marked_as_abandoned": "Item marked as abandoned successfully",
"item_marked_as_entered_in_error": "Item marked as entered in error successfully",
Expand Down Expand Up @@ -3178,7 +3179,7 @@
"live": "Live",
"live_monitoring": "Live Monitoring",
"live_patients_total_beds": "Live Patients / Total beds",
"load_from_order": "Load from order",
"load_from_order_with_items": "Load from order ({{count}} items)",
"load_more": "Load More",
Comment thread
NikhilA8606 marked this conversation as resolved.
"loading": "Loading...",
"loading_appointment_details": "Loading appointment details...",
Expand Down Expand Up @@ -3405,6 +3406,7 @@
"max": "Max",
"max_applicable_discounts": "Maximum Applicable Discounts",
"max_applicable_discounts_description": "The maximum number of discount components that can be applied to a single invoice. Set to 0 for no discount.",
"max_datapoints_per_upsert_limit": "You cannot add more than {{count}} items in a single delivery.",
Comment thread
NikhilA8606 marked this conversation as resolved.
"max_dosage_24_hrs": "Max. dosage in 24 hrs.",
"max_dosage_in_24hrs_gte_base_dosage_error": "Max. dosage in 24 hours must be greater than or equal to base dosage",
"max_favorites_reached": "You've reached the limit. Only {{count}} forms can be favourited.",
Expand Down
1 change: 1 addition & 0 deletions scripts/validate-env.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ const envSchema = z
})
.optional(),
REACT_MAX_FORM_DIALOG_FAVORITES: numberAsString.optional(),
REACT_MAX_DATAPOINTS_PER_UPSERT: numberAsString.optional(),
Comment thread
rithviknishad marked this conversation as resolved.
Comment thread
rithviknishad marked this conversation as resolved.
})
.superRefine(async (data, ctx) => {
if (data.REACT_CARE_API_URL === undefined && !data.REACT_CARE_URL_MAP) {
Expand Down
19 changes: 3 additions & 16 deletions src/CAREUI/display/Callout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import React from "react";
import React, { ReactNode } from "react";

import { cn } from "@/lib/utils";

interface CalloutProps {
variant?: "primary" | "secondary" | "warning" | "alert" | "danger";
className?: string;
badge: string;
badge: ReactNode;
children: React.ReactNode;
}

Expand All @@ -27,20 +27,7 @@ export default function Callout({
props.className,
Comment thread
NikhilA8606 marked this conversation as resolved.
)}
>
<div
className={cn(
"h-min rounded-full border bg-white px-2",
{
primary: "border-primary-200",
secondary: "border-secondary-300",
warning: "border-warning-300",
alert: "border-purple-300",
danger: "border-danger-300",
}[variant],
)}
>
<span className="font-medium">{props.badge}</span>
</div>
<span className="font-medium">{props.badge}</span>
Comment thread
NikhilA8606 marked this conversation as resolved.
Comment thread
NikhilA8606 marked this conversation as resolved.
Comment thread
NikhilA8606 marked this conversation as resolved.
Comment thread
NikhilA8606 marked this conversation as resolved.
Comment thread
NikhilA8606 marked this conversation as resolved.
Comment thread
NikhilA8606 marked this conversation as resolved.
Comment thread
NikhilA8606 marked this conversation as resolved.
<div className="flex-1">
<span className="font-medium">{props.children}</span>
</div>
Expand Down
73 changes: 52 additions & 21 deletions src/pages/Facility/services/inventory/externalSupply/deliveryOrder/AddSupplyDeliveryForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { zodResolver } from "@hookform/resolvers/zod";
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
import { PlusCircle, Trash2 } from "lucide-react";
import { PlusCircle, Trash2, TriangleAlert } from "lucide-react";
import { useQueryParams } from "raviger";
import { useCallback, useMemo, useState } from "react";
import { useFieldArray, useForm } from "react-hook-form";
Expand Down Expand Up @@ -43,6 +43,7 @@ import {
TableRow,
} from "@/components/ui/table";

import Callout from "@/CAREUI/display/Callout";
import {
getExtensionFieldsWithName,
processExtensions,
Expand Down Expand Up @@ -129,6 +130,8 @@ interface Props {
origin?: string;
destination: string;
onSuccess: () => void;
supplyDeliveriesCount: number;
isFetchingSupplyDeliveries: boolean;
}

export function AddSupplyDeliveryForm({
Expand All @@ -137,6 +140,8 @@ export function AddSupplyDeliveryForm({
origin,
destination,
onSuccess,
supplyDeliveriesCount,
isFetchingSupplyDeliveries,
}: Props) {
const { t } = useTranslation();
const queryClient = useQueryClient();
Expand Down Expand Up @@ -237,6 +242,12 @@ export function AddSupplyDeliveryForm({
name: "items",
});

const hasReachedUpsertLimit =
Comment thread
NikhilA8606 marked this conversation as resolved.

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two different formulas for "hit the limit" in the same component: hasReachedUpsertLimit only checks supplyDeliveriesCount, while the alert banner further down checks supplyDeliveriesCount + fields.length. So the "Add Item"/"Load from Order" buttons at the bottom stay enabled even after the in-form alert says you're capped. Pick one source of truth and use it everywhere, or this'll bite you in a "why can I still click add" bug report.

@github-actions github-actions Bot Aug 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two formulas for the same limit in one component: hasReachedUpsertLimit here uses supplyDeliveriesCount >= max, while the alert at ~912 uses supplyDeliveriesCount + fields.length >= max. Collapse to one derived value (remainingCapacity = max - supplyDeliveriesCount - fields.length) and drive both the disabled props and the alert from it — otherwise the "Add Item"/"Load from order" buttons and the alert can disagree about whether you're at the cap.

supplyDeliveriesCount >= careConfig.maxDatapointsPerUpsert;

const disableAddItem =
isProcessing || hasReachedUpsertLimit || isFetchingSupplyDeliveries;
Comment thread
NikhilA8606 marked this conversation as resolved.

const loadFromSupplyRequests = () => {
setIsSelectDialogOpen(true);
handleSelectAll(true);
Expand Down Expand Up @@ -269,6 +280,7 @@ export function AddSupplyDeliveryForm({
noOptionsMessage={t("no_orders_found")}
className="px-10"
popoverContentClassName="w-auto"
disabled={disableAddItem}
Comment thread
NikhilA8606 marked this conversation as resolved.
Comment thread
NikhilA8606 marked this conversation as resolved.
Comment thread
NikhilA8606 marked this conversation as resolved.
/>
);

Expand Down Expand Up @@ -897,29 +909,47 @@ export function AddSupplyDeliveryForm({
</div>
</div>

<div className="flex flex-row gap-2 mt-4 items-end">
<Button
type="button"
variant="outline"
onClick={handleAddAnotherItem}
{supplyDeliveriesCount + fields.length >=
Comment thread
NikhilA8606 marked this conversation as resolved.

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole limit thing is enforced purely client-side. Nothing stops someone from hammering the API directly past maxDatapointsPerUpsert. Fine if the backend has its own real cap (the .env comment implies it does), but make sure that's actually true and not just wishful thinking — a UI-only limit is not a limit.

@github-actions github-actions Bot Aug 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hiding the whole button row behind the limit means when the cap is hit the user also loses "Load from order" and any way to interact — and the identical supplyDeliveriesCount >= limit logic already lives in hasReachedUpsertLimit two hundred lines up. Reuse it (hasReachedUpsertLimit || supplyDeliveriesCount + fields.length >= limit) instead of re-deriving the condition inline.

Comment thread
NikhilA8606 marked this conversation as resolved.
careConfig.maxDatapointsPerUpsert ? (

@github-actions github-actions Bot Aug 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the limit is hit you replace the entire button row with a warning — including the "Load from order" button and, in the block below, nothing stops the already-added fields from being submitted. Hiding the button is not enforcement; the submit path should reject it too. Also, hasReachedUpsertLimit (line 245) uses a different formula than this one (count + fields.length); two competing definitions of the same rule is how bugs are born.

Comment thread
NikhilA8606 marked this conversation as resolved.
<Callout
variant="warning"
className="border border-amber-300 bg-amber-50 text-amber-800"
Comment thread
NikhilA8606 marked this conversation as resolved.
badge={
<TriangleAlert className="size-4 shrink-0 text-amber-600" />

@github-actions github-actions Bot Aug 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoded amber-* classes on a component that already has a warning variant. That's the whole point of having variants — so the design system, not this file, decides what warning looks like. Drop the className.

}
>
<PlusCircle className="mr-2 size-4" />
{t("add_another")}
</Button>
{supplyRequests?.results?.length &&
supplyRequests?.results?.length > 0 && (
<span className="flex items-center gap-2">
{t("max_datapoints_per_upsert_limit", {
count: careConfig.maxDatapointsPerUpsert,
})}
</span>
</Callout>
) : (
<div className="flex flex-row gap-2 mt-4 items-end">
<Button
type="button"
variant="outline"
onClick={handleAddAnotherItem}
Comment thread
NikhilA8606 marked this conversation as resolved.
disabled={disableAddItem}
>
<PlusCircle className="mr-2 size-4" />
{t("add_another")}
</Button>
{!!supplyRequests?.results?.length && (
<Button
type="button"
variant="secondary"
onClick={loadFromSupplyRequests}
disabled={disableAddItem}

@github-actions github-actions Bot Aug 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Broken (correctness gap) — the cap is enforced only on the entry controls, never on submit. handleSelectRequests (line 302) does form.setValue("items", itemsFromRequests) with every checked request, and handleSelectAll(true) in loadFromSupplyRequests pre-checks all of them. If the supply order has 150 requests, one "Load from order" click puts 150 rows in the form while disableAddItem was still false (nothing was over the cap before the click).

onSubmit (line 553) then fires Promise.allSettled over all 150 — the exact backend batch failure this PR is meant to fix.

The check that actually protects the API belongs in the two places that grow items: cap the selection in handleSelectRequests, and re-check data.items.length + supplyDeliveriesCount in onSubmit before Promise.allSettled, with a toast.error(t("max_datapoints_per_upsert_limit", { count: ... })). Disabling a button is a hint; this is the guard.

>
{t("load_from_order")} ({supplyRequests?.count}{" "}
{t("items")}
)
{t("load_from_order_with_items", {
count: supplyRequests.count,
})}
<ShortcutBadge actionId="load-from-order" />
Comment thread
rithviknishad marked this conversation as resolved.
</Button>
)}
</div>
</div>
)}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

<div className="flex justify-between">
<Button
Expand All @@ -945,17 +975,17 @@ export function AddSupplyDeliveryForm({
<p>{t("add_items_to_delivery_description")}</p>
<div className="flex flex-row gap-2 items-center mt-2">
{qParams.supplyOrder ? (
supplyRequests?.results?.length &&
supplyRequests?.results?.length > 0 && (
!!supplyRequests?.results?.length && (
<>
<Button
type="button"
variant="outline_primary"
onClick={loadFromSupplyRequests}
disabled={disableAddItem}
>
{t("load_from_order")} ({supplyRequests?.count}{" "}
{t("items")}
)
{t("load_from_order_with_items", {
count: supplyRequests.count,
})}
<ShortcutBadge actionId="load-from-order" />
</Button>
<p>- {t("or")} -</p>
Expand All @@ -971,6 +1001,7 @@ export function AddSupplyDeliveryForm({
type="button"
variant="outline_primary"
onClick={() => handleAddAnotherItem()}
disabled={disableAddItem}
>
<PlusCircle className="mr-2 size-4" />
{t("add_item")}
Expand Down Expand Up @@ -1007,7 +1038,7 @@ export function AddSupplyDeliveryForm({
{t("select_all")}
</label>
</div>
<div className="border rounded-md divide-y">
<div className="border rounded-md divide-y max-h-[50vh] overflow-y-auto">
{supplyRequests.results.map((request) => (
<div
key={request.id}
Expand Down
Loading
Loading