rohit/probo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Truncate access review roles with badge list

Browse Source 
Long role strings in the access review table broke row layout when
drivers joined many roles into one comma-separated value. Expose
roles as a string array in GraphQL by splitting the stored role at
the API layer, and render the first three roles as badges with a
"+X more" popover for the rest.

Closes ENG-459.

Signed-off-by: Ludovic Vielle <ludovic@probo.com>
This commit is contained in:
Ludovic Vielle
2026-06-12 18:23:04 +02:00
parent bf20ca1a90
commit 8094e7cfd0
80 changed files with 768 additions and 378 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
pkg/coredata/access_review_campaign_source_test.go
View File

@@ -41,7 +41,7 @@ func insertAccessReviewEntry(t *testing.T, ctx context.Context, client *pg.Clien
AccessReviewCampaignSourceID: fx.campaignSourceID,
Email: accountKey,
FullName: "Snapshot User",
Role: "member",
Roles: []string{"member"},
MFAStatus: coredata.MFAStatusUnknown,
AuthMethod: coredata.AccessReviewEntryAuthMethodUnknown,
AccountType: coredata.AccessReviewEntryAccountTypeUser,

22
pkg/coredata/access_review_entry.go
View File

@@ -37,7 +37,7 @@ type (
IdentityID *gid.GID `db:"identity_id"`
Email string `db:"email"`
FullName string `db:"full_name"`
Role string `db:"role"`
Roles []string `db:"roles"`
JobTitle string `db:"job_title"`
IsAdmin bool `db:"is_admin"`
MFAStatus MFAStatus `db:"mfa_status"`
@@ -125,7 +125,7 @@ SELECT
identity_id,
email,
full_name,
role,
roles,
job_title,
is_admin,
mfa_status,
@@ -192,7 +192,7 @@ INSERT INTO
identity_id,
email,
full_name,
role,
roles,
job_title,
is_admin,
mfa_status,
@@ -222,7 +222,7 @@ VALUES (
@identity_id,
@email,
@full_name,
@role,
COALESCE(@roles, '{}'::TEXT[]),
@job_title,
@is_admin,
@mfa_status,
@@ -254,7 +254,7 @@ VALUES (
"identity_id": e.IdentityID,
"email": e.Email,
"full_name": e.FullName,
"role": e.Role,
"roles": e.Roles,
"job_title": e.JobTitle,
"is_admin": e.IsAdmin,
"mfa_status": e.MFAStatus,
@@ -346,7 +346,7 @@ SELECT
identity_id,
email,
full_name,
role,
roles,
job_title,
is_admin,
mfa_status,
@@ -414,7 +414,7 @@ SELECT
identity_id,
email,
full_name,
role,
roles,
job_title,
is_admin,
mfa_status,
@@ -630,7 +630,7 @@ INSERT INTO access_review_entries (
identity_id,
email,
full_name,
role,
roles,
job_title,
is_admin,
mfa_status,
@@ -659,7 +659,7 @@ INSERT INTO access_review_entries (
@identity_id,
@email,
@full_name,
@role,
COALESCE(@roles, '{}'::TEXT[]),
@job_title,
@is_admin,
@mfa_status,
@@ -683,7 +683,7 @@ INSERT INTO access_review_entries (
ON CONFLICT (access_review_campaign_source_id, account_key) DO UPDATE SET
email = EXCLUDED.email,
full_name = EXCLUDED.full_name,
role = EXCLUDED.role,
roles = EXCLUDED.roles,
job_title = EXCLUDED.job_title,
is_admin = EXCLUDED.is_admin,
mfa_status = EXCLUDED.mfa_status,
@@ -706,7 +706,7 @@ ON CONFLICT (access_review_campaign_source_id, account_key) DO UPDATE SET
"identity_id": e.IdentityID,
"email": e.Email,
"full_name": e.FullName,
"role": e.Role,
"roles": e.Roles,
"job_title": e.JobTitle,
"is_admin": e.IsAdmin,
"mfa_status": e.MFAStatus,

68
pkg/coredata/access_review_entry_upsert_test.go
View File

@@ -19,6 +19,7 @@ import (
"testing"
"time"
"github.com/jackc/pgx/v5"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.gearno.de/kit/pg"
@@ -152,7 +153,7 @@ func TestAccessReviewEntry_Upsert_FreezesDecidedFields(t *testing.T) {
originalFlags := []coredata.AccessReviewEntryFlag{coredata.AccessReviewEntryFlagNew}
originalEmail := "old@example.com"
originalFullName := "Old Name"
originalRole := "viewer"
originalRoles := []string{"viewer"}
t0 := time.Now().UTC().Truncate(time.Microsecond)
@@ -165,7 +166,7 @@ func TestAccessReviewEntry_Upsert_FreezesDecidedFields(t *testing.T) {
AccessReviewCampaignSourceID: fx.campaignSourceID,
Email: originalEmail,
FullName: originalFullName,
Role: originalRole,
Roles: originalRoles,
JobTitle: "",
IsAdmin: false,
MFAStatus: coredata.MFAStatusUnknown,
@@ -214,7 +215,7 @@ func TestAccessReviewEntry_Upsert_FreezesDecidedFields(t *testing.T) {
t2 := decisionTime.Add(1 * time.Hour)
secondEmail := "new@example.com"
secondFullName := "New Name"
secondRole := "admin"
secondRoles := []string{"admin"}
refresh := &coredata.AccessReviewEntry{
ID: gid.New(tenantID, coredata.AccessReviewEntryEntityType), // ignored by ON CONFLICT
OrganizationID: fx.organizationID,
@@ -222,7 +223,7 @@ func TestAccessReviewEntry_Upsert_FreezesDecidedFields(t *testing.T) {
AccessReviewCampaignSourceID: fx.campaignSourceID,
Email: secondEmail,
FullName: secondFullName,
Role: secondRole,
Roles: secondRoles,
JobTitle: "",
IsAdmin: true,
MFAStatus: coredata.MFAStatusEnabled,
@@ -271,7 +272,7 @@ func TestAccessReviewEntry_Upsert_FreezesDecidedFields(t *testing.T) {
// Columns that ARE refreshed on every poll.
assert.Equal(t, secondEmail, loaded.Email)
assert.Equal(t, secondFullName, loaded.FullName)
assert.Equal(t, secondRole, loaded.Role)
assert.Equal(t, secondRoles, loaded.Roles)
assert.True(t, loaded.IsAdmin)
assert.Equal(t, coredata.MFAStatusEnabled, loaded.MFAStatus)
assert.Equal(t, coredata.AccessReviewEntryAuthMethodSSO, loaded.AuthMethod)
@@ -303,7 +304,7 @@ func TestAccessReviewEntry_Upsert_RefreshesSourceTrackingFields(t *testing.T) {
AccessReviewCampaignSourceID: fx.campaignSourceID,
Email: "old@example.com",
FullName: "Old Name",
Role: "viewer",
Roles: []string{"viewer"},
MFAStatus: coredata.MFAStatusUnknown,
AuthMethod: coredata.AccessReviewEntryAuthMethodUnknown,
AccountType: coredata.AccessReviewEntryAccountTypeUser,
@@ -329,7 +330,7 @@ func TestAccessReviewEntry_Upsert_RefreshesSourceTrackingFields(t *testing.T) {
AccessReviewCampaignSourceID: fx.campaignSourceID,
Email: "new@example.com",
FullName: "New Name",
Role: "admin",
Roles: []string{"admin"},
MFAStatus: coredata.MFAStatusEnabled,
AuthMethod: coredata.AccessReviewEntryAuthMethodSSO,
AccountType: coredata.AccessReviewEntryAccountTypeUser,
@@ -356,7 +357,7 @@ func TestAccessReviewEntry_Upsert_RefreshesSourceTrackingFields(t *testing.T) {
// Source-tracking columns advanced to the second poll's values.
assert.Equal(t, "new@example.com", loaded.Email)
assert.Equal(t, "New Name", loaded.FullName)
assert.Equal(t, "admin", loaded.Role)
assert.Equal(t, []string{"admin"}, loaded.Roles)
assert.Equal(t, coredata.MFAStatusEnabled, loaded.MFAStatus)
assert.Equal(t, coredata.AccessReviewEntryAuthMethodSSO, loaded.AuthMethod)
@@ -393,7 +394,7 @@ func TestAccessReviewEntry_Upsert_InsertsActiveAccount(t *testing.T) {
AccessReviewCampaignSourceID: fx.campaignSourceID,
Email: "active@example.com",
FullName: "Active User",
Role: "member",
Roles: []string{"member"},
MFAStatus: coredata.MFAStatusUnknown,
AuthMethod: coredata.AccessReviewEntryAuthMethodUnknown,
AccountType: coredata.AccessReviewEntryAccountTypeUser,
@@ -424,3 +425,52 @@ func TestAccessReviewEntry_Upsert_InsertsActiveAccount(t *testing.T) {
assert.Nil(t, loaded.DecidedBy)
assert.Nil(t, loaded.DecidedAt)
}
func TestAccessReviewEntry_Upsert_NilRolesWritesEmptyArray(t *testing.T) {
t.Parallel()
client := test.PGClient(t)
ctx := context.Background()
fx := seedAccessReviewEntryFixture(t, ctx, client)
tenantID := fx.scope.GetTenantID()
t0 := time.Now().UTC().Truncate(time.Microsecond)
entryID := gid.New(tenantID, coredata.AccessReviewEntryEntityType)
entry := &coredata.AccessReviewEntry{
ID: entryID,
OrganizationID: fx.organizationID,
AccessReviewCampaignID: fx.campaignID,
AccessReviewCampaignSourceID: fx.campaignSourceID,
Email: "nil-roles@example.com",
FullName: "Nil Roles User",
Roles: nil,
MFAStatus: coredata.MFAStatusUnknown,
AuthMethod: coredata.AccessReviewEntryAuthMethodUnknown,
AccountType: coredata.AccessReviewEntryAccountTypeUser,
ExternalID: "ext-nil-roles",
AccountKey: "nil-roles@example.com",
IncrementalTag: coredata.AccessReviewEntryIncrementalTagNew,
Flags: []coredata.AccessReviewEntryFlag{},
FlagReasons: []string{},
Decision: coredata.AccessReviewEntryDecisionPending,
CreatedAt: t0,
UpdatedAt: t0,
}
require.NoError(t, client.WithTx(ctx, func(ctx context.Context, tx pg.Tx) error {
return entry.Upsert(ctx, tx, fx.scope)
}))
var roles []string
require.NoError(t, client.WithConn(ctx, func(ctx context.Context, conn pg.Querier) error {
return conn.QueryRow(
ctx,
`SELECT roles FROM access_review_entries WHERE id = @id`,
pgx.StrictNamedArgs{"id": entryID},
).Scan(&roles)
}))
assert.Equal(t, []string{}, roles)
}

26
pkg/coredata/migrations/20260612T160000Z.sql Normal file
View File

@@ -0,0 +1,26 @@
-- Copyright (c) 2026 Probo Inc <hello@probo.com>.
--
-- Permission to use, copy, modify, and/or distribute this software for any
-- purpose with or without fee is hereby granted, provided that the above
-- copyright notice and this permission notice appear in all copies.
--
-- THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH
-- REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
-- AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT,
-- INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
-- LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR
-- OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
-- PERFORMANCE OF THIS SOFTWARE.
ALTER TABLE access_review_entries
ADD COLUMN roles TEXT[] NOT NULL DEFAULT '{}';
UPDATE access_review_entries
SET roles = CASE
WHEN role = '' THEN '{}'::TEXT[]
WHEN position(', ' in role) > 0 THEN string_to_array(role, ', ')
ELSE ARRAY[role]
END;
ALTER TABLE access_review_entries
DROP COLUMN role;