Skip to content

Commit 42b46be

Browse files
nass600robot-ci-heartex
authored andcommitted
fix: UTC-1442: Replace button incorrectly enabled when submission region is locked
GitOrigin-RevId: 6fef806e01fccb03ba3e60c51cb68026af0f32f1
1 parent 7089f42 commit 42b46be

2 files changed

Lines changed: 90 additions & 17 deletions

File tree

‎web/libs/ui/src/lib/collection-uploader/use-collection-submission.test.tsx‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -954,3 +954,59 @@ describe("server facts", () => {
954954
expect(member.meta?.facts).toEqual(["30 fps"]);
955955
});
956956
});
957+
958+
describe("locked regions", () => {
959+
const fireReplaceInput = async (refs: HarnessRefs, file: File) => {
960+
await act(async () =>
961+
refs.api!.replaceInput.onChange({
962+
target: { files: [file], value: "" } as unknown as HTMLInputElement,
963+
}),
964+
);
965+
};
966+
967+
it("a locked region offers neither Replace nor Remove while its neighbour offers both", async () => {
968+
const engine = new FakeEngine(() => undefined);
969+
const { refs, Host } = makeHarness({
970+
engine,
971+
initialRegions: [{ ...region(70, 0), locked: true }, region(71, 1)],
972+
});
973+
render(<Host />);
974+
await waitFor(() => expect(refs.api!.members).toHaveLength(2));
975+
expect(refs.api!.members[0].state).toBe("uploaded");
976+
expect(refs.api!.members[0].onReplace).toBeUndefined();
977+
expect(refs.api!.members[0].onRemove).toBeUndefined();
978+
expect(refs.api!.members[1].onReplace).toBeDefined();
979+
expect(refs.api!.members[1].onRemove).toBeDefined();
980+
});
981+
982+
it("a replace pick is ignored when the region was locked while the file dialog was open", async () => {
983+
const engine = new FakeEngine(() => undefined);
984+
const { refs, Host } = makeHarness({ engine, initialRegions: [region(70, 0)] });
985+
render(<Host />);
986+
await act(async () => refs.api!.members[0].onReplace!());
987+
await act(async () => refs.setRegions!([{ ...region(70, 0), locked: true }]));
988+
await fireReplaceInput(refs, pdf("new.pdf", 2222));
989+
await new Promise((resolve) => setTimeout(resolve, 20));
990+
expect(engine.rows).toHaveLength(0);
991+
expect(refs.api!.members).toHaveLength(1);
992+
expect(refs.regions[0]._submission!.upload_id).toBe(70);
993+
});
994+
995+
it("a region locked while its replacement uploads keeps the original and drops the new file", async () => {
996+
const engine = new FakeEngine(() => undefined);
997+
const { refs, Host } = makeHarness({ engine, initialRegions: [region(70, 0)] });
998+
render(<Host />);
999+
await act(async () => refs.api!.members[0].onReplace!());
1000+
await fireReplaceInput(refs, pdf("new.pdf", 2222));
1001+
await waitFor(() => expect(engine.rows).toHaveLength(1));
1002+
await act(async () => refs.setRegions!([{ ...region(70, 0), locked: true }]));
1003+
await act(async () => engine.complete(engine.rows[0].clientRef, 99));
1004+
await waitFor(() => expect(engine.discarded).toContain(99));
1005+
expect(engine.discarded).not.toContain(70);
1006+
expect(refs.regions).toHaveLength(1);
1007+
expect(refs.regions[0]._submission!.upload_id).toBe(70);
1008+
expect(refs.api!.members).toHaveLength(1);
1009+
expect(refs.api!.members[0].state).toBe("uploaded");
1010+
expect(refs.api!.members[0].file.name).toBe("f70.pdf");
1011+
});
1012+
});

‎web/libs/ui/src/lib/collection-uploader/use-collection-submission.ts‎

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,13 @@ export function useCollectionSubmission(options: UseCollectionSubmissionOptions)
485485
() => (props.visibleRegions || []).filter((r) => r.type === "submission" && r._submission),
486486
[props.visibleRegions],
487487
);
488+
// Upload completions read the lock state at that moment, not at pick time.
489+
const regionsRef = useRef(regions);
490+
regionsRef.current = regions;
491+
const isRegionLocked = useCallback(
492+
(regionId: string) => regionsRef.current.some((r) => r.id === regionId && r.locked === true),
493+
[],
494+
);
488495

