From 1422b6d5dd9259ed022416eb3ace96c0302df0ec Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 27 Jul 2026 13:13:11 +0000 Subject: [PATCH] Express campaign client errors in the service layer Remove status-to-error helpers and map loaded campaign data to accessreview sentinels at each business rule instead of wrapping coredata status values. Signed-off-by: Cursor Agent Co-authored-by: Bryan FRIMIN --- pkg/accessreview/campaign_service.go | 95 ++++++++++++++++++++-------- pkg/accessreview/errors_test.go | 50 --------------- 2 files changed, 70 insertions(+), 75 deletions(-) delete mode 100644 pkg/accessreview/errors_test.go diff --git a/pkg/accessreview/campaign_service.go b/pkg/accessreview/campaign_service.go index dd4bd1f93..56daa955a 100644 --- a/pkg/accessreview/campaign_service.go +++ b/pkg/accessreview/campaign_service.go @@ -158,8 +158,18 @@ func (s *Service) UpdateCampaign( return fmt.Errorf("cannot load campaign: %w", err) } - if campaign.Status != coredata.AccessReviewCampaignStatusDraft { - return campaignNotDraftError(campaign.Status) + 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 req.Name != nil && *req.Name != nil { @@ -211,7 +221,16 @@ func (s *Service) DeleteCampaign( if campaign.Status != coredata.AccessReviewCampaignStatusDraft && campaign.Status != coredata.AccessReviewCampaignStatusCancelled { - return fmt.Errorf("cannot delete campaign: status is %s, expected %s or %s", campaign.Status, coredata.AccessReviewCampaignStatusDraft, coredata.AccessReviewCampaignStatusCancelled) + switch campaign.Status { + case coredata.AccessReviewCampaignStatusInProgress: + return ErrCampaignInProgress + case coredata.AccessReviewCampaignStatusPendingActions: + return ErrCampaignPendingActions + case coredata.AccessReviewCampaignStatusCompleted: + return ErrCampaignCompleted + default: + return ErrCampaignInProgress + } } if err := campaign.Delete(ctx, conn, scope); err != nil { @@ -241,8 +260,18 @@ func (s *Service) AddCampaignSource( return fmt.Errorf("cannot load campaign: %w", err) } - if campaign.Status != coredata.AccessReviewCampaignStatusDraft { - return campaignNotDraftError(campaign.Status) + 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 := source.LoadByID(ctx, conn, scope, req.AccessReviewSourceID); err != nil { if errors.Is(err, coredata.ErrResourceNotFound) { @@ -288,8 +317,18 @@ func (s *Service) RemoveCampaignSource( return fmt.Errorf("cannot load campaign: %w", err) } - if campaign.Status != coredata.AccessReviewCampaignStatusDraft { - return campaignNotDraftError(campaign.Status) + 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 := campaignSource.DeleteByCampaignIDAndAccessReviewSourceID(ctx, conn, scope, campaign.ID, req.AccessReviewSourceID); err != nil { return fmt.Errorf("cannot delete campaign source: %w", err) @@ -395,8 +434,18 @@ func (s *Service) StartCampaign( return fmt.Errorf("cannot load campaign: %w", err) } - if campaign.Status != coredata.AccessReviewCampaignStatusDraft { - return campaignNotDraftError(campaign.Status) + 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 } var campaignSources coredata.AccessReviewCampaignSources @@ -450,7 +499,18 @@ func (s *Service) CloseCampaign( } if campaign.Status != coredata.AccessReviewCampaignStatusPendingActions { - return fmt.Errorf("cannot close campaign: status is %s, expected %s", campaign.Status, coredata.AccessReviewCampaignStatusPendingActions) + switch campaign.Status { + case coredata.AccessReviewCampaignStatusInProgress: + return ErrCampaignInProgress + case coredata.AccessReviewCampaignStatusCompleted: + return ErrCampaignCompleted + case coredata.AccessReviewCampaignStatusCancelled: + return ErrCampaignCancelled + case coredata.AccessReviewCampaignStatusDraft: + return ErrCampaignInProgress + default: + return ErrCampaignInProgress + } } entries := coredata.AccessReviewEntries{} @@ -730,18 +790,3 @@ func (s *Service) CountCampaignsForOrganizationID( return count, nil } - -func campaignNotDraftError(status coredata.AccessReviewCampaignStatus) error { - switch status { - case coredata.AccessReviewCampaignStatusInProgress: - return ErrCampaignInProgress - case coredata.AccessReviewCampaignStatusPendingActions: - return ErrCampaignPendingActions - case coredata.AccessReviewCampaignStatusCompleted: - return ErrCampaignCompleted - case coredata.AccessReviewCampaignStatusCancelled: - return ErrCampaignCancelled - default: - return ErrCampaignInProgress - } -} diff --git a/pkg/accessreview/errors_test.go b/pkg/accessreview/errors_test.go deleted file mode 100644 index 9c6e469b7..000000000 --- a/pkg/accessreview/errors_test.go +++ /dev/null @@ -1,50 +0,0 @@ -// 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 - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "go.probo.inc/probo/pkg/coredata" -) - -func TestCampaignNotDraftError(t *testing.T) { - t.Parallel() - - tests := []struct { - status coredata.AccessReviewCampaignStatus - want error - }{ - {status: coredata.AccessReviewCampaignStatusInProgress, want: ErrCampaignInProgress}, - {status: coredata.AccessReviewCampaignStatusPendingActions, want: ErrCampaignPendingActions}, - {status: coredata.AccessReviewCampaignStatusCompleted, want: ErrCampaignCompleted}, - {status: coredata.AccessReviewCampaignStatusCancelled, want: ErrCampaignCancelled}, - } - - for _, tt := range tests { - t.Run(string(tt.status), func(t *testing.T) { - t.Parallel() - - assert.ErrorIs(t, campaignNotDraftError(tt.status), tt.want) - }) - } -}