rohit/probo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Use sentinel errors for campaign validation failures

Browse Source 
Follow the cookiebanner pattern: grouped var Err* sentinels in the
service package, wrapped with fmt.Errorf where context is needed, and
explicit errors.Is checks in GraphQL resolvers.

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

Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
This commit is contained in:
Cursor Agent
2026-07-20 15:41:29 +00:00
parent a2b29b3d80
commit 0870e1dd15
4 changed files with 46 additions and 192 deletions
Download Patch File Download Diff File Expand all files Collapse all files

71
pkg/accessreview/campaign_service.go
View File

@@ -65,10 +65,11 @@ func (s *Service) CreateCampaign(
}
if source.OrganizationID != campaign.OrganizationID {
return &CampaignSourceOrganizationMismatchError{
Operation: "create",
SourceID: sourceID,
}
return fmt.Errorf(
"cannot create campaign: access source %s does not belong to the same organization: %w",
sourceID,
ErrCampaignSourceOrganizationMismatch,
)
}
if err := s.upsertCampaignSource(ctx, conn, scope, campaign.ID, source); err != nil {
@@ -157,11 +158,12 @@ func (s *Service) UpdateCampaign(
}
if campaign.Status != coredata.AccessReviewCampaignStatusDraft {
return &CampaignInvalidStatusError{
Operation: "update",
Status: campaign.Status,
Expected: coredata.AccessReviewCampaignStatusDraft,
}
return fmt.Errorf(
"cannot update campaign: status is %s, expected %s: %w",
campaign.Status,
coredata.AccessReviewCampaignStatusDraft,
ErrCampaignNotDraft,
)
}
if req.Name != nil && *req.Name != nil {
@@ -244,11 +246,12 @@ func (s *Service) AddCampaignSource(
}
if campaign.Status != coredata.AccessReviewCampaignStatusDraft {
return &CampaignInvalidStatusError{
Operation: "add scope source",
Status: campaign.Status,
Expected: coredata.AccessReviewCampaignStatusDraft,
}
return fmt.Errorf(
"cannot add scope source: campaign status is %s, expected %s: %w",
campaign.Status,
coredata.AccessReviewCampaignStatusDraft,
ErrCampaignNotDraft,
)
}
source := &coredata.AccessReviewSource{}
@@ -257,10 +260,11 @@ func (s *Service) AddCampaignSource(
}
if source.OrganizationID != campaign.OrganizationID {
return &CampaignSourceOrganizationMismatchError{
Operation: "add scope source",
SourceID: req.AccessReviewSourceID,
}
return fmt.Errorf(
"cannot add scope source: access source %q does not belong to the same organization: %w",
req.AccessReviewSourceID,
ErrCampaignSourceOrganizationMismatch,
)
}
if err := s.upsertCampaignSource(ctx, conn, scope, campaign.ID, source); err != nil {
@@ -296,11 +300,12 @@ func (s *Service) RemoveCampaignSource(
}
if campaign.Status != coredata.AccessReviewCampaignStatusDraft {
return &CampaignInvalidStatusError{
Operation: "remove scope source",
Status: campaign.Status,
Expected: coredata.AccessReviewCampaignStatusDraft,
}
return fmt.Errorf(
"cannot remove scope source: campaign status is %s, expected %s: %w",
campaign.Status,
coredata.AccessReviewCampaignStatusDraft,
ErrCampaignNotDraft,
)
}
campaignSource := &coredata.AccessReviewCampaignSource{}
@@ -348,10 +353,11 @@ func (s *Service) syncCampaignSources(
}
if source.OrganizationID != campaign.OrganizationID {
return &CampaignSourceOrganizationMismatchError{
Operation: "update",
SourceID: sourceID,
}
return fmt.Errorf(
"cannot update campaign: access source %s does not belong to the same organization: %w",
sourceID,
ErrCampaignSourceOrganizationMismatch,
)
}
if err := s.upsertCampaignSource(ctx, conn, scope, campaign.ID, source); err != nil {
@@ -408,11 +414,12 @@ func (s *Service) StartCampaign(
}
if campaign.Status != coredata.AccessReviewCampaignStatusDraft {
return &CampaignInvalidStatusError{
Operation: "start",
Status: campaign.Status,
Expected: coredata.AccessReviewCampaignStatusDraft,
}
return fmt.Errorf(
"cannot start campaign: status is %s, expected %s: %w",
campaign.Status,
coredata.AccessReviewCampaignStatusDraft,
ErrCampaignNotDraft,
)
}
var campaignSources coredata.AccessReviewCampaignSources

74
pkg/accessreview/errors.go
View File

@@ -20,80 +20,10 @@
package accessreview
import (
"errors"
"fmt"
"go.probo.inc/probo/pkg/coredata"
"go.probo.inc/probo/pkg/gid"
)
import "errors"
var (
ErrCampaignNoScopeSources = errors.New("cannot start campaign: no scope sources configured")
ErrCampaignInvalidStatus = errors.New("campaign status does not allow this operation")
ErrCampaignNotDraft = errors.New("campaign must be in draft status")
ErrCampaignSourceOrganizationMismatch = errors.New("access source does not belong to the same organization")
)
type CampaignInvalidStatusError struct {
Operation string
Status coredata.AccessReviewCampaignStatus
Expected coredata.AccessReviewCampaignStatus
}
func (e *CampaignInvalidStatusError) Error() string {
switch e.Operation {
case "add scope source", "remove scope source":
return fmt.Sprintf(
"cannot %s: campaign status is %s, expected %s",
e.Operation,
e.Status,
e.Expected,
)
default:
return fmt.Sprintf(
"cannot %s campaign: status is %s, expected %s",
e.Operation,
e.Status,
e.Expected,
)
}
}
func (e *CampaignInvalidStatusError) Is(target error) bool {
return target == ErrCampaignInvalidStatus
}
type CampaignSourceOrganizationMismatchError struct {
Operation string
SourceID gid.GID
}
func (e *CampaignSourceOrganizationMismatchError) Error() string {
switch e.Operation {
case "create":
return fmt.Sprintf(
"cannot create campaign: access source %s does not belong to the same organization",
e.SourceID,
)
case "update":
return fmt.Sprintf(
"cannot update campaign: access source %s does not belong to the same organization",
e.SourceID,
)
default:
return fmt.Sprintf(
"cannot add scope source: access source %q does not belong to the same organization",
e.SourceID,
)
}
}
func (e *CampaignSourceOrganizationMismatchError) Is(target error) bool {
return target == ErrCampaignSourceOrganizationMismatch
}
func IsCampaignClientError(err error) bool {
return errors.Is(err, ErrCampaignNoScopeSources) ||
errors.Is(err, ErrCampaignInvalidStatus) ||
errors.Is(err, ErrCampaignSourceOrganizationMismatch)
}

85
pkg/accessreview/errors_test.go
View File

@@ -1,85 +0,0 @@
// Copyright (c) 2026 Probo Inc <hello@probo.com>.
//
// 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 (
"errors"
"fmt"
"testing"
"github.com/stretchr/testify/assert"
"go.probo.inc/probo/pkg/coredata"
"go.probo.inc/probo/pkg/gid"
)
func TestCampaignClientErrors(t *testing.T) {
t.Parallel()
statusErr := &CampaignInvalidStatusError{
Operation: "start",
Status: coredata.AccessReviewCampaignStatusInProgress,
Expected: coredata.AccessReviewCampaignStatusDraft,
}
sourceErr := &CampaignSourceOrganizationMismatchError{
Operation: "update",
SourceID: gid.GID("source-id"),
}
tests := []struct {
name string
err error
want bool
}{
{
name: "no scope sources",
err: ErrCampaignNoScopeSources,
want: true,
},
{
name: "invalid status",
err: statusErr,
want: true,
},
{
name: "wrapped invalid status",
err: fmt.Errorf("cannot start access review campaign: %w", statusErr),
want: true,
},
{
name: "source organization mismatch",
err: sourceErr,
want: true,
},
{
name: "internal error",
err: errors.New("cannot lock campaign: timeout"),
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
assert.Equal(t, tt.want, IsCampaignClientError(tt.err))
})
}
}

8
pkg/server/api/console/v1/access_review_campaign_resolvers.go
View File

@@ -724,7 +724,7 @@ func (r *mutationResolver) CreateAccessReviewCampaign(ctx context.Context, input
},
)
if err != nil {
if accessreview.IsCampaignClientError(err) {
if errors.Is(err, accessreview.ErrCampaignSourceOrganizationMismatch) {
return nil, gqlutils.Invalid(ctx, err)
}
@@ -760,7 +760,8 @@ func (r *mutationResolver) UpdateAccessReviewCampaign(ctx context.Context, input
return nil, gqlutils.NotFound(ctx, err)
}
if accessreview.IsCampaignClientError(err) {
if errors.Is(err, accessreview.ErrCampaignNotDraft) ||
errors.Is(err, accessreview.ErrCampaignSourceOrganizationMismatch) {
return nil, gqlutils.Invalid(ctx, err)
}
@@ -805,7 +806,8 @@ func (r *mutationResolver) StartAccessReviewCampaign(ctx context.Context, input
campaign, err := r.accessReview.StartCampaign(ctx, scope, input.AccessReviewCampaignID)
if err != nil {
if accessreview.IsCampaignClientError(err) {
if errors.Is(err, accessreview.ErrCampaignNoScopeSources) ||
errors.Is(err, accessreview.ErrCampaignNotDraft) {
return nil, gqlutils.Invalid(ctx, err)
}