-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[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.
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
base: develop
Are you sure you want to change the base?
[ENG-887] Fix internal and external delivery batch failure and refine supply delivery limit UI #16661
Changes from all commits
8af2fcc
6b395a5
5914505
88ca1a9
79db63c
d5a0115
e0520a1
90f9e34
9432850
2692ceb
77d07d2
0ba9bea
5392e1a
a1b51bc
66c7c29
78188be
9445a5c
e09e007
dcc43a2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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"; | ||
|
|
@@ -43,6 +43,7 @@ import { | |
| TableRow, | ||
| } from "@/components/ui/table"; | ||
|
|
||
| import Callout from "@/CAREUI/display/Callout"; | ||
| import { | ||
| getExtensionFieldsWithName, | ||
| processExtensions, | ||
|
|
@@ -129,6 +130,8 @@ interface Props { | |
| origin?: string; | ||
| destination: string; | ||
| onSuccess: () => void; | ||
| supplyDeliveriesCount: number; | ||
| isFetchingSupplyDeliveries: boolean; | ||
| } | ||
|
|
||
| export function AddSupplyDeliveryForm({ | ||
|
|
@@ -137,6 +140,8 @@ export function AddSupplyDeliveryForm({ | |
| origin, | ||
| destination, | ||
| onSuccess, | ||
| supplyDeliveriesCount, | ||
| isFetchingSupplyDeliveries, | ||
| }: Props) { | ||
| const { t } = useTranslation(); | ||
| const queryClient = useQueryClient(); | ||
|
|
@@ -237,6 +242,12 @@ export function AddSupplyDeliveryForm({ | |
| name: "items", | ||
| }); | ||
|
|
||
| const hasReachedUpsertLimit = | ||
|
NikhilA8606 marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Two different formulas for "hit the limit" in the same component: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Two formulas for the same limit in one component: |
||
| supplyDeliveriesCount >= careConfig.maxDatapointsPerUpsert; | ||
|
|
||
| const disableAddItem = | ||
| isProcessing || hasReachedUpsertLimit || isFetchingSupplyDeliveries; | ||
|
NikhilA8606 marked this conversation as resolved.
|
||
|
|
||
| const loadFromSupplyRequests = () => { | ||
| setIsSelectDialogOpen(true); | ||
| handleSelectAll(true); | ||
|
|
@@ -269,6 +280,7 @@ export function AddSupplyDeliveryForm({ | |
| noOptionsMessage={t("no_orders_found")} | ||
| className="px-10" | ||
| popoverContentClassName="w-auto" | ||
| disabled={disableAddItem} | ||
|
NikhilA8606 marked this conversation as resolved.
NikhilA8606 marked this conversation as resolved.
NikhilA8606 marked this conversation as resolved.
|
||
| /> | ||
| ); | ||
|
|
||
|
|
@@ -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 >= | ||
|
NikhilA8606 marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
NikhilA8606 marked this conversation as resolved.
|
||
| careConfig.maxDatapointsPerUpsert ? ( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
NikhilA8606 marked this conversation as resolved.
|
||
| <Callout | ||
| variant="warning" | ||
| className="border border-amber-300 bg-amber-50 text-amber-800" | ||
|
NikhilA8606 marked this conversation as resolved.
|
||
| badge={ | ||
| <TriangleAlert className="size-4 shrink-0 text-amber-600" /> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hardcoded |
||
| } | ||
| > | ||
| <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} | ||
|
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} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
The check that actually protects the API belongs in the two places that grow |
||
| > | ||
| {t("load_from_order")} ({supplyRequests?.count}{" "} | ||
| {t("items")} | ||
| ) | ||
| {t("load_from_order_with_items", { | ||
| count: supplyRequests.count, | ||
| })} | ||
| <ShortcutBadge actionId="load-from-order" /> | ||
|
rithviknishad marked this conversation as resolved.
|
||
| </Button> | ||
| )} | ||
| </div> | ||
| </div> | ||
| )} | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| <div className="flex justify-between"> | ||
| <Button | ||
|
|
@@ -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> | ||
|
|
@@ -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")} | ||
|
|
@@ -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} | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.