feat(frontend): column-header sort & filter menu [IFC-2794]#9948
feat(frontend): column-header sort & filter menu [IFC-2794]#9948bilalabbad wants to merge 44 commits into
Conversation
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Shadow auto-approve: would auto-approve. Adds a frontend-only column-header sort menu, reusing existing sort/filter hooks and URL state; the behavior change is bounded to UI interaction, with full test coverage and no schema, API, or security changes.
Re-trigger cubic
There was a problem hiding this comment.
2 issues found and verified against the latest diff
Confidence score: 3/5
- In
frontend/app/src/entities/ipam/ip-addresses/api/get-ip-address-list-from-api.ts, filtered IPAM results lose the user-selected header sort when availability is included because expansion reorders nodes by address, which can surface inconsistent list ordering in the UI after merge — preserve the requestedorderthrough availability expansion and add a regression test for prefix-filtered + availability-enabled sorting before merging. - In
dev/specs/002-header-sort-menu/research.md, the stale R5 assumption sayssortis not threaded even though builders already pass it via__args, which risks future work being implemented against outdated guidance — update or remove that note to keep the spec aligned with current behavior.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/app/src/entities/ipam/ip-addresses/api/get-ip-address-list-from-api.ts">
<violation number="1" location="frontend/app/src/entities/ipam/ip-addresses/api/get-ip-address-list-from-api.ts:49">
P2: IPAM address results filtered to a prefix ignore the selected header sort whenever availability is included: the new `order` reaches the resolver, but availability expansion reorders all existing nodes by address. Sorting the combined existing/available result after expansion, while preserving pagination context, would keep header sorting effective.</violation>
</file>
<file name="dev/specs/002-header-sort-menu/research.md">
<violation number="1" location="dev/specs/002-header-sort-menu/research.md:59">
P3: This R5 note is stale: both IPAM GraphQL builders already thread `sort` into `__args`. Please update or remove the assumption so the spec doesn't send implementation in the wrong direction.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
| include_available: true, | ||
| ...(objectKind !== IP_ADDRESS_GENERIC ? { kinds: [objectKind] } : {}), | ||
| ...(filters ? addFiltersToRequest(filters) : {}), | ||
| ...(sort?.length ? addOrderByToRequest(sort) : {}), |
There was a problem hiding this comment.
P2: IPAM address results filtered to a prefix ignore the selected header sort whenever availability is included: the new order reaches the resolver, but availability expansion reorders all existing nodes by address. Sorting the combined existing/available result after expansion, while preserving pagination context, would keep header sorting effective.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/app/src/entities/ipam/ip-addresses/api/get-ip-address-list-from-api.ts, line 49:
<comment>IPAM address results filtered to a prefix ignore the selected header sort whenever availability is included: the new `order` reaches the resolver, but availability expansion reorders all existing nodes by address. Sorting the combined existing/available result after expansion, while preserving pagination context, would keep header sorting effective.</comment>
<file context>
@@ -42,6 +46,7 @@ export function getIpAddressListWithAvailabilityGraphQLQuery({
include_available: true,
...(objectKind !== IP_ADDRESS_GENERIC ? { kinds: [objectKind] } : {}),
...(filters ? addFiltersToRequest(filters) : {}),
+ ...(sort?.length ? addOrderByToRequest(sort) : {}),
},
edges: {
</file context>
| @@ -0,0 +1,76 @@ | |||
| # Research: Column-Header Sort & Filter Menu | |||
There was a problem hiding this comment.
P3: This R5 note is stale: both IPAM GraphQL builders already thread sort into __args. Please update or remove the assumption so the spec doesn't send implementation in the wrong direction.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/specs/002-header-sort-menu/research.md, line 59:
<comment>This R5 note is stale: both IPAM GraphQL builders already thread `sort` into `__args`. Please update or remove the assumption so the spec doesn't send implementation in the wrong direction.</comment>
<file context>
@@ -0,0 +1,76 @@
+
+## R5 — IPAM sort wiring (verified missing)
+
+**Decision**: IPAM IP-address and IP-prefix tables currently pass **no** order argument: `__args` in `get-ip-address-list-from-api.ts:39-45` / `get-ip-prefix-list-from-api.ts` contain only `limit, offset, include_available, kinds, filters`; use-cases and query hooks accept no sort; tables don't call `useSort`. Wire them the same way the object list is wired (`object-table.tsx:11-25` → `get-objects-from-api.ts:51`):
+1. API files: accept `sort` and spread `addOrderByToRequest(sort)` (`shared/api/graphql/utils.ts:169`) into `__args`.
+2. Use-cases: accept and thread a `sort: Sort[]` param.
</file context>
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 4 unresolved issues from previous reviews.
Re-trigger cubic
6e41e10 to
69ec32d
Compare
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 4 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/app/src/entities/nodes/sort/ui/hooks/use-sortable-fields.ts">
<violation number="1" location="frontend/app/src/entities/nodes/sort/ui/hooks/use-sortable-fields.ts:42">
P3: This product feature is currently targeting the stable release train, which can make it ship outside the normal minor-release flow. Please confirm the release vehicle or retarget the feature to `develop`; keep `stable` only if it must ship before the next minor.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| @@ -39,6 +39,7 @@ export function useSortableFields(schema: ModelSchema): SortableField[] { | |||
| .map((attribute) => ({ | |||
There was a problem hiding this comment.
P3: This product feature is currently targeting the stable release train, which can make it ship outside the normal minor-release flow. Please confirm the release vehicle or retarget the feature to develop; keep stable only if it must ship before the next minor.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/app/src/entities/nodes/sort/ui/hooks/use-sortable-fields.ts, line 42:
<comment>This product feature is currently targeting the stable release train, which can make it ship outside the normal minor-release flow. Please confirm the release vehicle or retarget the feature to `develop`; keep `stable` only if it must ship before the next minor.</comment>
<file context>
@@ -39,6 +39,7 @@ export function useSortableFields(schema: ModelSchema): SortableField[] {
.map((attribute) => ({
field: buildAttributeSortField(attribute.name),
label: attribute.label ?? attribute.name,
+ fieldSchema: attribute,
}));
</file context>
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 5 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 5 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 5 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 5 unresolved issues from previous reviews.
Re-trigger cubic
4f0f6a7 to
c39b58e
Compare
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 4 unresolved issues from previous reviews.
Re-trigger cubic
…2794] Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> docs(specs): add implementation plan and design artifacts for header sort menu [IFC-2794] Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> docs(specs): critique report + apply must-address fixes (IPAM scope, changelog gate) [IFC-2794] Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> docs(specs): add dependency-ordered tasks for header sort menu [IFC-2794] Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> docs(specs): baseline suites verified for header sort menu [IFC-2794] Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-2794] Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add pagination-preservation and disabled-header component tests, co-locate the relationship sort-field decode with its encoder, document setCustomSort clearing, and amend the contract scope for tables sharing the header without sort wiring. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ator [IFC-2794] The section wrapper added extra margin around the sort entries. Top-level direction items now mark the active direction with a check + sr-only text; the direction submenu keeps radio semantics via Menu-level selection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…FC-2794] Domain rules stay UI-agnostic: the rule matches a sort against a schema field, not a table column. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…C-2794] Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ship header submenu [IFC-2794] Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eld submenu [IFC-2794] Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
SortableField carries its source fieldSchema so the flat list can derive the kind/peer icon at the call site; metadata entries get the calendar. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…IFC-2794] activeFields stays optional; absence is handled at the filter sites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ration [IFC-2794] New test_object_header_sort.py and test_ip_prefix_list_sort.py; the object and IPAM filter specs now go through the header menu's Filter item. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…2794] Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> docs(dev): extract header-sort-menu spec into ADRs, knowledge, and guides [IFC-2794] Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> revert(dev): drop header-sort-menu extraction docs and un-archive spec [IFC-2794] Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…IFC-2794] Within-site order is a uuid tiebreaker, so only the site prefix is deterministic; the enum test anchors focus inside the popover before Escape, mirroring the python suite fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ac828af to
9bda295
Compare
pa-lem
left a comment
There was a problem hiding this comment.
Reviewed with parallel agents + manual testing. The core claims all check out in code: token-aware sort matching, replace/toggle-clear semantics, multi-field staying toolbar-only, IPAM wiring with query re-keying, cardinality gating. ?sort= is parsed and schema-allowlisted before reaching GraphQL. Tests are thorough and the TS/Python e2e mirrors are consistent. 👍
Also verified manually: sorting while deep in the list is fine — the infinite query refetches the loaded window so the result stays coherent (just a brief skeleton flash). No change needed there.
1 blocker + 1 decision + 3 small items inline. Nits, take or leave:
willResetsToDefault→willResetToDefault(sort-editor.tsx)- relationship "Sort by" submenu omits
variant="picker"unlike the direction submenu (peerSchema.attributes ?? []).filter(isSortableAttribute)+ weight-sort repeated inuse-sortable-fields.ts/add-sort-picker.tsx/table-column-header.tsx— could be onegetSortablePeerAttributesruleseedSortInUrl/seedFiltersInUrlduplicated across two test files- PR description says
getColumnActiveSort; the actual export isfindSortForField
There was a problem hiding this comment.
0 issues found across 10 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 4 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 4 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
pa-lem
left a comment
There was a problem hiding this comment.
is it possible to completely remove the .specify/feature.json file?
other than that seems good
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove the draft PR description and the implement/critique run reports; these are session artifacts, not durable spec docs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Closes IFC-2794
Spec:
specs/002-header-sort-menu/(spec.md, plan.md, contracts/header-menu-ui.md)Summary
Column headers in object lists and IPAM IP address/prefix lists now open a react-aria menu instead of directly opening the per-column filter popover. The menu offers:
The header shows an ↑/↓ indicator when it drives the active user-applied sort (never for the schema default). Sort state is shared with the toolbar Sort control through the existing
?sort=URL param viauseSort, so both surfaces always agree and sorted views survive reload and link-sharing. Cardinality-many or unresolvable-peer relationship columns offer Filter… only; columns that are neither sortable nor filterable keep their plain non-interactive header.The IPAM IP-address and IP-prefix lists previously ignored sorting entirely; this PR wires
sortend-to-end (API call → use-case → query hook → table) so the shared header's sort actions work there too (release-blocking per spec Assumptions).User-facing interaction change
Clicking a column header previously opened the filter form directly. It now opens the menu, and filtering sits under Filter… — one click further. The filter experience itself is unchanged and remains fully consistent with the toolbar path: same form, same active-filter tags, same
?filters=URL, editable/removable from either place (spec Story 3).A Towncrier fragment covers this:
changelog/+header-sort-menu.changed.md.Implementation notes
getColumnActiveSort(customSort, columnSchema)(src/entities/nodes/sort/domain/rules/get-column-active-sort.ts) decides when a column drives the active sort — token-aware matching (split on__), so relationshipsitenever matches attributesite_code.TableColumnHeaderreworked from a Radix popover to@infrahub/uiMenuTrigger+Menu, with a controlled popover for the filter form (pattern donor: the toolbar sort picker).addOrderByToRequest(sort)into the GraphQL__args, mirroring the object-list wiring.Accepted migration debt
TableColumnHeaderlocation (critique E2): the component stays underentities/nodes/object/ui/object-table/while now also imported byentities/ipam/tables. Moving it to a shared location is deliberate follow-up work outside this PR's scope; the cross-entity import is the accepted interim state.specs/ifc-2428-filters(spec Assumptions): that Draft spec's FR-001b ("column headers are no longer clickable filter triggers") is softened, not contradicted — headers now reuse the unified filter flow as a second entry point. The Draft spec should be amended when next worked on;specs/002-header-sort-menu/spec.mdis the current source of truth for header behavior.IPAM Prefix-column caveat
On the IPAM prefix list, the prefix value renders in the identifier column, which has no header menu (
prefixis excluded from the list columns by design). Sorting the prefix list therefore goes through the other columns (e.g. Description), which is what the E2E spec exercises.Test coverage
get-column-active-sort.test.ts— attribute/relationship match, near-miss names, multi-field sort → null.table-column-header.test.tsx— menu structure, full-replace write, toggle-clear, active direction marked, non-sortable kinds, relationship submenu contents, cardinality-many/unresolvable peer → Filter… only, filter form pre-fill and state parity.objects/object-header-sort.spec.ts(sort, persistence, toggle-clear, toolbar replace semantics, relationship submenu incl. keyboard-only path) andipam/ip-prefix-list-sort.spec.ts;objects/object-filters.spec.tsextended with header-menu paths and explicit header/toolbar parity assertions;objects/object-sort.spec.ts(toolbar sort) passes unmodified.🤖 Generated with Claude Code
Summary by cubic
Adds a column‑header menu to sort and filter in object lists and IPAM IP address/prefix lists. Sorting stays in sync with the toolbar via
?sort=, persists on reload, and IPAM lists now support sorting end‑to‑end; implements IFC‑2794.New Features
useSort,?sort=); header sort replaces the whole sort, multi‑field remains toolbar‑only.Refactors
TableColumnHeaderrebuilt with@infrahub/uiMenuTrigger/Menu; propdisabled→isDisabled; sharedSortableFieldMenuItem,SortableAttributeMenuItem, andSortDirectionMenuItemwith icons across header and the sort picker.useSort.setCustomSortacceptsnullto clear; addedfindSortForField;getValidSortskeeps schema‑default sub‑property fields (e.g.prefix__version)..specify/feature.json.Written for commit 8512033. Summary will update on new commits.