Return campaign status sentinels without wrapping

Drop CampaignStatusError from errors.go and map coredata status to
client errors in the service layer only. Cancel on terminal statuses
uses the same sentinels instead of fmt status strings.

Signed-off-by: Cursor Agent <cursoragent@cursor.com>

Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>

pkg/accessreview/campaign_service.go

@@ -159,7 +159,7 @@ func (s *Service) UpdateCampaign(
 			}
 
 			if campaign.Status != coredata.AccessReviewCampaignStatusDraft {
-				return fmt.Errorf("cannot update campaign: %w", CampaignStatusError(campaign.Status))
+				return campaignNotDraftError(campaign.Status)
 			}
 
 			if req.Name != nil && *req.Name != nil {
@@ -242,7 +242,7 @@ func (s *Service) AddCampaignSource(
 			}
 
 			if campaign.Status != coredata.AccessReviewCampaignStatusDraft {
-				return fmt.Errorf("cannot add scope source: %w", CampaignStatusError(campaign.Status))
+				return campaignNotDraftError(campaign.Status)
 			}
 			if err := source.LoadByID(ctx, conn, scope, req.AccessReviewSourceID); err != nil {
 				if errors.Is(err, coredata.ErrResourceNotFound) {
@@ -289,7 +289,7 @@ func (s *Service) RemoveCampaignSource(
 			}
 
 			if campaign.Status != coredata.AccessReviewCampaignStatusDraft {
-				return fmt.Errorf("cannot remove scope source: %w", CampaignStatusError(campaign.Status))
+				return campaignNotDraftError(campaign.Status)
 			}
 			if err := campaignSource.DeleteByCampaignIDAndAccessReviewSourceID(ctx, conn, scope, campaign.ID, req.AccessReviewSourceID); err != nil {
 				return fmt.Errorf("cannot delete campaign source: %w", err)
@@ -396,7 +396,7 @@ func (s *Service) StartCampaign(
 			}
 
 			if campaign.Status != coredata.AccessReviewCampaignStatusDraft {
-				return fmt.Errorf("cannot start campaign: %w", CampaignStatusError(campaign.Status))
+				return campaignNotDraftError(campaign.Status)
 			}
 
 			var campaignSources coredata.AccessReviewCampaignSources
@@ -574,9 +574,12 @@ func (s *Service) CancelCampaign(
 				return fmt.Errorf("cannot load campaign: %w", err)
 			}
 
-			if campaign.Status == coredata.AccessReviewCampaignStatusCompleted ||
+			if campaign.Status == coredata.AccessReviewCampaignStatusCompleted {
-				campaign.Status == coredata.AccessReviewCampaignStatusCancelled {
+				return ErrCampaignCompleted
-				return fmt.Errorf("cannot update campaign: already %s", campaign.Status)
+			}
+
+			if campaign.Status == coredata.AccessReviewCampaignStatusCancelled {
+				return ErrCampaignCancelled
 			}
 
 			now := time.Now()
@@ -727,3 +730,18 @@ 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
+	}
+}

pkg/accessreview/errors.go

@@ -20,11 +20,7 @@
 
 package accessreview
 
-import (
+import "errors"
-	"errors"
-
-	"go.probo.inc/probo/pkg/coredata"
-)
 
 var (
 	ErrCampaignMissingSources = errors.New("cannot start campaign: no scope sources configured")
@@ -33,18 +29,3 @@ var (
 	ErrCampaignCompleted      = errors.New("campaign is completed")
 	ErrCampaignCancelled      = errors.New("campaign is cancelled")
 )
-
-func CampaignStatusError(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
-	}
-}

pkg/accessreview/errors_test.go

@@ -21,15 +21,13 @@
 package accessreview
 
 import (
-	"errors"
-	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"go.probo.inc/probo/pkg/coredata"
 )
 
-func TestCampaignStatusError(t *testing.T) {
+func TestCampaignNotDraftError(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
@@ -46,8 +44,7 @@ func TestCampaignStatusError(t *testing.T) {
 		t.Run(string(tt.status), func(t *testing.T) {
 			t.Parallel()
 
-			err := fmt.Errorf("cannot start campaign: %w", CampaignStatusError(tt.status))
+			assert.ErrorIs(t, campaignNotDraftError(tt.status), tt.want)
-			assert.ErrorIs(t, err, tt.want)
 		})
 	}
 }