From 8b109c8454d4aacb98bdfa5b3de63d26b6a55ceb Mon Sep 17 00:00:00 2001 From: Quick <31828688+Quick104@users.noreply.github.com> Date: Mon, 28 Sep 2026 21:19:27 +0000 Subject: [PATCH] fix(requests): keep download server details out of requesters' requests The profile-scoped v2 request operations (createRequest, listMyRequests, getRequest, cancelRequest) gave a requester every target's download server id, kind and name, the server's own id and raw status, the routing rule and the target error, plus the request's integration kind, external fields and submission error. These are admin details. They show how the admin named their servers and routing rules, and the errors can carry a plugin's raw text, such as a server URL or a release title. mediaRequestOf now takes the viewer and fills those fields for an admin only. A requester keeps the request's state and outcome_reason and each target's quality, status and download progress. An admin still sees everything, on the profile-scoped operations and on the admin request operations. /api/v1 is frozen and unchanged. The fields were already optional, so the contract diff reports no change; their descriptions now say admins only. Refs #1646 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../api/v2/fixtures/admin_requests_ok.json | 18 +++ .../api/v2/fixtures/cancel_request_ok.json | 1 - .../api/v2/fixtures/create_request_ok.json | 1 - .../api/v2/fixtures/get_system_info_ok.json | 2 +- .../api/v2/fixtures/list_my_requests_ok.json | 2 - contracts/api/v2/openapi.json | 13 +- docs/architecture/media-requests.md | 17 ++ internal/apiv2/admin_requests.go | 4 +- internal/apiv2/admin_requests_test.go | 39 ++++- internal/apiv2/request_lifecycle.go | 5 +- internal/apiv2/requests.go | 60 +++---- internal/apiv2/requests_test.go | 146 +++++++++++++++++- web/src/api/types.ts | 2 + web/src/api/v2/schema.ts | 21 ++- 14 files changed, 287 insertions(+), 44 deletions(-) diff --git a/contracts/api/v2/fixtures/admin_requests_ok.json b/contracts/api/v2/fixtures/admin_requests_ok.json index 6d2e7fb825..551cd3f907 100644 --- a/contracts/api/v2/fixtures/admin_requests_ok.json +++ b/contracts/api/v2/fixtures/admin_requests_ok.json @@ -20,9 +20,15 @@ { "id": "42", "request_id": "r-3", + "integration_id": "integration-1", + "integration_kind": "radarr", + "instance_name": "Radarr", "quality": "1080p", "is_anime": false, + "external_id": "7", + "external_status": "queued", "status": "queued", + "route_name": "Movies", "created_at": "2026-01-02T03:04:05.678Z", "updated_at": "2026-01-02T03:04:05.678Z" } @@ -51,9 +57,15 @@ { "id": "42", "request_id": "r-2", + "integration_id": "integration-1", + "integration_kind": "radarr", + "instance_name": "Radarr", "quality": "1080p", "is_anime": false, + "external_id": "7", + "external_status": "queued", "status": "queued", + "route_name": "Movies", "created_at": "2026-01-02T03:04:05.678Z", "updated_at": "2026-01-02T03:04:05.678Z" } @@ -82,9 +94,15 @@ { "id": "42", "request_id": "r-1", + "integration_id": "integration-1", + "integration_kind": "radarr", + "instance_name": "Radarr", "quality": "1080p", "is_anime": false, + "external_id": "7", + "external_status": "queued", "status": "queued", + "route_name": "Movies", "created_at": "2026-01-02T03:04:05.678Z", "updated_at": "2026-01-02T03:04:05.678Z" } diff --git a/contracts/api/v2/fixtures/cancel_request_ok.json b/contracts/api/v2/fixtures/cancel_request_ok.json index d1c73648b3..212803d679 100644 --- a/contracts/api/v2/fixtures/cancel_request_ok.json +++ b/contracts/api/v2/fixtures/cancel_request_ok.json @@ -12,7 +12,6 @@ "season_progress": [], "requested_by_user_id": "1", "requested_by_profile_id": "p-owner", - "integration_kind": "radarr", "is_anime": false, "targets": [ { diff --git a/contracts/api/v2/fixtures/create_request_ok.json b/contracts/api/v2/fixtures/create_request_ok.json index 6f706cbf8a..9f05dc9465 100644 --- a/contracts/api/v2/fixtures/create_request_ok.json +++ b/contracts/api/v2/fixtures/create_request_ok.json @@ -12,7 +12,6 @@ "season_progress": [], "requested_by_user_id": "1", "requested_by_profile_id": "p-owner", - "integration_kind": "radarr", "is_anime": false, "targets": [], "created_at": "2026-01-02T03:04:05.678Z", diff --git a/contracts/api/v2/fixtures/get_system_info_ok.json b/contracts/api/v2/fixtures/get_system_info_ok.json index 1d645f16f0..e94c54a5f9 100644 --- a/contracts/api/v2/fixtures/get_system_info_ok.json +++ b/contracts/api/v2/fixtures/get_system_info_ok.json @@ -1,7 +1,7 @@ { "server_version": "unavailable", "api_major": 2, - "contract_digest": "8348e2ab6b2f48273ea3734b2c955923f5a8cb5e218c1be06af7c465b294c575", + "contract_digest": "2d062f1cc2421ddb2af274137f1c243c82eedfc881dce028c2f2f93def30a6ad", "links": { "openapi": "/api/v2/openapi.json", "capabilities": "/api/v2/capabilities", diff --git a/contracts/api/v2/fixtures/list_my_requests_ok.json b/contracts/api/v2/fixtures/list_my_requests_ok.json index 01624f3db0..67a73c0b0a 100644 --- a/contracts/api/v2/fixtures/list_my_requests_ok.json +++ b/contracts/api/v2/fixtures/list_my_requests_ok.json @@ -14,7 +14,6 @@ "season_progress": [], "requested_by_user_id": "1", "requested_by_profile_id": "p-owner", - "integration_kind": "radarr", "is_anime": false, "targets": [ { @@ -45,7 +44,6 @@ "season_progress": [], "requested_by_user_id": "1", "requested_by_profile_id": "p-owner", - "integration_kind": "radarr", "is_anime": false, "targets": [ { diff --git a/contracts/api/v2/openapi.json b/contracts/api/v2/openapi.json index 03e7db0d33..41d2d90e99 100644 --- a/contracts/api/v2/openapi.json +++ b/contracts/api/v2/openapi.json @@ -28544,9 +28544,11 @@ "description": "How far the request's downloads are over all its servers (1080p and 4K together), while any reports them: bytes summed, the phase that needs the most attention, the latest estimate, and the oldest report's time" }, "external_id": { + "description": "Admins only: the integration's own identifier", "type": "string" }, "external_status": { + "description": "Admins only: the status as the download server reports it", "type": "string" }, "id": { @@ -28564,6 +28566,7 @@ "type": "string" }, "integration_kind": { + "description": "Admins only: the download server's kind", "examples": [ "radarr" ], @@ -28573,6 +28576,7 @@ "type": "boolean" }, "last_error": { + "description": "Admins only: why the last submission to a download server failed. It can name servers and routing rules", "type": "string" }, "library_content_id": { @@ -36939,10 +36943,11 @@ "description": "How far this target's downloads are, while its download server reports them" }, "external_id": { - "description": "The integration's own identifier", + "description": "Admins only: the integration's own identifier", "type": "string" }, "external_status": { + "description": "Admins only: the status as the download server reports it", "type": "string" }, "id": { @@ -36954,12 +36959,15 @@ "type": "string" }, "instance_name": { + "description": "Admins only: the download server's name", "type": "string" }, "integration_id": { + "description": "Admins only: the download server holding this target", "type": "string" }, "integration_kind": { + "description": "Admins only: the download server's kind", "examples": [ "radarr" ], @@ -36969,6 +36977,7 @@ "type": "boolean" }, "last_error": { + "description": "Admins only: why the download server failed this target", "type": "string" }, "quality": { @@ -36986,7 +36995,7 @@ "type": "string" }, "route_name": { - "description": "The routing rule that sent this target to its server, as named when it was sent", + "description": "Admins only: the routing rule that sent this target to its server, as named when it was sent", "type": "string" }, "status": { diff --git a/docs/architecture/media-requests.md b/docs/architecture/media-requests.md index fa61253337..e2fd20cf11 100644 --- a/docs/architecture/media-requests.md +++ b/docs/architecture/media-requests.md @@ -465,6 +465,23 @@ accounts' failed requests are left alone as those users' history. Retrying one of them after someone else has requested the title answers `ErrAlreadyRequested`, since only one active request per title may exist. +## What a requester sees + +The v2 request operations a profile calls (`createRequest`, `listMyRequests`, +`getRequest` and `cancelRequest`) give a viewer who is not an admin the request +without its download server details: the request's `integration_kind`, +`external_id`, `external_status` and `last_error`, and each target's +`integration_id`, `integration_kind`, `instance_name`, `external_id`, +`external_status`, `route_name` and `last_error`. These name the admin's +download servers and routing rules and carry the servers' raw statuses and +errors, none of which a requester can act on. The request's `last_error` is +written for the admin who fixes the submission: it can name a server or a +routing rule, or pass on a plugin's own error text. A requester still sees the +request's state and `outcome_reason`, and each target's quality, status and +`download`. An admin sees every field, on those operations and on the +`/api/v2/admin/requests` operations. The frozen `/api/v1` request routes still +return them to everyone. + ## Admin queue The admin queue groups requests by what an admin does next, from status and diff --git a/internal/apiv2/admin_requests.go b/internal/apiv2/admin_requests.go index d5434156a9..eebffd2266 100644 --- a/internal/apiv2/admin_requests.go +++ b/internal/apiv2/admin_requests.go @@ -250,7 +250,7 @@ func registerAdminRequests(reg *Registry) { if err != nil { return nil, requestProblem(err) } - return &MediaRequestOutput{Body: mediaRequestOf(r)}, nil + return &MediaRequestOutput{Body: mediaRequestOf(r, v)}, nil }) } Register(reg, op(http.MethodGet, "/admin/request-settings", opGetAdminRequestSettings, false), reg.getAdminRequestSettings) @@ -306,7 +306,7 @@ func (reg *Registry) listAdminRequests(ctx context.Context, cursors *Cursors, in } items := make([]MediaRequest, 0, len(rows)) for _, r := range rows { - items = append(items, mediaRequestOf(r)) + items = append(items, mediaRequestOf(r, v)) } return &MediaRequestCollectionOutput{Body: MediaRequestCollection{Collection: Paginated(items, next)}}, nil } diff --git a/internal/apiv2/admin_requests_test.go b/internal/apiv2/admin_requests_test.go index dc91b3013e..adbf765c49 100644 --- a/internal/apiv2/admin_requests_test.go +++ b/internal/apiv2/admin_requests_test.go @@ -22,6 +22,9 @@ type fakeAdminRequests struct { filter mediarequests.ListFilter action, reason, requestID string probedBaseURL string + // failed gives every returned request the errors a failed submission + // leaves (withSubmissionErrors). + failed bool } func fixtureAdminRequests() *fakeAdminRequests { @@ -130,7 +133,7 @@ func TestAdminRequestOptionsUnreachableIntegration(t *testing.T) { func (f *fakeAdminRequests) ListAdmin(_ context.Context, v mediarequests.Viewer, filter mediarequests.ListFilter) ([]*mediarequests.Request, error) { f.viewer = v f.filter = filter - rows := []*mediarequests.Request{fixtureMediaRequest("r-3", 3), fixtureMediaRequest("r-2", 2), fixtureMediaRequest("r-1", 1)} + rows := []*mediarequests.Request{f.request("r-3", 3), f.request("r-2", 2), f.request("r-1", 1)} out := []*mediarequests.Request{} for _, r := range rows { if filter.Before != nil && r.ID >= filter.Before.ID { @@ -147,7 +150,14 @@ func (f *fakeAdminRequests) moderate(v mediarequests.Viewer, action, id, reason f.viewer = v f.action, f.requestID, f.reason = action, id, reason f.writes++ - return fixtureMediaRequest(id, 1), nil + return f.request(id, 1), nil +} +func (f *fakeAdminRequests) request(id string, tmdbID int) *mediarequests.Request { + r := fixtureMediaRequest(id, tmdbID) + if f.failed { + withSubmissionErrors(r) + } + return r } func (f *fakeAdminRequests) Approve(_ context.Context, v mediarequests.Viewer, id string) (*mediarequests.Request, error) { return f.moderate(v, "approve", id, "") @@ -319,6 +329,31 @@ func TestAdminRequestLimitsModerationAndOptions(t *testing.T) { t.Fatalf("validation %+v", p) } } + +// The admin request operations carry every download server detail. +func TestAdminRequestsCarryDownloadServerDetails(t *testing.T) { + f := fixtureAdminRequests() + f.failed = true + h := adminRequestsHandler(f) + rec := do(t, h, http.MethodGet, Prefix+"/admin/requests", "", actingRequestAdmin) + var page struct { + Items []map[string]any `json:"items"` + } + decodeBody(t, rec.Body, &page) + if rec.Code != http.StatusOK || len(page.Items) != 3 { + t.Fatalf("listAdminRequests: %d %s", rec.Code, rec.Body.String()) + } + for _, item := range page.Items { + if targets, _ := item["targets"].([]any); len(targets) == 0 { + t.Fatalf("listAdminRequests: no targets in %v", item) + } + requireAdminMembers(t, "listAdminRequests", item, adminRequestMembers, adminTargetMembers) + } + for _, action := range []string{"approve", "decline", "cancel", "retry"} { + got := requireTargets(t, action, do(t, h, http.MethodPost, Prefix+"/admin/requests/r-1/"+action, `{}`, actingRequestAdmin)) + requireAdminMembers(t, action, got, adminRequestMembers, adminTargetMembers) + } +} func TestAdminRequestCursorBoundaries(t *testing.T) { f := fixtureAdminRequests() h := adminRequestsHandler(f) diff --git a/internal/apiv2/request_lifecycle.go b/internal/apiv2/request_lifecycle.go index 21fad4600c..6ba0f69473 100644 --- a/internal/apiv2/request_lifecycle.go +++ b/internal/apiv2/request_lifecycle.go @@ -194,11 +194,12 @@ func registerRequestLifecycle(reg *Registry, requests RequestLifecycleService, p if requests == nil { return nil, unavailable("requests") } - result, err := requests.Cancel(ctx, lifecycleViewer(ctx), string(in.ID), in.Body.Reason) + viewer := lifecycleViewer(ctx) + result, err := requests.Cancel(ctx, viewer, string(in.ID), in.Body.Reason) if err != nil { return nil, requestProblem(err) } - return &MediaRequestOutput{Body: mediaRequestOf(result)}, nil + return &MediaRequestOutput{Body: mediaRequestOf(result, viewer)}, nil }) scope := func(ctx context.Context) (int, string, error) { if providers == nil { diff --git a/internal/apiv2/requests.go b/internal/apiv2/requests.go index 7b5554050b..39e4151e7e 100644 --- a/internal/apiv2/requests.go +++ b/internal/apiv2/requests.go @@ -184,20 +184,21 @@ type DiscoverBrowsePage struct { } // RequestTarget is one fulfillment of a request against one integration -// instance at one quality. +// instance at one quality. The download server details are for admins only: +// see mediaRequestOf. type RequestTarget struct { ID ID `json:"id" example:"42"` RequestID ID `json:"request_id" example:"1834729"` - IntegrationID string `json:"integration_id,omitempty"` - IntegrationKind string `json:"integration_kind,omitempty" example:"radarr"` - InstanceName string `json:"instance_name,omitempty"` + IntegrationID string `json:"integration_id,omitempty" doc:"Admins only: the download server holding this target"` + IntegrationKind string `json:"integration_kind,omitempty" doc:"Admins only: the download server's kind" example:"radarr"` + InstanceName string `json:"instance_name,omitempty" doc:"Admins only: the download server's name"` Quality string `json:"quality" example:"1080p"` IsAnime bool `json:"is_anime"` - ExternalID string `json:"external_id,omitempty" doc:"The integration's own identifier"` - ExternalStatus string `json:"external_status,omitempty"` + ExternalID string `json:"external_id,omitempty" doc:"Admins only: the integration's own identifier"` + ExternalStatus string `json:"external_status,omitempty" doc:"Admins only: the status as the download server reports it"` Status string `json:"status" example:"queued"` - LastError string `json:"last_error,omitempty"` - RouteName string `json:"route_name,omitempty" doc:"The routing rule that sent this target to its server, as named when it was sent"` + LastError string `json:"last_error,omitempty" doc:"Admins only: why the download server failed this target"` + RouteName string `json:"route_name,omitempty" doc:"Admins only: the routing rule that sent this target to its server, as named when it was sent"` CreatedAt Instant `json:"created_at" example:"2026-01-02T03:04:05.000Z"` UpdatedAt Instant `json:"updated_at" example:"2026-01-02T03:04:05.000Z"` // Download is set while the target's router plugin reports progress. @@ -225,13 +226,13 @@ type MediaRequest struct { OutcomeReason string `json:"outcome_reason,omitempty" doc:"Why the request was declined or withdrawn, when a reason was given"` RequestedByUserID ID `json:"requested_by_user_id,omitempty" example:"1"` RequestedByProfileID ID `json:"requested_by_profile_id,omitempty" example:"p-owner"` - IntegrationKind string `json:"integration_kind,omitempty" example:"radarr"` + IntegrationKind string `json:"integration_kind,omitempty" doc:"Admins only: the download server's kind" example:"radarr"` IsAnime bool `json:"is_anime"` Targets []RequestTarget `json:"targets" doc:"Empty, never null"` - ExternalID string `json:"external_id,omitempty"` - ExternalStatus string `json:"external_status,omitempty"` + ExternalID string `json:"external_id,omitempty" doc:"Admins only: the integration's own identifier"` + ExternalStatus string `json:"external_status,omitempty" doc:"Admins only: the status as the download server reports it"` LibraryContentID string `json:"library_content_id,omitempty" doc:"The catalog item once the media is in the library"` - LastError string `json:"last_error,omitempty"` + LastError string `json:"last_error,omitempty" doc:"Admins only: why the last submission to a download server failed. It can name servers and routing rules"` CreatedAt Instant `json:"created_at" example:"2026-01-02T03:04:05.000Z"` UpdatedAt Instant `json:"updated_at" example:"2026-01-02T03:04:05.000Z"` ApprovedAt *Instant `json:"approved_at,omitempty"` @@ -522,7 +523,7 @@ func (reg *Registry) createRequest(ctx context.Context, in *MediaRequestCreateIn if err != nil { return nil, requestProblem(err) } - return &MediaRequestOutput{Body: mediaRequestOf(req)}, nil + return &MediaRequestOutput{Body: mediaRequestOf(req, viewer)}, nil } // listMyRequests pages by the last emitted creation time and unique request ID. @@ -568,7 +569,7 @@ func (reg *Registry) listMyRequests(ctx context.Context, cursors *Cursors, in *M } items := make([]MediaRequest, 0, len(rows)) for _, r := range rows { - items = append(items, mediaRequestOf(r)) + items = append(items, mediaRequestOf(r, viewer)) } return &MediaRequestCollectionOutput{Body: MediaRequestCollection{Collection: Paginated(items, next)}}, nil } @@ -583,7 +584,7 @@ func (reg *Registry) getRequest(ctx context.Context, in *MediaRequestGetInput) ( if err != nil { return nil, requestProblem(err) } - return &MediaRequestOutput{Body: mediaRequestOf(req)}, nil + return &MediaRequestOutput{Body: mediaRequestOf(req, viewer)}, nil } // searchRequestMedia is v1 GET /requests/search. @@ -799,7 +800,12 @@ func requestProblem(err error) *Problem { return NewProblem(TypeInternalError, "An unexpected error occurred.") } -func mediaRequestOf(r *mediarequests.Request) MediaRequest { +// mediaRequestOf maps a request for the viewer. The download server details +// (which server and routing rule took each target, the server's own ids and +// raw statuses, and the submission and target errors, which can name servers +// and routing rules) go to an admin only. A requester keeps each target's +// quality, status and download progress. +func mediaRequestOf(r *mediarequests.Request, viewer mediarequests.Viewer) MediaRequest { out := MediaRequest{ ID: ID(r.ID), Provider: r.Provider, @@ -818,31 +824,33 @@ func mediaRequestOf(r *mediarequests.Request) MediaRequest { Seasons: NonNil(r.Seasons), SeasonProgress: requestSeasonProgressOf(r.SeasonProgress), OutcomeReason: r.OutcomeReason, - IntegrationKind: r.IntegrationKind, IsAnime: r.IsAnime, Targets: make([]RequestTarget, 0, len(r.Targets)), - ExternalID: r.ExternalID, - ExternalStatus: r.ExternalStatus, LibraryContentID: r.LibraryContentID, - LastError: r.LastError, CreatedAt: NewInstant(r.CreatedAt), UpdatedAt: NewInstant(r.UpdatedAt), ApprovedAt: instantPtr(r.ApprovedAt), CompletedAt: instantPtr(r.CompletedAt), Download: requestDownloadOf(r.Download()), } + if viewer.IsAdmin { + out.IntegrationKind, out.ExternalID, out.ExternalStatus, out.LastError = r.IntegrationKind, r.ExternalID, r.ExternalStatus, r.LastError + } if r.RequestedByUserID != 0 { out.RequestedByUserID = IDFromInt(int64(r.RequestedByUserID)) } out.RequestedByProfileID = ID(r.RequestedByProfileID) for _, t := range r.Targets { - out.Targets = append(out.Targets, RequestTarget{ - ID: IDFromInt(t.ID), RequestID: ID(t.RequestID), IntegrationID: t.IntegrationID, - IntegrationKind: t.IntegrationKind, InstanceName: t.InstanceName, Quality: string(t.Quality), - IsAnime: t.IsAnime, ExternalID: t.ExternalID, ExternalStatus: t.ExternalStatus, Status: string(t.Status), - LastError: t.LastError, RouteName: t.RouteName, CreatedAt: NewInstant(t.CreatedAt), UpdatedAt: NewInstant(t.UpdatedAt), + target := RequestTarget{ + ID: IDFromInt(t.ID), RequestID: ID(t.RequestID), Quality: string(t.Quality), IsAnime: t.IsAnime, + Status: string(t.Status), CreatedAt: NewInstant(t.CreatedAt), UpdatedAt: NewInstant(t.UpdatedAt), Download: requestDownloadOf(t.Download), - }) + } + if viewer.IsAdmin { + target.IntegrationID, target.IntegrationKind, target.InstanceName = t.IntegrationID, t.IntegrationKind, t.InstanceName + target.ExternalID, target.ExternalStatus, target.LastError, target.RouteName = t.ExternalID, t.ExternalStatus, t.LastError, t.RouteName + } + out.Targets = append(out.Targets, target) } return out } diff --git a/internal/apiv2/requests_test.go b/internal/apiv2/requests_test.go index e2a4eb93ac..697dec25b0 100644 --- a/internal/apiv2/requests_test.go +++ b/internal/apiv2/requests_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "net/http" + "net/http/httptest" "slices" "strings" "testing" @@ -28,13 +29,20 @@ type fakeRequests struct { lastArgs []any } +// fixtureMediaRequest is an approved movie request with one queued target. +// The target names its download server and routing rule, which only an +// admin sees. func fixtureMediaRequest(id string, tmdbID int) *mediarequests.Request { year := 1995 approved := fixedTime() return &mediarequests.Request{ ID: id, Provider: "tmdb", MediaType: mediarequests.MediaTypeMovie, TMDBID: tmdbID, Title: "Heat", Year: &year, Status: mediarequests.StatusApproved, Outcome: mediarequests.OutcomeActive, RequestedByUserID: 1, RequestedByProfileID: "p-owner", - IntegrationKind: "radarr", Targets: []mediarequests.Target{{ID: 42, RequestID: id, Quality: mediarequests.Quality1080p, Status: mediarequests.StatusQueued, CreatedAt: fixedTime(), UpdatedAt: fixedTime()}}, + IntegrationKind: "radarr", Targets: []mediarequests.Target{{ + ID: 42, RequestID: id, IntegrationID: "integration-1", IntegrationKind: "radarr", InstanceName: "Radarr", + Quality: mediarequests.Quality1080p, ExternalID: "7", ExternalStatus: "queued", Status: mediarequests.StatusQueued, + RouteName: "Movies", CreatedAt: fixedTime(), UpdatedAt: fixedTime(), + }}, CreatedAt: fixedTime(), UpdatedAt: fixedTime(), ApprovedAt: &approved, } } @@ -375,6 +383,142 @@ func TestGetRequest(t *testing.T) { } } +// A request's download server details are for admins only: the servers and +// routing rules a request went to, the servers' own ids and raw statuses, and +// errors that can name them. +var ( + adminRequestMembers = []string{"integration_kind", "external_id", "external_status", "last_error"} + adminTargetMembers = []string{"integration_id", "integration_kind", "instance_name", "external_id", "external_status", "last_error", "route_name"} + // serverRequestMembers and serverTargetMembers are the ones + // fixtureMediaRequest fills; withSubmissionErrors fills the rest. + serverRequestMembers = []string{"integration_kind"} + serverTargetMembers = []string{"integration_id", "integration_kind", "instance_name", "external_id", "external_status", "route_name"} + // requesterTargetMembers are what every viewer gets on a target. + requesterTargetMembers = []string{"id", "request_id", "quality", "is_anime", "status", "created_at", "updated_at"} +) + +// withSubmissionErrors adds the rest of the admin details: the errors a failed +// submission leaves, which name a server and a routing rule, and the request's +// own server fields. +func withSubmissionErrors(r *mediarequests.Request) { + r.ExternalID, r.ExternalStatus = "3", "5" + r.LastError = `route "Movies" sends to "Radarr", which is disabled` + for i := range r.Targets { + r.Targets[i].LastError = `Post "http://radarr.lan:7878/api/v3/movie": connection refused` + } +} + +// requireAdminMembers checks that a request body carries exactly the wanted +// admin-only members, on the request and on each of its targets, and that its +// targets keep what a requester sees. +func requireAdminMembers(t *testing.T, label string, req map[string]any, wantRequest, wantTarget []string) { + t.Helper() + for _, m := range adminRequestMembers { + if _, got := req[m]; got != slices.Contains(wantRequest, m) { + t.Errorf("%s: request %s present = %v", label, m, got) + } + } + targets, _ := req["targets"].([]any) + for _, raw := range targets { + target, _ := raw.(map[string]any) + for _, m := range adminTargetMembers { + if _, got := target[m]; got != slices.Contains(wantTarget, m) { + t.Errorf("%s: target %s present = %v", label, m, got) + } + } + for _, m := range requesterTargetMembers { + if _, ok := target[m]; !ok { + t.Errorf("%s: target lost %s", label, m) + } + } + } +} + +// requireTargets decodes one request body and checks it has targets. +func requireTargets(t *testing.T, label string, rec *httptest.ResponseRecorder) map[string]any { + t.Helper() + if rec.Code != http.StatusOK { + t.Fatalf("%s: %d %s", label, rec.Code, rec.Body.String()) + } + var req map[string]any + decodeBody(t, rec.Body, &req) + if targets, _ := req["targets"].([]any); len(targets) == 0 { + t.Fatalf("%s: no targets in %s", label, rec.Body.String()) + } + return req +} + +// The profile-scoped request operations leave the download server details +// out for a requester and keep them for an admin. +func TestRequestDownloadServerDetailsAreForAdmins(t *testing.T) { + svc := fixtureRequests() + for _, r := range svc.requests { + withSubmissionErrors(r) + } + deps := requestDeps(svc) + deps.RequestLifecycle = &fakeLifecycle{} + h := newTestHandler(t, deps) + admin := with(bearer(adminToken), "X-Profile-Id", "p-primary") + + // A requester sees none of them, on any operation that answers with a + // request. + created := do(t, h, http.MethodPost, "/api/v2/requests", `{"media_type":"movie","tmdb_id":7,"title":"Heat"}`, requestOwner) + if created.Code != http.StatusCreated { + t.Fatalf("createRequest: %d %s", created.Code, created.Body.String()) + } + var body map[string]any + decodeBody(t, created.Body, &body) + requireAdminMembers(t, "createRequest", body, nil, nil) + + var mine struct { + Items []map[string]any `json:"items"` + } + rec := do(t, h, http.MethodGet, "/api/v2/requests/mine", "", requestOwner) + decodeBody(t, rec.Body, &mine) + if rec.Code != http.StatusOK || len(mine.Items) != 2 { + t.Fatalf("listMyRequests: %d %s", rec.Code, rec.Body.String()) + } + for _, item := range mine.Items { + requireAdminMembers(t, "listMyRequests", item, nil, nil) + } + got := requireTargets(t, "getRequest", do(t, h, http.MethodGet, "/api/v2/requests/r-1", "", requestOwner)) + requireAdminMembers(t, "getRequest", got, nil, nil) + target := got["targets"].([]any)[0].(map[string]any) + if target["quality"] != "1080p" || target["status"] != "downloading" || target["download"] == nil { + t.Fatalf("getRequest: target = %v, want its quality, status and download", target) + } + got = requireTargets(t, "cancelRequest", do(t, h, http.MethodPost, "/api/v2/requests/r-1/cancel", `{}`, requestOwner)) + requireAdminMembers(t, "cancelRequest", got, nil, nil) + + // An admin sees every one the request has on the same operations. The + // create and cancel fakes answer with a fresh fixture, which carries no + // errors and, once created, no targets. + created = do(t, h, http.MethodPost, "/api/v2/requests", `{"media_type":"movie","tmdb_id":8,"title":"Heat"}`, admin) + if created.Code != http.StatusCreated { + t.Fatalf("admin createRequest: %d %s", created.Code, created.Body.String()) + } + var adminBody map[string]any + decodeBody(t, created.Body, &adminBody) + requireAdminMembers(t, "admin createRequest", adminBody, serverRequestMembers, nil) + var adminMine struct { + Items []map[string]any `json:"items"` + } + rec = do(t, h, http.MethodGet, "/api/v2/requests/mine", "", admin) + decodeBody(t, rec.Body, &adminMine) + if rec.Code != http.StatusOK || len(adminMine.Items) != 1 { + t.Fatalf("admin listMyRequests: %d %s", rec.Code, rec.Body.String()) + } + requireAdminMembers(t, "admin listMyRequests", adminMine.Items[0], adminRequestMembers, adminTargetMembers) + got = requireTargets(t, "admin getRequest", do(t, h, http.MethodGet, "/api/v2/requests/r-1", "", admin)) + requireAdminMembers(t, "admin getRequest", got, adminRequestMembers, adminTargetMembers) + target = got["targets"].([]any)[0].(map[string]any) + if target["instance_name"] != "Radarr" || target["route_name"] != "Movies" || target["last_error"] != `Post "http://radarr.lan:7878/api/v3/movie": connection refused` { + t.Fatalf("admin getRequest: target = %v", target) + } + got = requireTargets(t, "admin cancelRequest", do(t, h, http.MethodPost, "/api/v2/requests/r-1/cancel", `{}`, admin)) + requireAdminMembers(t, "admin cancelRequest", got, serverRequestMembers, serverTargetMembers) +} + func TestSearchRequestMedia(t *testing.T) { svc := fixtureRequests() h := newTestHandler(t, requestDeps(svc)) diff --git a/web/src/api/types.ts b/web/src/api/types.ts index a6c777d6c8..ec27f00776 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -2029,6 +2029,7 @@ export interface CreateMediaRequestInput { seasons?: number[]; } +/** The download server details (integration_*, instance_name, route_name, external_*, last_error) reach admins only. */ export interface RequestTarget { id: number; request_id: string; @@ -2048,6 +2049,7 @@ export interface RequestTarget { updated_at: string; } +/** integration_kind, external_id, external_status and last_error reach admins only. */ export interface MediaRequest { id: string; provider: string; diff --git a/web/src/api/v2/schema.ts b/web/src/api/v2/schema.ts index 9b34031718..b85b1bdbea 100644 --- a/web/src/api/v2/schema.ts +++ b/web/src/api/v2/schema.ts @@ -21200,7 +21200,9 @@ export interface components { created_at: string; /** @description How far the request's downloads are over all its servers (1080p and 4K together), while any reports them: bytes summed, the phase that needs the most attention, the latest estimate, and the oldest report's time */ download?: components["schemas"]["RequestDownload"]; + /** @description Admins only: the integration's own identifier */ external_id?: string; + /** @description Admins only: the status as the download server reports it */ external_status?: string; /** * @description Opaque identifier @@ -21209,9 +21211,13 @@ export interface components { id: string; /** @example tt0113277 */ imdb_id?: string; - /** @example radarr */ + /** + * @description Admins only: the download server's kind + * @example radarr + */ integration_kind?: string; is_anime: boolean; + /** @description Admins only: why the last submission to a download server failed. It can name servers and routing rules */ last_error?: string; /** @description The catalog item once the media is in the library */ library_content_id?: string; @@ -24687,19 +24693,26 @@ export interface components { created_at: string; /** @description How far this target's downloads are, while its download server reports them */ download?: components["schemas"]["RequestDownload"]; - /** @description The integration's own identifier */ + /** @description Admins only: the integration's own identifier */ external_id?: string; + /** @description Admins only: the status as the download server reports it */ external_status?: string; /** * @description Opaque identifier * @example 42 */ id: string; + /** @description Admins only: the download server's name */ instance_name?: string; + /** @description Admins only: the download server holding this target */ integration_id?: string; - /** @example radarr */ + /** + * @description Admins only: the download server's kind + * @example radarr + */ integration_kind?: string; is_anime: boolean; + /** @description Admins only: why the download server failed this target */ last_error?: string; /** @example 1080p */ quality: string; @@ -24708,7 +24721,7 @@ export interface components { * @example 1834729 */ request_id: string; - /** @description The routing rule that sent this target to its server, as named when it was sent */ + /** @description Admins only: the routing rule that sent this target to its server, as named when it was sent */ route_name?: string; /** @example queued */ status: string;