Skip to content

feat: Objects Accordion for Details redesign BED-6844 #2160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
urangel merged 13 commits into from
Jan 5, 2026
Merged

feat: Objects Accordion for Details redesign BED-6844 #2160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
579c5bc
feat: Objects Accordion for Details redesign BED-6844
urangel Dec 9, 2025
ba1c627
chore: test id typo fix; swap wip component
urangel Dec 12, 2025
d82d964
chore: remove skeleton array rendering in tag list
urangel Dec 15, 2025
0fa588b
fix: change `kind` to `primary_kind` to match API
urangel Dec 15, 2025
bec017f
test: add rule and tag based unit tests
urangel Dec 23, 2025
f5e8d98
fix: resolve circular import issue
urangel Dec 23, 2025
d727575
chore: count queries and rule count handler
urangel Dec 23, 2025
f5e6390
chore: uncomment rulemembercounts call
urangel Jan 5, 2026
a820295
test: update unit tests
urangel Jan 5, 2026
be0a9c0
fix: truncation, superfluous calls, sort button visibility
urangel Jan 5, 2026
33c16b8
test: update unit tests with enabled param expectation
urangel Jan 5, 2026
9d8c38d
chore: title to tooltip
urangel Jan 5, 2026
c7d63d3
chore: revert to original details view
urangel Jan 5, 2026
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
2 changes: 0 additions & 2 deletions packages/javascript/bh-shared-ui/src/components/ExploreTable/ExploreTable.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -679,8 +679,6 @@ describe('ExploreTable', async () => {

//fire sort
await user.click(screen.getByText('Name'));
const testElem = screen.getByRole('button', { name: /name/i });
screen.debug(testElem);

//up arrow visible, down arrow removed
expect(within(screen.getByRole('button', { name: /name/i })).getByText('app-icon-sort-asc')).toBeVisible();
Expand Down
66 changes: 50 additions & 16 deletions packages/javascript/bh-shared-ui/src/hooks/useAssetGroupTags/useAssetGroupTags.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,19 @@ export const privilegeZonesKeys = {
ruleDetail: (tagId: string | number, ruleId: string | number) =>
[...privilegeZonesKeys.rulesByTag(tagId), 'ruleId', ruleId] as const,
members: () => [...privilegeZonesKeys.all, 'members'] as const,
membersByTag: (tagId: string | number, sortOrder: SortOrder, environments: string[] = []) =>
[...privilegeZonesKeys.members(), 'tag', tagId, sortOrder, ...environments] as const,
membersByTag: (
tagId: string | number,
sortOrder: SortOrder,
environments: string[] = [],
primary_kind: string = 'all'
) => [...privilegeZonesKeys.members(), 'tag', tagId, primary_kind, sortOrder, ...environments] as const,
membersByTagAndRule: (
tagId: string | number,
ruleId: string | number | undefined,
sortOrder: SortOrder,
environments: string[] = []
) => ['tag', tagId, 'rule', ruleId, sortOrder, ...environments] as const,
environments: string[] = [],
primary_kind: string = 'all'
) => ['tag', tagId, 'rule', ruleId, primary_kind, sortOrder, ...environments] as const,
Comment on lines +83 to +95
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get the exact content of useAssetGroupTags.tsx to verify the claims

echo "=== Lines 83-95 (query key factories) ==="
sed -n '83,95p' packages/javascript/bh-shared-ui/src/hooks/useAssetGroupTags/useAssetGroupTags.tsx

echo -e "\n=== Lines 271, 292, 321, 344 (function signatures) ==="
sed -n '271p;292p;321p;344p' packages/javascript/bh-shared-ui/src/hooks/useAssetGroupTags/useAssetGroupTags.tsx

echo -e "\n=== Line 298 (query key call example) ==="
sed -n '298p' packages/javascript/bh-shared-ui/src/hooks/useAssetGroupTags/useAssetGroupTags.tsx

echo -e "\n=== Full context of function implementations (265-312, 314-366) ==="
sed -n '265,312p' packages/javascript/bh-shared-ui/src/hooks/useAssetGroupTags/useAssetGroupTags.tsx

echo -e "\n=== Second section ==="
sed -n '314,366p' packages/javascript/bh-shared-ui/src/hooks/useAssetGroupTags/useAssetGroupTags.tsx

Repository: SpecterOps/BloodHound

Length of output: 4253

🏁 Script executed:

#!/bin/bash
# Check API client implementation for primary_kind parameter handling

echo "=== API client getAssetGroupTagMembers implementation ==="
rg -n "getAssetGroupTagMembers" -A 10 packages/javascript/js-client-library/src/client.ts

echo -e "\n=== Check line 375 and surrounding context ==="
sed -n '370,380p' packages/javascript/js-client-library/src/client.ts

Repository: SpecterOps/BloodHound

Length of output: 1583

🏁 Script executed:

#!/bin/bash
# Verify how primary_kind is actually passed to the API

echo "=== How primary_kind is passed in useAssetGroupTags ==="
rg -n "apiClient\.|getAssetGroupTag" -B 2 -A 2 packages/javascript/bh-shared-ui/src/hooks/useAssetGroupTags/useAssetGroupTags.tsx | grep -A 5 -B 5 "primary_kind"

Repository: SpecterOps/BloodHound

Length of output: 120

Fix cache key consistency with API parameter handling.

The query key factories and the API calls create inconsistent caching when primary_kind is undefined:

  • Query key factories (lines 83-88, 89-95) define primary_kind: string = 'all' with a default value
  • The functions (lines 271, 292, 321, 344) accept primary_kind?: string without a default
  • When primary_kind is undefined, calling the key factory uses the default 'all', but the API call omits the parameter entirely (line 375 only sends when truthy)

This means the same query key represents two different API requests:

// Both produce cache key [..., 'all', ...] but different API calls:
useTagMembersInfiniteQuery(42, 'asc', ['env-1'])          // key: [..., 'all', ...], API: primary_kind not sent
useTagMembersInfiniteQuery(42, 'asc', ['env-1'], 'all')   // key: [..., 'all', ...], API: primary_kind=eq:all

To resolve, either:

  1. Remove the default from the key factories and explicitly pass primary_kind ?? 'all' at call sites, or
  2. Ensure the key factories always receive an explicit value (never undefined)

Applies to both membersByTag and membersByTagAndRule key factories, and their corresponding hooks and fetcher functions.

🤖 Prompt for AI Agents 
In
@packages/javascript/bh-shared-ui/src/hooks/useAssetGroupTags/useAssetGroupTags.tsx
around lines 83-95, The query key factories membersByTag and membersByTagAndRule
currently default primary_kind to 'all' causing mismatch with fetchers that
accept primary_kind?: string; change the key factories to accept primary_kind?:
string (remove the '= \'all\'') and update all call sites (e.g., where you call
membersByTag/membersByTagAndRule from useTagMembersInfiniteQuery and the related
fetcher functions) to pass primary_kind ?? 'all' explicitly when building the
API request and keys so the cache key exactly matches whether the API sends
primary_kind or not.

memberDetail: (tagId: string | number, memberId: string | number) =>
[...privilegeZonesKeys.tagDetail(tagId), 'memberId', memberId] as const,

Expand Down Expand Up @@ -262,11 +267,19 @@ export const getAssetGroupTagMembers = (
skip = 0,
limit = PAGE_SIZE,
sortOrder: SortOrder = 'asc',
environments?: string[]
environments?: string[],
primary_kind?: string
) =>
createPaginatedFetcher<AssetGroupTagMemberListItem>(
() =>
apiClient.getAssetGroupTagMembers(tagId, skip, limit, sortOrder === 'asc' ? 'name' : '-name', environments),
apiClient.getAssetGroupTagMembers(
tagId,
skip,
limit,
sortOrder === 'asc' ? 'name' : '-name',
environments,
primary_kind
),
'members',
skip,
limit
Expand All @@ -275,19 +288,28 @@ export const getAssetGroupTagMembers = (
export const useTagMembersInfiniteQuery = (
tagId: number | string | undefined,
sortOrder: SortOrder,
environments?: string[]
environments?: string[],
primary_kind?: string,
enabled?: boolean
) =>
useInfiniteQuery<{
items: AssetGroupTagMemberListItem[];
nextPageParam?: PageParam;
}>({
queryKey: privilegeZonesKeys.membersByTag(tagId!, sortOrder, environments),
queryKey: privilegeZonesKeys.membersByTag(tagId!, sortOrder, environments, primary_kind),
queryFn: ({ pageParam = { skip: 0, limit: PAGE_SIZE } }) => {
if (!tagId) return Promise.reject('No tag ID provided for tag members request');
return getAssetGroupTagMembers(tagId, pageParam.skip, pageParam.limit, sortOrder, environments);
return getAssetGroupTagMembers(
tagId,
pageParam.skip,
pageParam.limit,
sortOrder,
environments,
primary_kind
);
},
getNextPageParam: (lastPage) => lastPage.nextPageParam,
enabled: tagId !== undefined,
enabled: tagId !== undefined && (enabled === undefined ? true : enabled),
});

export const getAssetGroupTagRuleMembers = (
Expand All @@ -296,7 +318,8 @@ export const getAssetGroupTagRuleMembers = (
skip: number = 0,
limit: number = PAGE_SIZE,
sortOrder: SortOrder = 'asc',
environments?: string[]
environments?: string[],
primary_kind?: string
) =>
createPaginatedFetcher(
() =>
Expand All @@ -306,7 +329,8 @@ export const getAssetGroupTagRuleMembers = (
skip,
limit,
sortOrder === 'asc' ? 'name' : '-name',
environments
environments,
primary_kind
),
'members',
skip,
Expand All @@ -317,20 +341,30 @@ export const useRuleMembersInfiniteQuery = (
tagId: number | string | undefined,
ruleId: number | string | undefined,
sortOrder: SortOrder,
environments?: string[]
environments?: string[],
primary_kind?: string,
enabled?: boolean
) =>
useInfiniteQuery<{
items: AssetGroupTagMemberListItem[];
nextPageParam?: PageParam;
}>({
queryKey: privilegeZonesKeys.membersByTagAndRule(tagId!, ruleId, sortOrder, environments),
queryKey: privilegeZonesKeys.membersByTagAndRule(tagId!, ruleId, sortOrder, environments, primary_kind),
queryFn: ({ pageParam = { skip: 0, limit: PAGE_SIZE } }) => {
if (!tagId) return Promise.reject('No tag ID available to get rule members');
if (!ruleId) return Promise.reject('No rule ID available to get rule members');
return getAssetGroupTagRuleMembers(tagId, ruleId, pageParam.skip, pageParam.limit, sortOrder, environments);
return getAssetGroupTagRuleMembers(
tagId,
ruleId,
pageParam.skip,
pageParam.limit,
sortOrder,
environments,
primary_kind
);
},
getNextPageParam: (lastPage) => lastPage.nextPageParam,
enabled: tagId !== undefined && ruleId !== undefined,
enabled: tagId !== undefined && ruleId !== undefined && (enabled === undefined ? true : enabled),
});

export const useMemberInfo = (tagId: string = '', memberId: string = '') =>
Expand Down
8 changes: 4 additions & 4 deletions packages/javascript/bh-shared-ui/src/mocks/factories/privilegeZones.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import {
export const createAssetGroupTag = (tagId: number = 0, name?: string, type?: AssetGroupTagType): AssetGroupTag => {
return {
id: tagId,
name: name ? name : `Tier-${tagId - 1}`,
name: name ? name : `tag-${tagId - 1}`,
kind_id: faker.datatype.number(),
glyph: null,
type: type ?? AssetGroupTagTypeZone,
Expand Down Expand Up @@ -73,7 +73,7 @@ export const createRule = (tagId: number = 0, ruleId: number = 0) => {
const data: AssetGroupTagSelector = {
id: ruleId,
asset_group_tag_id: tagId,
name: `tier-${tagId - 1}-rule-${ruleId}`,
name: `tag-${tagId - 1}-rule-${ruleId}`,
allow_disable: faker.datatype.boolean(),
description: faker.random.words(),
is_default: faker.datatype.boolean(),
Expand Down Expand Up @@ -146,8 +146,8 @@ export const createObjects = (
if (i === count) break;

const name = Number.isNaN(ruleId)
? `tier-${assetGroupId - 1}-object-${i}`
: `tier-${assetGroupId - 1}-rule-${ruleId}-object-${i}`;
? `tag-${assetGroupId - 1}-object-${i}`
: `tag-${assetGroupId - 1}-rule-${ruleId}-object-${i}`;

data.push({
id: i,
Expand Down
6 changes: 3 additions & 3 deletions packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/Details.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ describe('Details', async () => {
const objectsListItems = await within(objects).findAllByTestId('sort-button');
expect(objectsListItems.length).toBeGreaterThan(0);

const object5 = await screen.findByText('tier-0-object-5');
const object5 = await screen.findByText('tag-0-object-5');
await user.click(object5);

await waitFor(async () => {
Expand All @@ -134,7 +134,7 @@ describe('Details', async () => {

const rules = await screen.findByTestId('privilege-zones_details_rules-list');
await within(rules).findAllByTestId('sort-button');
const rule7 = await within(rules).findByText('tier-0-rule-7');
const rule7 = await within(rules).findByText('tag-0-rule-7');

await user.click(rule7);

Expand Down Expand Up @@ -165,7 +165,7 @@ describe('Details', async () => {

const zones = await screen.findByTestId('privilege-zones_details_zones-list');
await within(zones).findAllByTestId('privilege-zones_details_zones-list_static-order');
const zone = await within(zones).findByText('Tier-2');
const zone = await within(zones).findByText('tag-2');
expect(zone).toBeInTheDocument();
await user.click(zone);

Expand Down
2 changes: 1 addition & 1 deletion packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/DynamicDetails.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ import { LuxonFormat } from '../../../utils';
import { Cypher } from '../Cypher/Cypher';
import { PrivilegeZonesContext } from '../PrivilegeZonesContext';
import { ZoneIcon } from '../ZoneIcon';
import { getRuleSeedType, isRule, isTag } from '../utils';
import ObjectCountPanel from './ObjectCountPanel';
import { getRuleSeedType, isRule, isTag } from './utils';

const DetailField: FC<{ label: string; value: string }> = ({ label, value }) => {
return (
Expand Down
2 changes: 1 addition & 1 deletion packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/MembersList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ import { NodeIcon, SortableHeader } from '../../../components';
import { InfiniteQueryFixedList, InfiniteQueryFixedListProps } from '../../../components/InfiniteQueryFixedList';
import { SortOrder } from '../../../types';
import { cn } from '../../../utils';
import { getListHeight } from '../utils';
import { SelectedHighlight } from './SelectedHighlight';
import { getListHeight } from './utils';

interface MembersListProps {
listQuery: UseInfiniteQueryResult<{
Expand Down
2 changes: 1 addition & 1 deletion packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/RulesList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import { SortableHeader } from '../../../components';
import { InfiniteQueryFixedList, InfiniteQueryFixedListProps } from '../../../components/InfiniteQueryFixedList';
import { SortOrder } from '../../../types';
import { cn } from '../../../utils';
import { getListHeight } from '../utils';
import { SelectedHighlight } from './SelectedHighlight';
import { getListHeight } from './utils';

const LoadingRow = (_: number, style: React.CSSProperties) => (
<div
Expand Down
2 changes: 1 addition & 1 deletion packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/SearchBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { useQuery } from 'react-query';
import { AppIcon } from '../../../components';
import { useDebouncedValue, usePZPathParams } from '../../../hooks';
import { apiClient, cn, useAppNavigate } from '../../../utils';
import { isRule, isTag } from './utils';
import { isRule, isTag } from '../utils';

type SectorMap =
| { Zones: 'tags'; Rules: 'selectors'; Members: 'members' } // 'selectors' is the key in the API response so should not be updated to 'rules'
Expand Down
4 changes: 2 additions & 2 deletions ...i/src/views/PrivilegeZones/Details/SelectedDetailsTabs/SelectedDetailsTabContent.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@ describe('Selected Details Tab Content', () => {
it('renders the Zone/Labels Tab content when first tab is chosen', async () => {
render(<SelectedDetailsTabContent currentDetailsTab={TagTabValue} tagId='1' />);

const zoneTitle = await screen.findByText(/tier-0/i); // can find the structure of title in mocks/factories/privilegeZones
const zoneTitle = await screen.findByText(/tag-0/i); // can find the structure of title in mocks/factories/privilegeZones

expect(zoneTitle).toBeInTheDocument();
});
it('renders the Rule Tab content when Rule tab is chosen', async () => {
render(<SelectedDetailsTabContent currentDetailsTab={RuleTabValue} tagId='1' ruleId='2' />);

const ruleTitle = await screen.findByText(/tier-0-rule-2/i); // can find the structure of title in mocks/factories/privilegeZones
const ruleTitle = await screen.findByText(/tag-0-rule-2/i); // can find the structure of title in mocks/factories/privilegeZones

expect(ruleTitle).toBeInTheDocument();
});
Expand Down
2 changes: 1 addition & 1 deletion packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/TagList.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ describe('List', async () => {

render(<TagList title='Labels' listQuery={testQuery} selected={'1'} onSelect={() => {}} />);

expect(screen.getAllByTestId('privilege-zones_labels-list_loading-skeleton')).toHaveLength(3);
expect(screen.getAllByTestId('privilege-zones_tags-list_loading-skeleton')).toHaveLength(3);
});

it('handles data fetching errors', async () => {
Expand Down
23 changes: 17 additions & 6 deletions packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/TagList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
//
// SPDX-License-Identifier: Apache-2.0

import { Button } from '@bloodhoundenterprise/doodleui';
import { Button, Skeleton } from '@bloodhoundenterprise/doodleui';
import { AssetGroupTag } from 'js-client-library';
import { FC, useState } from 'react';
import { UseQueryResult } from 'react-query';
Expand All @@ -23,9 +23,18 @@ import { useHighestPrivilegeTagId, usePZPathParams, usePrivilegeZoneAnalysis } f
import { SortOrder } from '../../../types';
import { cn } from '../../../utils';
import { ZoneIcon } from '../ZoneIcon';
import { itemSkeletons } from '../utils';
import { isTag } from '../utils';
import { SelectedHighlight } from './SelectedHighlight';
import { isTag } from './utils';

const ItemSkeleton = () => {
return (
<li
data-testid={`privilege-zones_tags-list_loading-skeleton`}
className='border-y border-neutral-light-3 dark:border-neutral-dark-3 relative w-full'>
<Skeleton className='h-10 rounded-none' />
</li>
);
};

type TagListProps = {
title: 'Zones' | 'Labels';
Expand Down Expand Up @@ -73,9 +82,11 @@ export const TagList: FC<TagListProps> = ({ title, listQuery, selected, onSelect
)}
<ul>
{listQuery.isLoading ? (
itemSkeletons.map((skeleton, index) => {
return skeleton(title, index);
})
<>
<ItemSkeleton />
<ItemSkeleton />
<ItemSkeleton />
</>
) : listQuery.isError ? (
<li className='border-y border-neutral-3 relative h-10 pl-2'>
<span className='text-base'>There was an error fetching this data</span>
Expand Down
3 changes: 0 additions & 3 deletions packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,3 @@ export { EntityRulesInformation, SelectedDetails };

import Details from './Details';
export default Details;

// import NewDetails from './NewDetails';
// export default NewDetails;
Loading