Sort IAM role assignments with groups first, then by name

OMICRON_VERSION

@@ -1 +1 @@
-bce0f5ddf7f9f3be498fa9afb1fee8d3336c1f30
+2ea34d0369fd26bc4f438ea0b3c8ab1b50314048

app/pages/OrgAccessPage.tsx

@@ -5,6 +5,7 @@ import type { LoaderFunctionArgs } from 'react-router-dom'
 
 import {
   apiQueryClient,
+  byGroupThenName,
   getEffectiveRole,
   setUserRole,
   useApiMutation,
@@ -23,7 +24,7 @@ import {
   TableActions,
   TableEmptyBox,
 } from '@oxide/ui'
-import { groupBy, isTruthy, sortBy } from '@oxide/util'
+import { groupBy, isTruthy } from '@oxide/util'
 
 import { AccessNameCell } from 'app/components/AccessNameCell'
 import { RoleBadgeCell } from 'app/components/RoleBadgeCell'
@@ -50,6 +51,7 @@ OrgAccessPage.loader = async ({ params }: LoaderFunctionArgs) => {
     }),
     // used to resolve user names
     apiQueryClient.prefetchQuery('userList', {}),
+    apiQueryClient.prefetchQuery('groupList', {}),
   ])
 }
 
@@ -77,8 +79,8 @@ export function OrgAccessPage() {
   const orgRows = useUserRows(orgPolicy?.roleAssignments, 'org')
 
   const rows = useMemo(() => {
-    const users = groupBy(siloRows.concat(orgRows), (u) => u.id).map(
-      ([userId, userAssignments]) => {
+    return groupBy(siloRows.concat(orgRows), (u) => u.id)
+      .map(([userId, userAssignments]) => {
         const siloRole = userAssignments.find((a) => a.roleSource === 'silo')?.roleName
         const orgRole = userAssignments.find((a) => a.roleSource === 'org')?.roleName
 
@@ -97,9 +99,8 @@ export function OrgAccessPage() {
         }
 
         return row
-      }
-    )
-    return sortBy(users, (u) => u.name)
+      })
+      .sort(byGroupThenName)
   }, [siloRows, orgRows])
 
   const queryClient = useApiQueryClient()

app/pages/SiloAccessPage.tsx

@@ -4,6 +4,7 @@ import { useMemo, useState } from 'react'
 
 import {
   apiQueryClient,
+  byGroupThenName,
   getEffectiveRole,
   setUserRole,
   useApiMutation,
@@ -22,7 +23,7 @@ import {
   TableActions,
   TableEmptyBox,
 } from '@oxide/ui'
-import { groupBy, isTruthy, sortBy } from '@oxide/util'
+import { groupBy, isTruthy } from '@oxide/util'
 
 import { AccessNameCell } from 'app/components/AccessNameCell'
 import { RoleBadgeCell } from 'app/components/RoleBadgeCell'
@@ -48,6 +49,7 @@ SiloAccessPage.loader = async () => {
     apiQueryClient.prefetchQuery('policyView', {}),
     // used to resolve user names
     apiQueryClient.prefetchQuery('userList', {}),
+    apiQueryClient.prefetchQuery('groupList', {}),
   ])
 }
 
