diff --git a/.ci/migration-hashes.json b/.ci/migration-hashes.json index 37fe205ce..af08c317a 100644 --- a/.ci/migration-hashes.json +++ b/.ci/migration-hashes.json @@ -63,6 +63,7 @@ "057_principal_relationships.sql": "124f72d04f718f615ac665f47b8500479c84db7d39969159d55afd95e0525717", "058_rename_entra_app_role.sql": "b86da2c0c2b0fc2934affcab1298886b4e5d23644742b2c433abc08fc3d1c57f", "059_context_acyclic_trigger.sql": "b5cd241b6a5df8deb5834f56e020ba8e0be6ca02f324d8054f0042b6fd2eccd7", - "060_auth_role_change_log.sql": "a03e9ab49545a9e9d18b9fe9160f298205e640875cfeeb5cde2982570d4e4c8b" + "060_auth_role_change_log.sql": "a03e9ab49545a9e9d18b9fe9160f298205e640875cfeeb5cde2982570d4e4c8b", + "061_business_role_covers_itself.sql": "0dc53ad1e6691691d43a80d929b5f55a207b267831465ba8abb67569eb10e6f6" } } diff --git a/app/api/contract-tests/governedIntentGap.contract.test.js b/app/api/contract-tests/governedIntentGap.contract.test.js index 6bc23cd9c..ea1dced8f 100644 --- a/app/api/contract-tests/governedIntentGap.contract.test.js +++ b/app/api/contract-tests/governedIntentGap.contract.test.js @@ -91,3 +91,46 @@ describe('matrix matview — managedByAccessPackage from governance coverage', ( expect(rows).toHaveLength(0); }); }); + +// Migration 061: holding a business role is governed access, so the role's own +// row must be covered by the role itself — otherwise the matrix paints the role +// row ungoverned and the scope statistics count it as an ungoverned assignment. +describe('business-role coverage — a role covers its own membership row (061)', () => { + async function coverage(principalId) { + await pool.query(`REFRESH MATERIALIZED VIEW "vw_UserPermissionAssignmentViaBusinessRole"`); + const r = await pool.query( + `SELECT "resourceId", "businessRoleId" + FROM "vw_UserPermissionAssignmentViaBusinessRole" + WHERE "userId" = $1 + ORDER BY "resourceId"`, + [principalId], + ); + return r.rows; + } + + it('reports both the role itself and the resources it Contains', async () => { + await assign({ resourceId: BR, principalId: U_OK, governed: true }); + const rows = await coverage(U_OK); + expect(rows).toEqual(expect.arrayContaining([ + { resourceId: BR, businessRoleId: BR }, + { resourceId: G, businessRoleId: BR }, + ])); + expect(rows).toHaveLength(2); + }); + + it('leaves a subject who does not hold the role uncovered', async () => { + await assign({ resourceId: G, principalId: U_UN, governed: false }); + expect(await coverage(U_UN)).toEqual([]); + }); + + it('drops the self row again when the role assignment is soft-deleted', async () => { + await assign({ resourceId: BR, principalId: U_GAP, governed: true }); + expect(await coverage(U_GAP)).toHaveLength(2); + await pool.query( + `UPDATE "ResourceAssignments" SET "deletedAt" = now() + WHERE "resourceId" = $1 AND "principalId" = $2`, + [BR, U_GAP], + ); + expect(await coverage(U_GAP)).toEqual([]); + }); +}); diff --git a/app/api/src/db/migrations/061_business_role_covers_itself.sql b/app/api/src/db/migrations/061_business_role_covers_itself.sql new file mode 100644 index 000000000..26cb9a1d1 --- /dev/null +++ b/app/api/src/db/migrations/061_business_role_covers_itself.sql @@ -0,0 +1,59 @@ +-- 061: a business role also covers its own membership row +-- +-- "vw_UserPermissionAssignmentViaBusinessRole" answers one question: which +-- business role(s) account for this (subject, resource) cell? Migration 049 +-- built it purely from the `Contains` relationships, so it listed the resources +-- a role grants but never the role itself — even though holding a business role +-- IS governed access (a Direct assignment carrying governed=true, see 049). +-- +-- The business role is a resource row in the matrix like any other, so that +-- omission surfaced everywhere the view is the governed signal: +-- * the role's own row rendered ungoverned — no business-role colour on its +-- cells, and it disappeared from the Governed view entirely; +-- * the scope statistics counted every business-role membership as an +-- ungoverned assignment, understating the governed percentage; +-- * a role that grants no resources at all never appeared in the roll-ups. +-- +-- Fixed at the source: the view gains a self arm, so every consumer (matrix +-- colouring, the Governed filter, scope statistics, the roll-up builders) +-- agrees without any of them special-casing the role row. + +DROP MATERIALIZED VIEW IF EXISTS "vw_UserPermissionAssignmentViaBusinessRole" CASCADE; +CREATE MATERIALIZED VIEW "vw_UserPermissionAssignmentViaBusinessRole" AS +-- Arm 1 (unchanged): the resources a governance resource Contains. +SELECT + bru."principalId" AS "userId", + rr."childResourceId" AS "groupId", + rr."childResourceId" AS "resourceId", + rr."parentResourceId" AS "businessRoleId" +FROM "ResourceRelationships" rr +JOIN "Resources" gov + ON gov.id = rr."parentResourceId" AND gov."governanceResource" +JOIN "ResourceAssignments" bru + ON bru."resourceId" = rr."parentResourceId" + AND bru."principalId" IS NOT NULL + AND bru."deletedAt" IS NULL +WHERE rr."relationshipType" = 'Contains' +UNION +-- Arm 2 (new): the governance resource covers its own membership cell. +SELECT + bru."principalId" AS "userId", + gov.id AS "groupId", + gov.id AS "resourceId", + gov.id AS "businessRoleId" +FROM "Resources" gov +JOIN "ResourceAssignments" bru + ON bru."resourceId" = gov.id + AND bru."principalId" IS NOT NULL + AND bru."deletedAt" IS NULL +WHERE gov."governanceResource" +WITH NO DATA; + +CREATE UNIQUE INDEX "ix_vw_UPABR_pk" + ON "vw_UserPermissionAssignmentViaBusinessRole" ("userId", "groupId", "businessRoleId"); +CREATE INDEX "ix_vw_UPABR_userId" + ON "vw_UserPermissionAssignmentViaBusinessRole" ("userId"); +CREATE INDEX "ix_vw_UPABR_groupId" + ON "vw_UserPermissionAssignmentViaBusinessRole" ("groupId"); + +REFRESH MATERIALIZED VIEW "vw_UserPermissionAssignmentViaBusinessRole"; diff --git a/app/api/src/db/migrations/061_business_role_covers_itself.test.js b/app/api/src/db/migrations/061_business_role_covers_itself.test.js new file mode 100644 index 000000000..1ccec6929 --- /dev/null +++ b/app/api/src/db/migrations/061_business_role_covers_itself.test.js @@ -0,0 +1,46 @@ +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'fs'; +import { join, dirname } from 'path'; +import { fileURLToPath } from 'url'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const sql = readFileSync(join(__dirname, '061_business_role_covers_itself.sql'), 'utf8'); + +// Text-level guard, mirroring 054's/058's. The behavioural proof (a real +// Postgres returning the self row) lives in +// app/api/contract-tests/governedIntentGap.contract.test.js. +describe('migration 061 — a business role covers its own membership row', () => { + it('rebuilds the business-role coverage matview', () => { + expect(sql).toMatch( + /DROP MATERIALIZED VIEW IF EXISTS "vw_UserPermissionAssignmentViaBusinessRole" CASCADE/, + ); + expect(sql).toMatch( + /CREATE MATERIALIZED VIEW "vw_UserPermissionAssignmentViaBusinessRole"/, + ); + }); + + it('keeps the Contains arm and adds a self arm', () => { + expect(sql).toContain(`WHERE rr."relationshipType" = 'Contains'`); + // The self arm reports the governance resource as its own role AND resource. + expect(sql).toMatch(/gov\.id\s+AS "resourceId",\s*\n\s*gov\.id\s+AS "businessRoleId"/); + expect(sql.match(/^UNION$/m)).not.toBeNull(); + }); + + it('counts only effective assignments — soft-deleted ones stay out of both arms', () => { + const guards = sql.match(/bru\."deletedAt" IS NULL/g); + expect(guards).toHaveLength(2); + }); + + it('restricts the self arm to governance resources', () => { + expect(sql).toContain(`WHERE gov."governanceResource"`); + }); + + it('recreates the indexes the matview is queried through and populates it', () => { + for (const ix of ['ix_vw_UPABR_pk', 'ix_vw_UPABR_userId', 'ix_vw_UPABR_groupId']) { + expect(sql).toContain(`"${ix}"`); + } + expect(sql).toMatch( + /REFRESH MATERIALIZED VIEW "vw_UserPermissionAssignmentViaBusinessRole"/, + ); + }); +}); diff --git a/app/api/src/matrix/scopeHistory.js b/app/api/src/matrix/scopeHistory.js index 29d50f33f..00543ddb0 100644 --- a/app/api/src/matrix/scopeHistory.js +++ b/app/api/src/matrix/scopeHistory.js @@ -258,7 +258,14 @@ export function buildScopeAsofSql({ filter, principalColSet, resourceColSet, con -- assignment type, from 049 on as a normal Direct membership on a resource -- flagged governanceResource. The derived governed rows are not history -- tracked, so coverage is reconstructed from membership and Contains facts. + -- + -- Two arms, mirroring "vw_UserPermissionAssignmentViaBusinessRole" (049 + + -- 061) so the as-of numbers use the same definition of governed as the live + -- scope statistics. Without arm 2 the history path reported every + -- business-role membership row as ungoverned while the live path counted it, + -- so the latest timeseries point disagreed with live scope-stats. coverage AS ( + -- Arm 1: the resources a governance resource Contains. SELECT DISTINCT ga.pid AS "userId", rr.child AS "groupId" FROM asof_assign ga JOIN asof_contains rr ON rr.parent = ga.rid @@ -268,6 +275,16 @@ export function buildScopeAsofSql({ filter, principalColSet, resourceColSet, con WHERE (ar.state->>'id') = ga.rid AND COALESCE((ar.state->>'governanceResource')::boolean, false) ) + UNION + -- Arm 2: the governance resource covers its own membership cell — + -- holding a business role IS governed access. + SELECT DISTINCT ga.pid AS "userId", ga.rid AS "groupId" + FROM asof_assign ga + WHERE EXISTS ( + SELECT 1 FROM asof_resources ar + WHERE (ar.state->>'id') = ga.rid + AND COALESCE((ar.state->>'governanceResource')::boolean, false) + ) ), sp AS ( SELECT (sp.state->>'id')::uuid AS id diff --git a/app/api/src/matrix/scopeHistory.test.js b/app/api/src/matrix/scopeHistory.test.js index 4a12f2222..fb6a2ecd5 100644 --- a/app/api/src/matrix/scopeHistory.test.js +++ b/app/api/src/matrix/scopeHistory.test.js @@ -65,6 +65,26 @@ describe('buildScopeAsofSql', () => { expect(scopeMode).toBe('attribute'); }); + // The live scope statistics read vw_UserPermissionAssignmentViaBusinessRole, + // which since migration 061 has a second arm: holding a business role is + // itself governed access. The as-of path rebuilds that definition in SQL + // rather than reading the view, so it needs the same arm — without it the + // latest timeseries point reported a lower governed count than live + // scope-stats for the very same instant. + it('counts a governance resource membership as governed in its own right (061 parity)', () => { + const { sql } = build(EMPTY); + const coverage = sql.slice(sql.indexOf('coverage AS ('), sql.indexOf('sp AS (')); + + // Arm 1 stays: coverage via the Contains relationship. + expect(coverage).toContain('asof_contains'); + // Arm 2: the role's own cell, keyed on the assignment's own resource + // (ga.rid) rather than a Contains child. + expect(coverage).toMatch(/UNION/); + expect(coverage).toMatch(/ga\.rid AS "groupId"/); + // Both arms gate on the resource being a governance resource. + expect(coverage.match(/governanceResource/g)).toHaveLength(2); + }); + it('excludes group-shaped principals from the subject count', () => { const { sql } = build(EMPTY); expect(sql).toContain('#microsoft.graph.group'); diff --git a/app/ui/e2e/export-validation.spec.js b/app/ui/e2e/export-validation.spec.js index c51dea802..167723ac0 100644 --- a/app/ui/e2e/export-validation.spec.js +++ b/app/ui/e2e/export-validation.spec.js @@ -88,9 +88,13 @@ test.describe('Matrix Excel export — access-package columns match the grid', ( for (const tr of table.querySelectorAll('tbody tr')) { const tds = [...tr.querySelectorAll('td')]; if (tds.length !== headers.length) continue; // spacer / message row - // The name cell carries an expander glyph for expandable rows; the - // export writes the bare display name. - const resource = (tds[1]?.innerText || '').replace(/^[▶▼]\s*/, '').trim(); + // The export writes the bare display name, but the name cell also holds + // the fold/expand toggles, the nesting elbow and the "BR" overlap chips + // — so read the name element itself and only fall back to scraping the + // cell (older rows, and the rotated views) when it isn't there. + const nameEl = tds[1]?.querySelector('[data-row-name]'); + const resource = (nameEl?.textContent ?? tds[1]?.innerText ?? '') + .replace(/^[▶▼└\s]+/, '').trim(); const type = (tds[headers.length - 2]?.innerText || '').trim(); const cells = []; for (const col of apCols) { diff --git a/app/ui/e2e/matrix.spec.js b/app/ui/e2e/matrix.spec.js index af0fa5513..c8d642da2 100644 --- a/app/ui/e2e/matrix.spec.js +++ b/app/ui/e2e/matrix.spec.js @@ -112,6 +112,520 @@ test.describe('Matrix View', () => { }); }); +// ─── Folding a business role's resources away (#370) ────────────────────────── +// +// A business role row can hide the rows of the resources it grants, so the grid +// reduces to "business roles + resources no role covers". Fold state is pure +// view state and sticks per matrix. These run against the demo dataset; when it +// holds no business role that grants a visible resource there is no fold +// affordance at all (the zero case), and the tests skip rather than fail. +test.describe('Matrix — fold business-role resources', () => { + test.setTimeout(90000); + + const ALL_DATA_FILTER = { + rowType: 'principal', + orientation: 'rows-as-resources', + subject: { include: [], exclude: [] }, + resource: { include: [], exclude: [] }, + }; + const matrixUrl = (filter) => '/#matrix?filter=' + encodeURIComponent(JSON.stringify(filter)); + + // Load a matrix slice in a fresh document. The app reads `?filter=` once, at + // mount — the matrix hash is a share/bookmark entry point, not a live route — + // so a hash-only change would leave the previous slice on screen. Going via + // about:blank forces the real navigation (localStorage survives it, which is + // what the fold-persistence assertions rely on). + async function gotoSlice(page, filter) { + await page.goto('about:blank'); + await page.goto(matrixUrl(filter)); + await page.waitForLoadState('networkidle'); + } + + // Open the all-data matrix. Returns false when no grid renders (no data here). + async function openGrid(page) { + await gotoSlice(page, ALL_DATA_FILTER); + try { + await expect(page.locator('table').first()).toBeVisible({ timeout: 40000 }); + } catch { + return false; + } + await page.waitForTimeout(1000); // let the virtualizer settle + return true; + } + + const foldAll = (page) => page.getByRole('button', { name: 'Fold roles', exact: true }); + const unfoldAll = (page) => page.getByRole('button', { name: 'Unfold roles', exact: true }); + + // Total height of the (virtualised) row list — it shrinks when rows fold away. + const rowsHeight = (page) => page.evaluate(() => { + const tbody = document.querySelector('table tbody'); + return tbody ? Math.round(tbody.getBoundingClientRect().height) : 0; + }); + + // Value shown in a scope-statistics tile ("Resources", "Assignments"). Each + // tile is a group named after its metric; the number is its first span. Waits + // out the em-dash placeholder the tile shows until scope-stats has loaded. + async function statValue(page, label) { + const value = page.getByRole('group', { name: label, exact: true }).locator('span').first(); + await expect(value).not.toHaveText('—', { timeout: 30000 }); + return value.innerText(); + } + + async function openFoldableGrid(page) { + const rendered = await openGrid(page); + test.skip(!rendered, 'matrix grid did not render (no data) — cannot exercise the fold'); + const foldable = await foldAll(page).count(); + test.skip(foldable === 0, 'no business role grants a visible resource in this dataset'); + } + + test('"Fold roles" hides the resources roles grant, "Unfold roles" restores them', async ({ page }) => { + await openFoldableGrid(page); + + const before = await rowsHeight(page); + await foldAll(page).click(); + await expect(unfoldAll(page)).toBeVisible(); + await expect.poll(() => rowsHeight(page)).toBeLessThan(before); + // The folded roles say how many rows they took with them. + await expect(page.getByText(/\d+ resources? folded/).first()).toBeVisible(); + + await unfoldAll(page).click(); + await expect.poll(() => rowsHeight(page)).toBe(before); + await expect(unfoldAll(page)).toHaveCount(0); + }); + + test('a per-role chevron folds only that role, and is labelled for screen readers', async ({ page }) => { + await openFoldableGrid(page); + + const chevron = page.getByRole('button', { name: 'Fold business role resources' }).first(); + await expect(chevron).toBeVisible(); + + const before = await rowsHeight(page); + await chevron.click(); + await expect.poll(() => rowsHeight(page)).toBeLessThan(before); + // The same control now offers the reverse action. + const unfoldOne = page.getByRole('button', { name: 'Unfold business role resources' }).first(); + await expect(unfoldOne).toBeVisible(); + await unfoldOne.click(); + await expect.poll(() => rowsHeight(page)).toBe(before); + }); + + test('folding changes no number in the scope-statistics panel', async ({ page }) => { + await openFoldableGrid(page); + + const before = { + resources: await statValue(page, 'Resources'), + assignments: await statValue(page, 'Assignments'), + }; + await foldAll(page).click(); + await expect(unfoldAll(page)).toBeVisible(); + + expect(await statValue(page, 'Resources')).toBe(before.resources); + expect(await statValue(page, 'Assignments')).toBe(before.assignments); + }); + + test('fold state is restored when the same matrix is re-opened', async ({ page }) => { + await openFoldableGrid(page); + + await foldAll(page).click(); + await expect(unfoldAll(page)).toBeVisible(); + const folded = await rowsHeight(page); + + await page.reload(); + await page.waitForLoadState('networkidle'); + await expect(page.locator('table').first()).toBeVisible({ timeout: 40000 }); + await expect(unfoldAll(page)).toBeVisible({ timeout: 20000 }); + await expect.poll(() => rowsHeight(page)).toBe(folded); + + // A different matrix slice keeps its own (expanded) state. + await gotoSlice(page, { ...ALL_DATA_FILTER, rowType: 'identity' }); + await expect(unfoldAll(page)).toHaveCount(0); + + // Leave the browser profile clean for the next test. + await gotoSlice(page, ALL_DATA_FILTER); + if (await unfoldAll(page).count()) await unfoldAll(page).click(); + }); + + // The resources a role grants hang under it with the same indent + elbow an + // expanded nested group uses, so "what is in this role" reads off the grid. + const childElbows = (page) => page.locator('tbody tr td span', { hasText: /^└$/ }).count(); + + test('a role\'s resources hang under it as child rows, and go with it when it folds', async ({ page }) => { + await openFoldableGrid(page); + + const before = await childElbows(page); + test.skip(before === 0, 'no role and one of its resources are on screen together here'); + + await foldAll(page).click(); + await expect(unfoldAll(page)).toBeVisible(); + await expect.poll(() => childElbows(page)).toBeLessThan(before); + + await unfoldAll(page).click(); + await expect.poll(() => childElbows(page)).toBe(before); + }); + + test('a folded role counts the access it hides but does not account for', async ({ page }) => { + await openFoldableGrid(page); + + // Nothing is folded yet, so the marker cannot be on screen. + const marker = page.locator('tbody span[title*="does not account for"]'); + await expect(marker).toHaveCount(0); + + await foldAll(page).click(); + await expect(unfoldAll(page)).toBeVisible(); + + if (await marker.count()) { + // Every marker states a count of ungoverned assignments it stands for. + await expect(marker.first()).toHaveText(/^[1-9]\d*$/); + await expect(marker.first()).toHaveAttribute( + 'title', /assignments? on the folded resources that this business role does not account for/, + ); + } + await unfoldAll(page).click(); + await expect(marker).toHaveCount(0); + }); + + // Requestor feedback on #370: the grid showed only over-granting. The demo + // dataset's BR-Service-Desk carries both directions (see DemoRoleDrift.ps1). + test('a folded role counts what it assigns that the subject does not have', async ({ page }) => { + await openFoldableGrid(page); + + const marker = page.locator('tbody span[title*="does not have"]'); + await expect(marker).toHaveCount(0); + + await foldAll(page).click(); + await expect(unfoldAll(page)).toBeVisible(); + + if (await marker.count()) { + await expect(marker.first()).toHaveText(/^[1-9]\d*$/); + await expect(marker.first()).toHaveAttribute( + 'title', /assignments? on the folded resources that this business role assigns but this subject does not have/, + ); + } + await unfoldAll(page).click(); + await expect(marker).toHaveCount(0); + }); + + test('marks a standing membership where the role only grants eligibility', async ({ page }) => { + await openFoldableGrid(page); + + // Unfolded, the deviation sits on the resource's own cell. + const overGrant = page.locator('tbody span[title*="More than the business role assigns"]'); + if (await overGrant.count()) { + await expect(overGrant.first()).toHaveText('+'); + await expect(overGrant.first()).toHaveAttribute('title', /just-in-time/); + } + }); + + // Requestor feedback on #370: BR-Engineering-Tools grants SG-VPN-Access, and + // the two SysAdmins hold that group without holding the role. Folded, the + // role counted them in red; unfolded, the very same cells said nothing at all. + test('a membership held outside the role that grants it is marked on the resource row', async ({ page }) => { + await openFoldableGrid(page); + + // The demo dataset puts SG-VPN-Access near the top of the grid; wait for the + // virtualizer to paint it before concluding the dataset has no such case. + const outside = page.locator('tbody span[title*="Held outside"]'); + const present = await outside.first().waitFor({ state: 'attached', timeout: 15000 }) + .then(() => true, () => false); + test.skip(!present, 'no visible row in this dataset is held outside the business role that grants it'); + + await expect(outside.first()).toHaveText(/^[1-9]\d*$/); + // The marker reports what it evaluated — the granting role's assignments — + // and never asserts a role membership it did not check (requestor feedback + // on #370). + await expect(outside.first()).toHaveAttribute( + 'title', /^⚠ Held outside business-role governance: (no business role assigns this resource to this subject|this subject holds a business role that grants this resource)/, + ); + await expect(outside.first()).toHaveAttribute( + 'title', /It is granted by (business role .+|\d+ business roles.*), (which carries no assignment|none of which carries an assignment) of it for this subject\.$/, + ); + // The marker explains the access — it never replaces it, so the badge stays. + await expect(page.locator('tbody td:has(span[title*="Held outside"])').first()) + .toContainText(/[DIE]/); + + // Folding the roles takes those rows away, and the same finding reappears as + // the folded role's own red count — the statement is never lost. + await foldAll(page).click(); + await expect(unfoldAll(page)).toBeVisible(); + await expect(outside).toHaveCount(0); + const foldedCount = page.locator('tbody span[title*="does not account for"]'); + if (await foldedCount.count()) await expect(foldedCount.first()).toHaveText(/^[1-9]\d*$/); + + await unfoldAll(page).click(); + await expect.poll(() => outside.count()).toBeGreaterThan(0); + }); + + // The exact cell the requestor checked: SG-VPN-Access under + // BR-Engineering-Tools. The old tooltip closed on "the subject does not hold + // that role" — a claim about role membership the marker never established, + // and one the requestor read (correctly) as wrong. What it did establish is + // that no business role carries an assignment of this resource for the + // subject, and that is all it may say. + test('the held-outside marker reports the missing role assignment, not a missing role', async ({ page }) => { + await openFoldableGrid(page); + + const vpnRow = page.locator('tbody tr').filter({ has: page.getByTitle(/^SG-VPN-Access/) }); + const present = await vpnRow.first().waitFor({ state: 'attached', timeout: 15000 }) + .then(() => true, () => false); + test.skip(!present, 'SG-VPN-Access is not part of this matrix slice'); + + const marker = vpnRow.first().locator('span[title*="Held outside"]'); + test.skip(await marker.count() === 0, 'nobody in this slice holds SG-VPN-Access outside the role that grants it'); + + const title = await marker.first().getAttribute('title'); + expect(title).toContain('carries no assignment of it for this subject'); + expect(title).toContain('BR-Engineering-Tools'); + // The old wording, which asserted a role membership the marker never checked. + expect(title).not.toContain('does not hold'); + }); + + // Requestor feedback on #370: what happens to a group / app role that two + // business roles grant. The demo dataset's BR-Service-Desk and + // BR-IT-Operations share exactly that (see DemoSharedGrants.ps1). + test('a resource two roles grant has a row under each of them', async ({ page }) => { + await openFoldableGrid(page); + + // Find a resource granted by two roles that both have a row — the scenario. + const pairs = await (await page.request.get('/api/access-package-groups')).json(); + const byResource = new Map(); + for (const r of pairs) { + if (!r.resourceId) continue; + const key = String(r.resourceId).toUpperCase(); + if (!byResource.has(key)) byResource.set(key, []); + byResource.get(key).push(r); + } + const shared = [...byResource.values()].find(rows => rows.length > 1); + test.skip(!shared, 'no resource in this dataset is granted by more than one business role'); + + // Fold everything first: with only the role rows left the grid is short, so + // the virtualizer paints every row and the counts below are exact. + await foldAll(page).click(); + await expect(unfoldAll(page)).toBeVisible(); + // Ownership resources are named after their group, so pin the rows to the + // ones that actually carry the "granted by a business role" tooltip. + const nameCellSelector = + `td[title^="${shared[0].resourceName}"][title*="Granted by business role:"]`; + const sharedRows = page.locator(`tbody tr:has(${nameCellSelector})`); + await expect(sharedRows).toHaveCount(0); + + // Unfold the roles that grant it, one at a time: each one brings its own row + // for the shared resource. + const roleRow = (name) => page.locator('tbody tr', { hasText: name }) + .filter({ has: page.getByRole('button', { name: 'Unfold business role resources' }) }).first(); + for (const [i, pair] of shared.entries()) { + const row = roleRow(pair.accessPackageName); + test.skip(await row.count() === 0, `the ${pair.accessPackageName} row is not rendered in this grid`); + await row.getByRole('button', { name: 'Unfold business role resources' }).click(); + await expect(sharedRows).toHaveCount(i + 1); + } + + // Every one of those rows names all the granting roles, and carries the + // "BR+N" chip that points at the others. + const nameCell = page.locator(`tbody ${nameCellSelector}`).first(); + for (const pair of shared) { + await expect(nameCell) + .toHaveAttribute('title', new RegExp(`Granted by business role:.*${pair.accessPackageName}`, 's')); + } + await expect(sharedRows.first().locator('button[title^="Also granted by business role:"]')) + .toHaveText(/^BR(\+\d+)?$/); + + // Folding one of them takes away only that role's copy; the other stays. + await page.locator('tbody tr', { hasText: shared[0].accessPackageName }) + .filter({ has: page.getByRole('button', { name: 'Fold business role resources' }) }).first() + .getByRole('button', { name: 'Fold business role resources' }).click(); + await expect(sharedRows).toHaveCount(shared.length - 1); + // A fold always takes exactly what its role grants, so the chip never hedges. + await expect(page.getByText(/\d+ of \d+ resources folded/)).toHaveCount(0); + await expect(page.getByText(/\d+ resources? folded/).first()).toBeVisible(); + + // Leave the browser profile clean for the next test. + await unfoldAll(page).click(); + await expect(unfoldAll(page)).toHaveCount(0); + }); + + // A resource lives under the role(s) that grant it, whatever order the rows + // were saved in — so it can never be orphaned from its role. + test('a resource stays under its business role whatever the saved row order', async ({ page }) => { + await openFoldableGrid(page); + + // Every resource a role grants answers "which role?" from its row tooltip. + const named = page.locator('tbody td[title*="Granted by business role:"]'); + await expect.poll(() => named.count()).toBeGreaterThan(0); + + // Persist a row order the way the drag handle does — clicking the "#" + // header sorts by member count, which writes the current ids to storage. + await page.locator('th[title="Sort by member count (descending)"]').click(); + + // Then move one role-granted resource to the very top, above every role row. + const pairs = await (await page.request.get('/api/access-package-groups')).json(); + const moved = await page.evaluate((rows) => { + const key = Object.keys(localStorage).find(k => k.startsWith('fgraph-roworder-')); + if (!key) return null; + const saved = JSON.parse(localStorage.getItem(key)); + const order = saved.order.map(id => String(id).toUpperCase()); + // A pair whose role AND resource both have a row — only then is there a + // role in the grid to file it under. + const pair = rows.find(r => r.resourceId + && order.includes(String(r.accessPackageId).toUpperCase()) + && order.includes(String(r.resourceId).toUpperCase())); + if (!pair) return null; + const child = saved.order.find(id => String(id).toUpperCase() === String(pair.resourceId).toUpperCase()); + localStorage.setItem(key, JSON.stringify({ + ...saved, + order: [child, ...saved.order.filter(id => id !== child)], + })); + return { role: pair.accessPackageName, resource: pair.resourceName }; + }, pairs); + test.skip(!moved, 'no business role and one of its resources share this grid'); + + await page.reload(); + await page.waitForLoadState('networkidle'); + await expect(page.locator('table').first()).toBeVisible({ timeout: 40000 }); + + // The row is drawn under its role again — indented, with the elbow — and + // still names it in the tooltip. + const row = page.locator( + `tbody tr:has(td[title^="${moved.resource}"][title*="Granted by business role:"])`).first(); + await expect(row).toBeVisible({ timeout: 20000 }); + await expect(row.locator('td span', { hasText: /^└$/ }).first()).toBeVisible(); + await expect(page.locator(`tbody td[title*="Granted by business role:"][title*="${moved.role}"]`).first()) + .toBeVisible(); + + // Leave the browser profile clean for the next test. + await page.evaluate(() => { + for (const k of Object.keys(localStorage)) { + if (k.startsWith('fgraph-roworder-')) localStorage.removeItem(k); + } + }); + }); + + // Requestor feedback on #370: the white "covered by N business roles" bubble + // was drawn over the labels of the cells around it. Every marker now lives in + // a strip the cell reserves for it, so no marker can reach another cell — or + // the badge underneath it. + test('no cell marker is drawn over another label', async ({ page }) => { + await openFoldableGrid(page); + // Fold the roles so the deviation counts are on screen alongside the + // role-count bubbles and the gap markers — the busiest the grid ever gets. + await foldAll(page).click(); + await expect(unfoldAll(page)).toBeVisible(); + + const overlaps = await page.evaluate(() => { + const markers = [...document.querySelectorAll('tbody td span.absolute > span')] + .filter(s => s.textContent.trim() !== ''); + const escaped = []; + const inside = (m, cell) => { + const a = m.getBoundingClientRect(); + const b = cell.getBoundingClientRect(); + // 1px of slack for sub-pixel rounding and collapsed borders. + return a.left >= b.left - 1 && a.right <= b.right + 1 + && a.top >= b.top - 1 && a.bottom <= b.bottom + 1; + }; + for (const m of markers) { + const cell = m.closest('td'); + if (!inside(m, cell)) escaped.push({ text: m.textContent, cls: m.className }); + // The badge row starts below the strip, so a marker can never sit on it. + for (const badge of cell.querySelectorAll(':scope > span:not(.absolute)')) { + const a = m.getBoundingClientRect(); + const b = badge.getBoundingClientRect(); + const hit = a.left < b.right - 1 && a.right > b.left + 1 + && a.top < b.bottom - 1 && a.bottom > b.top + 1; + if (hit) escaped.push({ text: m.textContent, over: badge.textContent }); + } + } + return { count: markers.length, escaped }; + }); + + expect(overlaps.count, 'the folded grid must show at least one marker').toBeGreaterThan(0); + expect(overlaps.escaped).toEqual([]); + + await unfoldAll(page).click(); + }); +}); + +// ─── Resizing the matrix (#370) ─────────────────────────────────────────────── +// +// The measured "fit the rest of the window" height is a default, not a verdict: +// how much of the window the grid deserves next to the panels above it is the +// analyst's call. The grip under the grid makes it theirs, and remembers it. +test.describe('Matrix — resizing the grid height', () => { + test.setTimeout(90000); + + const gridHeight = (page) => page.evaluate(() => { + const el = document.querySelector('div[style*="max-height"]'); + return el ? Math.round(el.getBoundingClientRect().height) : 0; + }); + + const grip = (page) => page.getByRole('button', { name: 'Resize the matrix height' }); + + async function openMatrix(page) { + const filter = { + rowType: 'principal', + orientation: 'rows-as-resources', + subject: { include: [], exclude: [] }, + resource: { include: [], exclude: [] }, + }; + await page.goto('about:blank'); + await page.setViewportSize({ width: 1280, height: 900 }); + await page.goto('/#matrix?filter=' + encodeURIComponent(JSON.stringify(filter))); + await page.waitForLoadState('networkidle'); + try { + await expect(page.locator('table').first()).toBeVisible({ timeout: 40000 }); + } catch { + return false; + } + // Collapse "How to read this matrix" so the chrome leaves the grid a + // measurable cap to start from (the same setup the scrollbar spec uses). + const legend = page.getByRole('button', { name: /How to read this matrix/i }).first(); + await expect(legend).toBeVisible({ timeout: 20000 }); + if (await legend.getAttribute('aria-expanded') === 'true') await legend.click(); + await page.waitForTimeout(1500); // let the measuring effect settle + return true; + } + + test('dragging the grip resizes the grid, and the height is remembered', async ({ page }) => { + test.skip(!await openMatrix(page), 'matrix grid did not render (no data)'); + + const before = await gridHeight(page); + const box = await grip(page).boundingBox(); + await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2); + await page.mouse.down(); + await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2 - 200, { steps: 10 }); + await page.mouse.up(); + + await expect.poll(() => gridHeight(page)).toBeLessThan(before); + const shrunk = await gridHeight(page); + + // Still exactly one scroller — a resized grid is not a broken layout. + expect(await page.evaluate(() => { + const de = document.documentElement; + return de.scrollHeight - de.clientHeight > 4; + })).toBe(false); + + // The choice survives a reload. + await page.reload(); + await page.waitForLoadState('networkidle'); + await expect(page.locator('table').first()).toBeVisible({ timeout: 40000 }); + await expect.poll(() => gridHeight(page)).toBe(shrunk); + + // "Fit to window" hands the decision back to the measured fit. + await page.getByRole('button', { name: 'Fit to window' }).click(); + await expect.poll(() => gridHeight(page)).toBe(before); + await expect(page.getByRole('button', { name: 'Fit to window' })).toHaveCount(0); + }); + + test('the arrow keys resize it too, so the grip is not mouse-only', async ({ page }) => { + test.skip(!await openMatrix(page), 'matrix grid did not render (no data)'); + + const before = await gridHeight(page); + await grip(page).focus(); + await page.keyboard.press('ArrowUp'); + await expect.poll(() => gridHeight(page)).toBeLessThan(before); + await page.keyboard.press('Escape'); + await expect.poll(() => gridHeight(page)).toBe(before); + }); +}); + // ─── Contexts column (#870) ──────────────────────────────────────────────────── // // Each resource row carries the Contexts it belongs to (group category, tags, @@ -415,18 +929,24 @@ test.describe('Matrix — adjust without changing anything', () => { // the chrome height. The real chrome (auth banner + scope-statistics + "How to // read") is taller, so the grid was too tall and the PAGE got a second // scrollbar next to the grid's own (measured ~310px page overflow). The fix -// measures the remaining viewport and caps the grid to fit, so only the grid -// scrolls. This test renders a tall grid (all data) and asserts the page itself -// does not overflow. +// measures the space really left below the grid and caps it to fit — and when +// less than a usable grid is left, drops the cap so the page is the single +// scroller instead. Either way exactly one of the two scrolls. test.describe('Matrix — no double scrollbar', () => { test.setTimeout(90000); - test('the page does not scroll when the grid does (only one scrollbar)', async ({ page }) => { - // A viewport tall enough that the fix has room (it floors the grid at 240px), - // but where a full-data grid is far taller than the space left for it — so - // the OLD fixed cap would push the page past the viewport. - await page.setViewportSize({ width: 1280, height: 800 }); + // Measures which of the two scrolls: the grid (internally) or the page. + const readScrollState = (page) => page.evaluate(() => { + const de = document.documentElement; + const gridScrolls = [...document.querySelectorAll('div')].some((el) => { + const s = getComputedStyle(el); + return /auto|scroll/.test(s.overflowY) && el.scrollHeight > el.clientHeight + 2; + }); + // A few px of slack for sub-pixel rounding. + return { pageScrolls: de.scrollHeight - de.clientHeight > 4, gridScrolls }; + }); + async function openFullMatrix(page) { // Apply an all-data filter directly via the hash (resources as rows → many // rows → a grid taller than the viewport). Bypasses the wizard. const filter = { @@ -441,32 +961,41 @@ test.describe('Matrix — no double scrollbar', () => { // Need the grid to actually render. If it doesn't (no demo data in this // environment), the scrollbar path can't be exercised — skip rather than // fail on an unrelated data condition. - const table = page.locator('table').first(); try { - await expect(table).toBeVisible({ timeout: 40000 }); + await expect(page.locator('table').first()).toBeVisible({ timeout: 40000 }); } catch { - test.skip(true, 'matrix grid did not render (no data) — cannot exercise the scrollbar path'); - return; + return false; } await page.waitForTimeout(1500); // let the height-measuring effect settle + return true; + } - const m = await page.evaluate(() => { - const de = document.documentElement; - // The grid is the lone vertical-overflow scroll container. - const gridScrolls = [...document.querySelectorAll('div')].some((el) => { - const s = getComputedStyle(el); - return /auto|scroll/.test(s.overflowY) && el.scrollHeight > el.clientHeight + 2; - }); - return { pageOverflow: de.scrollHeight - de.clientHeight, gridScrolls }; - }); + test('the grid and the page never scroll at the same time', async ({ page }) => { + // Short viewport + the "How to read this matrix" panel open: the chrome eats + // most of the window, which is exactly the case the old fixed cap got wrong. + await page.setViewportSize({ width: 1280, height: 800 }); + test.skip(!await openFullMatrix(page), 'matrix grid did not render (no data)'); - if (!m.gridScrolls) { - test.skip(true, 'grid is not taller than the viewport in this dataset — nothing to assert'); - return; - } + const m = await readScrollState(page); + expect(m.gridScrolls && m.pageScrolls, 'only one of grid/page may scroll').toBe(false); + // Something must scroll — a full-data grid cannot fit an 800px window. This + // keeps the assertion above from passing vacuously. + expect(m.gridScrolls || m.pageScrolls, 'the full-data matrix must scroll somewhere').toBe(true); + }); - // The grid scrolls internally; the page must NOT (a few px of slack for - // sub-pixel rounding). On the old fixed-cap code this overflowed by ~200px. - expect(m.pageOverflow, 'page should not scroll when only the grid does').toBeLessThanOrEqual(4); + test('with room for the grid, only the grid scrolls', async ({ page }) => { + await page.setViewportSize({ width: 1280, height: 800 }); + test.skip(!await openFullMatrix(page), 'matrix grid did not render (no data)'); + + // Collapse the legend to free the ~270px that keeps the grid from getting a + // usable height. The measuring hook re-measures and caps the grid. + const legend = page.getByRole('button', { name: /How to read this matrix/i }).first(); + await expect(legend).toBeVisible({ timeout: 20000 }); + if (await legend.getAttribute('aria-expanded') === 'true') await legend.click(); + await page.waitForTimeout(1000); + + const m = await readScrollState(page); + expect(m.gridScrolls, 'the grid should scroll internally').toBe(true); + expect(m.pageScrolls, 'the page should not scroll when the grid does').toBe(false); }); }); diff --git a/app/ui/src/components/MatrixView.jsx b/app/ui/src/components/MatrixView.jsx index a83f17729..238e9981f 100644 --- a/app/ui/src/components/MatrixView.jsx +++ b/app/ui/src/components/MatrixView.jsx @@ -1,4 +1,4 @@ -import { useMemo, useState, useReducer, useCallback, useEffect, useLayoutEffect, useRef } from 'react'; +import { useMemo, useState, useReducer, useCallback, useEffect, useRef } from 'react'; // useState-equivalent backed by useReducer (supports value + functional // updates): dispatch isn't flagged by react-hooks/set-state-in-effect, so the @@ -7,6 +7,9 @@ const setStateReducer = (s, a) => (typeof a === 'function' ? a(s) : a); import { useAuth } from '@ui/auth/AuthGate'; import { useMatrixRowOrder } from '@ui/hooks/useMatrixRowOrder'; import { useNestedGroupExpand, MAX_NEST_LEVEL } from '@ui/hooks/useNestedGroupExpand'; +import { useBusinessRoleFold } from '@ui/hooks/useBusinessRoleFold'; +import useResizableGridHeight from '@ui/hooks/useResizableGridHeight'; +import GridResizeHandle from './matrix/GridResizeHandle'; import MatrixToolbar from './matrix/MatrixToolbar'; import MatrixLegend from './matrix/MatrixLegend'; import MatrixFilterSummary from './matrix/MatrixFilterSummary'; @@ -14,6 +17,7 @@ import MatrixScopePanel from './matrix/MatrixScopePanel'; import MatrixColumnHeaders from './matrix/MatrixColumnHeaders'; import { makeUserComparator, buildSortKeys } from './matrix/sortUsers'; import MatrixGroupRow from './matrix/MatrixGroupRow'; +import { buildRoleDeviationCounts, cellDeviation, NO_ROLE_DEVIATIONS } from './matrix/coverageDeviation'; import { buildResourceContextMap, contextsFor } from '@ui/utils/resourceContexts'; // Inline arrayMove so MatrixView doesn't depend on @dnd-kit @@ -63,6 +67,20 @@ export const AGG_SENTINEL = '@@AGG@@'; // Above this many assignments, an 'auto' fold-on-load matrix opens folded. const FOLD_AUTO_THRESHOLD = 5000; +// The AP-staircase bucket a resource row falls into: the leftmost access-package +// column that grants it, or accessPackages.length ("unmanaged") when none does. +// Owner rows only match AP columns whose role is Owner. +function leftmostApBucket(group, accessPackages, apGroupMap) { + const gidUpper = (group.realGroupId || group.id).toUpperCase(); + const isOwnerRow = !!group.realGroupId; + for (let i = 0; i < accessPackages.length; i++) { + const role = apGroupMap.get(`${gidUpper}|${accessPackages[i].id.toLowerCase()}`); + if (!role) continue; + if (isOwnerRow === role.toLowerCase().includes('owner')) return i; + } + return accessPackages.length; +} + // Short label for a manager-hierarchy node name ("A · B · C (Manager)" → "C"). function orgShort(name) { const noMgr = String(name || '').replace(/\s*\([^)]*\)\s*$/, '').trim(); @@ -96,6 +114,50 @@ function makeAccountCol(parent, acc, sortKeys) { }; } +// Which business role grants which resource row, from the (role, resource) +// payload — the SOLL side of the grid. Two things earn a role a column: it +// grants a resource that is on screen, or its OWN row is (holding the role IS a +// governed Direct assignment on it, which is not a `Contains` relationship and +// so can never arrive as a pair — without this the role's column was blank on +// its own row). Returns the roles keyed by id plus a "RESOURCE|role" → roleName +// mapping. +function buildApMapping(accessPackageGroups, visibleGroupIds) { + const apMap = new Map(); + const mapping = new Map(); + for (const row of accessPackageGroups) { + const gid = (row.resourceId || row.groupId)?.toUpperCase(); + const selfGid = row.accessPackageId?.toUpperCase(); + const selfVisible = !!selfGid && visibleGroupIds.has(selfGid); + const childVisible = !!gid && visibleGroupIds.has(gid); + if (!selfVisible && !childVisible) continue; + if (!apMap.has(row.accessPackageId)) { + apMap.set(row.accessPackageId, { + id: row.accessPackageId, + displayName: row.accessPackageName, + catalogName: row.catalogName, + totalAssignments: row.totalAssignments || 0, + categoryName: row.categoryName || null, + categoryColor: row.categoryColor || null, + }); + } + const apKey = row.accessPackageId.toLowerCase(); + if (selfVisible) mapping.set(`${selfGid}|${apKey}`, 'Member'); + if (childVisible) mapping.set(`${gid}|${apKey}`, row.roleName || 'Member'); + } + return { apMap, mapping }; +} + +// Column order: by category name, then by total assignments descending within +// each category; uncategorized roles go last. +function compareAccessPackages(a, b) { + const aCat = a.categoryName; + const bCat = b.categoryName; + if (aCat && !bCat) return -1; + if (!aCat && bCat) return 1; + if (aCat && bCat && aCat !== bCat) return aCat.localeCompare(bCat); + return b.totalAssignments - a.totalAssignments || a.displayName.localeCompare(b.displayName); +} + export default function MatrixView({ data, accessPackageGroups = [], managedByPackages = [], resourceContexts, filter, @@ -431,24 +493,7 @@ export default function MatrixView({ } const visibleGroupIds = new Set(groups.map(g => (g.realGroupId || g.id).toUpperCase())); const visibleUserIds = new Set(users.map(u => u.id.toLowerCase())); - const apMap = new Map(); - const mapping = new Map(); // "groupId|apId" -> roleName - - for (const row of accessPackageGroups) { - const gid = (row.resourceId || row.groupId)?.toUpperCase(); - if (!gid || !visibleGroupIds.has(gid)) continue; - if (!apMap.has(row.accessPackageId)) { - apMap.set(row.accessPackageId, { - id: row.accessPackageId, - displayName: row.accessPackageName, - catalogName: row.catalogName, - totalAssignments: row.totalAssignments || 0, - categoryName: row.categoryName || null, - categoryColor: row.categoryColor || null, - }); - } - mapping.set(`${gid}|${row.accessPackageId.toLowerCase()}`, row.roleName || 'Member'); - } + const { apMap, mapping } = buildApMapping(accessPackageGroups, visibleGroupIds); // Filter to APs that have at least one visible user assignment const apIdsWithAssignments = new Set(); @@ -466,20 +511,7 @@ export default function MatrixView({ } } - // Sort access packages: by category name first, then by total assignments - // descending within each category. Uncategorized APs go at the end. - const accessPackages = [...apMap.values()].sort((a, b) => { - const aCat = a.categoryName; - const bCat = b.categoryName; - // Uncategorized after all categorized - if (aCat && !bCat) return -1; - if (!aCat && bCat) return 1; - // Both categorized: sort by category name - if (aCat && bCat && aCat !== bCat) return aCat.localeCompare(bCat); - // Same category (or both uncategorized): sort by total assignments descending - return b.totalAssignments - a.totalAssignments || a.displayName.localeCompare(b.displayName); - }); - return { accessPackages, apGroupMap: mapping }; + return { accessPackages: [...apMap.values()].sort(compareAccessPackages), apGroupMap: mapping }; }, [accessPackageGroups, groups, users, managedApMap]); // AP ID (lowercase) -> sorted index (for consistent color lookup) @@ -498,39 +530,33 @@ export default function MatrixView({ if (managedFilter === 'unmanaged') return groups; if (accessPackages.length === 0) return groups; // no APs, keep member count sort - // Assign each group to the AP bucket of its leftmost AP column + // Assign each group to the AP bucket of its leftmost AP column. A business + // role's OWN row is promoted into its own bucket so it sits directly above + // the resources it grants — folding is only coherent when a parent row is + // adjacent to the children it hides. const groupApBucket = new Map(); + const roleRowIds = new Set(); for (const g of groups) { - let bucket = accessPackages.length; // unmanaged = after all APs - const gidUpper = (g.realGroupId || g.id).toUpperCase(); // use realGroupId for owner rows - const isOwnerRow = !!g.realGroupId; - for (let i = 0; i < accessPackages.length; i++) { - const mapKey = `${gidUpper}|${accessPackages[i].id.toLowerCase()}`; - if (apGroupMap.has(mapKey)) { - // Owner rows only match AP buckets where the role is Owner - const role = apGroupMap.get(mapKey); - const roleIsOwner = (role || '').toLowerCase().includes('owner'); - if (isOwnerRow ? roleIsOwner : !roleIsOwner) { - bucket = i; - break; - } - } - } - groupApBucket.set(g.id, bucket); + const selfBucket = g.realGroupId ? undefined : apIdToIndex.get(g.id.toLowerCase()); + if (selfBucket != null) roleRowIds.add(g.id); + groupApBucket.set(g.id, selfBucket ?? leftmostApBucket(g, accessPackages, apGroupMap)); } return [...groups].sort((a, b) => { const aBucket = groupApBucket.get(a.id); const bBucket = groupApBucket.get(b.id); if (aBucket !== bBucket) return aBucket - bBucket; - // Same bucket: sort by type priority (Direct > Eligible > Indirect) + // Same bucket: the business role row itself comes first, above its resources + const aRole = roleRowIds.has(a.id); + if (aRole !== roleRowIds.has(b.id)) return aRole ? -1 : 1; + // Then by type priority (Direct > Eligible > Indirect) const directCmp = (b.directCount || 0) - (a.directCount || 0); if (directCmp !== 0) return directCmp; const eligibleCmp = (b.eligibleCount || 0) - (a.eligibleCount || 0); if (eligibleCmp !== 0) return eligibleCmp; return b.memberCount - a.memberCount; }); - }, [groups, accessPackages, apGroupMap, managedFilter]); + }, [groups, accessPackages, apGroupMap, apIdToIndex, managedFilter]); // Apply custom drag-row order on top of the default AP staircase sort. All // subject/resource selection happens through the filter wizard, so there @@ -639,26 +665,30 @@ export default function MatrixView({ }); if (groupAps.length === 0) return false; + // A row is a gap row when some subject is short of what a role covering + // this cell assigns — the same comparison the cell markers use, so the + // Gaps view and the amber "!" can never disagree. const groupApIdSetLower = new Set(groupAps.map(ap => ap.id.toLowerCase())); return users.some(user => { const cellKeyLower = `${realGid.toLowerCase()}|${user.id.toLowerCase()}`; - const userApIds = (managedApMap?.get(cellKeyLower) || []).filter(id => groupApIdSetLower.has(id)); - if (userApIds.length === 0) return false; - - const cellKey = `${group.id}|${user.id}`; - const cellTypes = displayMemberships.get(cellKey); - return userApIds.some(apId => { - const apObj = groupAps.find(a => a.id.toLowerCase() === apId); - const role = apObj ? (apGroupMap?.get(`${lookupGid}|${apObj.id.toLowerCase()}`) || 'Member') : 'Member'; - const lower = role.toLowerCase(); - if (lower.includes('owner')) return !cellTypes?.has('Owner'); - if (lower.includes('eligible')) return !cellTypes?.has('Eligible'); - return !cellTypes?.has('Direct'); - }); + const apIds = (managedApMap?.get(cellKeyLower) || []).filter(id => groupApIdSetLower.has(id)); + const types = displayMemberships.get(`${group.id}|${user.id}`); + return cellDeviation({ types, apIds, apGroupMap, resourceKey: lookupGid }).missing.length > 0; }); }); }, [displayGroups, managedFilter, accessPackages, apGroupMap, users, managedApMap, displayMemberships]); + // ─── Business-role fold ───────────────────────────────────────── + // Folding a business role hides the rows of the resources it grants, so the + // grid can be reduced to "business roles + resources no role covers". Applied + // last in the row pipeline, so it composes with the All/Governed/Non-governed/ + // Gaps toggles and with the injected nested sub-rows. + const { + visibleRows: foldedGroups, foldedChildRows, exportRows, + foldableRoles, foldedRoles, roleFoldInfo, + toggleRoleFold, foldAllRoles, unfoldAllRoles, canFoldRoles, hasFoldedRoles, + } = useBusinessRoleFold({ accessPackageGroups, rows: visibleGroups, storageKey }); + // Lazy-load SortableMatrixBody (contains @dnd-kit + @tanstack/react-virtual) const [SortableBody, setSortableBody] = useState(null); useEffect(() => { @@ -685,7 +715,10 @@ export default function MatrixView({ const { exportToExcel } = await import('../utils/exportToExcel'); exportToExcel({ users, - orderedGroups, + // The rows the grid lays out, always unfolded: a resource appears under + // every business role that grants it (so the file matches what is on + // screen), but a folded role never omits its resources from the export. + orderedGroups: exportRows, memberships, managedApMap, apIdToIndex, @@ -696,7 +729,7 @@ export default function MatrixView({ shareUrl, sortAttributes: sortAttrs, }); - }, [users, orderedGroups, memberships, managedApMap, apIdToIndex, accessPackages, apGroupMap, shareUrl, sortAttrs]); + }, [users, exportRows, memberships, managedApMap, apIdToIndex, accessPackages, apGroupMap, shareUrl, sortAttrs]); // Share: copy URL to clipboard const handleShare = useCallback(async () => { @@ -813,6 +846,16 @@ export default function MatrixView({ return counts; }, [colMemberships, userToAgg, collapsedGroups]); + // Per (folded role, subject column): how the rows a role folded away deviate + // from what the role assigns — more than it grants (red) and fewer (amber). + // Folding a role otherwise hides exactly what a role-mining review is looking + // for, in both directions, so the folded row keeps both counts. Coverage comes + // from managedApMap (the server's business-role → cell mapping), never from a + // client-side guess at what a role ought to grant. + const roleDeviations = useMemo(() => buildRoleDeviationCounts({ + foldedChildRows, users, memberships: colMemberships, managedApMap, apGroupMap, userToAgg, + }) || NO_ROLE_DEVIATIONS, [foldedChildRows, users, colMemberships, managedApMap, apGroupMap, userToAgg]); + // Fold every top-level (first sort attribute) group into one aggregate column; // unfold clears all collapses. There's something to fold only when the first // attribute has more than one distinct value. @@ -900,44 +943,11 @@ export default function MatrixView({ const filterIsApplied = filter !== null && filter !== undefined; // Cap the grid's height to the remaining viewport so ONLY the grid scrolls, - // never the page too. A fixed viewport-minus-fixed-pixels max-height guesses - // the chrome height; the real chrome (auth banner + scope stats + "How to - // read") is taller, so the grid sat too low and the page got a second - // scrollbar. Measure the grid's real document-top instead and re-measure on - // any layout change (header content loads late, panels toggle). + // never the page too — until the analyst drags the grip below the grid to a + // height of their own, which then wins and is remembered. const rootRef = useRef(null); - const [gridMaxH, setGridMaxH] = useState(null); - useLayoutEffect(() => { - const measure = () => { - const el = scrollRef.current; - if (!el) return; - // Reserve room for the app footer (below
) + main's bottom padding. - const footer = document.querySelector('footer'); - const below = (footer ? footer.getBoundingClientRect().height : 0) + 28; - // clientHeight = real layout height; document-relative top (rect.top is - // viewport-relative, so a scrolled page would read too small and cap the - // grid too tall — a self-sustaining overflow). scrollY corrects that. - const vh = document.documentElement.clientHeight; - const gridTop = el.getBoundingClientRect().top + window.scrollY; - // Fit the grid into the remaining viewport so ONLY the grid scrolls. Use - // the available space directly (so the page never gets a second - // scrollbar); a fixed 240px floor on a short viewport with tall chrome - // (e.g. gridTop ~530 on an 800px viewport leaves ~206px) overflowed the - // page by ~30px. A small 160px floor keeps the grid usable without - // re-introducing the overflow in any realistic viewport. - const avail = vh - gridTop - below; - setGridMaxH(Math.max(160, avail)); - }; - measure(); - const raf = requestAnimationFrame(measure); - window.addEventListener('resize', measure); - let ro; - if (typeof ResizeObserver !== 'undefined') { - ro = new ResizeObserver(measure); // body: anything above the grid shifts it down - ro.observe(document.body); - } - return () => { cancelAnimationFrame(raf); window.removeEventListener('resize', measure); if (ro) ro.disconnect(); }; - }, [filterIsApplied, users.length]); + const gridHeight = useResizableGridHeight(scrollRef, [filterIsApplied, users.length]); + const gridMaxH = gridHeight.height; return (
@@ -966,6 +976,10 @@ export default function MatrixView({ isFolded={collapsedGroups.size > 0} onFoldAllColumns={foldAllColumns} onUnfoldAllColumns={unfoldAllColumns} + canFoldRoles={canFoldRoles} + hasFoldedRoles={hasFoldedRoles} + onFoldAllRoles={foldAllRoles} + onUnfoldAllRoles={unfoldAllRoles} /> {filterIsApplied && } @@ -977,6 +991,7 @@ export default function MatrixView({ No assignments match the current filter. Adjust the subjects or resources to widen the view.
) : ( + <>
{refreshing && (
@@ -992,8 +1007,7 @@ export default function MatrixView({ {SortableBody ? ( ) : ( {columnHeaders} - {visibleGroups.map(group => ( + {foldedGroups.map(group => ( ))}
)}
+ + )} {pathExplain && (
({ props: null })); + +// Capture what the Excel exporter is handed, without pulling in ExcelJS. +const excel = vi.hoisted(() => ({ calls: [] })); +vi.mock('../utils/exportToExcel', () => ({ + exportToExcel: (payload) => { excel.calls.push(payload); }, +})); + vi.mock('./matrix/SortableMatrixBody', () => ({ - default: ({ columnHeaders, orderedGroups = [] }) => - h('table', null, + default: (props) => { + body.props = props; + const { columnHeaders, orderedGroups = [] } = props; + return h('table', null, columnHeaders, h('tbody', null, orderedGroups.map(g => @@ -26,7 +39,8 @@ vi.mock('./matrix/SortableMatrixBody', () => ({ ), ), ), - ), + ); + }, })); // A small but realistic matrix dataset: two resources, three subjects across two @@ -40,6 +54,47 @@ function makeData() { ]; } +// A matrix that contains a business role (br-1) granting one of the resource +// rows (res-1) — the shape the business-role fold operates on. res-2 is granted +// by no role, so it must survive every fold. +function makeRoleData() { + return [ + ...makeData(), + { memberId: 'u1', memberDisplayName: 'Alice Eng', department: 'Engineering', memberType: 'User', resourceId: 'br-1', resourceDisplayName: 'HR Manager Role', resourceType: 'BusinessRole', membershipType: 'Direct' }, + ]; +} +const roleProps = { + data: makeRoleData(), + accessPackageGroups: [ + { accessPackageId: 'br-1', accessPackageName: 'HR Manager Role', resourceId: 'res-1', roleName: 'Member', totalAssignments: 1 }, + ], + // Server-side business-role coverage: the role covers the resource it + // Contains AND its own membership row (migration 061). + managedByPackages: [ + { resourceId: 'res-1', memberId: 'u1', accessPackageIds: ['br-1'] }, + { resourceId: 'br-1', memberId: 'u1', accessPackageIds: ['br-1'] }, + ], +}; + +// The same matrix plus Carol, who holds the role but not the resource it grants +// — the "fewer than the role assigns" side. Its own filter, so the fold state a +// previous test persisted (per filter) can't carry into it. +const driftProps = { + ...roleProps, + data: [ + ...makeRoleData(), + { memberId: 'u3', memberDisplayName: 'Carol Sales', department: 'Sales', memberType: 'User', resourceId: 'br-1', resourceDisplayName: 'HR Manager Role', resourceType: 'BusinessRole', membershipType: 'Direct' }, + ], + managedByPackages: [ + ...roleProps.managedByPackages, + { resourceId: 'res-1', memberId: 'u3', accessPackageIds: ['br-1'] }, + { resourceId: 'br-1', memberId: 'u3', accessPackageIds: ['br-1'] }, + ], +}; + +const rowLabels = () => + screen.queryAllByTestId('row-label').filter(el => el.isConnected).map(el => el.textContent); + const baseFilter = { rowType: 'user', subject: { include: [], exclude: [] }, @@ -232,6 +287,131 @@ describe('MatrixView (mounted)', () => { ); }); + it('folds a business role\'s resources away and back from the toolbar', async () => { + renderView(roleProps); + const user = userEvent.setup(); + await expectRowVisible('HR Manager Role'); + await expectRowVisible('Finance App'); + + await user.click(await screen.findByText('Fold roles')); + // Only the role row and the resource no role grants remain. + await waitFor(() => expect(rowLabels()).not.toContain('Finance App')); + expect(rowLabels()).toContain('HR Manager Role'); + expect(rowLabels()).toContain('HR Portal'); + + await user.click(await screen.findByText('Unfold roles')); + await expectRowVisible('Finance App'); + }); + + it('promotes the business-role row directly above the resources it grants', async () => { + renderView(roleProps); + await expectRowVisible('HR Manager Role'); + const labels = rowLabels(); + expect(labels.indexOf('HR Manager Role')).toBe(labels.indexOf('Finance App') - 1); + }); + + it('grants a business role its own SOLL cell, so its column is not blank on its own row', async () => { + renderView(roleProps); + await expectRowVisible('HR Manager Role'); + // Holding the role IS the assignment; the diagonal cell renders it as a + // Member (D) grant in the role's own column. + expect(body.props.apGroupMap.get('BR-1|br-1')).toBe('Member'); + expect(body.props.apGroupMap.get('RES-1|br-1')).toBe('Member'); + }); + + it('keeps a business role\'s column when only its own row is on screen', async () => { + renderView({ + ...roleProps, + // The role grants a resource that is outside this matrix slice. + accessPackageGroups: [ + { accessPackageId: 'br-1', accessPackageName: 'HR Manager Role', resourceId: 'res-99', roleName: 'Member', totalAssignments: 1 }, + ], + managedByPackages: [{ resourceId: 'br-1', memberId: 'u1', accessPackageIds: ['br-1'] }], + }); + await expectRowVisible('HR Manager Role'); + expect(body.props.accessPackages.map(ap => ap.id)).toEqual(['br-1']); + expect(body.props.apGroupMap.get('BR-1|br-1')).toBe('Member'); + }); + + it('shows the resources a role grants as that role\'s children', async () => { + renderView(roleProps); + await expectRowVisible('Finance App'); + const rows = new Map(body.props.orderedGroups.map(g => [g.displayName, g])); + expect(rows.get('Finance App').roleParentId).toBe('BR-1'); + // A resource no role grants stays a plain top-level row. + expect(rows.get('HR Portal').roleParentId).toBeUndefined(); + }); + + it('tallies the access a folded role hides but does not grant', async () => { + renderView(roleProps); + const user = userEvent.setup(); + await expectRowVisible('Finance App'); + expect(body.props.roleExtraCounts).toBeNull(); + + await user.click(await screen.findByText('Fold roles')); + await waitFor(() => expect(body.props.roleExtraCounts).not.toBeNull()); + // Alice holds Finance App through the role — covered, so not counted. + expect(body.props.roleExtraCounts.get('BR-1|u1')).toBeUndefined(); + // Bob's Indirect membership on the same resource is not covered by it. + expect(body.props.roleExtraCounts.get('BR-1|u2')).toBe(1); + }); + + // Requestor feedback on #370: folding must summarise both directions of drift + // — Carol holds the role but not the resource it grants (fewer), while Bob + // holds that resource without the role (more). + it('tallies what a folded role assigns that the subject does not have', async () => { + renderView({ ...driftProps, filter: { ...baseFilter, sortAttributes: [{ attribute: 'jobTitle', dir: 'asc' }] } }); + const user = userEvent.setup(); + await expectRowVisible('Finance App'); + expect(body.props.roleMissingCounts).toBeNull(); + + await user.click(await screen.findByText('Fold roles')); + await waitFor(() => expect(body.props.roleMissingCounts).not.toBeNull()); + expect(body.props.roleMissingCounts.get('BR-1|u3')).toBe(1); + expect(body.props.roleMissingCounts.get('BR-1|u1')).toBeUndefined(); + // ...and the opposite drift is still counted in the same folded row. + expect(body.props.roleExtraCounts.get('BR-1|u2')).toBe(1); + }); + + it('keeps only the rows a subject is short on in the Gaps view', async () => { + renderView({ ...driftProps, managedFilter: 'gaps' }); + // Carol holds the role but not the resource it grants — the only gap here. + await expectRowVisible('Finance App'); + expect(rowLabels()).not.toContain('HR Portal'); // no role covers it + expect(rowLabels()).not.toContain('HR Manager Role'); // everyone holding it has it + }); + + // Main's #949 pinned "the export matches the matrix". #370 changed what the + // matrix shows, so the export follows the grid's row model — but never its + // fold state, or a folded role would silently drop resources from an + // access-review artifact. + it('exports the resources of a folded business role anyway', async () => { + excel.calls.length = 0; + // Its own filter: fold state is persisted per matrix, so this test must not + // inherit (or leave behind) a fold from another one. + renderView({ ...roleProps, filter: { ...baseFilter, sortAttributes: [{ attribute: 'email', dir: 'asc' }] } }); + const user = userEvent.setup(); + await expectRowVisible('Finance App'); + + await user.click(await screen.findByText('Fold roles')); + await waitFor(() => expect(rowLabels()).not.toContain('Finance App')); + + await user.click(screen.getByRole('button', { name: /Export Excel/i })); + await waitFor(() => expect(excel.calls).toHaveLength(1)); + + const exported = excel.calls[0].orderedGroups.map(g => g.displayName); + // On screen the role's resource is folded away; in the file it is not. + expect(exported).toContain('HR Manager Role'); + expect(exported).toContain('Finance App'); + }); + + it('offers no fold controls in a matrix without business-role rows', async () => { + renderView(); + await expectRowVisible('Finance App'); + expect(screen.queryByText('Fold roles')).not.toBeInTheDocument(); + expect(screen.queryByText('Unfold roles')).not.toBeInTheDocument(); + }); + it('threads the resourceContexts sidecar onto the matching resource rows', async () => { renderView({ resourceContexts: [ diff --git a/app/ui/src/components/MatrixView.scrollbar.test.js b/app/ui/src/components/MatrixView.scrollbar.test.js index 5f485c8ca..bc2ab3389 100644 --- a/app/ui/src/components/MatrixView.scrollbar.test.js +++ b/app/ui/src/components/MatrixView.scrollbar.test.js @@ -8,14 +8,18 @@ import { describe, it, expect } from 'vitest'; // The bug: the grid used a fixed `max-h-[calc(100vh-280px)]` that GUESSES the // height of the chrome above it. The real chrome (auth banner + scope-statistics // panel + "How to read") is taller than 280px, so the grid was too tall and the -// page got a second scrollbar next to the grid's own. The fix measures the real -// remaining viewport (clientHeight minus the grid's document-top) and caps the -// grid's height with an inline maxHeight — in BOTH matrix orientations. +// page got a second scrollbar next to the grid's own. The fix caps the grid with +// a measured inline maxHeight — from the shared useResizableGridHeight hook, +// which wraps useViewportFitHeight — in EVERY matrix orientation. The hooks' +// own behaviour (including that the fit never returns more than the available +// space) is covered by src/hooks/useViewportFitHeight.test.jsx and +// src/hooks/useResizableGridHeight.test.jsx. const here = dirname(fileURLToPath(import.meta.url)); const sources = { MatrixView: readFileSync(join(here, 'MatrixView.jsx'), 'utf8'), RotatedMatrixView: readFileSync(join(here, 'RotatedMatrixView.jsx'), 'utf8'), + RollupMatrixView: readFileSync(join(here, 'RollupMatrixView.jsx'), 'utf8'), }; describe('matrix grid height — no double scrollbar', () => { @@ -24,12 +28,23 @@ describe('matrix grid height — no double scrollbar', () => { it('does not use the fixed max-h-[calc(100vh-280px)] magic number', () => { expect(src).not.toContain('max-h-[calc(100vh-280px)]'); }); - it('measures the real layout height (documentElement.clientHeight)', () => { - expect(src).toContain('document.documentElement.clientHeight'); + it('takes its cap from the shared measuring hook', () => { + expect(src).toContain("import useResizableGridHeight from '@ui/hooks/useResizableGridHeight'"); + expect(src).toMatch(/const gridHeight = useResizableGridHeight\(/); + expect(src).toMatch(/const gridMaxH = gridHeight\.height/); }); it('caps the grid with a measured inline maxHeight', () => { expect(src).toMatch(/maxHeight: gridMaxH/); }); + // Requestor feedback on #370: the measured fit is a default, not a + // verdict — every orientation offers the same grip to resize it. + it('offers the shared resize grip', () => { + expect(src).toContain("import GridResizeHandle from './matrix/GridResizeHandle'"); + expect(src).toMatch(/ { + expect(src).not.toMatch(/Math\.max\(\s*\d+\s*,\s*(vh|avail)/); + }); }); } }); diff --git a/app/ui/src/components/RollupMatrixView.jsx b/app/ui/src/components/RollupMatrixView.jsx index 6776aefa2..bf66cb265 100644 --- a/app/ui/src/components/RollupMatrixView.jsx +++ b/app/ui/src/components/RollupMatrixView.jsx @@ -1,5 +1,6 @@ -import { useMemo, useState, useCallback, useRef, useLayoutEffect } from 'react'; +import { useMemo, useState, useCallback, useRef } from 'react'; import { useAuth } from '@ui/auth/AuthGate'; +import useResizableGridHeight from '@ui/hooks/useResizableGridHeight'; import { friendlyLabel } from '@ui/utils/formatters'; import { getAccessPackageColor } from '@ui/utils/colors'; import { exportRollupToExcel } from '@ui/utils/exportRollupToExcel'; @@ -9,6 +10,7 @@ import MatrixScopePanel from './matrix/MatrixScopePanel'; import MatrixLegend from './matrix/MatrixLegend'; import MatrixFilterSummary from './matrix/MatrixFilterSummary'; import MatrixToolbar from './matrix/MatrixToolbar'; +import GridResizeHandle from './matrix/GridResizeHandle'; // Roll-up matrix: the subject (column) axis is aggregated by an attribute (e.g. // department). Rows are resources; each cell is the count of distinct subjects @@ -301,24 +303,8 @@ export default function RollupMatrixView({ // (matches MatrixView). overflow-auto then gives both scrollbars, including // the horizontal one when the columns are wider than the screen. const scrollRef = useRef(null); - const [gridMaxH, setGridMaxH] = useState(null); - useLayoutEffect(() => { - const measure = () => { - const el = scrollRef.current; - if (!el) return; - const footer = document.querySelector('footer'); - const below = (footer ? footer.getBoundingClientRect().height : 0) + 28; - const vh = document.documentElement.clientHeight; - const gridTop = el.getBoundingClientRect().top + window.scrollY; - setGridMaxH(Math.max(240, vh - gridTop - below)); - }; - measure(); - const raf = requestAnimationFrame(measure); - window.addEventListener('resize', measure); - let ro; - if (typeof ResizeObserver !== 'undefined') { ro = new ResizeObserver(measure); ro.observe(document.body); } - return () => { cancelAnimationFrame(raf); window.removeEventListener('resize', measure); if (ro) ro.disconnect(); }; - }, [columns.length, visibleRoles.length]); + const gridHeight = useResizableGridHeight(scrollRef, [columns.length, visibleRoles.length]); + const gridMaxH = gridHeight.height; const trailingCols = visibleRoles.length + 3; // resource + # + Description (+ roles handled separately) @@ -721,6 +707,13 @@ export default function RollupMatrixView({
+ +
); } diff --git a/app/ui/src/components/RollupMatrixView.mount.test.jsx b/app/ui/src/components/RollupMatrixView.mount.test.jsx index 755036836..90009ccf2 100644 --- a/app/ui/src/components/RollupMatrixView.mount.test.jsx +++ b/app/ui/src/components/RollupMatrixView.mount.test.jsx @@ -158,6 +158,64 @@ describe('RollupMatrixView (mounted)', () => { expect(screen.getByText(/No assignments match the current filter/i)).toBeInTheDocument(); }); + // A context (org-tree) roll-up: columns are context nodes rather than plain + // attribute values, and the drill path is navigable. + describe('context roll-up (org tree)', () => { + const contextRollup = (overrides = {}) => makeRollup({ + rollupKind: 'context', + attribute: 'manager-hierarchy', + groupValues: ['node-eng', 'node-sales'], + counts: [ + { resourceId: 'res-1', groupValue: 'node-eng', directCount: 5, governedCount: 2 }, + { resourceId: 'res-2', groupValue: 'node-sales', directCount: 2, governedCount: 0 }, + ], + groupTotals: [{ groupValue: 'node-eng', total: 20 }, { groupValue: 'node-sales', total: 9 }], + nodes: [ + { id: 'node-eng', displayName: 'Acme · Engineering (Manager, Ada)', total: 20, childCount: 3, directMembers: 4, depth: 2, pathIds: ['root', 'node-eng'], pathNames: ['Acme', 'Acme · Engineering'] }, + { id: 'node-sales', displayName: 'Acme · Sales (Manager, Bo)', total: 9, childCount: 0, directMembers: 9, depth: 2, pathIds: ['root', 'node-sales'], pathNames: ['Acme', 'Acme · Sales'] }, + ], + breadcrumb: [{ id: 'root', displayName: 'Acme' }, { id: 'node-eng', displayName: 'Acme · Engineering (Manager, Ada)' }], + ...overrides, + }); + + it('labels the columns with the deepest org segment and shows the drill path', () => { + renderView({ rollup: contextRollup() }); + expect(screen.getByText('Drill path:')).toBeInTheDocument(); + // Column headers use the short label, not the full "A · B (Manager)" path. + expect(screen.getAllByText('Engineering').length).toBeGreaterThan(0); + expect(screen.getByText('Sales')).toBeInTheDocument(); + }); + + it('zooms out to an earlier step of the drill path', async () => { + const { onFilterChange } = renderView({ rollup: contextRollup() }); + await userEvent.setup().click(screen.getByTitle('Zoom out to Acme')); + expect(onFilterChange).toHaveBeenCalledWith(expect.objectContaining({ rollupPath: [] })); + }); + + it('zooms into a node that has sub-teams', async () => { + const { onFilterChange } = renderView({ rollup: contextRollup() }); + await userEvent.setup().click(screen.getByTitle(/Zoom into Engineering/)); + expect(onFilterChange).toHaveBeenCalledWith( + expect.objectContaining({ rollupPath: ['node-eng'] }), + ); + }); + + it('expands and collapses an org in place in the layered view', async () => { + const { onFilterChange } = renderView({ rollup: contextRollup({ layered: true, maxDepth: 2 }) }); + const user = userEvent.setup(); + // The team header splits the team into its sub-teams… + await user.click(screen.getByTitle(/Click to split Engineering into its 3 sub-teams/)); + expect(onFilterChange).toHaveBeenCalledWith( + expect.objectContaining({ rollupExpanded: ['node-eng'] }), + ); + // …and the merged ancestor header collapses the branch back. + await user.click(screen.getAllByTitle(/Collapse Acme back into one column/)[0]); + expect(onFilterChange).toHaveBeenCalledWith( + expect.objectContaining({ rollupExpanded: [] }), + ); + }); + }); + it('renders the roles-only variant with business roles on the rows', () => { renderView({ rollup: makeRollup({ diff --git a/app/ui/src/components/RotatedMatrixView.jsx b/app/ui/src/components/RotatedMatrixView.jsx index ad8eca91f..36821742e 100644 --- a/app/ui/src/components/RotatedMatrixView.jsx +++ b/app/ui/src/components/RotatedMatrixView.jsx @@ -13,7 +13,9 @@ // Everything else (filter chip, share link, Excel export hook, basic // per-cell membership-type badges) works the same as the default view. -import { useMemo, useCallback, useState, useLayoutEffect, useRef } from 'react'; +import { useMemo, useCallback, useState, useRef } from 'react'; +import useResizableGridHeight from '@ui/hooks/useResizableGridHeight'; +import GridResizeHandle from './matrix/GridResizeHandle'; import MatrixToolbar from './matrix/MatrixToolbar'; import MatrixFilterSummary from './matrix/MatrixFilterSummary'; import MatrixCell from './matrix/MatrixCell'; @@ -61,31 +63,12 @@ export default function RotatedMatrixView({ const filterIsApplied = filter !== null && filter !== undefined; // Cap the grid to the remaining viewport so only the grid scrolls, not the - // page too (mirrors MatrixView). Measure the grid's real document-top rather - // than guessing the chrome height with a fixed max-h. + // page too — and let the analyst drag it to a height of their own (mirrors + // MatrixView). const rootRef = useRef(null); const gridRef = useRef(null); - const [gridMaxH, setGridMaxH] = useState(null); - useLayoutEffect(() => { - const measure = () => { - const el = gridRef.current; - if (!el) return; - const footer = document.querySelector('footer'); - const below = (footer ? footer.getBoundingClientRect().height : 0) + 28; - const vh = document.documentElement.clientHeight; - const gridTop = el.getBoundingClientRect().top + window.scrollY; - setGridMaxH(Math.max(240, vh - gridTop - below)); - }; - measure(); - const raf = requestAnimationFrame(measure); - window.addEventListener('resize', measure); - let ro; - if (typeof ResizeObserver !== 'undefined') { - ro = new ResizeObserver(measure); - ro.observe(document.body); - } - return () => { cancelAnimationFrame(raf); window.removeEventListener('resize', measure); if (ro) ro.disconnect(); }; - }, [filterIsApplied]); + const gridHeight = useResizableGridHeight(gridRef, [filterIsApplied]); + const gridMaxH = gridHeight.height; // Same client-side managed-state toggle as MatrixView. const filteredData = useMemo(() => { @@ -203,6 +186,7 @@ export default function RotatedMatrixView({ No assignments match the current matrix. Adjust the subjects or resources to widen the view. ) : ( + <>
{refreshing && (
@@ -333,6 +317,13 @@ export default function RotatedMatrixView({
+ + )}
); diff --git a/app/ui/src/components/matrix/CellMarkerStrip.jsx b/app/ui/src/components/matrix/CellMarkerStrip.jsx new file mode 100644 index 000000000..8b51afbfd --- /dev/null +++ b/app/ui/src/components/matrix/CellMarkerStrip.jsx @@ -0,0 +1,102 @@ +import { + extraAccessTitle, missingAccessTitle, overGrantTitle, heldOutsideTitle, hasCellMarkers, +} from './cellMarkers'; + +// Every marker an intersection cell can carry is drawn here, in ONE strip along +// the top of the cell, above the D/I/E badge — see cellMarkers.js for the +// geometry that reserves the strip and for why the markers left the corners. + +function Marker({ className, title, children }) { + return ( + + {children} + + ); +} + +// Keeps the three slots in place when a cell carries only some of them. +function EmptySlot() { + return