489496
const pdfjsLib = deps?.documentAI?.pdfjsLib;
490497
const memberPdf = useCallback(
@@ -689,12 +696,19 @@ export function useCollectionSubmission(options: UseCollectionSubmissionOptions)
689696
? pending.clientRef === row.clientRef
690697
: pending.name === row.filename && pending.size === row.size);
691698
if (pending && pendingMatches) {
699+
replacePendingRef.current = null;
700+
const engine = engineRef.current;
701+
// The host rejects updates to a locked region, so the original stays
702+
// and the replacement is dropped instead of lingering as an extra card.
703+
if (isRegionLocked(pending.regionId)) {
704+
removedUploadIds.current.add(String(row.uploadId));
705+
if (engine) engine.discard(row.uploadId).catch(() => undefined);
706+
return;
707+
}
692708
// Swap in place: same slot, new file. The old upload is tombstoned so
693709
// recovery never resurrects it, and discarded best-effort - the server
694710
// refuses while an annotation or history revision still needs it.
695-
replacePendingRef.current = null;
696711
removedUploadIds.current.add(String(pending.oldUploadId));
697-
const engine = engineRef.current;
698712
if (engine && pending.oldUploadId) engine.discard(pending.oldUploadId).catch(() => undefined);
699713
if (pending.url && row.key) {
700714
previewCache(taskId)[row.key] = {
@@ -741,7 +755,7 @@ export function useCollectionSubmission(options: UseCollectionSubmissionOptions)
741755
previewCache(taskId)[row.key] = { url: picked.url, kind: submissionMediaKind(row.filename, row.contentType) };
742756
}
743757
},
744-
[visibleRegions, addRegion, updateRegion, taskId, bounds.max, rules],
758+
[visibleRegions, addRegion, updateRegion, isRegionLocked, taskId, bounds.max, rules],
745759
);
746760

747761
// Bind the in-flight replace to its engine row once, by clientRef. Name+size
@@ -955,6 +969,7 @@ export function useCollectionSubmission(options: UseCollectionSubmissionOptions)
955969
replaceTargetRef.current = null;
956970
const engine = getEngine();
957971
if (!file || !target || !engine) return;
972+
if (isRegionLocked(target.regionId)) return;
958973
validateAndHandle(file, (validFile, meta, url) => {
959974
const priorRefs = new Set((engine.rows || []).map((row) => row.clientRef));
960975
const pendingEntry = {
@@ -986,7 +1001,7 @@ export function useCollectionSubmission(options: UseCollectionSubmissionOptions)
9861001
});
9871002
});
9881003
},
989-
[getEngine, validateAndHandle, startUpload, attachRow, taskId],
1004+
[getEngine, isRegionLocked, validateAndHandle, startUpload, attachRow, taskId],
9901005
);
9911006

9921007
const replaceInput = useMemo(
@@ -1074,6 +1089,7 @@ export function useCollectionSubmission(options: UseCollectionSubmissionOptions)
10741089

10751090
const storedMembers: CollectionMember[] = regions.map((region) => {
10761091
const sub = region._submission!;
1092+
const locked = region.locked === true;
10771093
const server = serverByUploadId[String(sub.upload_id)];
10781094
const submitted =
10791095
!!(server && server.referenced_by_annotation) ||
@@ -1147,22 +1163,23 @@ export function useCollectionSubmission(options: UseCollectionSubmissionOptions)
11471163
submitted,
11481164
storedHint: true,
11491165
duplicate: (server && server.duplicate) || undefined,
1150-
onReplace: readonly
1151-
? undefined
1152-
: () => {
1153-
replaceTargetRef.current = {
1154-
regionId: region.id,
1155-
index: sub.index || 0,
1156-
oldUploadId: sub.upload_id,
1157-
oldKey: sub.key,
1158-
};
1159-
replacePendingRef.current = null;
1160-
if (replaceInputElRef.current) replaceInputElRef.current.click();
1161-
},
1166+
onReplace:
1167+
readonly || locked
1168+
? undefined
1169+
: () => {
1170+
replaceTargetRef.current = {
1171+
regionId: region.id,
1172+
index: sub.index || 0,
1173+
oldUploadId: sub.upload_id,
1174+
oldKey: sub.key,
1175+
};
1176+
replacePendingRef.current = null;
1177+
if (replaceInputElRef.current) replaceInputElRef.current.click();
1178+
},
11621179
// Submitted members are Replace-only - except when the collection is
11631180
// over the (lowered) capacity: trimming must stay possible, and the old
11641181
// revision keeps its files regardless (removal lands on Update).
1165-
onRemove: readonly || (submitted && !capacityOver) ? undefined : () => removeRegion(region),
1182+
onRemove: readonly || locked || (submitted && !capacityOver) ? undefined : () => removeRegion(region),
11661183
onRetryPreview: sub.key
11671184
? () =>
11681185
setPreviewRetryNonce((prior) => ({ ...prior, [sub.key as string]: (prior[sub.key as string] || 0) + 1 }))

0 commit comments

Comments
 (0)