feat(requests): add cancel endpoint and tighten state guards
Owners can now POST /requests/{id}/cancel to withdraw a pending
request; admins can cancel any active request that has not entered
the fulfillment pipeline. The route is mounted on both the user
group (with profile required) and the admin group. The cancelled
outcome was already reserved in the migration's CHECK constraint
but was unreachable from any handler.
Decline now also rejects approved requests — between Approve setting
StatusApproved and the reconciler picking the request up, an admin
could declare the request declined while submission was about to
fire. The reconciler's outcome filter would skip the request, but
the narrow window meant external state could diverge from Silo's
view. Refuse decline once a request is approved; callers should
wait for completion or use the failed/retry path.
Reconcile now emits a slog.WarnContext at the per-request failure
site with request id, media type, tmdb id, status, and integration
kind. Aggregated counters in ReconcileResult are unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
860978cf2f
commit
38868b2128
@@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"io"
|
||||
"net/http"
|
||||
"strconv"
|
||||
"strings"
|
||||
@@ -26,6 +27,7 @@ type RequestService interface {
|
||||
GetRequest(ctx context.Context, viewer mediarequests.Viewer, id string) (*mediarequests.Request, error)
|
||||
Approve(ctx context.Context, viewer mediarequests.Viewer, id string) (*mediarequests.Request, error)
|
||||
Decline(ctx context.Context, viewer mediarequests.Viewer, id, reason string) (*mediarequests.Request, error)
|
||||
Cancel(ctx context.Context, viewer mediarequests.Viewer, id, reason string) (*mediarequests.Request, error)
|
||||
Retry(ctx context.Context, viewer mediarequests.Viewer, id string) (*mediarequests.Request, error)
|
||||
GetSettings(ctx context.Context, viewer mediarequests.Viewer) (mediarequests.Settings, error)
|
||||
UpdateSettings(ctx context.Context, viewer mediarequests.Viewer, settings mediarequests.Settings) (mediarequests.Settings, error)
|
||||
@@ -325,6 +327,28 @@ func (h *RequestsHandler) HandleDecline(w http.ResponseWriter, r *http.Request)
|
||||
writeJSON(w, http.StatusOK, req)
|
||||
}
|
||||
|
||||
func (h *RequestsHandler) HandleCancel(w http.ResponseWriter, r *http.Request) {
|
||||
viewer, ok := requestViewer(w, r, false)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
var body struct {
|
||||
Reason string `json:"reason"`
|
||||
}
|
||||
if r.Body != nil {
|
||||
if err := json.NewDecoder(r.Body).Decode(&body); err != nil && !errors.Is(err, io.EOF) {
|
||||
writeError(w, http.StatusBadRequest, "bad_request", "Invalid request body")
|
||||
return
|
||||
}
|
||||
}
|
||||
req, err := h.service.Cancel(r.Context(), viewer, chi.URLParam(r, "id"), body.Reason)
|
||||
if err != nil {
|
||||
writeRequestServiceError(w, err)
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, req)
|
||||
}
|
||||
|
||||
func (h *RequestsHandler) HandleRetry(w http.ResponseWriter, r *http.Request) {
|
||||
viewer, ok := requestViewer(w, r, false)
|
||||
if !ok {
|
||||
|
||||
@@ -1407,6 +1407,7 @@ func NewRouter(deps Dependencies) chi.Router {
|
||||
r.Post("/", requestHandler.HandleCreate)
|
||||
r.Get("/mine", requestHandler.HandleListMine)
|
||||
r.Get("/{id}", requestHandler.HandleGet)
|
||||
r.Post("/{id}/cancel", requestHandler.HandleCancel)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -1909,6 +1910,7 @@ func NewRouter(deps Dependencies) chi.Router {
|
||||
r.Get("/requests", requestHandler.HandleAdminList)
|
||||
r.Post("/requests/{id}/approve", requestHandler.HandleApprove)
|
||||
r.Post("/requests/{id}/decline", requestHandler.HandleDecline)
|
||||
r.Post("/requests/{id}/cancel", requestHandler.HandleCancel)
|
||||
r.Post("/requests/{id}/retry", requestHandler.HandleRetry)
|
||||
r.Get("/request-settings", requestHandler.HandleGetSettings)
|
||||
r.Put("/request-settings", requestHandler.HandleUpdateSettings)
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
@@ -373,7 +374,10 @@ func (s *Service) Decline(ctx context.Context, viewer Viewer, id, reason string)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
// Approved requests are pending submission by the reconciler; declining
|
||||
// while submission may be in flight risks a divergent external state.
|
||||
if req.Outcome != OutcomeActive ||
|
||||
req.Status == StatusApproved ||
|
||||
req.Status == StatusCompleted ||
|
||||
req.Status == StatusQueued ||
|
||||
req.Status == StatusDownloading ||
|
||||
@@ -384,6 +388,34 @@ func (s *Service) Decline(ctx context.Context, viewer Viewer, id, reason string)
|
||||
return s.store.SetOutcome(ctx, req.ID, OutcomeDeclined, viewer, reason)
|
||||
}
|
||||
|
||||
// Cancel withdraws a request that has not yet been submitted to a downstream
|
||||
// integration. Owners can cancel their own pending requests; admins can cancel
|
||||
// any active request that has not entered the fulfillment pipeline. Requests
|
||||
// already approved, queued, downloading, or completed cannot be cancelled —
|
||||
// callers should decline (admin) or wait for completion in those cases.
|
||||
func (s *Service) Cancel(ctx context.Context, viewer Viewer, id, reason string) (*Request, error) {
|
||||
if viewer.UserID == 0 {
|
||||
return nil, ErrForbidden
|
||||
}
|
||||
req, err := s.store.GetRequest(ctx, strings.TrimSpace(id))
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if !viewer.IsAdmin && req.RequestedByUserID != viewer.UserID {
|
||||
return nil, ErrForbidden
|
||||
}
|
||||
if req.Outcome != OutcomeActive ||
|
||||
req.Status == StatusApproved ||
|
||||
req.Status == StatusCompleted ||
|
||||
req.Status == StatusQueued ||
|
||||
req.Status == StatusDownloading ||
|
||||
strings.TrimSpace(req.ExternalID) != "" ||
|
||||
strings.TrimSpace(req.IntegrationKind) != "" {
|
||||
return nil, ErrInvalidState
|
||||
}
|
||||
return s.store.SetOutcome(ctx, req.ID, OutcomeCancelled, viewer, reason)
|
||||
}
|
||||
|
||||
func (s *Service) Retry(ctx context.Context, viewer Viewer, id string) (*Request, error) {
|
||||
if !viewer.IsAdmin {
|
||||
return nil, ErrForbidden
|
||||
@@ -425,6 +457,14 @@ func (s *Service) ReconcileRequests(ctx context.Context, limit int) (ReconcileRe
|
||||
}
|
||||
change, err := s.reconcileRequest(ctx, *req)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "request reconcile failed",
|
||||
"request_id", req.ID,
|
||||
"media_type", req.MediaType,
|
||||
"tmdb_id", req.TMDBID,
|
||||
"status", req.Status,
|
||||
"integration_kind", req.IntegrationKind,
|
||||
"err", err,
|
||||
)
|
||||
result.Errors++
|
||||
continue
|
||||
}
|
||||
|
||||
@@ -649,6 +649,106 @@ func TestReconcileRequestsMarksDownloadingFromAdapter(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestCancelOwnerCanWithdrawPendingRequest(t *testing.T) {
|
||||
store := newFakeStore()
|
||||
store.requests["req-mine"] = &Request{
|
||||
ID: "req-mine",
|
||||
MediaType: MediaTypeMovie,
|
||||
TMDBID: 550,
|
||||
Status: StatusPending,
|
||||
Outcome: OutcomeActive,
|
||||
RequestedByUserID: 7,
|
||||
}
|
||||
service := newTestService(store)
|
||||
|
||||
req, err := service.Cancel(context.Background(), Viewer{UserID: 7, ProfileID: "profile-1"}, "req-mine", "no longer want")
|
||||
if err != nil {
|
||||
t.Fatalf("Cancel returned error: %v", err)
|
||||
}
|
||||
if req.Outcome != OutcomeCancelled {
|
||||
t.Fatalf("Outcome = %q, want cancelled", req.Outcome)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCancelNonOwnerForbidden(t *testing.T) {
|
||||
store := newFakeStore()
|
||||
store.requests["req-someone-else"] = &Request{
|
||||
ID: "req-someone-else",
|
||||
MediaType: MediaTypeMovie,
|
||||
TMDBID: 550,
|
||||
Status: StatusPending,
|
||||
Outcome: OutcomeActive,
|
||||
RequestedByUserID: 7,
|
||||
}
|
||||
service := newTestService(store)
|
||||
|
||||
_, err := service.Cancel(context.Background(), Viewer{UserID: 8, ProfileID: "profile-2"}, "req-someone-else", "")
|
||||
if !errors.Is(err, ErrForbidden) {
|
||||
t.Fatalf("err = %v, want ErrForbidden", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCancelAdminCanCancelAnyPending(t *testing.T) {
|
||||
store := newFakeStore()
|
||||
store.requests["req-other"] = &Request{
|
||||
ID: "req-other",
|
||||
MediaType: MediaTypeMovie,
|
||||
TMDBID: 550,
|
||||
Status: StatusPending,
|
||||
Outcome: OutcomeActive,
|
||||
RequestedByUserID: 7,
|
||||
}
|
||||
service := newTestService(store)
|
||||
|
||||
_, err := service.Cancel(context.Background(), Viewer{UserID: 99, IsAdmin: true}, "req-other", "house cleaning")
|
||||
if err != nil {
|
||||
t.Fatalf("admin Cancel returned error: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCancelRejectsRequestsAlreadyInFulfillment(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
req Request
|
||||
}{
|
||||
{"approved", Request{Status: StatusApproved, Outcome: OutcomeActive}},
|
||||
{"queued", Request{Status: StatusQueued, Outcome: OutcomeActive, IntegrationKind: "radarr", ExternalID: "42"}},
|
||||
{"downloading", Request{Status: StatusDownloading, Outcome: OutcomeActive, IntegrationKind: "radarr", ExternalID: "42"}},
|
||||
{"completed", Request{Status: StatusCompleted, Outcome: OutcomeActive}},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
store := newFakeStore()
|
||||
tc.req.ID = "req-x"
|
||||
tc.req.RequestedByUserID = 7
|
||||
store.requests["req-x"] = &tc.req
|
||||
service := newTestService(store)
|
||||
|
||||
_, err := service.Cancel(context.Background(), Viewer{UserID: 7, ProfileID: "profile-1"}, "req-x", "")
|
||||
if !errors.Is(err, ErrInvalidState) {
|
||||
t.Fatalf("err = %v, want ErrInvalidState for %s", err, tc.name)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestDeclineRejectsApprovedRequests(t *testing.T) {
|
||||
store := newFakeStore()
|
||||
store.requests["req-approved"] = &Request{
|
||||
ID: "req-approved",
|
||||
MediaType: MediaTypeMovie,
|
||||
TMDBID: 550,
|
||||
Status: StatusApproved,
|
||||
Outcome: OutcomeActive,
|
||||
}
|
||||
service := newTestService(store)
|
||||
|
||||
_, err := service.Decline(context.Background(), Viewer{UserID: 1, IsAdmin: true}, "req-approved", "changed mind")
|
||||
if !errors.Is(err, ErrInvalidState) {
|
||||
t.Fatalf("err = %v, want ErrInvalidState (approved is owned by the reconciler)", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDeclineRejectsQueuedRequests(t *testing.T) {
|
||||
store := newFakeStore()
|
||||
store.requests["req-1"] = &Request{
|
||||
|
||||
Reference in New Issue
Block a user