From 352c551e53892621a652a081407725f10ab8dc4d Mon Sep 17 00:00:00 2001
From: Cursor Agent <cursoragent@cursor.com>
Date: Mon, 27 Jul 2026 13:06:39 +0000
Subject: [PATCH] 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 | 32 ++++++++++++++++++++++------
 pkg/accessreview/errors.go           | 21 +-----------------
 pkg/accessreview/errors_test.go      |  7 ++----
 3 files changed, 28 insertions(+), 32 deletions(-)

diff --git a/pkg/accessreview/campaign_service.go b/pkg/accessreview/campaign_service.go
index 4e531634d..dd4bd1f93 100644
--- a/pkg/accessreview/campaign_service.go
+++ b/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 ||
-				campaign.Status == coredata.AccessReviewCampaignStatusCancelled {
-				return fmt.Errorf("cannot update campaign: already %s", campaign.Status)
+			if campaign.Status == coredata.AccessReviewCampaignStatusCompleted {
+				return ErrCampaignCompleted
+			}
+
+			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
+	}
+}
diff --git a/pkg/accessreview/errors.go b/pkg/accessreview/errors.go
index d6259a090..8f1432761 100644
--- a/pkg/accessreview/errors.go
+++ b/pkg/accessreview/errors.go
@@ -20,11 +20,7 @@
 
 package accessreview
 
-import (
-	"errors"
-
-	"go.probo.inc/probo/pkg/coredata"
-)
+import "errors"
 
 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
-	}
-}
diff --git a/pkg/accessreview/errors_test.go b/pkg/accessreview/errors_test.go
index 8e81749eb..9c6e469b7 100644
--- a/pkg/accessreview/errors_test.go
+++ b/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, err, tt.want)
+			assert.ErrorIs(t, campaignNotDraftError(tt.status), tt.want)
 		})
 	}
 }