mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-03 05:45:04 +03:00
feat(combos): add select all / unselect all in Browse Catalog (#8526)
* feat(combos): add select all / unselect all in Browse Catalog * fix(dashboard): guard combo Select all + test the real batch handlers (#8526) Select all had no cap — with "Show configured only" off, or a large provider catalog, one click could add hundreds of models to a combo. ModelSelectModal now confirms above SELECT_ALL_CONFIRM_THRESHOLD (20) before batch-adding, matching the native confirm() pattern already used for bulk/destructive actions elsewhere in the dashboard. Also extracts ComboFormModal's handleAddModels/handleDeselectModels batching logic into computeBatchAddModelSteps/computeBatchDeselectModelSteps (src/lib/combos/builderDraft.ts) so unit tests exercise the real implementation instead of a hand-maintained mirror that could drift from the component and stay green while production code broke. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> Co-authored-by: Diego Rodrigues de Sa e Souza <diegosouza.pw@gmail.com>
This commit is contained in:
@@ -28,6 +28,8 @@ import {
|
||||
buildManualComboModelStep,
|
||||
buildPrecisionComboModelStep,
|
||||
canAccessComboBuilderStage,
|
||||
computeBatchAddModelSteps,
|
||||
computeBatchDeselectModelSteps,
|
||||
findNextSuggestedConnectionId,
|
||||
getComboBuilderStageChecks,
|
||||
getComboBuilderStages,
|
||||
@@ -2530,6 +2532,26 @@ function ComboFormModal({ isOpen, combo, onClose, onSave, activeProviders, combo
|
||||
setBuilderError("");
|
||||
};
|
||||
|
||||
// Batch add for ModelSelectModal "Select all" — delegates to the pure
|
||||
// computeBatchAddModelSteps (src/lib/combos/builderDraft.ts) which applies
|
||||
// every candidate against a growing list in one pass, otherwise N× onSelect
|
||||
// would each close over the same stale `models` snapshot and keep only the
|
||||
// last entry. Extracted so tests exercise this real implementation instead
|
||||
// of a hand-maintained mirror (#8526).
|
||||
const handleAddModels = (selected) => {
|
||||
const { next, addedAny } = computeBatchAddModelSteps(models, selected, builderProviders);
|
||||
if (!addedAny) return;
|
||||
setModels(next);
|
||||
setBuilderError("");
|
||||
};
|
||||
|
||||
const handleDeselectModels = (toRemove) => {
|
||||
const next = computeBatchDeselectModelSteps(models, toRemove);
|
||||
if (next === models) return;
|
||||
setModels(next);
|
||||
setBuilderError("");
|
||||
};
|
||||
|
||||
const handleWeightChange = (index, weight) => {
|
||||
const newModels = [...models];
|
||||
newModels[index] = {
|
||||
@@ -4610,6 +4632,8 @@ function ComboFormModal({ isOpen, combo, onClose, onSave, activeProviders, combo
|
||||
onClose={() => setShowModelSelect(false)}
|
||||
onSelect={handleAddModel}
|
||||
onDeselect={handleDeselectModel}
|
||||
onSelectMany={handleAddModels}
|
||||
onDeselectMany={handleDeselectModels}
|
||||
activeProviders={activeProviders}
|
||||
modelAliases={modelAliases}
|
||||
title={t("addModelToCombo")}
|
||||
|
||||
@@ -168,6 +168,10 @@
|
||||
"noModelsFound": "No models found",
|
||||
"clear": "Clear",
|
||||
"done": "Done",
|
||||
"selectAll": "Select all",
|
||||
"unselectAll": "Unselect all",
|
||||
"visibleModels": "visible",
|
||||
"selectAllConfirm": "Add {count} models to this combo?",
|
||||
"errorOccurred": "Error Occurred",
|
||||
"comboDeleted": "Combo Deleted",
|
||||
"hide": "Hide",
|
||||
|
||||
@@ -267,3 +267,105 @@ export function getPreviousComboBuilderStage(
|
||||
if (stageIndex <= 0) return "basics";
|
||||
return stages[stageIndex - 1];
|
||||
}
|
||||
|
||||
export type ComboBuilderModelCandidate = {
|
||||
value?: unknown;
|
||||
providerId?: unknown;
|
||||
};
|
||||
|
||||
export type ComboBuilderDraftModelStep = {
|
||||
model: string;
|
||||
providerId?: string;
|
||||
weight: number;
|
||||
};
|
||||
|
||||
/**
|
||||
* Resolve the providerId for one "Select all" candidate: an exact/alias/prefix
|
||||
* match in `builderProviders` wins, falling back to whatever provider prefix
|
||||
* the qualified model string itself carries. Split out of
|
||||
* `computeBatchAddModelSteps` to keep that loop under the complexity budget.
|
||||
*/
|
||||
function resolveBatchCandidateProviderId(
|
||||
model: ComboBuilderModelCandidate,
|
||||
parsedModel: { providerId: string; modelId: string } | null,
|
||||
builderProviders: ComboBuilderProviderIdentity[]
|
||||
): string | null {
|
||||
return (
|
||||
resolveComboBuilderProviderId(model?.providerId, builderProviders) ||
|
||||
resolveComboBuilderProviderId(parsedModel?.providerId, builderProviders) ||
|
||||
(typeof model?.providerId === "string" && model.providerId.trim()) ||
|
||||
parsedModel?.providerId ||
|
||||
null
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Batch-add handler for ModelSelectModal's "Select all" (`onSelectMany`) in
|
||||
* the combo builder — must apply every candidate against a growing list in
|
||||
* ONE pass. Looping the single-add handler N times would have each call
|
||||
* close over the same stale `models` snapshot and only the last selected
|
||||
* model would survive (#8526).
|
||||
*
|
||||
* This is the real implementation `ComboFormModal::handleAddModels` in
|
||||
* `page.tsx` delegates to, so unit tests exercise actual production logic
|
||||
* instead of a hand-maintained copy that can drift from the component.
|
||||
*/
|
||||
export function computeBatchAddModelSteps(
|
||||
models: ComboBuilderDraftModelStep[],
|
||||
selected: ComboBuilderModelCandidate[],
|
||||
builderProviders: ComboBuilderProviderIdentity[] = []
|
||||
): { next: ComboBuilderDraftModelStep[]; addedAny: boolean } {
|
||||
if (!Array.isArray(selected) || selected.length === 0) {
|
||||
return { next: models, addedAny: false };
|
||||
}
|
||||
const next = [...models];
|
||||
let addedAny = false;
|
||||
for (const model of selected) {
|
||||
const qualifiedModel = typeof model?.value === "string" ? model.value : "";
|
||||
if (!qualifiedModel) continue;
|
||||
const parsedModel = parseQualifiedModel(qualifiedModel);
|
||||
const resolvedProviderId = resolveBatchCandidateProviderId(
|
||||
model,
|
||||
parsedModel,
|
||||
builderProviders
|
||||
);
|
||||
const nextEntry: ComboBuilderDraftModelStep = {
|
||||
model: qualifiedModel,
|
||||
...(resolvedProviderId ? { providerId: resolvedProviderId } : {}),
|
||||
weight: 0,
|
||||
};
|
||||
if (hasExactModelStepDuplicate(next, nextEntry)) continue;
|
||||
next.push(nextEntry);
|
||||
addedAny = true;
|
||||
}
|
||||
return { next, addedAny };
|
||||
}
|
||||
|
||||
/**
|
||||
* Batch-remove handler for ModelSelectModal's "Unselect all" (`onDeselectMany`)
|
||||
* in the combo builder — same stale-snapshot reasoning as
|
||||
* `computeBatchAddModelSteps` above. Accepts either `{ value }` candidate
|
||||
* objects (from the modal) or raw qualified-model strings.
|
||||
*
|
||||
* Returns the same `models` reference (no-op) when there is nothing to
|
||||
* remove, so callers can skip the `setModels` call.
|
||||
*/
|
||||
export function computeBatchDeselectModelSteps(
|
||||
models: ComboBuilderDraftModelStep[],
|
||||
toRemove: Array<{ value?: unknown } | string>
|
||||
): ComboBuilderDraftModelStep[] {
|
||||
if (!Array.isArray(toRemove) || toRemove.length === 0) return models;
|
||||
const values = new Set(
|
||||
toRemove
|
||||
.map((model) =>
|
||||
typeof (model as { value?: unknown })?.value === "string"
|
||||
? (model as { value: string }).value
|
||||
: typeof model === "string"
|
||||
? model
|
||||
: ""
|
||||
)
|
||||
.filter(Boolean)
|
||||
);
|
||||
if (values.size === 0) return models;
|
||||
return models.filter((m) => !values.has(m.model));
|
||||
}
|
||||
|
||||
@@ -3,7 +3,11 @@
|
||||
import { useState, useMemo, useEffect } from "react";
|
||||
import { useTranslations } from "next-intl";
|
||||
import Modal from "./Modal";
|
||||
import { buildPassthroughAliasModels, buildNodeAliasModels } from "./modelSelectModalHelpers";
|
||||
import {
|
||||
buildPassthroughAliasModels,
|
||||
buildNodeAliasModels,
|
||||
shouldConfirmSelectAll,
|
||||
} from "./modelSelectModalHelpers";
|
||||
import { getModelsByProviderId, PROVIDER_ID_TO_ALIAS } from "@/shared/constants/models";
|
||||
import { getCompatibleFallbackModels } from "@/lib/providers/managedAvailableModels";
|
||||
import {
|
||||
@@ -38,6 +42,16 @@ type ModelSelectModalProps = {
|
||||
* decolua/9router#889 (Fajar Hidayat).
|
||||
*/
|
||||
onDeselect?: (model: unknown) => void;
|
||||
/**
|
||||
* Batch add for "Select all" — callers that keep the modal open (combo
|
||||
* builder) must use this instead of looping `onSelect`, because each
|
||||
* single-add handler closes over the same models snapshot.
|
||||
*/
|
||||
onSelectMany?: (models: unknown[]) => void;
|
||||
/**
|
||||
* Batch remove for "Unselect all" — same stale-state reason as onSelectMany.
|
||||
*/
|
||||
onDeselectMany?: (models: unknown[]) => void;
|
||||
selectedModel?: string;
|
||||
selectedModels?: string[];
|
||||
activeProviders?: Array<{
|
||||
@@ -72,6 +86,8 @@ export default function ModelSelectModal({
|
||||
onClose,
|
||||
onSelect,
|
||||
onDeselect,
|
||||
onSelectMany,
|
||||
onDeselectMany,
|
||||
selectedModel,
|
||||
selectedModels = [],
|
||||
activeProviders = [],
|
||||
@@ -85,6 +101,11 @@ export default function ModelSelectModal({
|
||||
}: ModelSelectModalProps) {
|
||||
const t = useTranslations("common");
|
||||
const resolvedTitle = title ?? t("selectModel");
|
||||
const labelOrFallback = (key: string, fallback: string, values?: Record<string, unknown>) =>
|
||||
typeof (t as { has?: (k: string) => boolean }).has === "function" &&
|
||||
(t as { has: (k: string) => boolean }).has(key)
|
||||
? (t as unknown as (k: string, v?: Record<string, unknown>) => string)(key, values)
|
||||
: fallback;
|
||||
const [searchQuery, setSearchQuery] = useState("");
|
||||
const [combos, setCombos] = useState<any[]>([]);
|
||||
const [providerNodes, setProviderNodes] = useState<any[]>([]);
|
||||
@@ -460,6 +481,63 @@ export default function ModelSelectModal({
|
||||
return result;
|
||||
}, [filteredGroups, showConfiguredOnly, activeProviders]);
|
||||
|
||||
// Flat list of currently visible provider models (respects search + configured-only).
|
||||
// Used by Select all / Unselect all — does not include the Combos section.
|
||||
const visibleModels = useMemo(() => {
|
||||
const models: any[] = [];
|
||||
Object.values(connectionFilteredGroups).forEach((group: any) => {
|
||||
if (Array.isArray(group?.models)) {
|
||||
models.push(...group.models);
|
||||
}
|
||||
});
|
||||
return models;
|
||||
}, [connectionFilteredGroups]);
|
||||
|
||||
const addedModelValueSet = useMemo(() => new Set(addedModelValues), [addedModelValues]);
|
||||
|
||||
const allVisibleSelected =
|
||||
visibleModels.length > 0 &&
|
||||
visibleModels.every(
|
||||
(model) => typeof model?.value === "string" && addedModelValueSet.has(model.value)
|
||||
);
|
||||
|
||||
const showSelectAllToggle =
|
||||
keepOpenOnSelect &&
|
||||
!multiSelect &&
|
||||
typeof onSelectMany === "function" &&
|
||||
typeof onDeselectMany === "function" &&
|
||||
visibleModels.length > 0;
|
||||
|
||||
const handleToggleSelectAllVisible = () => {
|
||||
if (!showSelectAllToggle) return;
|
||||
if (allVisibleSelected) {
|
||||
const toRemove = visibleModels.filter(
|
||||
(model) => typeof model?.value === "string" && addedModelValueSet.has(model.value)
|
||||
);
|
||||
if (toRemove.length > 0) onDeselectMany!(toRemove);
|
||||
return;
|
||||
}
|
||||
const toAdd = visibleModels.filter(
|
||||
(model) => typeof model?.value === "string" && !addedModelValueSet.has(model.value)
|
||||
);
|
||||
if (toAdd.length === 0) return;
|
||||
// Guard against a single click adding hundreds of models (e.g. with
|
||||
// "Show configured only" off) — see modelSelectModalHelpers.ts (#8526).
|
||||
if (
|
||||
shouldConfirmSelectAll(toAdd.length) &&
|
||||
!confirm(
|
||||
labelOrFallback(
|
||||
"selectAllConfirm",
|
||||
`Add ${toAdd.length} models to this combo?`,
|
||||
{ count: toAdd.length }
|
||||
)
|
||||
)
|
||||
) {
|
||||
return;
|
||||
}
|
||||
onSelectMany!(toAdd);
|
||||
};
|
||||
|
||||
const resolvedSelectedModels = multiSelect
|
||||
? selectedModels
|
||||
: selectedModel
|
||||
@@ -541,22 +619,40 @@ export default function ModelSelectModal({
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<label className="flex items-center gap-1.5 mt-1.5 text-xs text-text-muted cursor-pointer">
|
||||
<input
|
||||
type="checkbox"
|
||||
checked={showConfiguredOnly}
|
||||
onChange={(e) => setShowConfiguredOnly(e.target.checked)}
|
||||
className="rounded border-border"
|
||||
/>
|
||||
{t("showConfiguredOnly")}
|
||||
</label>
|
||||
<div className="mt-1.5 mb-2 flex items-center justify-between gap-2">
|
||||
<label className="flex items-center gap-1.5 text-xs text-text-muted cursor-pointer min-w-0">
|
||||
<input
|
||||
type="checkbox"
|
||||
checked={showConfiguredOnly}
|
||||
onChange={(e) => setShowConfiguredOnly(e.target.checked)}
|
||||
className="rounded border-border"
|
||||
/>
|
||||
<span className="truncate">{t("showConfiguredOnly")}</span>
|
||||
</label>
|
||||
|
||||
{showSelectAllToggle && (
|
||||
<button
|
||||
type="button"
|
||||
onClick={handleToggleSelectAllVisible}
|
||||
data-testid="model-select-toggle-all-visible"
|
||||
className="shrink-0 px-2 py-1 text-xs font-medium rounded border border-border bg-surface text-text-main hover:border-primary/50 hover:bg-primary/5 transition-colors"
|
||||
>
|
||||
{allVisibleSelected
|
||||
? labelOrFallback("unselectAll", "Unselect all")
|
||||
: labelOrFallback("selectAll", "Select all")}
|
||||
<span className="ml-1 text-[10px] text-text-muted font-normal">
|
||||
({visibleModels.length})
|
||||
</span>
|
||||
</button>
|
||||
)}
|
||||
</div>
|
||||
|
||||
{/* Models grouped by provider - compact */}
|
||||
<div className="max-h-[300px] overflow-y-auto space-y-3">
|
||||
<div className="max-h-[300px] overflow-y-auto space-y-3 isolate">
|
||||
{/* Combos section - always first */}
|
||||
{showCombos && filteredCombos.length > 0 && (
|
||||
<div>
|
||||
<div className="flex items-center gap-1.5 mb-1.5 sticky top-0 bg-surface py-0.5">
|
||||
<div className="flex items-center gap-1.5 mb-1.5 sticky top-0 z-10 bg-surface py-1">
|
||||
<span className="material-symbols-outlined text-primary text-[14px]">layers</span>
|
||||
<span className="text-xs font-medium text-primary">{t("combos")}</span>
|
||||
<span className="text-[10px] text-text-muted">({filteredCombos.length})</span>
|
||||
@@ -590,9 +686,12 @@ export default function ModelSelectModal({
|
||||
{/* Provider models */}
|
||||
{Object.entries(connectionFilteredGroups).map(([providerId, group]: [string, any]) => (
|
||||
<div key={providerId}>
|
||||
{/* Provider header */}
|
||||
<div className="flex items-center gap-1.5 mb-1.5 sticky top-0 bg-surface py-0.5">
|
||||
<div className="w-2 h-2 rounded-full" style={{ backgroundColor: group.color }} />
|
||||
{/* Provider header — z-10 + opaque bg so scrolling chips don't bleed through */}
|
||||
<div className="flex items-center gap-1.5 mb-1.5 sticky top-0 z-10 bg-surface py-1">
|
||||
<div
|
||||
className="w-2 h-2 rounded-full shrink-0"
|
||||
style={{ backgroundColor: group.color }}
|
||||
/>
|
||||
<span className="text-xs font-medium text-primary">{group.name}</span>
|
||||
<span className="text-[10px] text-text-muted">({group.models.length})</span>
|
||||
</div>
|
||||
|
||||
@@ -73,3 +73,23 @@ export function buildNodeAliasModels(
|
||||
source: "alias" as const,
|
||||
}));
|
||||
}
|
||||
|
||||
/**
|
||||
* "Select all" adds every currently-visible model to the combo in one click,
|
||||
* with no cap — turning off "Show configured only" (or just having a large
|
||||
* provider catalog) can put hundreds of candidates behind a single click.
|
||||
* Above this threshold the caller must confirm before batch-adding (#8526).
|
||||
*
|
||||
* A native `confirm()` — not a bespoke modal — matches the existing bulk /
|
||||
* destructive-action pattern already used in this codebase (e.g.
|
||||
* `ReasoningRoutingRules.tsx::deleteConfirm`, `combos/page.tsx::deleteConfirm`),
|
||||
* so no new UI primitive is needed for this one interaction.
|
||||
*/
|
||||
export const SELECT_ALL_CONFIRM_THRESHOLD = 20;
|
||||
|
||||
export function shouldConfirmSelectAll(
|
||||
candidateCount: number,
|
||||
threshold: number = SELECT_ALL_CONFIRM_THRESHOLD
|
||||
): boolean {
|
||||
return Number.isFinite(candidateCount) && candidateCount > threshold;
|
||||
}
|
||||
|
||||
100
tests/unit/combo-select-all-batch-8526.test.ts
Normal file
100
tests/unit/combo-select-all-batch-8526.test.ts
Normal file
@@ -0,0 +1,100 @@
|
||||
// Regression guard for #8526 (combo Browse Catalog "Select all"/"Unselect
|
||||
// all"). This is the BLOCKING copy — tests/unit/*.test.ts is collected by
|
||||
// `npm run test:unit` (node:test), unlike tests/unit/ui/*.test.tsx which is
|
||||
// only collected by the advisory `test:vitest:ui`
|
||||
// (see CONTRIBUTING.md "Both test runners must pass").
|
||||
//
|
||||
// It exercises the exact functions `ComboFormModal::handleAddModels` /
|
||||
// `handleDeselectModels` (src/app/(dashboard)/dashboard/combos/page.tsx)
|
||||
// delegate to — not a hand-maintained mirror of the batching logic — so a
|
||||
// broken extraction fails this suite, not just an advisory one.
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
const builderDraft = await import("../../src/lib/combos/builderDraft.ts");
|
||||
|
||||
test("computeBatchAddModelSteps adds every candidate against a growing list in one pass", () => {
|
||||
const { next, addedAny } = builderDraft.computeBatchAddModelSteps(
|
||||
[],
|
||||
[
|
||||
{ value: "openai/gpt-4o" },
|
||||
{ value: "openai/gpt-4o-mini" },
|
||||
{ value: "anthropic/claude-3-5-sonnet" },
|
||||
],
|
||||
[]
|
||||
);
|
||||
assert.equal(addedAny, true);
|
||||
assert.deepEqual(
|
||||
next.map((m) => m.model),
|
||||
["openai/gpt-4o", "openai/gpt-4o-mini", "anthropic/claude-3-5-sonnet"]
|
||||
);
|
||||
});
|
||||
|
||||
test("computeBatchAddModelSteps skips exact provider/model/account duplicates", () => {
|
||||
const existing = [{ model: "openai/gpt-4o", providerId: "openai", weight: 0 }];
|
||||
const { next, addedAny } = builderDraft.computeBatchAddModelSteps(
|
||||
existing,
|
||||
[{ value: "openai/gpt-4o", providerId: "openai" }, { value: "openai/gpt-4o-mini" }],
|
||||
[]
|
||||
);
|
||||
assert.equal(addedAny, true);
|
||||
assert.deepEqual(
|
||||
next.map((m) => m.model),
|
||||
["openai/gpt-4o", "openai/gpt-4o-mini"]
|
||||
);
|
||||
});
|
||||
|
||||
test("computeBatchAddModelSteps resolves providerId via builderProviders alias/prefix", () => {
|
||||
const builderProviders = [{ providerId: "openai", alias: "oa", prefix: "gpt" }];
|
||||
const { next } = builderDraft.computeBatchAddModelSteps(
|
||||
[],
|
||||
[{ value: "gpt/gpt-4o", providerId: "gpt" }],
|
||||
builderProviders
|
||||
);
|
||||
assert.equal(next[0].providerId, "openai");
|
||||
});
|
||||
|
||||
test("computeBatchAddModelSteps ignores empty/missing values and is a no-op with nothing selected", () => {
|
||||
const { next, addedAny } = builderDraft.computeBatchAddModelSteps(
|
||||
[],
|
||||
[{ value: "" }, {}, { value: "openai/gpt-4o" }],
|
||||
[]
|
||||
);
|
||||
assert.equal(addedAny, true);
|
||||
// No matching entry in builderProviders, so providerId falls back to the
|
||||
// provider prefix parsed out of the qualified model string.
|
||||
assert.deepEqual(next, [{ model: "openai/gpt-4o", providerId: "openai", weight: 0 }]);
|
||||
|
||||
const existing = [{ model: "openai/gpt-4o", weight: 0 }];
|
||||
const noop = builderDraft.computeBatchAddModelSteps(existing, [], []);
|
||||
assert.equal(noop.addedAny, false);
|
||||
assert.equal(noop.next, existing);
|
||||
});
|
||||
|
||||
test("computeBatchDeselectModelSteps removes every matching qualified model in one pass", () => {
|
||||
const models = [
|
||||
{ model: "openai/gpt-4o", weight: 0 },
|
||||
{ model: "openai/gpt-4o-mini", weight: 0 },
|
||||
{ model: "anthropic/claude-3-5-sonnet", weight: 0 },
|
||||
];
|
||||
const next = builderDraft.computeBatchDeselectModelSteps(models, [
|
||||
{ value: "openai/gpt-4o" },
|
||||
{ value: "openai/gpt-4o-mini" },
|
||||
]);
|
||||
assert.deepEqual(next, [{ model: "anthropic/claude-3-5-sonnet", weight: 0 }]);
|
||||
});
|
||||
|
||||
test("computeBatchDeselectModelSteps accepts raw string identifiers", () => {
|
||||
const models = [
|
||||
{ model: "openai/gpt-4o", weight: 0 },
|
||||
{ model: "openai/gpt-4o-mini", weight: 0 },
|
||||
];
|
||||
const next = builderDraft.computeBatchDeselectModelSteps(models, ["openai/gpt-4o"]);
|
||||
assert.deepEqual(next, [{ model: "openai/gpt-4o-mini", weight: 0 }]);
|
||||
});
|
||||
|
||||
test("computeBatchDeselectModelSteps is a no-op (same reference) when there is nothing to remove", () => {
|
||||
const models = [{ model: "openai/gpt-4o", weight: 0 }];
|
||||
assert.equal(builderDraft.computeBatchDeselectModelSteps(models, []), models);
|
||||
assert.equal(builderDraft.computeBatchDeselectModelSteps(models, [{ value: "" }]), models);
|
||||
});
|
||||
33
tests/unit/model-select-all-confirm-threshold-8526.test.ts
Normal file
33
tests/unit/model-select-all-confirm-threshold-8526.test.ts
Normal file
@@ -0,0 +1,33 @@
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import {
|
||||
SELECT_ALL_CONFIRM_THRESHOLD,
|
||||
shouldConfirmSelectAll,
|
||||
} from "../../src/shared/components/modelSelectModalHelpers.ts";
|
||||
|
||||
// Regression guard for #8526: ModelSelectModal's "Select all" (Browse
|
||||
// Catalog, combo builder) had no cap — turning off "Show configured only",
|
||||
// or just having a large provider catalog, could add hundreds of models to a
|
||||
// combo in one click. Above SELECT_ALL_CONFIRM_THRESHOLD the caller must
|
||||
// confirm before batch-adding.
|
||||
|
||||
test("shouldConfirmSelectAll: does not require confirmation at or below the threshold", () => {
|
||||
assert.equal(shouldConfirmSelectAll(0), false);
|
||||
assert.equal(shouldConfirmSelectAll(1), false);
|
||||
assert.equal(shouldConfirmSelectAll(SELECT_ALL_CONFIRM_THRESHOLD), false);
|
||||
});
|
||||
|
||||
test("shouldConfirmSelectAll: requires confirmation above the threshold", () => {
|
||||
assert.equal(shouldConfirmSelectAll(SELECT_ALL_CONFIRM_THRESHOLD + 1), true);
|
||||
assert.equal(shouldConfirmSelectAll(500), true);
|
||||
});
|
||||
|
||||
test("shouldConfirmSelectAll: respects a caller-supplied threshold override", () => {
|
||||
assert.equal(shouldConfirmSelectAll(5, 10), false);
|
||||
assert.equal(shouldConfirmSelectAll(11, 10), true);
|
||||
});
|
||||
|
||||
test("shouldConfirmSelectAll: tolerates non-finite counts without throwing", () => {
|
||||
assert.equal(shouldConfirmSelectAll(Number.NaN), false);
|
||||
assert.equal(shouldConfirmSelectAll(Number.POSITIVE_INFINITY), false);
|
||||
});
|
||||
89
tests/unit/ui/combo-form-select-all-handlers.test.tsx
Normal file
89
tests/unit/ui/combo-form-select-all-handlers.test.tsx
Normal file
@@ -0,0 +1,89 @@
|
||||
// Exercises the REAL Select all / Unselect all batch handlers ComboFormModal
|
||||
// (src/app/(dashboard)/dashboard/combos/page.tsx::handleAddModels /
|
||||
// handleDeselectModels) delegates to. Previously this test re-implemented the
|
||||
// batch logic by hand as a parallel copy — that copy could stay green while
|
||||
// the real component logic broke (#8526). The regression guard that actually
|
||||
// blocks CI is the node:test copy of this file at
|
||||
// tests/unit/combo-select-all-batch-8526.test.ts (this file's location,
|
||||
// tests/unit/ui/*.test.tsx, is only collected by the advisory
|
||||
// `test:vitest:ui` — see CONTRIBUTING.md "Both test runners must pass").
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
computeBatchAddModelSteps,
|
||||
computeBatchDeselectModelSteps,
|
||||
} from "@/lib/combos/builderDraft";
|
||||
|
||||
describe("ComboFormModal Select all / Unselect all handlers (real implementation)", () => {
|
||||
it("adds every candidate in one pass (no stale single-add overwrite)", () => {
|
||||
const { next, addedAny } = computeBatchAddModelSteps(
|
||||
[],
|
||||
[
|
||||
{ value: "openai/gpt-4o" },
|
||||
{ value: "openai/gpt-4o-mini" },
|
||||
{ value: "anthropic/claude-3-5-sonnet" },
|
||||
],
|
||||
[]
|
||||
);
|
||||
expect(addedAny).toBe(true);
|
||||
expect(next.map((m) => m.model)).toEqual([
|
||||
"openai/gpt-4o",
|
||||
"openai/gpt-4o-mini",
|
||||
"anthropic/claude-3-5-sonnet",
|
||||
]);
|
||||
});
|
||||
|
||||
it("skips models already present (exact duplicate)", () => {
|
||||
const existing = [{ model: "openai/gpt-4o", providerId: "openai", weight: 0 }];
|
||||
const { next } = computeBatchAddModelSteps(
|
||||
existing,
|
||||
[{ value: "openai/gpt-4o", providerId: "openai" }, { value: "openai/gpt-4o-mini" }],
|
||||
[]
|
||||
);
|
||||
expect(next.map((m) => m.model)).toEqual(["openai/gpt-4o", "openai/gpt-4o-mini"]);
|
||||
});
|
||||
|
||||
it("ignores empty / missing values", () => {
|
||||
const { next, addedAny } = computeBatchAddModelSteps(
|
||||
[],
|
||||
[{ value: "" }, {}, { value: "openai/gpt-4o" }],
|
||||
[]
|
||||
);
|
||||
expect(addedAny).toBe(true);
|
||||
expect(next).toEqual([{ model: "openai/gpt-4o", providerId: "openai", weight: 0 }]);
|
||||
});
|
||||
|
||||
it("is a no-op when nothing is selected", () => {
|
||||
const existing = [{ model: "openai/gpt-4o", weight: 0 }];
|
||||
const { next, addedAny } = computeBatchAddModelSteps(existing, [], []);
|
||||
expect(addedAny).toBe(false);
|
||||
expect(next).toBe(existing);
|
||||
});
|
||||
|
||||
it("removes every matching qualified model in one pass", () => {
|
||||
const models = [
|
||||
{ model: "openai/gpt-4o", weight: 0 },
|
||||
{ model: "openai/gpt-4o-mini", weight: 0 },
|
||||
{ model: "anthropic/claude-3-5-sonnet", weight: 0 },
|
||||
];
|
||||
const next = computeBatchDeselectModelSteps(models, [
|
||||
{ value: "openai/gpt-4o" },
|
||||
{ value: "openai/gpt-4o-mini" },
|
||||
]);
|
||||
expect(next).toEqual([{ model: "anthropic/claude-3-5-sonnet", weight: 0 }]);
|
||||
});
|
||||
|
||||
it("accepts raw string identifiers in the deselect batch", () => {
|
||||
const models = [
|
||||
{ model: "openai/gpt-4o", weight: 0 },
|
||||
{ model: "openai/gpt-4o-mini", weight: 0 },
|
||||
];
|
||||
const next = computeBatchDeselectModelSteps(models, ["openai/gpt-4o"]);
|
||||
expect(next).toEqual([{ model: "openai/gpt-4o-mini", weight: 0 }]);
|
||||
});
|
||||
|
||||
it("is a no-op (same reference) when nothing to remove", () => {
|
||||
const models = [{ model: "openai/gpt-4o", weight: 0 }];
|
||||
expect(computeBatchDeselectModelSteps(models, [])).toBe(models);
|
||||
expect(computeBatchDeselectModelSteps(models, [{ value: "" }])).toBe(models);
|
||||
});
|
||||
});
|
||||
231
tests/unit/ui/model-select-modal-select-all.test.tsx
Normal file
231
tests/unit/ui/model-select-modal-select-all.test.tsx
Normal file
@@ -0,0 +1,231 @@
|
||||
// @vitest-environment jsdom
|
||||
//
|
||||
// Select all / Unselect all for currently visible models in ModelSelectModal.
|
||||
// Used by combo Create/Edit "Browse Catalog" (keepOpenOnSelect + batch callbacks).
|
||||
import React from "react";
|
||||
import { act } from "react";
|
||||
import { createRoot } from "react-dom/client";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
vi.mock("next-intl", () => ({
|
||||
useTranslations: () => {
|
||||
const t = (key: string) => key;
|
||||
t.has = () => false;
|
||||
return t;
|
||||
},
|
||||
}));
|
||||
|
||||
const { default: ModelSelectModal } = await import("@/shared/components/ModelSelectModal");
|
||||
|
||||
const containers: HTMLElement[] = [];
|
||||
|
||||
async function renderModal(props: Partial<React.ComponentProps<typeof ModelSelectModal>> = {}) {
|
||||
const container = document.createElement("div");
|
||||
document.body.appendChild(container);
|
||||
containers.push(container);
|
||||
|
||||
const root = createRoot(container);
|
||||
await act(async () => {
|
||||
root.render(
|
||||
<ModelSelectModal
|
||||
isOpen={true}
|
||||
onClose={() => {}}
|
||||
onSelect={() => {}}
|
||||
showCombos={false}
|
||||
activeProviders={[{ provider: "openai" }]}
|
||||
keepOpenOnSelect
|
||||
{...props}
|
||||
/>
|
||||
);
|
||||
});
|
||||
|
||||
await act(async () => {});
|
||||
return { container, root };
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
(
|
||||
globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean }
|
||||
).IS_REACT_ACT_ENVIRONMENT = true;
|
||||
// These tests exercise selection bookkeeping, not the confirm-above-threshold
|
||||
// UX (see modelSelectModalHelpers.ts::shouldConfirmSelectAll) — stub confirm
|
||||
// to always accept so assertions don't depend on how many models the fixture
|
||||
// catalog happens to expose.
|
||||
vi.stubGlobal("confirm", vi.fn(() => true));
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async (url: string) => {
|
||||
if (url === "/api/provider-nodes") {
|
||||
return { ok: true, json: async () => ({ nodes: [] }) };
|
||||
}
|
||||
if (url === "/api/provider-models") {
|
||||
return { ok: true, json: async () => ({ models: {} }) };
|
||||
}
|
||||
if (url === "/api/combos") {
|
||||
return { ok: true, json: async () => ({ combos: [] }) };
|
||||
}
|
||||
return { ok: true, json: async () => ({}) };
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
while (containers.length > 0) {
|
||||
containers.pop()?.remove();
|
||||
}
|
||||
document.body.innerHTML = "";
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
describe("ModelSelectModal — Select all / Unselect all visible models", () => {
|
||||
it("does not render the toggle without batch callbacks", async () => {
|
||||
const { container } = await renderModal();
|
||||
expect(container.querySelector('[data-testid="model-select-toggle-all-visible"]')).toBeNull();
|
||||
});
|
||||
|
||||
it("renders Select all when keepOpenOnSelect + batch callbacks are set", async () => {
|
||||
const { container } = await renderModal({
|
||||
onSelectMany: vi.fn(),
|
||||
onDeselectMany: vi.fn(),
|
||||
addedModelValues: [],
|
||||
});
|
||||
|
||||
const toggle = container.querySelector(
|
||||
'[data-testid="model-select-toggle-all-visible"]'
|
||||
) as HTMLButtonElement | null;
|
||||
expect(toggle, "expected Select all toggle").not.toBeNull();
|
||||
expect(toggle!.textContent).toMatch(/Select all/i);
|
||||
});
|
||||
|
||||
it("calls onSelectMany with every visible model that is not yet added", async () => {
|
||||
const onSelectMany = vi.fn();
|
||||
const onDeselectMany = vi.fn();
|
||||
|
||||
const { container } = await renderModal({
|
||||
onSelectMany,
|
||||
onDeselectMany,
|
||||
addedModelValues: [],
|
||||
});
|
||||
|
||||
const toggle = container.querySelector(
|
||||
'[data-testid="model-select-toggle-all-visible"]'
|
||||
) as HTMLButtonElement;
|
||||
expect(toggle).not.toBeNull();
|
||||
|
||||
await act(async () => {
|
||||
toggle.click();
|
||||
});
|
||||
|
||||
expect(onSelectMany).toHaveBeenCalledTimes(1);
|
||||
expect(onDeselectMany).not.toHaveBeenCalled();
|
||||
const selected = onSelectMany.mock.calls[0][0] as Array<{ value: string }>;
|
||||
expect(Array.isArray(selected)).toBe(true);
|
||||
expect(selected.length).toBeGreaterThan(1);
|
||||
expect(selected.every((m) => typeof m.value === "string" && m.value.length > 0)).toBe(true);
|
||||
});
|
||||
|
||||
it("shows Unselect all and calls onDeselectMany when every visible model is already added", async () => {
|
||||
const probeSelect = vi.fn();
|
||||
const probe = await renderModal({
|
||||
onSelectMany: probeSelect,
|
||||
onDeselectMany: vi.fn(),
|
||||
addedModelValues: [],
|
||||
});
|
||||
const probeToggle = probe.container.querySelector(
|
||||
'[data-testid="model-select-toggle-all-visible"]'
|
||||
) as HTMLButtonElement;
|
||||
await act(async () => {
|
||||
probeToggle.click();
|
||||
});
|
||||
const allValues = (probeSelect.mock.calls[0][0] as Array<{ value: string }>).map(
|
||||
(m) => m.value
|
||||
);
|
||||
expect(allValues.length).toBeGreaterThan(1);
|
||||
|
||||
const onSelectMany = vi.fn();
|
||||
const onDeselectMany = vi.fn();
|
||||
const { container } = await renderModal({
|
||||
onSelectMany,
|
||||
onDeselectMany,
|
||||
addedModelValues: allValues,
|
||||
});
|
||||
|
||||
const toggle = container.querySelector(
|
||||
'[data-testid="model-select-toggle-all-visible"]'
|
||||
) as HTMLButtonElement;
|
||||
expect(toggle.textContent).toMatch(/Unselect all/i);
|
||||
|
||||
await act(async () => {
|
||||
toggle.click();
|
||||
});
|
||||
|
||||
expect(onDeselectMany).toHaveBeenCalledTimes(1);
|
||||
expect(onSelectMany).not.toHaveBeenCalled();
|
||||
const removed = onDeselectMany.mock.calls[0][0] as Array<{ value: string }>;
|
||||
expect(removed.map((m) => m.value).sort()).toEqual([...allValues].sort());
|
||||
});
|
||||
|
||||
it("Select all only adds models that are not already in addedModelValues", async () => {
|
||||
const probeSelect = vi.fn();
|
||||
const probe = await renderModal({
|
||||
onSelectMany: probeSelect,
|
||||
onDeselectMany: vi.fn(),
|
||||
addedModelValues: [],
|
||||
});
|
||||
const probeToggle = probe.container.querySelector(
|
||||
'[data-testid="model-select-toggle-all-visible"]'
|
||||
) as HTMLButtonElement;
|
||||
await act(async () => {
|
||||
probeToggle.click();
|
||||
});
|
||||
const allValues = (probeSelect.mock.calls[0][0] as Array<{ value: string }>).map(
|
||||
(m) => m.value
|
||||
);
|
||||
const alreadyAdded = [allValues[0]];
|
||||
|
||||
const onSelectMany = vi.fn();
|
||||
const { container } = await renderModal({
|
||||
onSelectMany,
|
||||
onDeselectMany: vi.fn(),
|
||||
addedModelValues: alreadyAdded,
|
||||
});
|
||||
|
||||
const toggle = container.querySelector(
|
||||
'[data-testid="model-select-toggle-all-visible"]'
|
||||
) as HTMLButtonElement;
|
||||
await act(async () => {
|
||||
toggle.click();
|
||||
});
|
||||
|
||||
const selected = onSelectMany.mock.calls[0][0] as Array<{ value: string }>;
|
||||
expect(selected.some((m) => m.value === alreadyAdded[0])).toBe(false);
|
||||
expect(selected.length).toBe(allValues.length - 1);
|
||||
});
|
||||
|
||||
it("asks for confirmation before Select all above the threshold, and honors decline", async () => {
|
||||
const confirmSpy = vi.fn(() => false);
|
||||
vi.stubGlobal("confirm", confirmSpy);
|
||||
|
||||
const onSelectMany = vi.fn();
|
||||
// openai (20 models) + anthropic (10 models) = 30 candidates, comfortably
|
||||
// above SELECT_ALL_CONFIRM_THRESHOLD (20) — see modelSelectModalHelpers.ts.
|
||||
const { container } = await renderModal({
|
||||
activeProviders: [{ provider: "openai" }, { provider: "anthropic" }],
|
||||
onSelectMany,
|
||||
onDeselectMany: vi.fn(),
|
||||
addedModelValues: [],
|
||||
});
|
||||
|
||||
const toggle = container.querySelector(
|
||||
'[data-testid="model-select-toggle-all-visible"]'
|
||||
) as HTMLButtonElement;
|
||||
|
||||
await act(async () => {
|
||||
toggle.click();
|
||||
});
|
||||
|
||||
expect(confirmSpy).toHaveBeenCalledTimes(1);
|
||||
// Declining the confirmation must abort the batch add entirely.
|
||||
expect(onSelectMany).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user