@@ -69,25 +71,26 @@ export function SiloAccessPage() {
   const siloRows = useUserRows(siloPolicy?.roleAssignments, 'silo')
 
   const rows = useMemo(() => {
-    const users = groupBy(siloRows, (u) => u.id).map(([userId, userAssignments]) => {
-      const siloRole = userAssignments.find((a) => a.roleSource === 'silo')?.roleName
-
-      const roles = [siloRole].filter(isTruthy)
-
-      const { name, identityType } = userAssignments[0]
-
-      const row: UserRow = {
-        id: userId,
-        identityType,
-        name,
-        siloRole,
-        // we know there has to be at least one
-        effectiveRole: getEffectiveRole(roles)!,
-      }
-
-      return row
-    })
-    return sortBy(users, (u) => u.name)
+    return groupBy(siloRows, (u) => u.id)
+      .map(([userId, userAssignments]) => {
+        const siloRole = userAssignments.find((a) => a.roleSource === 'silo')?.roleName
+
+        const roles = [siloRole].filter(isTruthy)
+
+        const { name, identityType } = userAssignments[0]
+
+        const row: UserRow = {
+          id: userId,
+          identityType,
+          name,
+          siloRole,
+          // we know there has to be at least one
+          effectiveRole: getEffectiveRole(roles)!,
+        }
+
+        return row
+      })
+      .sort(byGroupThenName)
   }, [siloRows])
 
   const queryClient = useApiQueryClient()

app/pages/__tests__/org-access.e2e.ts

@@ -1,8 +1,14 @@
 import { test } from '@playwright/test'
 
-import { user1, user2, user3 } from '@oxide/api-mocks'
+import { user1, user2, user3, userGroup1 } from '@oxide/api-mocks'
+import { userGroups } from '@oxide/api-mocks'
 
-import { expectNotVisible, expectRowVisible, expectVisible } from 'app/test/e2e'
+import {
+  expectNotVisible,
+  expectRowVisible,
+  expectSimultaneous,
+  expectVisible,
+} from 'app/test/e2e'
 
 test('Click through org access page', async ({ page }) => {
   await page.goto('/orgs/maze-war')
@@ -11,7 +17,24 @@ test('Click through org access page', async ({ page }) => {
 
   // page is there, we see user 1 and 2 but not 3
   await page.click('role=link[name*="Access & IAM"]')
+
+  // has to be before anything else is checked. ensures we've prefetched
+  // users list and groups list properly
+  await expectSimultaneous(page, [
+    `role=cell[name="${userGroup1.id}"]`,
+    'role=cell[name="web-devs Group"]',
+    `role=cell[name="${user1.id}"]`,
+    'role=cell[name="Hannah Arendt"]',
+  ])
+
   await expectVisible(page, ['role=heading[name*="Access & IAM"]'])
+  await expectRowVisible(table, {
+    ID: userGroups[0].id,
+    // no space because expectRowVisible uses textContent, not accessible name
+    Name: 'web-devsGroup',
+    'Silo role': '',
+    'Org role': 'collaborator',
+  })
   await expectRowVisible(table, {
     ID: user1.id,
     Name: 'Hannah Arendt',

app/pages/__tests__/project-access.e2e.ts

@@ -1,13 +1,27 @@
 import { test } from '@playwright/test'
 
-import { user1, user2, user3, user4 } from '@oxide/api-mocks'
+import { user1, user2, user3, user4, userGroup1, userGroup2 } from '@oxide/api-mocks'
 
-import { expectNotVisible, expectRowVisible, expectVisible } from 'app/test/e2e'
+import {
+  expectNotVisible,
+  expectRowVisible,
+  expectSimultaneous,
+  expectVisible,
+} from 'app/test/e2e'
 
 test('Click through project access page', async ({ page }) => {
   await page.goto('/orgs/maze-war/projects/mock-project')
   await page.click('role=link[name*="Access & IAM"]')
 
+  // has to be before anything else is checked. ensures we've prefetched
+  // users list and groups list properly
+  await expectSimultaneous(page, [
+    `role=cell[name="${userGroup1.id}"]`,
+    'role=cell[name="web-devs Group"]',
+    `role=cell[name="${user1.id}"]`,
+    'role=cell[name="Hannah Arendt"]',
+  ])
+
   // page is there, we see user 1-3 but not 4
   await expectVisible(page, ['role=heading[name*="Access & IAM"]'])
   const table = page.locator('table')
@@ -32,6 +46,22 @@ test('Click through project access page', async ({ page }) => {
     'Org role': '',
     'Project role': 'collaborator',
   })
+  await expectRowVisible(table, {
+    ID: userGroup1.id,
+    // no space because expectRowVisible uses textContent, not accessible name
+    Name: 'web-devsGroup',
+    'Silo role': '',
+    'Org role': 'collaborator',
+  })
+  await expectRowVisible(table, {
+    ID: userGroup2.id,
+    // no space because expectRowVisible uses textContent, not accessible name
+    Name: 'kernel-devsGroup',
+    'Silo role': '',
+    'Org role': '',
+    'Project role': 'viewer',
+  })
+
   await expectNotVisible(page, [`role=cell[name="${user4.id}"]`])
 
   // Add user 4 as collab

app/pages/__tests__/silo-access.e2e.ts

@@ -0,0 +1,99 @@
+import { test } from '@playwright/test'
+
+import { user1, user3, user4, userGroup3 } from '@oxide/api-mocks'
+
+import {
+  expectNotVisible,
+  expectRowVisible,
+  expectSimultaneous,
+  expectVisible,
+} from 'app/test/e2e'
+
+test('Click through silo access page', async ({ page }) => {
+  await page.goto('/orgs')
+
+  const table = page.locator('role=table')
+
+  // page is there, we see user 1 and 2 but not 3
+  await page.click('role=link[name*="Access & IAM"]')
+
+  // has to be before anything else is checked. ensures we've prefetched
+  // users list and groups list properly
+  await expectSimultaneous(page, [
+    `role=cell[name="${userGroup3.id}"]`,
+    'role=cell[name="real-estate-devs Group"]',
+    `role=cell[name="${user1.id}"]`,
+    'role=cell[name="Hannah Arendt"]',
+  ])
+
+  await expectVisible(page, ['role=heading[name*="Access & IAM"]'])
+  await expectRowVisible(table, {
+    ID: userGroup3.id,
+    // no space because expectRowVisible uses textContent, not accessible name
+    Name: 'real-estate-devsGroup',
+    'Silo role': 'admin',
+  })
+  await expectRowVisible(table, {
+    ID: user1.id,
+    Name: 'Hannah Arendt',
+    'Silo role': 'admin',
+  })
+  await expectNotVisible(page, [`role=cell[name="${user4.id}"]`])
+
+  // Add user 2 as collab
+  await page.click('role=button[name="Add user or group"]')
+  await expectVisible(page, ['role=heading[name*="Add user or group"]'])
+
+  await page.click('role=button[name="User"]')
+  // only users not already on the org should be visible
+  await expectNotVisible(page, ['role=option[name="Hannah Arendt"]'])
+  await expectVisible(page, [
+    'role=option[name="Hans Jonas"]',
+    'role=option[name="Jacob Klein"]',
+    'role=option[name="Simone de Beauvoir"]',
+  ])
+
+  await page.click('role=option[name="Jacob Klein"]')
+
+  await page.click('role=button[name="Role"]')
+  await expectVisible(page, [
+    'role=option[name="Admin"]',
+    'role=option[name="Collaborator"]',
+    'role=option[name="Viewer"]',
+  ])
+
+  await page.click('role=option[name="Collaborator"]')
+  await page.click('role=button[name="Add user"]')
+
+  // User 3 shows up in the table
+  await expectRowVisible(table, {
+    ID: user3.id,
+    Name: 'Jacob Klein',
+    'Silo role': 'collaborator',
+  })
+
+  // now change user 3's role from collab to viewer
+  await page
+    .locator('role=row', { hasText: user3.id })
+    .locator('role=button[name="Row actions"]')
+    .click()
+  await page.click('role=menuitem[name="Change role"]')
+
+  await expectVisible(page, ['role=heading[name*="Change user role"]'])
+  await expectVisible(page, ['button:has-text("Collaborator")'])
+
+  await page.click('role=button[name="Role"]')
+  await page.click('role=option[name="Viewer"]')
+  await page.click('role=button[name="Update role"]')
+
+  await expectRowVisible(table, { ID: user3.id, 'Silo role': 'viewer' })
+
+  // now delete user 3
+  await page
+    .locator('role=row', { hasText: user3.id })
+    .locator('role=button[name="Row actions"]')
+    .click()
+  await expectVisible(page, [`role=cell[name="${user3.id}"]`])
+  await page.click('role=menuitem[name="Delete"]')
+  await expectNotVisible(page, [`role=cell[name="${user3.id}"]`])
+})

app/pages/project/access/ProjectAccessPage.tsx

@@ -5,6 +5,7 @@ import type { LoaderFunctionArgs } from 'react-router-dom'
 
 import {
   apiQueryClient,
+  byGroupThenName,
   getEffectiveRole,
   setUserRole,
   useApiMutation,
@@ -23,7 +24,7 @@ import {
   TableActions,
   TableEmptyBox,
 } from '@oxide/ui'
-import { groupBy, isTruthy, sortBy } from '@oxide/util'
+import { groupBy, isTruthy } from '@oxide/util'
 
 import { AccessNameCell } from 'app/components/AccessNameCell'
 import { RoleBadgeCell } from 'app/components/RoleBadgeCell'
@@ -53,6 +54,7 @@ ProjectAccessPage.loader = async ({ params }: LoaderFunctionArgs) => {
     apiQueryClient.prefetchQuery('projectPolicyView', { path: { orgName, projectName } }),
     // used to resolve user names
     apiQueryClient.prefetchQuery('userList', {}),
+    apiQueryClient.prefetchQuery('groupList', {}),
   ])
 }
 
@@ -84,8 +86,8 @@ export function ProjectAccessPage() {
   const projectRows = useUserRows(projectPolicy?.roleAssignments, 'project')
 
   const rows = useMemo(() => {
-    const users = groupBy(siloRows.concat(orgRows, projectRows), (u) => u.id).map(
-      ([userId, userAssignments]) => {
+    return groupBy(siloRows.concat(orgRows, projectRows), (u) => u.id)
+      .map(([userId, userAssignments]) => {
         const siloRole = userAssignments.find((a) => a.roleSource === 'silo')?.roleName
         const orgRole = userAssignments.find((a) => a.roleSource === 'org')?.roleName
         const projectRole = userAssignments.find(
@@ -108,9 +110,8 @@ export function ProjectAccessPage() {
         }
 
         return row
-      }
-    )
-    return sortBy(users, (u) => u.name)
+      })
+      .sort(byGroupThenName)
   }, [siloRows, orgRows, projectRows])
 
   const queryClient = useApiQueryClient()

app/test/e2e/utils.ts

@@ -85,9 +85,10 @@ async function timeToAppear(page: Page, selector: string): Promise<number> {
 }
 
 /**
- * Assert two elements appeared within 20ms of each other
+ * Assert a set of elements all appeared within a 20ms range
  */
-export async function expectSimultaneous(page: Page, selectors: [string, string]) {
-  const [t1, t2] = await Promise.all(selectors.map((sel) => timeToAppear(page, sel)))
-  expect(Math.abs(t1 - t2)).toBeLessThan(20)
+export async function expectSimultaneous(page: Page, selectors: string[]) {
+  const times = await Promise.all(selectors.map((sel) => timeToAppear(page, sel)))
+  times.sort()
+  expect(times[times.length - 1] - times[0]).toBeLessThan(20)
 }