feat(ui): address alan feedback

This commit is contained in:
Pablo F.G
2026-06-23 09:24:31 +02:00
parent f08d7c9f15
commit d90e4449f9
11 changed files with 340 additions and 74 deletions
@@ -120,4 +120,19 @@ describe("getAllProviderGroups", () => {
expect(result).toBeUndefined();
});
it("returns undefined instead of a truncated list when the max-page cap is hit", async () => {
// Given an API that always reports more pages than the 50-page safety cap
handleApiResponseMock.mockImplementation((response: Response) => {
void response;
return Promise.resolve(makePage([makeGroup("g", "Group")], 1, 9999));
});
// When fetching every page
const result = await getAllProviderGroups();
// Then it must not return a partial/truncated list; bail out instead
expect(result).toBeUndefined();
expect(fetchMock).toHaveBeenCalledTimes(50);
});
});
+8 -1
View File
@@ -59,7 +59,6 @@ export const getProviderGroups = async ({
export const getAllProviderGroups = async (): Promise<
ProviderGroupsResponse | undefined
> => {
const headers = await getAuthHeaders({ contentType: false });
const pageSize = 100; // Larger page size to minimize API calls
const maxPages = 50; // Safety limit: 50 pages × 100 = 5000 groups max
let currentPage = 1;
@@ -68,6 +67,7 @@ export const getAllProviderGroups = async (): Promise<
let hasMorePages = true;
try {
const headers = await getAuthHeaders({ contentType: false });
while (hasMorePages && currentPage <= maxPages) {
const url = new URL(`${apiBaseUrl}/provider-groups`);
url.searchParams.append("page[number]", currentPage.toString());
@@ -102,6 +102,13 @@ export const getAllProviderGroups = async (): Promise<
}
}
if (hasMorePages && currentPage > maxPages) {
console.error(
`Error fetching all provider groups: exceeded max page limit (${maxPages})`,
);
return undefined;
}
if (lastResponse) {
return {
...lastResponse,
@@ -0,0 +1,162 @@
import { describe, expect, it } from "vitest";
import { ProviderProps } from "@/types/providers";
import {
filterProvidersByScope,
parseFilterIds,
scopeProvidersByGroup,
} from "./provider-scope";
const makeProvider = (
id: string,
provider: string,
groupIds: string[] = [],
): ProviderProps =>
({
id,
attributes: { provider },
relationships: {
provider_groups: {
data: groupIds.map((gid) => ({ type: "provider-groups", id: gid })),
},
},
}) as unknown as ProviderProps;
describe("parseFilterIds", () => {
it("returns an empty array for undefined", () => {
// Given / When / Then
expect(parseFilterIds(undefined)).toEqual([]);
});
it("returns an empty array for an empty string", () => {
// Given an empty param value (e.g. "filter[provider_groups__in]=")
// When / Then it must not produce a [""] match
expect(parseFilterIds("")).toEqual([]);
});
it("drops whitespace-only and empty segments", () => {
// Given a blank/whitespace value
// When / Then
expect(parseFilterIds(" ")).toEqual([]);
expect(parseFilterIds(",")).toEqual([]);
expect(parseFilterIds("a,,b")).toEqual(["a", "b"]);
});
it("splits and trims comma-separated ids", () => {
expect(parseFilterIds(" a , b ")).toEqual(["a", "b"]);
});
it("normalizes array param values", () => {
expect(parseFilterIds(["a", "", "b"])).toEqual(["a", "b"]);
});
});
describe("scopeProvidersByGroup", () => {
const providers = [
makeProvider("p1", "aws", ["g1"]),
makeProvider("p2", "gcp", ["g2"]),
makeProvider("p3", "azure", []),
];
it("returns every provider when no group is selected", () => {
expect(scopeProvidersByGroup(providers, [])).toEqual(providers);
});
it("keeps only providers that belong to a selected group", () => {
// When scoping to g1
const result = scopeProvidersByGroup(providers, ["g1"]);
// Then only the g1 member remains
expect(result.map((p) => p.id)).toEqual(["p1"]);
});
it("excludes providers with no group memberships", () => {
expect(scopeProvidersByGroup(providers, ["g2"]).map((p) => p.id)).toEqual([
"p2",
]);
});
});
describe("filterProvidersByScope", () => {
const providers = [
makeProvider("p1", "aws", ["g1"]),
makeProvider("p2", "gcp", ["g1"]),
makeProvider("p3", "aws", ["g2"]),
makeProvider("p4", "azure", []),
];
it("returns every provider when no dimension is set", () => {
const result = filterProvidersByScope(providers, {
providerIds: [],
providerTypes: [],
providerGroupIds: [],
});
expect(result).toEqual(providers);
});
it("filters by provider id", () => {
const result = filterProvidersByScope(providers, {
providerIds: ["p2"],
providerTypes: [],
providerGroupIds: [],
});
expect(result.map((p) => p.id)).toEqual(["p2"]);
});
it("filters by provider type case-insensitively", () => {
const result = filterProvidersByScope(providers, {
providerIds: [],
providerTypes: ["AWS"],
providerGroupIds: [],
});
expect(result.map((p) => p.id)).toEqual(["p1", "p3"]);
});
it("filters by provider group", () => {
const result = filterProvidersByScope(providers, {
providerIds: [],
providerTypes: [],
providerGroupIds: ["g1"],
});
expect(result.map((p) => p.id)).toEqual(["p1", "p2"]);
});
it("composes group AND type (the risk-plot regression)", () => {
// Given both a group and a type filter are active
// When combining group g1 with type aws
const result = filterProvidersByScope(providers, {
providerIds: [],
providerTypes: ["aws"],
providerGroupIds: ["g1"],
});
// Then only providers matching BOTH survive (p1), not all aws or all g1
expect(result.map((p) => p.id)).toEqual(["p1"]);
});
it("composes id AND group", () => {
// p3 is aws/g2; selecting it together with group g1 yields nothing
const result = filterProvidersByScope(providers, {
providerIds: ["p3"],
providerTypes: [],
providerGroupIds: ["g1"],
});
expect(result).toEqual([]);
});
it("composes all three dimensions", () => {
const result = filterProvidersByScope(providers, {
providerIds: ["p1", "p2"],
providerTypes: ["aws"],
providerGroupIds: ["g1"],
});
expect(result.map((p) => p.id)).toEqual(["p1"]);
});
});
@@ -0,0 +1,71 @@
import { ProviderProps } from "@/types/providers";
export interface ProviderScopeFilters {
providerIds: string[];
providerTypes: string[];
providerGroupIds: string[];
}
/**
* Normalize a comma-separated filter param into trimmed, non-empty ids.
* Guards against blank values (e.g. an empty "filter[...]=" param) so they are
* treated as "no filter" instead of matching against an empty-string id.
*/
export const parseFilterIds = (
value: string | string[] | undefined,
): string[] => {
if (value === undefined) return [];
const raw = Array.isArray(value) ? value.join(",") : value;
return raw
.split(",")
.map((id) => id.trim())
.filter((id) => id.length > 0);
};
const belongsToGroup = (provider: ProviderProps, groupIds: string[]): boolean =>
provider.relationships.provider_groups?.data?.some((group) =>
groupIds.includes(group.id),
) ?? false;
/**
* Keep only providers belonging to one of the selected groups. An empty group
* list means "no group filter" and returns every provider unchanged.
*/
export const scopeProvidersByGroup = (
providers: ProviderProps[],
groupIds: string[],
): ProviderProps[] =>
groupIds.length === 0
? providers
: providers.filter((p) => belongsToGroup(p, groupIds));
/**
* Filter providers by every active scope dimension (id, type, group) combined
* with AND. Each empty dimension is skipped, so a provider is kept only when it
* satisfies all the filters that are actually set.
*/
export const filterProvidersByScope = (
providers: ProviderProps[],
{ providerIds, providerTypes, providerGroupIds }: ProviderScopeFilters,
): ProviderProps[] => {
const normalizedTypes = providerTypes.map((type) => type.toLowerCase());
return providers.filter((provider) => {
if (providerIds.length > 0 && !providerIds.includes(provider.id)) {
return false;
}
if (
normalizedTypes.length > 0 &&
!normalizedTypes.includes(provider.attributes.provider.toLowerCase())
) {
return false;
}
if (
providerGroupIds.length > 0 &&
!belongsToGroup(provider, providerGroupIds)
) {
return false;
}
return true;
});
};
@@ -9,6 +9,10 @@ import { SankeyChart } from "@/components/graphs/sankey-chart";
import { SearchParamsProps } from "@/types";
import { pickFilterParams } from "../../_lib/filter-params";
import {
parseFilterIds,
scopeProvidersByGroup,
} from "../../_lib/provider-scope";
export async function RiskPipelineViewSSR({
searchParams,
@@ -27,18 +31,8 @@ export async function RiskPipelineViewSSR({
// Scope the provider set to the selected groups so we enumerate only their
// provider types below (the per-type API calls also carry the group filter).
const selectedGroupIds = providerGroupsFilter
? String(providerGroupsFilter)
.split(",")
.map((id) => id.trim())
: [];
const scopedProviders = selectedGroupIds.length
? allProviders.filter((p) =>
p.relationships.provider_groups?.data?.some((group) =>
selectedGroupIds.includes(group.id),
),
)
: allProviders;
const selectedGroupIds = parseFilterIds(providerGroupsFilter);
const scopedProviders = scopeProvidersByGroup(allProviders, selectedGroupIds);
// Build severityByProviderType based on filters
const severityByProviderType: SeverityByProviderType = {};
@@ -46,9 +40,7 @@ export async function RiskPipelineViewSSR({
if (providerIdFilter) {
// Case: Accounts are selected - group by provider type and make parallel calls
const selectedAccountIds = String(providerIdFilter)
.split(",")
.map((id) => id.trim());
const selectedAccountIds = parseFilterIds(providerIdFilter);
// Group selected accounts by provider type
const accountsByType: Record<string, string[]> = {};
@@ -87,9 +79,9 @@ export async function RiskPipelineViewSSR({
}
} else if (providerTypeFilter) {
// Case: Provider types are selected - make parallel calls for each type
selectedProviderTypes = String(providerTypeFilter)
.split(",")
.map((t) => t.trim().toLowerCase());
selectedProviderTypes = parseFilterIds(providerTypeFilter).map((type) =>
type.toLowerCase(),
);
const severityPromises = selectedProviderTypes.map(async (providerType) => {
const response = await getFindingsBySeverity({
@@ -1,5 +1,6 @@
import { Info } from "lucide-react";
import { OVERVIEW_FILTER_PARAM } from "@/actions/overview/overview-filters";
import {
adaptToRiskPlotData,
getProvidersRiskData,
@@ -8,6 +9,10 @@ import { getAllProviders } from "@/actions/providers";
import { SearchParamsProps } from "@/types";
import { pickFilterParams } from "../../_lib/filter-params";
import {
filterProvidersByScope,
parseFilterIds,
} from "../../_lib/provider-scope";
import { RiskPlotClient } from "./risk-plot-client";
export async function RiskPlotSSR({
@@ -17,42 +22,19 @@ export async function RiskPlotSSR({
}) {
const filters = pickFilterParams(searchParams);
const providerTypeFilter = filters["filter[provider_type__in]"];
const providerIdFilter = filters["filter[provider_id__in]"];
const providerGroupsFilter = filters["filter[provider_groups__in]"];
// Fetch all providers
const providersListResponse = await getAllProviders();
const allProviders = providersListResponse?.data || [];
// Filter providers based on search params
let filteredProviders = allProviders;
if (providerIdFilter) {
// Filter by specific provider IDs
const selectedIds = String(providerIdFilter)
.split(",")
.map((id) => id.trim());
filteredProviders = allProviders.filter((p) => selectedIds.includes(p.id));
} else if (providerGroupsFilter) {
// Filter by provider group membership
const selectedGroupIds = String(providerGroupsFilter)
.split(",")
.map((id) => id.trim());
filteredProviders = allProviders.filter((p) =>
p.relationships.provider_groups?.data?.some((group) =>
selectedGroupIds.includes(group.id),
),
);
} else if (providerTypeFilter) {
// Filter by provider types
const selectedTypes = String(providerTypeFilter)
.split(",")
.map((t) => t.trim().toLowerCase());
filteredProviders = allProviders.filter((p) =>
selectedTypes.includes(p.attributes.provider.toLowerCase()),
);
}
// Compose every active provider-scope filter with AND so combining e.g. a
// group and a type narrows to providers matching both.
const filteredProviders = filterProvidersByScope(allProviders, {
providerIds: parseFilterIds(filters[OVERVIEW_FILTER_PARAM.PROVIDER_ID]),
providerTypes: parseFilterIds(filters[OVERVIEW_FILTER_PARAM.PROVIDER_TYPE]),
providerGroupIds: parseFilterIds(
filters[OVERVIEW_FILTER_PARAM.PROVIDER_GROUPS],
),
});
// No providers to show
if (filteredProviders.length === 0) {
@@ -3,6 +3,7 @@
import { useRouter, useSearchParams } from "next/navigation";
import { useState } from "react";
import { OVERVIEW_FILTER_PARAM } from "@/actions/overview/overview-filters";
import { getSeverityTrendsByTimeRange } from "@/actions/overview/severity-trends";
import { LineChart } from "@/components/graphs/line-chart";
import { LineConfig, LineDataPoint } from "@/components/graphs/types";
@@ -42,12 +43,16 @@ export const FindingSeverityOverTime = ({
const getActiveProviderFilters = (): Record<string, string> => {
const filters: Record<string, string> = {};
const providerType = searchParams.get("filter[provider_type__in]");
const providerId = searchParams.get("filter[provider_id__in]");
const providerGroups = searchParams.get("filter[provider_groups__in]");
if (providerType) filters["filter[provider_type__in]"] = providerType;
if (providerId) filters["filter[provider_id__in]"] = providerId;
if (providerGroups) filters["filter[provider_groups__in]"] = providerGroups;
const providerType = searchParams.get(OVERVIEW_FILTER_PARAM.PROVIDER_TYPE);
const providerId = searchParams.get(OVERVIEW_FILTER_PARAM.PROVIDER_ID);
const providerGroups = searchParams.get(
OVERVIEW_FILTER_PARAM.PROVIDER_GROUPS,
);
if (providerType)
filters[OVERVIEW_FILTER_PARAM.PROVIDER_TYPE] = providerType;
if (providerId) filters[OVERVIEW_FILTER_PARAM.PROVIDER_ID] = providerId;
if (providerGroups)
filters[OVERVIEW_FILTER_PARAM.PROVIDER_GROUPS] = providerGroups;
return filters;
};
@@ -1,7 +1,10 @@
import type { FindingsFilterParam } from "@/actions/findings/findings-filters";
import type { FilterChip } from "@/components/filters/filter-summary-strip";
import { formatLabel, getCategoryLabel, getGroupLabel } from "@/lib/categories";
import { getScanEntityLabel } from "@/lib/helper-filters";
import {
getProviderGroupDisplayValue,
getScanEntityLabel,
} from "@/lib/helper-filters";
import { FINDING_STATUS_DISPLAY_NAMES } from "@/types";
import { ProviderGroup } from "@/types/components";
import { getProviderDisplayName, ProviderProps } from "@/types/providers";
@@ -31,14 +34,6 @@ function getProviderAccountDisplayValue(
return provider.attributes.alias || provider.attributes.uid || providerId;
}
function getProviderGroupDisplayValue(
groupId: string,
groups: ProviderGroup[],
): string {
const group = groups.find((item) => item.id === groupId);
return group?.attributes.name || groupId;
}
function getScanDisplayValue(
scanId: string,
scans: Array<{ [scanId: string]: ScanEntity }>,
@@ -1,6 +1,7 @@
import type { ResourcesFilterParam } from "@/actions/resources/resources-filters";
import type { FilterChip } from "@/components/filters/filter-summary-strip";
import { formatLabel, getGroupLabel } from "@/lib/categories";
import { getProviderGroupDisplayValue } from "@/lib/helper-filters";
import type { ProviderGroup } from "@/types/components";
import type { ProviderProps } from "@/types/providers";
import { getProviderDisplayName } from "@/types/providers";
@@ -27,14 +28,6 @@ function getProviderAccountDisplayValue(
return provider.attributes.alias || provider.attributes.uid || providerId;
}
function getProviderGroupDisplayValue(
groupId: string,
groups: ProviderGroup[],
): string {
const group = groups.find((item) => item.id === groupId);
return group?.attributes.name || groupId;
}
export function getResourcesFilterDisplayValue(
filterKey: string,
value: string,
+30
View File
@@ -1,14 +1,23 @@
import { describe, expect, it } from "vitest";
import type { ProviderGroup } from "@/types/components";
import type { ScanEntity } from "@/types/scans";
import {
getProviderGroupDisplayValue,
getScanEntityLabel,
hasDateFilter,
hasDateOrScanFilter,
hasHistoricalFindingFilter,
} from "./helper-filters";
const makeProviderGroup = (id: string, name: string): ProviderGroup =>
({
type: "provider-groups",
id,
attributes: { name, inserted_at: "", updated_at: "" },
}) as ProviderGroup;
function makeScan(overrides: Partial<ScanEntity> = {}): ScanEntity {
return {
id: "scan-1",
@@ -25,6 +34,27 @@ function makeScan(overrides: Partial<ScanEntity> = {}): ScanEntity {
};
}
describe("getProviderGroupDisplayValue", () => {
const groups = [
makeProviderGroup("g1", "Production"),
makeProviderGroup("g2", "Staging"),
];
it("resolves the group name when the id matches", () => {
expect(getProviderGroupDisplayValue("g1", groups)).toBe("Production");
});
it("falls back to the raw id when the group is not found", () => {
expect(getProviderGroupDisplayValue("unknown", groups)).toBe("unknown");
});
it("falls back to the raw id when the group name is empty", () => {
expect(
getProviderGroupDisplayValue("g3", [makeProviderGroup("g3", "")]),
).toBe("g3");
});
});
describe("hasDateOrScanFilter", () => {
it("returns true for scan filters", () => {
expect(hasDateOrScanFilter({ "filter[scan__in]": "scan-1" })).toBe(true);
+14
View File
@@ -1,4 +1,5 @@
import { ProviderProps, ProvidersApiResponse, ScanProps } from "@/types";
import { ProviderGroup } from "@/types/components";
import { FilterEntity } from "@/types/filters";
import {
getProviderDisplayName,
@@ -119,6 +120,19 @@ export function getScanEntityLabel(scan: ScanEntity): string {
return providerLabel || scanName;
}
/**
* Resolves the display name for a provider group filter value, falling back to
* the raw id when the group can't be resolved. Shared by the findings and
* resources filter utils so their chips stay in sync.
*/
export function getProviderGroupDisplayValue(
groupId: string,
groups: ProviderGroup[],
): string {
const group = groups.find((item) => item.id === groupId);
return group?.attributes.name || groupId;
}
/**
* Creates a scan details mapping for filters from completed scans.
* Used to provide detailed information for scan filters in the UI.