From 6708cc901ca80c8c7699cb2e170434d8551766e9 Mon Sep 17 00:00:00 2001 From: Bryan Frimin Date: Thu, 7 May 2026 09:14:51 +0200 Subject: [PATCH] Drop FAILED status from access review campaigns Source fetch failures stay surfaced on the source fetch (status and last error) instead of failing the entire campaign, so reviewers can proceed on the sources that succeeded. Signed-off-by: Bryan Frimin --- .../_components/accessReviewHelpers.tsx | 3 -- cmd/probod/CHANGELOG.md | 4 ++ pkg/accessreview/campaign_service.go | 5 +-- pkg/accessreview/worker.go | 11 +---- pkg/coredata/access_review_campaign_status.go | 3 -- .../access_review_campaign_status_test.go | 1 - pkg/coredata/migrations/20260507T061947Z.sql | 40 +++++++++++++++++++ .../v1/graphql/access_review_campaign.graphql | 4 -- pkg/server/api/mcp/v1/specification.yaml | 1 - 9 files changed, 47 insertions(+), 25 deletions(-) create mode 100644 pkg/coredata/migrations/20260507T061947Z.sql diff --git a/apps/console/src/pages/organizations/access-reviews/_components/accessReviewHelpers.tsx b/apps/console/src/pages/organizations/access-reviews/_components/accessReviewHelpers.tsx index 09895037d..873426e61 100644 --- a/apps/console/src/pages/organizations/access-reviews/_components/accessReviewHelpers.tsx +++ b/apps/console/src/pages/organizations/access-reviews/_components/accessReviewHelpers.tsx @@ -24,7 +24,6 @@ export function statusBadgeVariant(status: string): BadgeVariant { return "warning"; case "COMPLETED": return "success"; - case "FAILED": case "CANCELLED": return "danger"; default: @@ -45,8 +44,6 @@ export function statusLabel( return __("Pending actions"); case "COMPLETED": return __("Completed"); - case "FAILED": - return __("Failed"); case "CANCELLED": return __("Cancelled"); default: diff --git a/cmd/probod/CHANGELOG.md b/cmd/probod/CHANGELOG.md index 6099a8387..386138d91 100644 --- a/cmd/probod/CHANGELOG.md +++ b/cmd/probod/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to `probod` (the server, including the bundled `@probo/conso ## Unreleased +### Changed + +- Access review campaigns no longer transition to `FAILED` when individual sources fail to fetch; the failure stays surfaced on the source fetch (status + last error) and reviewers can proceed on the sources that succeeded (breaking: removed `FAILED` from `AccessReviewCampaignStatus`) + ## [0.182.0] - 2026-05-06 ### Added diff --git a/pkg/accessreview/campaign_service.go b/pkg/accessreview/campaign_service.go index 0fe5d768b..442e151b9 100644 --- a/pkg/accessreview/campaign_service.go +++ b/pkg/accessreview/campaign_service.go @@ -301,9 +301,8 @@ func (s *CampaignService) Start( return fmt.Errorf("cannot load campaign: %w", err) } - if campaign.Status != coredata.AccessReviewCampaignStatusDraft && - campaign.Status != coredata.AccessReviewCampaignStatusFailed { - return fmt.Errorf("cannot start campaign: status is %s, expected %s or %s", campaign.Status, coredata.AccessReviewCampaignStatusDraft, coredata.AccessReviewCampaignStatusFailed) + if campaign.Status != coredata.AccessReviewCampaignStatusDraft { + return fmt.Errorf("cannot start campaign: status is %s, expected %s", campaign.Status, coredata.AccessReviewCampaignStatusDraft) } var sources coredata.AccessSources diff --git a/pkg/accessreview/worker.go b/pkg/accessreview/worker.go index 1366a82cf..3a08fd112 100644 --- a/pkg/accessreview/worker.go +++ b/pkg/accessreview/worker.go @@ -242,22 +242,13 @@ func (h *sourceFetchHandler) finalizeCampaignFetchLifecycle( return nil } - hasFailure := false for _, fetch := range fetches { if !fetch.Status.IsTerminal() { return nil } - if fetch.Status == coredata.AccessReviewCampaignSourceFetchStatusFailed { - hasFailure = true - } - } - - if hasFailure { - campaign.Status = coredata.AccessReviewCampaignStatusFailed - } else { - campaign.Status = coredata.AccessReviewCampaignStatusPendingActions } + campaign.Status = coredata.AccessReviewCampaignStatusPendingActions campaign.UpdatedAt = time.Now() return campaign.Update(ctx, tx, scope) }, diff --git a/pkg/coredata/access_review_campaign_status.go b/pkg/coredata/access_review_campaign_status.go index 27eccab50..a1892a8e8 100644 --- a/pkg/coredata/access_review_campaign_status.go +++ b/pkg/coredata/access_review_campaign_status.go @@ -25,7 +25,6 @@ const ( AccessReviewCampaignStatusDraft AccessReviewCampaignStatus = "DRAFT" AccessReviewCampaignStatusInProgress AccessReviewCampaignStatus = "IN_PROGRESS" AccessReviewCampaignStatusPendingActions AccessReviewCampaignStatus = "PENDING_ACTIONS" - AccessReviewCampaignStatusFailed AccessReviewCampaignStatus = "FAILED" AccessReviewCampaignStatusCompleted AccessReviewCampaignStatus = "COMPLETED" AccessReviewCampaignStatusCancelled AccessReviewCampaignStatus = "CANCELLED" ) @@ -52,8 +51,6 @@ func (s *AccessReviewCampaignStatus) Scan(value any) error { *s = AccessReviewCampaignStatusInProgress case "PENDING_ACTIONS": *s = AccessReviewCampaignStatusPendingActions - case "FAILED": - *s = AccessReviewCampaignStatusFailed case "COMPLETED": *s = AccessReviewCampaignStatusCompleted case "CANCELLED": diff --git a/pkg/coredata/access_review_campaign_status_test.go b/pkg/coredata/access_review_campaign_status_test.go index 1924b586b..7fefc7f6a 100644 --- a/pkg/coredata/access_review_campaign_status_test.go +++ b/pkg/coredata/access_review_campaign_status_test.go @@ -28,7 +28,6 @@ func TestAccessReviewCampaignStatusScan(t *testing.T) { {name: "draft string", input: "DRAFT", want: AccessReviewCampaignStatusDraft}, {name: "in_progress string", input: "IN_PROGRESS", want: AccessReviewCampaignStatusInProgress}, {name: "pending_actions string", input: "PENDING_ACTIONS", want: AccessReviewCampaignStatusPendingActions}, - {name: "failed string", input: "FAILED", want: AccessReviewCampaignStatusFailed}, {name: "completed string", input: "COMPLETED", want: AccessReviewCampaignStatusCompleted}, {name: "cancelled bytes", input: []byte("CANCELLED"), want: AccessReviewCampaignStatusCancelled}, {name: "invalid value", input: "BOGUS", wantErr: true}, diff --git a/pkg/coredata/migrations/20260507T061947Z.sql b/pkg/coredata/migrations/20260507T061947Z.sql new file mode 100644 index 000000000..9252584e5 --- /dev/null +++ b/pkg/coredata/migrations/20260507T061947Z.sql @@ -0,0 +1,40 @@ +-- Copyright (c) 2026 Probo Inc . +-- +-- Permission to use, copy, modify, and/or distribute this software for any +-- purpose with or without fee is hereby granted, provided that the above +-- copyright notice and this permission notice appear in all copies. +-- +-- THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH +-- REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +-- AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, +-- INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +-- LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR +-- OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +-- PERFORMANCE OF THIS SOFTWARE. + +-- Drop the FAILED status from access_review_campaign_status. A campaign is +-- no longer marked as failed when individual sources fail to fetch: the +-- failure stays surfaced on the source fetch (status + last_error) and the +-- review can still be performed on the sources that succeeded. + +UPDATE access_review_campaigns +SET status = 'PENDING_ACTIONS' +WHERE status = 'FAILED'; + +ALTER TYPE access_review_campaign_status RENAME TO access_review_campaign_status_old; + +CREATE TYPE access_review_campaign_status AS ENUM ( + 'DRAFT', + 'IN_PROGRESS', + 'PENDING_ACTIONS', + 'COMPLETED', + 'CANCELLED' +); + +ALTER TABLE access_review_campaigns + ALTER COLUMN status DROP DEFAULT, + ALTER COLUMN status TYPE access_review_campaign_status + USING status::text::access_review_campaign_status, + ALTER COLUMN status SET DEFAULT 'DRAFT'; + +DROP TYPE access_review_campaign_status_old; diff --git a/pkg/server/api/console/v1/graphql/access_review_campaign.graphql b/pkg/server/api/console/v1/graphql/access_review_campaign.graphql index ba69e97e7..3229f89b1 100644 --- a/pkg/server/api/console/v1/graphql/access_review_campaign.graphql +++ b/pkg/server/api/console/v1/graphql/access_review_campaign.graphql @@ -14,10 +14,6 @@ enum AccessReviewCampaignStatus @goEnum( value: "go.probo.inc/probo/pkg/coredata.AccessReviewCampaignStatusPendingActions" ) - FAILED - @goEnum( - value: "go.probo.inc/probo/pkg/coredata.AccessReviewCampaignStatusFailed" - ) COMPLETED @goEnum( value: "go.probo.inc/probo/pkg/coredata.AccessReviewCampaignStatusCompleted" diff --git a/pkg/server/api/mcp/v1/specification.yaml b/pkg/server/api/mcp/v1/specification.yaml index 39281dc8d..97a108c79 100644 --- a/pkg/server/api/mcp/v1/specification.yaml +++ b/pkg/server/api/mcp/v1/specification.yaml @@ -7189,7 +7189,6 @@ components: - DRAFT - IN_PROGRESS - PENDING_ACTIONS - - FAILED - COMPLETED - CANCELLED go.probo.inc/mcpgen/type: go.probo.inc/probo/pkg/coredata.AccessReviewCampaignStatus