From 5dca6bb883a816218eb4973996c2e3e0a240fbb5 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 27 Jul 2026 13:16:31 +0000 Subject: [PATCH] Use struct campaign errors with campaign ID in message Business-layer errors carry the access review campaign GID in Error() and match GraphQL sentinels via Is(). Signed-off-by: Cursor Agent Co-authored-by: Bryan FRIMIN --- pkg/accessreview/campaign_service.go | 101 +++++++++++--------------- pkg/accessreview/errors.go | 102 +++++++++++++++++++++++++-- pkg/accessreview/errors_test.go | 88 +++++++++++++++++++++++ 3 files changed, 225 insertions(+), 66 deletions(-) create mode 100644 pkg/accessreview/errors_test.go diff --git a/pkg/accessreview/campaign_service.go b/pkg/accessreview/campaign_service.go index 56daa955a..c7721e5ac 100644 --- a/pkg/accessreview/campaign_service.go +++ b/pkg/accessreview/campaign_service.go @@ -158,18 +158,8 @@ func (s *Service) UpdateCampaign( return fmt.Errorf("cannot load campaign: %w", err) } - switch campaign.Status { - case coredata.AccessReviewCampaignStatusDraft: - case coredata.AccessReviewCampaignStatusInProgress: - return ErrCampaignInProgress - case coredata.AccessReviewCampaignStatusPendingActions: - return ErrCampaignPendingActions - case coredata.AccessReviewCampaignStatusCompleted: - return ErrCampaignCompleted - case coredata.AccessReviewCampaignStatusCancelled: - return ErrCampaignCancelled - default: - return ErrCampaignInProgress + if err := errUnlessDraftCampaign(campaign); err != nil { + return err } if req.Name != nil && *req.Name != nil { @@ -223,13 +213,13 @@ func (s *Service) DeleteCampaign( campaign.Status != coredata.AccessReviewCampaignStatusCancelled { switch campaign.Status { case coredata.AccessReviewCampaignStatusInProgress: - return ErrCampaignInProgress + return NewCampaignInProgressError(campaign.ID) case coredata.AccessReviewCampaignStatusPendingActions: - return ErrCampaignPendingActions + return NewCampaignPendingActionsError(campaign.ID) case coredata.AccessReviewCampaignStatusCompleted: - return ErrCampaignCompleted + return NewCampaignCompletedError(campaign.ID) default: - return ErrCampaignInProgress + return NewCampaignInProgressError(campaign.ID) } } @@ -260,18 +250,8 @@ func (s *Service) AddCampaignSource( return fmt.Errorf("cannot load campaign: %w", err) } - switch campaign.Status { - case coredata.AccessReviewCampaignStatusDraft: - case coredata.AccessReviewCampaignStatusInProgress: - return ErrCampaignInProgress - case coredata.AccessReviewCampaignStatusPendingActions: - return ErrCampaignPendingActions - case coredata.AccessReviewCampaignStatusCompleted: - return ErrCampaignCompleted - case coredata.AccessReviewCampaignStatusCancelled: - return ErrCampaignCancelled - default: - return ErrCampaignInProgress + if err := errUnlessDraftCampaign(campaign); err != nil { + return err } if err := source.LoadByID(ctx, conn, scope, req.AccessReviewSourceID); err != nil { if errors.Is(err, coredata.ErrResourceNotFound) { @@ -317,19 +297,11 @@ func (s *Service) RemoveCampaignSource( return fmt.Errorf("cannot load campaign: %w", err) } - switch campaign.Status { - case coredata.AccessReviewCampaignStatusDraft: - case coredata.AccessReviewCampaignStatusInProgress: - return ErrCampaignInProgress - case coredata.AccessReviewCampaignStatusPendingActions: - return ErrCampaignPendingActions - case coredata.AccessReviewCampaignStatusCompleted: - return ErrCampaignCompleted - case coredata.AccessReviewCampaignStatusCancelled: - return ErrCampaignCancelled - default: - return ErrCampaignInProgress + if err := errUnlessDraftCampaign(campaign); err != nil { + return err } + + campaignSource := &coredata.AccessReviewCampaignSource{} if err := campaignSource.DeleteByCampaignIDAndAccessReviewSourceID(ctx, conn, scope, campaign.ID, req.AccessReviewSourceID); err != nil { return fmt.Errorf("cannot delete campaign source: %w", err) } @@ -434,18 +406,8 @@ func (s *Service) StartCampaign( return fmt.Errorf("cannot load campaign: %w", err) } - switch campaign.Status { - case coredata.AccessReviewCampaignStatusDraft: - case coredata.AccessReviewCampaignStatusInProgress: - return ErrCampaignInProgress - case coredata.AccessReviewCampaignStatusPendingActions: - return ErrCampaignPendingActions - case coredata.AccessReviewCampaignStatusCompleted: - return ErrCampaignCompleted - case coredata.AccessReviewCampaignStatusCancelled: - return ErrCampaignCancelled - default: - return ErrCampaignInProgress + if err := errUnlessDraftCampaign(campaign); err != nil { + return err } var campaignSources coredata.AccessReviewCampaignSources @@ -454,7 +416,7 @@ func (s *Service) StartCampaign( } if len(campaignSources) == 0 { - return ErrCampaignMissingSources + return NewCampaignMissingSourcesError(campaign.ID) } now := time.Now() @@ -501,15 +463,15 @@ func (s *Service) CloseCampaign( if campaign.Status != coredata.AccessReviewCampaignStatusPendingActions { switch campaign.Status { case coredata.AccessReviewCampaignStatusInProgress: - return ErrCampaignInProgress + return NewCampaignInProgressError(campaign.ID) case coredata.AccessReviewCampaignStatusCompleted: - return ErrCampaignCompleted + return NewCampaignCompletedError(campaign.ID) case coredata.AccessReviewCampaignStatusCancelled: - return ErrCampaignCancelled + return NewCampaignCancelledError(campaign.ID) case coredata.AccessReviewCampaignStatusDraft: - return ErrCampaignInProgress + return NewCampaignInProgressError(campaign.ID) default: - return ErrCampaignInProgress + return NewCampaignInProgressError(campaign.ID) } } @@ -635,11 +597,11 @@ func (s *Service) CancelCampaign( } if campaign.Status == coredata.AccessReviewCampaignStatusCompleted { - return ErrCampaignCompleted + return NewCampaignCompletedError(campaign.ID) } if campaign.Status == coredata.AccessReviewCampaignStatusCancelled { - return ErrCampaignCancelled + return NewCampaignCancelledError(campaign.ID) } now := time.Now() @@ -790,3 +752,22 @@ func (s *Service) CountCampaignsForOrganizationID( return count, nil } + +func errUnlessDraftCampaign(campaign *coredata.AccessReviewCampaign) error { + if campaign.Status == coredata.AccessReviewCampaignStatusDraft { + return nil + } + + switch campaign.Status { + case coredata.AccessReviewCampaignStatusInProgress: + return NewCampaignInProgressError(campaign.ID) + case coredata.AccessReviewCampaignStatusPendingActions: + return NewCampaignPendingActionsError(campaign.ID) + case coredata.AccessReviewCampaignStatusCompleted: + return NewCampaignCompletedError(campaign.ID) + case coredata.AccessReviewCampaignStatusCancelled: + return NewCampaignCancelledError(campaign.ID) + default: + return NewCampaignInProgressError(campaign.ID) + } +} diff --git a/pkg/accessreview/errors.go b/pkg/accessreview/errors.go index 8f1432761..ee87b1859 100644 --- a/pkg/accessreview/errors.go +++ b/pkg/accessreview/errors.go @@ -20,12 +20,102 @@ package accessreview -import "errors" +import ( + "errors" + "fmt" + + "go.probo.inc/probo/pkg/gid" +) var ( - ErrCampaignMissingSources = errors.New("cannot start campaign: no scope sources configured") - ErrCampaignInProgress = errors.New("campaign is in progress") - ErrCampaignPendingActions = errors.New("campaign is pending actions") - ErrCampaignCompleted = errors.New("campaign is completed") - ErrCampaignCancelled = errors.New("campaign is cancelled") + ErrCampaignMissingSources = errors.New("access review campaign missing scope sources") + ErrCampaignInProgress = errors.New("access review campaign in progress") + ErrCampaignPendingActions = errors.New("access review campaign pending actions") + ErrCampaignCompleted = errors.New("access review campaign completed") + ErrCampaignCancelled = errors.New("access review campaign cancelled") ) + +type ( + CampaignMissingSourcesError struct { + CampaignID gid.GID + } + + CampaignInProgressError struct { + CampaignID gid.GID + } + + CampaignPendingActionsError struct { + CampaignID gid.GID + } + + CampaignCompletedError struct { + CampaignID gid.GID + } + + CampaignCancelledError struct { + CampaignID gid.GID + } +) + +func NewCampaignMissingSourcesError(campaignID gid.GID) error { + return &CampaignMissingSourcesError{CampaignID: campaignID} +} + +func (e *CampaignMissingSourcesError) Error() string { + return fmt.Sprintf( + "access review campaign %q cannot be started: no scope sources configured", + e.CampaignID, + ) +} + +func (e *CampaignMissingSourcesError) Is(target error) bool { + return target == ErrCampaignMissingSources +} + +func NewCampaignInProgressError(campaignID gid.GID) error { + return &CampaignInProgressError{CampaignID: campaignID} +} + +func (e *CampaignInProgressError) Error() string { + return fmt.Sprintf("access review campaign %q is in progress", e.CampaignID) +} + +func (e *CampaignInProgressError) Is(target error) bool { + return target == ErrCampaignInProgress +} + +func NewCampaignPendingActionsError(campaignID gid.GID) error { + return &CampaignPendingActionsError{CampaignID: campaignID} +} + +func (e *CampaignPendingActionsError) Error() string { + return fmt.Sprintf("access review campaign %q is pending actions", e.CampaignID) +} + +func (e *CampaignPendingActionsError) Is(target error) bool { + return target == ErrCampaignPendingActions +} + +func NewCampaignCompletedError(campaignID gid.GID) error { + return &CampaignCompletedError{CampaignID: campaignID} +} + +func (e *CampaignCompletedError) Error() string { + return fmt.Sprintf("access review campaign %q is completed", e.CampaignID) +} + +func (e *CampaignCompletedError) Is(target error) bool { + return target == ErrCampaignCompleted +} + +func NewCampaignCancelledError(campaignID gid.GID) error { + return &CampaignCancelledError{CampaignID: campaignID} +} + +func (e *CampaignCancelledError) Error() string { + return fmt.Sprintf("access review campaign %q is cancelled", e.CampaignID) +} + +func (e *CampaignCancelledError) Is(target error) bool { + return target == ErrCampaignCancelled +} diff --git a/pkg/accessreview/errors_test.go b/pkg/accessreview/errors_test.go new file mode 100644 index 000000000..05a112f67 --- /dev/null +++ b/pkg/accessreview/errors_test.go @@ -0,0 +1,88 @@ +// Copyright (c) 2026 Probo Inc . +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package accessreview_test + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "go.probo.inc/probo/pkg/accessreview" + "go.probo.inc/probo/pkg/coredata" + "go.probo.inc/probo/pkg/gid" +) + +func TestCampaignClientErrors(t *testing.T) { + t.Parallel() + + tenantID := gid.NewTenantID() + campaignID := gid.New(tenantID, coredata.AccessReviewCampaignEntityType) + + tests := []struct { + name string + err error + wantText string + sentinel error + }{ + { + name: "missing sources", + err: accessreview.NewCampaignMissingSourcesError(campaignID), + wantText: fmt.Sprintf( + "access review campaign %q cannot be started: no scope sources configured", + campaignID, + ), + sentinel: accessreview.ErrCampaignMissingSources, + }, + { + name: "in progress", + err: accessreview.NewCampaignInProgressError(campaignID), + wantText: fmt.Sprintf("access review campaign %q is in progress", campaignID), + sentinel: accessreview.ErrCampaignInProgress, + }, + { + name: "pending actions", + err: accessreview.NewCampaignPendingActionsError(campaignID), + wantText: fmt.Sprintf("access review campaign %q is pending actions", campaignID), + sentinel: accessreview.ErrCampaignPendingActions, + }, + { + name: "completed", + err: accessreview.NewCampaignCompletedError(campaignID), + wantText: fmt.Sprintf("access review campaign %q is completed", campaignID), + sentinel: accessreview.ErrCampaignCompleted, + }, + { + name: "cancelled", + err: accessreview.NewCampaignCancelledError(campaignID), + wantText: fmt.Sprintf("access review campaign %q is cancelled", campaignID), + sentinel: accessreview.ErrCampaignCancelled, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + assert.Equal(t, tt.wantText, tt.err.Error()) + assert.ErrorIs(t, tt.err, tt.sentinel) + }) + } +}