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
129 changes: 129 additions & 0 deletions src/frontend/src/components/portal/portal-topbar.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
// @vitest-environment jsdom
/**
* The seam between the catalog and the control.
*
* `cohortOptions` is tested on its own and so is `SliceSelect`; what neither
* covers is that the bar actually asks the catalog and hands the answer down.
* This test therefore runs the real query hook against a real QueryClient and
* stubs only the network — so a wiring mistake here fails rather than hiding
* behind a stubbed hook.
*/
vi.mock("@tanstack/react-router", async () => {
const { portalRouterMock } = await import("@/test/portal-router");
return portalRouterMock();
});

import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import { SidebarProvider } from "@/components/ui/sidebar";
import { render, screen, waitFor } from "@testing-library/react";
import { beforeEach, describe, expect, it, vi } from "vitest";

import type { MetricDefinitionListResponse } from "@/api/metric-definitions-client";
import { identityPerson, pid } from "@/test/identity";

const mocks = vi.hoisted(() => ({
definitions: { metrics: [] } as MetricDefinitionListResponse,
}));

// Only the network is stubbed; the query hook itself is the real one.
vi.mock("@/api/metric-definitions-client", () => ({
listMetricDefinitions: () => Promise.resolve(mocks.definitions),
}));
vi.mock("@/queries/ic-dashboard", () => ({
// A viewer with two reports who differ by division: enough for the roster
// walk to offer something, so a catalog winning is a real preference and
// not the absence of an alternative.
useIcPerson: () => ({
data: identityPerson("boss", {
person_id: pid("boss"),
division: "Alpha",
subordinates: [
identityPerson("one", { person_id: pid("one"), division: "Alpha" }),
identityPerson("two", { person_id: pid("two"), division: "Beta" }),
],
}),
}),
}));
vi.mock("@/auth", () => ({ useViewer: () => ({ personId: pid("boss") }) }));
vi.mock("@/hooks/use-portal-period", () => ({
usePortalPeriod: () => ({
period: "month",
customRange: null,
setPeriod: vi.fn(),
setCustomRange: vi.fn(),
}),
}));
vi.mock("@/components/portal/scope-select", () => ({
ScopeSelect: () => <div />,
}));
vi.mock("@/components/widgets/period-selector-bar", () => ({
PeriodSelectorBar: () => <div />,
}));
// Stands in for the control so the test can read what it was given.
vi.mock("@/components/portal/slice-select", () => ({
SliceSelect: ({ dims }: { dims: { key: string; label: string }[] }) => (
<div data-testid="dims">{dims.map((d) => d.key).join(",")}</div>
),
}));

import { PortalTopBar } from "./portal-topbar";

function bar() {
const client = new QueryClient({
defaultOptions: { queries: { retry: false } },
});
return render(
<QueryClientProvider client={client}>
<SidebarProvider>
<PortalTopBar />
</SidebarProvider>
</QueryClientProvider>,
);
}

beforeEach(() => {
window.matchMedia ??= ((query: string) => ({
matches: false,
media: query,
onchange: null,
addEventListener: () => {},
removeEventListener: () => {},
addListener: () => {},
removeListener: () => {},
dispatchEvent: () => false,
})) as unknown as typeof window.matchMedia;
mocks.definitions = { metrics: [] };
});

describe("PortalTopBar", () => {
it("offers what the catalog allows once the response carries it", async () => {
mocks.definitions = {
metrics: [],
comparison_attributes: [{ id: "job_title", label: "Title" }],
} as MetricDefinitionListResponse;
bar();
await waitFor(() =>
expect(screen.getByTestId("dims")).toHaveTextContent("job_title"),
);
});

it("prefers the catalog over what the roster happens to support", async () => {
// The roster here would offer `division`; a governed answer wins, and the
// two are never shown side by side.
mocks.definitions = {
metrics: [],
comparison_attributes: [{ id: "job_title", label: "Title" }],
} as MetricDefinitionListResponse;
bar();
await waitFor(() =>
expect(screen.getByTestId("dims")).not.toHaveTextContent("division"),
);
});

it("falls back to the roster while no catalog exists", async () => {
bar();
await waitFor(() =>
expect(screen.getByTestId("dims")).toHaveTextContent("division"),
);
});
});
17 changes: 5 additions & 12 deletions src/frontend/src/components/portal/portal-topbar.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import { HelpCircle } from "lucide-react";
import { useMemo } from "react";

