Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions changelog/+common-parent-relationship-filter.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed relationship selectors in object forms not honoring the `common_parent` schema property. The options are now filtered to peers that share the same parent as the value picked for the referenced relationship in the same form, instead of listing every peer. Changing that parent clears a now-invalid selection, and the inline "Add new" form pre-fills the parent when one is already selected so a created peer stays valid.
18 changes: 9 additions & 9 deletions frontend/app/.betterer.results

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,20 @@ import { useState } from "react";
import SlideOver, { SlideOverTitle } from "@/shared/components/display/slide-over";
import ObjectForm, { type ObjectFormProps } from "@/shared/components/form/object-form";

import type { NodeFieldsWithMetadata } from "@/entities/nodes/types";
import { useSchema } from "@/entities/schema/ui/hooks/useSchema";

export interface AddRelationshipActionProps {
peer: string;
onSuccess?: ObjectFormProps["onSuccess"];
// Pre-fills the create form (e.g. the common_parent so a created peer satisfies the constraint).
initialObject?: NodeFieldsWithMetadata;
}

export const AddRelationshipAction: React.FC<AddRelationshipActionProps> = ({
peer,
onSuccess,
initialObject,
}) => {
const { schema } = useSchema(peer);
const [open, setOpen] = useState(false);
Expand Down Expand Up @@ -45,6 +49,7 @@ export const AddRelationshipAction: React.FC<AddRelationshipActionProps> = ({
>
<ObjectForm
kind={peer}
currentObject={initialObject}
onSuccess={async (newNode) => {
setOpen(false);
if (!onSuccess) return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,34 @@ describe("RelationshipComboboxList", () => {
const lastCall = useRelationshipsMock.mock.calls.at(-1)?.[0];
expect(lastCall.filterQuery).toEqual({ ids: ["17a4cdef-1234-4abc-8def-0123456789ab"] });
});

test("keeps the caller filterQuery on a UUID search when enforceFilterQueryOnIdSearch is set", async () => {
useRelationshipsMock.mockReturnValue(setupReturn());
useSchemaMock.mockReturnValue({ schema: { label: "Device" } });

const component = await render(
<RelationshipComboboxList
peer="InfraDevice"
onSelect={vi.fn()}
filterQuery={{ device__ids: ["dev-1"] }}
enforceFilterQueryOnIdSearch
/>
);

// Clear previous calls from initial render
useRelationshipsMock.mockClear();

const input = component.getByRole("combobox");
await input.click();
await input.fill("17a4cdef-1234-4abc-8def-0123456789ab");

await new Promise((resolve) => setTimeout(resolve, 350));

// The id restriction and the enforced parent filter are both applied.
const lastCall = useRelationshipsMock.mock.calls.at(-1)?.[0];
expect(lastCall.filterQuery).toEqual({
device__ids: ["dev-1"],
ids: ["17a4cdef-1234-4abc-8def-0123456789ab"],
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ export interface RelationshipComboboxListProps
value?: RelationshipNode | null;
filterItem?: (relationshipNode: RelationshipNode) => boolean;
filterQuery?: Record<string, string | number | boolean | string[]>;
// Keep filterQuery applied even on a UUID search. Used when the filter is a hard constraint
// (e.g. common_parent) that a UUID lookup must not bypass.
enforceFilterQueryOnIdSearch?: boolean;
}

export const RelationshipComboboxList = ({
Expand All @@ -33,19 +36,23 @@ export const RelationshipComboboxList = ({
onSelect,
filterItem,
filterQuery,
enforceFilterQueryOnIdSearch,
...props
}: RelationshipComboboxListProps) => {
const [search, setSearch] = React.useState("");
const { schema } = useSchema(peer);
// When the user types or pastes a UUID, switch the underlying query from a
// label search to an ids filter. UUID is a maximally specific match, so it
// intentionally overrides any caller-provided filterQuery.
// When the user types or pastes a UUID, switch the underlying query from a label search to an
// ids filter. UUID is a maximally specific match, so it overrides a caller-provided filterQuery
// by default — unless enforceFilterQueryOnIdSearch keeps the filter as a hard constraint.
const isUuidSearch = search.length > 0 && isUuid(search);
const idSearchFilterQuery = enforceFilterQueryOnIdSearch
? { ...filterQuery, ids: [search.trim()] }
: { ids: [search.trim()] };
const { isPending, data, error, fetchNextPage, hasNextPage, isFetchingNextPage } =
useRelationships({
peer,
search: isUuidSearch ? undefined : search,
filterQuery: isUuidSearch ? { ids: [search.trim()] } : filterQuery,
filterQuery: isUuidSearch ? idSearchFilterQuery : filterQuery,
});

if (error) return <ErrorScreen message={error.message} />;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,30 @@ import {
type RelationshipComboboxListProps,
} from "@/entities/nodes/relationships/ui/relationship-combobox-list";
import { RelationshipHierarchicalComboboxList } from "@/entities/nodes/relationships/ui/relationship-hierarchical-combobox-list";

export interface RelationshipHierarchicalContentProps extends RelationshipComboboxListProps {}
import type { NodeFieldsWithMetadata } from "@/entities/nodes/types";

export interface RelationshipHierarchicalContentProps extends RelationshipComboboxListProps {
// The tree explorer browses the peer's own hierarchy and cannot honor an external filterQuery,
// so it is dropped when a filter must be enforced (e.g. common_parent).
hideExplore?: boolean;
// Pre-fills the "Add new" create form so a created peer satisfies an enforced filter.
addNewInitialObject?: NodeFieldsWithMetadata;
}

export const RelationshipHierarchicalContent = ({
hideExplore,
addNewInitialObject,
...props
}: RelationshipHierarchicalContentProps) => {
if (hideExplore) {
return (
<ComboboxContent>
<RelationshipComboboxList {...props} />
<AddRelationshipAction {...props} initialObject={addNewInitialObject} />
</ComboboxContent>
);
}

return (
<ComboboxContent>
<PopoverTabs defaultValue="list">
Expand All @@ -44,7 +62,7 @@ export const RelationshipHierarchicalContent = ({

<PopoverTabsContent value="list">
<RelationshipComboboxList {...props} />
<AddRelationshipAction {...props} />
<AddRelationshipAction {...props} initialObject={addNewInitialObject} />
</PopoverTabsContent>

<PopoverTabsContent value="tree">
Expand All @@ -61,13 +79,21 @@ export interface RelationshipHierarchicalInputProps
onChange?: (value: RelationshipNode | null) => void;
value?: RelationshipNode | null;
peer: string;
filterQuery?: Record<string, string | number | boolean | string[]>;
hideExplore?: boolean;
addNewInitialObject?: NodeFieldsWithMetadata;
enforceFilterQueryOnIdSearch?: boolean;
}

export const RelationshipHierarchicalInput = ({
ref,
value,
onChange,
peer,
filterQuery,
hideExplore,
addNewInitialObject,
enforceFilterQueryOnIdSearch,
...props
}: RelationshipHierarchicalInputProps) => {
const [open, setOpen] = React.useState(false);
Expand All @@ -83,7 +109,15 @@ export const RelationshipHierarchicalInput = ({
{value ? getNodeLabel(value) : ""}
</ComboboxTrigger>

<RelationshipHierarchicalContent peer={peer} onSelect={handleSelect} value={value} />
<RelationshipHierarchicalContent
peer={peer}
onSelect={handleSelect}
value={value}
filterQuery={filterQuery}
hideExplore={hideExplore}
addNewInitialObject={addNewInitialObject}
enforceFilterQueryOnIdSearch={enforceFilterQueryOnIdSearch}
/>
</Combobox>
);
};
Expand All @@ -94,6 +128,10 @@ export interface RelationshipHierarchicalManyInputProps
onChange: (value: RelationshipNode[]) => void;
value?: RelationshipNode[] | null;
peer: string;
filterQuery?: Record<string, string | number | boolean | string[]>;
hideExplore?: boolean;
addNewInitialObject?: NodeFieldsWithMetadata;
enforceFilterQueryOnIdSearch?: boolean;
}

export const RelationshipHierarchicalManyInput = ({
Expand All @@ -102,6 +140,10 @@ export const RelationshipHierarchicalManyInput = ({
onChange,
peer,
className,
filterQuery,
hideExplore,
addNewInitialObject,
enforceFilterQueryOnIdSearch,
...props
}: RelationshipHierarchicalManyInputProps) => {
const [open, setOpen] = React.useState(false);
Expand Down Expand Up @@ -159,6 +201,10 @@ export const RelationshipHierarchicalManyInput = ({
peer={peer}
onSelect={handleSelect}
filterItem={(node) => !value?.some((v) => v.id === node.id)}
filterQuery={filterQuery}
hideExplore={hideExplore}
addNewInitialObject={addNewInitialObject}
enforceFilterQueryOnIdSearch={enforceFilterQueryOnIdSearch}
/>
</Combobox>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
import { afterEach, describe, expect, test, vi } from "vitest";

import { TestForm } from "../../../../../tests/components/form.story";
import { render } from "../../../../../tests/components/render";
import { generateRelationshipSchema } from "../../../../../tests/fake/schema";
import RelationshipManyField from "./relationships/relationship-many.field";

// Capture the props handed to the input so we can assert the common_parent wiring
// without driving the whole combobox/query stack.
let lastFilterQuery: unknown;
let lastAddNewInitialObject: unknown;
let lastEnforceOnIdSearch: unknown;
vi.mock("@/shared/components/inputs/relationship-many", () => ({
RelationshipManyInput: (props: {
filterQuery?: unknown;
addNewInitialObject?: unknown;
enforceFilterQueryOnIdSearch?: unknown;
}) => {
lastFilterQuery = props.filterQuery;
lastAddNewInitialObject = props.addNewInitialObject;
lastEnforceOnIdSearch = props.enforceFilterQueryOnIdSearch;
return <input data-testid="many-input" />;
},
}));

describe("RelationshipManyField - common_parent filtering", () => {
afterEach(() => {
lastFilterQuery = undefined;
lastAddNewInitialObject = undefined;
lastEnforceOnIdSearch = undefined;
vi.clearAllMocks();
});

test("filters the peer options by the common_parent chosen in a sibling field", async () => {
// GIVEN a relationship declaring common_parent: device, with the sibling device picked
const relationship = generateRelationshipSchema({
name: "profile_one",
peer: "TestProfile",
common_parent: "device",
});
const deviceValue = {
source: { type: "user" as const },
value: { id: "dev-1", display_label: "dc1-device", __typename: "TestDevice" },
};

// WHEN the field renders with that sibling value seeded into the form
await render(
<TestForm defaultValues={{ device: deviceValue }}>
<RelationshipManyField
type="relationship"
name="profile_one"
label="Profile One"
peer="TestProfile"
relationship={relationship}
/>
</TestForm>
);

// THEN the input receives a single-hop filter on the chosen parent; the UUID-search override
// is closed, and "Add new" is pre-filled with that parent so a created peer stays valid.
await expect.poll(() => lastFilterQuery).toEqual({ device__ids: ["dev-1"] });
expect(lastEnforceOnIdSearch).toBe(true);
expect(lastAddNewInitialObject).toEqual({
device: { node: { id: "dev-1", display_label: "dc1-device", __typename: "TestDevice" } },
});
});

test("passes no filter when the sibling common_parent field is empty", async () => {
const relationship = generateRelationshipSchema({
name: "profile_one",
peer: "TestProfile",
common_parent: "device",
});

await render(
<TestForm>
<RelationshipManyField
type="relationship"
name="profile_one"
label="Profile One"
peer="TestProfile"
relationship={relationship}
/>
</TestForm>
);

await expect.poll(() => lastFilterQuery).toBeUndefined();
});

test("passes no filter when the schema declares no common_parent", async () => {
const relationship = generateRelationshipSchema({ name: "tags", peer: "TestProfile" });

await render(
<TestForm>
<RelationshipManyField
type="relationship"
name="tags"
label="Tags"
peer="TestProfile"
relationship={relationship}
/>
</TestForm>
);

await expect.poll(() => lastFilterQuery).toBeUndefined();
});
});
Loading
Loading