Return conflict when removing a referenced person
Deleting a profile still referenced elsewhere (for example as an asset owner) surfaced an internal error. PostgreSQL reports ON DELETE RESTRICT blocks as SQLSTATE 23001, not 23503; map both in profile delete and propagate ErrProfileInUse through removeUser as CONFLICT. Signed-off-by: Ludovic Vielle <ludovic@probo.com>
This commit is contained in:
committed by
Bryan Frimin
parent
9e47aba2b6
commit
888fa4d63a
@@ -20,6 +20,7 @@ import (
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"go.probo.inc/probo/e2e/internal/factory"
|
||||
"go.probo.inc/probo/e2e/internal/testutil"
|
||||
)
|
||||
|
||||
@@ -324,6 +325,67 @@ func TestUser_RemoveUser(t *testing.T) {
|
||||
assert.False(t, removedUserFound, "Should not find removed user")
|
||||
}
|
||||
|
||||
func TestUser_RemoveUser_ProfileInUse(t *testing.T) {
|
||||
t.Parallel()
|
||||
owner := testutil.NewClient(t, testutil.RoleOwner)
|
||||
profileID := factory.CreateUser(owner)
|
||||
|
||||
const createAssetMutation = `
|
||||
mutation($input: CreateAssetInput!) {
|
||||
createAsset(input: $input) {
|
||||
assetEdge {
|
||||
node {
|
||||
id
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
`
|
||||
|
||||
var createAssetResult struct {
|
||||
CreateAsset struct {
|
||||
AssetEdge struct {
|
||||
Node struct {
|
||||
ID string `json:"id"`
|
||||
} `json:"node"`
|
||||
} `json:"assetEdge"`
|
||||
} `json:"createAsset"`
|
||||
}
|
||||
|
||||
err := owner.Execute(createAssetMutation, map[string]any{
|
||||
"input": map[string]any{
|
||||
"organizationId": owner.GetOrganizationID().String(),
|
||||
"name": "Production Database Server",
|
||||
"amount": 1,
|
||||
"ownerId": profileID,
|
||||
"assetType": "VIRTUAL",
|
||||
"dataTypesStored": "Customer PII",
|
||||
},
|
||||
}, &createAssetResult)
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, createAssetResult.CreateAsset.AssetEdge.Node.ID)
|
||||
|
||||
const removeUserMutation = `
|
||||
mutation($input: RemoveUserInput!) {
|
||||
removeUser(input: $input) {
|
||||
deletedProfileId
|
||||
}
|
||||
}
|
||||
`
|
||||
|
||||
err = owner.ExecuteConnect(removeUserMutation, map[string]any{
|
||||
"input": map[string]any{
|
||||
"organizationId": owner.GetOrganizationID().String(),
|
||||
"profileId": profileID,
|
||||
},
|
||||
}, nil)
|
||||
testutil.RequireErrorCode(t, err, "CONFLICT")
|
||||
|
||||
var gqlErrors testutil.GraphQLErrors
|
||||
require.ErrorAs(t, err, &gqlErrors)
|
||||
assert.Contains(t, gqlErrors[0].Message, "referenced by other resources")
|
||||
}
|
||||
|
||||
func TestUser_ArchiveUser(t *testing.T) {
|
||||
t.Parallel()
|
||||
owner := testutil.NewClient(t, testutil.RoleOwner)
|
||||
|
||||
@@ -323,7 +323,7 @@ WHERE %s AND id = @id
|
||||
_, err := conn.Exec(ctx, q, args)
|
||||
if err != nil {
|
||||
if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok {
|
||||
if pgErr.Code == "23503" {
|
||||
if pgErr.Code == "23503" || pgErr.Code == "23001" {
|
||||
return ErrResourceInUse
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1263,7 +1263,7 @@ WHERE
|
||||
_, err := conn.Exec(ctx, q, args)
|
||||
if err != nil {
|
||||
if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok {
|
||||
if pgErr.Code == "23503" {
|
||||
if pgErr.Code == "23503" || pgErr.Code == "23001" {
|
||||
return ErrResourceInUse
|
||||
}
|
||||
}
|
||||
|
||||
@@ -180,6 +180,18 @@ func (e ErrLastActiveOwner) Error() string {
|
||||
return fmt.Sprintf("cannot remove profile %q: last active owner of the organization", e.MembershipID)
|
||||
}
|
||||
|
||||
type ErrProfileInUse struct {
|
||||
ProfileID gid.GID
|
||||
}
|
||||
|
||||
func NewProfileInUseError(profileID gid.GID) error {
|
||||
return &ErrProfileInUse{ProfileID: profileID}
|
||||
}
|
||||
|
||||
func (e ErrProfileInUse) Error() string {
|
||||
return fmt.Sprintf("cannot remove profile %q: referenced by other resources", e.ProfileID)
|
||||
}
|
||||
|
||||
type ErrOrganizationNotFound struct{ OrganizationID gid.GID }
|
||||
|
||||
func NewOrganizationNotFoundError(organizationID gid.GID) error {
|
||||
|
||||
@@ -354,6 +354,10 @@ func (s *OrganizationService) RemoveUser(
|
||||
}
|
||||
|
||||
if err := profile.Delete(ctx, tx, scope, profileID); err != nil {
|
||||
if errors.Is(err, coredata.ErrResourceInUse) {
|
||||
return NewProfileInUseError(profileID)
|
||||
}
|
||||
|
||||
return fmt.Errorf("cannot delete profile: %w", err)
|
||||
}
|
||||
|
||||
|
||||
@@ -145,6 +145,10 @@ func (r *mutationResolver) RemoveUser(ctx context.Context, input types.RemoveUse
|
||||
return nil, gqlutils.Conflictf(ctx, "cannot remove last active owner")
|
||||
}
|
||||
|
||||
if _, ok := errors.AsType[*iam.ErrProfileInUse](err); ok {
|
||||
return nil, gqlutils.Conflictf(ctx, "cannot remove person: referenced by other resources")
|
||||
}
|
||||
|
||||
r.logger.ErrorCtx(ctx, "cannot remove user from organization", log.Error(err))
|
||||
|
||||
return nil, gqlutils.Internal(ctx)
|
||||
|
||||
@@ -2954,6 +2954,10 @@ func (r *Resolver) RemoveUserTool(ctx context.Context, req *mcp.CallToolRequest,
|
||||
return nil, types.RemoveUserOutput{}, fmt.Errorf("cannot remove last active owner: %w", err)
|
||||
}
|
||||
|
||||
if _, ok := errors.AsType[*iam.ErrProfileInUse](err); ok {
|
||||
return nil, types.RemoveUserOutput{}, fmt.Errorf("cannot remove person: referenced by other resources: %w", err)
|
||||
}
|
||||
|
||||
return nil, types.RemoveUserOutput{}, fmt.Errorf("remove user: %w", err)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user