import { useViewer } from "@/auth";
import { ScopeSelect } from "@/components/portal/scope-select";
import { SliceSelect } from "@/components/portal/slice-select";
import { SidebarTrigger } from "@/components/ui/sidebar";
Expand All @@ -13,11 +11,8 @@ import {
} from "@/components/ui/tooltip";
import { PeriodSelectorBar } from "@/components/widgets/period-selector-bar";
import { usePortalPeriod } from "@/hooks/use-portal-period";
import { availableSlices } from "@/lib/insight/slices";
import { collectRosterAttrs } from "@/lib/insight/slices";
import { normalizePersonId } from "@/lib/metrics/entity";
import { useActiveZone } from "@/lib/portal/use-active-zone";
import { useIcPerson } from "@/queries/ic-dashboard";
import { useCohortOptions } from "@/lib/portal/use-cohort-options";

/**
* Global portal bar — the two cross-cutting controls live here so every zone
Expand All @@ -28,18 +23,16 @@ import { useIcPerson } from "@/queries/ic-dashboard";
*/
export function PortalTopBar() {
const { period, customRange, setPeriod, setCustomRange } = usePortalPeriod();
const { personId } = useViewer();
// The Person zone is about ONE person, and it reads nothing from the org
// scope. Leaving the control up meant the bar named a different person than
// the page — a reader saw two names and had to work out which one the
// numbers belonged to.
const { activeZone } = useActiveZone();
const scoped = activeZone !== "person";
const tree = useIcPerson(personId ?? "").data ?? null;
const dims = useMemo(
() => availableSlices(collectRosterAttrs(tree, normalizePersonId).values()),
[tree],
);
// The server's catalog when it has one, the viewer's roster until then —
// decided in `useCohortOptions`, which also owns which selection is in
// effect so this control and the comparison cannot disagree.
const { dims } = useCohortOptions();

return (
// Sticky to the scroll container (`SidebarInset` owns the overflow): scope,
Expand Down
5 changes: 5 additions & 0 deletions src/frontend/src/components/portal/shell-layout.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ vi.mock("@/components/widgets/period-selector-bar", () => ({
vi.mock("@/queries/ic-dashboard", () => ({
useIcPerson: () => ({ data: null }),
}));
// The topbar asks the catalog which attributes a comparison may use; this
// test is about the shell's layout, not about that answer.
vi.mock("@/queries/metric-definitions", () => ({
useMetricDefinitionsResponse: () => ({ data: undefined }),
}));
vi.mock("@/hooks/use-portal-period", () => ({
usePortalPeriod: () => ({
period: "month",
Expand Down
40 changes: 40 additions & 0 deletions src/frontend/src/components/portal/slice-select.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// @vitest-environment jsdom
/**
* What the control says when there is nothing to choose.
*
* Slices are discovered by enumerating people and grouping them by one of
* their attributes, and identity serves a viewer only their own subtree — so a
* viewer with no reports has a roster of one person and no attribute with a
* second value. Comparisons still happen (the peer view compares within the
* organization unit, server-side); the choice is what is missing, and a
* control offering exactly one option claims the reader chose it.
*/
import { render, screen } from "@testing-library/react";
import { describe, expect, it, vi } from "vitest";

vi.mock("@/lib/portal/portal-nav", () => ({
usePortalNavActions: () => ({ setSlice: vi.fn() }),
usePortalSlice: () => "",
}));

import { SliceSelect } from "./slice-select";

describe("SliceSelect", () => {
it("does not offer a choice when the roster supports none", () => {
render(<SliceSelect dims={[]} />);
const trigger = screen.getByLabelText("Cohort");
expect(trigger).toBeDisabled();
// And says why, rather than leaving a dead control.
expect(trigger).toHaveAttribute(
"title",
expect.stringContaining("organization unit"),
);
});

it("is a live control as soon as the roster supports one dimension", () => {
render(<SliceSelect dims={[{ key: "division", label: "Division" }]} />);
const trigger = screen.getByLabelText("Cohort");
expect(trigger).not.toBeDisabled();
expect(trigger).not.toHaveAttribute("title");
});
});
25 changes: 20 additions & 5 deletions src/frontend/src/components/portal/slice-select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,8 @@ import {
} from "@/components/ui/select";
import type { SliceDim } from "@/lib/insight/slices";
import { PLANNED_SLICES } from "@/lib/insight/slices";
import {
usePortalNavActions,
usePortalSlice,
} from "@/lib/portal/portal-nav";
import { NO_COHORT_REASON } from "@/lib/portal/cohort-options";
import { usePortalNavActions, usePortalSlice } from "@/lib/portal/portal-nav";

/**
* "No slice" — the whole roster is one cohort and views stay per-person. The
Expand All @@ -35,6 +33,17 @@ export function SliceSelect({ dims }: { dims: SliceDim[] }) {
const { setSlice } = usePortalNavActions();
const slice = usePortalSlice();
const all = [TEAM_SLICE, ...dims, ...PLANNED_SLICES];
// Slices are discovered by enumerating people and grouping them by an
// attribute, so a viewer whose roster holds only themselves has nothing to
// group: identity serves a viewer their own subtree, and an individual
// contributor's subtree is one person. Their attributes are all there —
// there is simply no second value for any of them.
//
// The comparisons on screen still happen: the peer view compares within the
// person's organization unit, decided server-side. What is missing is the
// CHOICE, and a control offering exactly one option states the opposite —
// it reads as a setting the reader picked, not as the only thing available.
const hasChoice = dims.length + PLANNED_SLICES.length > 0;
const current = slice || TEAM_KEY;
const value = all.some((d) => d.key === current) ? current : TEAM_KEY;
const label = all.find((d) => d.key === value)?.label ?? "Team (all)";
Expand All @@ -46,7 +55,13 @@ export function SliceSelect({ dims }: { dims: SliceDim[] }) {
{/* The trigger carries the VALUE only; the word "Cohort" labels the
control from outside, next to the tooltip that explains it. Inside,
it read as part of the value and repeated on every option list. */}
<SelectTrigger size="sm" aria-label="Cohort" className="w-32 md:w-44">
<SelectTrigger
size="sm"
aria-label="Cohort"
className="w-32 md:w-44"
disabled={!hasChoice}
title={hasChoice ? undefined : NO_COHORT_REASON}
>
<SelectValue>{label}</SelectValue>
</SelectTrigger>
<SelectContent align="end">
Expand Down
94 changes: 94 additions & 0 deletions src/frontend/src/lib/portal/cohort-options.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import { describe, expect, it } from "vitest";

import type { MetricDefinitionListResponse } from "@/api/metric-definitions-client";
import type { SliceAttr } from "@/lib/insight/slices";
import { catalogAttributes, cohortOptions } from "./cohort-options";

/** A roster row as `collectRosterAttrs` produces it. */
const person = (division: string): Record<string, SliceAttr> => ({
division: { key: "division", label: "Division", value: division },
});

const response = (extra: object = {}): MetricDefinitionListResponse =>
({ metrics: [], ...extra }) as MetricDefinitionListResponse;

describe("catalogAttributes", () => {
it("reads the attributes a response carries", () => {
expect(
catalogAttributes(
response({
comparison_attributes: [
{ id: "job_title", label: "Title" },
{ id: "division", label: "Division" },
],
}),
),
).toEqual([
{ id: "job_title", label: "Title" },
{ id: "division", label: "Division" },
]);
});

it("treats a response without one as a normal state", () => {
// This client runs against installations on both sides of the change, so
// the field's absence is expected rather than an error to surface.
expect(catalogAttributes(response())).toEqual([]);
expect(catalogAttributes(undefined)).toEqual([]);
});

it("drops entries it cannot render or send back", () => {
expect(
catalogAttributes(
response({
comparison_attributes: [
{ id: "ok", label: "Fine" },
{ id: "no_label" },
{ label: "no id" },
null,
"not an object",
],
}),
),
).toEqual([{ id: "ok", label: "Fine" }]);
});
});

describe("cohortOptions", () => {
it("follows the catalog when there is one", () => {
const options = cohortOptions(
[{ id: "job_title", label: "Title" }],
[person("Alpha"), person("Alpha"), person("Beta")],
);
expect(options.source).toBe("catalog");
expect(options.dims).toEqual([{ key: "job_title", label: "Title" }]);
});

it("does not mix the two sources", () => {
// A locally-derived attribute standing beside a governed one is
// indistinguishable to the reader, who cannot tell which they picked.
const options = cohortOptions(
[{ id: "job_title", label: "Title" }],
[person("Alpha"), person("Alpha"), person("Beta")],
);
expect(options.dims.map((d) => d.key)).not.toContain("division");
});

it("falls back to the roster until the catalog exists", () => {
// Three people, two divisions: an attribute that splits them without
// being near-unique, which `availableSlices` reads as an identifier.
const options = cohortOptions(
[],
[person("Alpha"), person("Alpha"), person("Beta")],
);
expect(options.source).toBe("roster");
expect(options.dims.map((d) => d.key)).toEqual(["division"]);
});

it("reports having nothing when neither source can offer anything", () => {
// A viewer with no reports: one person in the roster, so no attribute
// takes a second value and none qualifies.
const options = cohortOptions([], [person("Alpha")]);
expect(options.source).toBe("none");
expect(options.dims).toEqual([]);
});
});
Loading
Loading