From 4f7654280b8e243df0650e54ff8aa70eae223e00 Mon Sep 17 00:00:00 2001 From: Lovesmile Small Date: Mon, 24 Aug 2026 11:20:56 +0000 Subject: [PATCH] feat(api): add GET /streams/:id/tags and wire the contract end to end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shared type contract and the dashboard's useStreamTags hook both documented GET /streams/:id/tags, but StreamTagsController only defined POST/DELETE — every tag-chip load 404'd. Add the GET handler under the existing StreamOwnershipGuard, reusing the batch listForStreamIds path (issue #330) so a single stream costs one round-trip, and return the PagedTags envelope the app hook already parses so no app-side shape change is needed. Pin the endpoint with a contract test on both sides (provider spec seeds a tag; the SDK gains getStreamTags and a consumer test), add Swagger docs with the ownership/403 semantics, and cover listForStream with service/controller/hook tests. --- api/src/contract-provider.spec.ts | 15 +++++- api/src/tags/tags.controller.spec.ts | 43 ++++++++++++++++- api/src/tags/tags.controller.ts | 32 ++++++++++++- api/src/tags/tags.service.spec.ts | 47 +++++++++++++++++++ api/src/tags/tags.service.ts | 25 +++++++++- app/hooks/useStreams.test.tsx | 42 ++++++++++++++++- tests/contracts/src/schemas.ts | 25 ++++++++++ tests/contracts/src/streams.contract.ts | 26 +++++++++- .../__tests__/contract.consumer.test.ts | 31 ++++++++++++ xstreamroll-sdk/src/client.ts | 12 ++++- xstreamroll-sdk/src/types.ts | 16 ++++++- 11 files changed, 301 insertions(+), 13 deletions(-) diff --git a/api/src/contract-provider.spec.ts b/api/src/contract-provider.spec.ts index 29693dc..0044b49 100644 --- a/api/src/contract-provider.spec.ts +++ b/api/src/contract-provider.spec.ts @@ -25,21 +25,23 @@ import { type Contract, } from "@xstreamroll/contract-tests" import request from "supertest" + import { AuditService } from "./audit/audit.service" import { AuthController } from "./auth/auth.controller" import { AuthService } from "./auth/auth.service" import { PasswordResetService } from "./auth/password-reset.service" import { TokenDenylistService } from "./auth/token-denylist.service" import { User, UsersRepository } from "./auth/users.repository" -import createJwtConfig, { createRefreshJwtConfig } from "./config/jwt.config" import { AuthGuard } from "./common/guards/auth.guard" import { JwtExtractorService } from "./common/guards/jwt-extractor.service" import { StreamOwnershipGuard } from "./common/guards/stream-ownership.guard" import { StreamOwnershipService } from "./common/guards/stream-ownership.service" +import createJwtConfig, { createRefreshJwtConfig } from "./config/jwt.config" import { StreamsRepository } from "./streams/repository/streams.repository" import { StreamsController } from "./streams/streams.controller" import { StreamsService } from "./streams/streams.service" import { TagsRepository } from "./tags/repository/tags.repository" +import { StreamTagsController } from "./tags/tags.controller" import { TagsService } from "./tags/tags.service" import { WebhooksService } from "./webhooks/webhooks.service" @@ -108,7 +110,7 @@ describe("Contract provider verification (api)", () => { JwtModule.registerAsync({ useFactory: () => createJwtConfig() }), CacheModule.register(), ], - controllers: [StreamsController, AuthController], + controllers: [StreamsController, StreamTagsController, AuthController], providers: [ StreamsService, TagsService, @@ -176,6 +178,15 @@ describe("Contract provider verification (api)", () => { description: "Seeded for contract verification", }) existingStreamId = String(stream.id) + + // Attach one tag so `list-stream-tags` exercises a non-empty + // response (the schema pins the Tag shape, not just the empty case). + const tagsRepository = moduleFixture.get(TagsRepository) + const seededTag = await tagsRepository.upsertBySlug( + "Live Streaming", + "live-streaming", + ) + await tagsRepository.attachToStream(stream.id, seededTag.id) }) afterAll(async () => { diff --git a/api/src/tags/tags.controller.spec.ts b/api/src/tags/tags.controller.spec.ts index e555995..176001b 100644 --- a/api/src/tags/tags.controller.spec.ts +++ b/api/src/tags/tags.controller.spec.ts @@ -1,9 +1,9 @@ import { Test, TestingModule } from "@nestjs/testing" -import { StreamOwnershipGuard } from "../common/guards/stream-ownership.guard" import { Tag } from "./tag.entity" import { StreamTagsController, TagsListController } from "./tags.controller" import { TagsService } from "./tags.service" +import { StreamOwnershipGuard } from "../common/guards/stream-ownership.guard" // Mock the env config before any imports that transitively load it. // StreamOwnershipGuard → StreamOwnershipService → config/env → validateEnv() @@ -89,11 +89,12 @@ describe("TagsListController", () => { describe("StreamTagsController", () => { let controller: StreamTagsController let tagsService: jest.Mocked< - Pick + Pick > beforeEach(async () => { tagsService = { + listForStream: jest.fn(), attachToStream: jest.fn(), detachFromStream: jest.fn(), } @@ -109,6 +110,44 @@ describe("StreamTagsController", () => { controller = module.get(StreamTagsController) }) + // ── list (issue #517) ────────────────────────────────────────────────── + + it("delegates list to tagsService.listForStream with defaults", async () => { + const tag = makeTag() + tagsService.listForStream.mockResolvedValue({ + data: [tag], + page: 1, + limit: 50, + total: 1, + hasMore: false, + }) + + const result = await controller.list(1, {}) + + expect(tagsService.listForStream).toHaveBeenCalledWith(1, 1, 50) + expect(result).toEqual({ + data: [tag], + page: 1, + limit: 50, + total: 1, + hasMore: false, + }) + }) + + it("forwards explicit page and limit from query", async () => { + tagsService.listForStream.mockResolvedValue({ + data: [], + page: 2, + limit: 10, + total: 0, + hasMore: false, + }) + + await controller.list(1, { page: 2, limit: 10 }) + + expect(tagsService.listForStream).toHaveBeenCalledWith(1, 2, 10) + }) + // ── @UseGuards reflection ───────────────────────────────────────────── it("has StreamOwnershipGuard applied to the controller class", () => { diff --git a/api/src/tags/tags.controller.ts b/api/src/tags/tags.controller.ts index 566870f..b976355 100644 --- a/api/src/tags/tags.controller.ts +++ b/api/src/tags/tags.controller.ts @@ -11,10 +11,16 @@ import { Query, UseGuards, } from "@nestjs/common" -import { StreamOwnershipGuard } from "../common/guards/stream-ownership.guard" +import { + ApiForbiddenResponse, + ApiOkResponse, + ApiOperation, +} from "@nestjs/swagger" + import { CreateTagDto } from "./dto/create-tag.dto" import { ListTagsQueryDto } from "./dto/list-tags.query.dto" import { TagsService } from "./tags.service" +import { StreamOwnershipGuard } from "../common/guards/stream-ownership.guard" /** * Public, paginated list of all tags in the system. @@ -34,9 +40,10 @@ export class TagsListController { } /** - * Stream-scoped tag management. Both endpoints require ownership of the + * Stream-scoped tag management. All endpoints require ownership of the * referenced stream, enforced via {@link StreamOwnershipGuard}. * + * GET /streams/:id/tags -> PagedTags * POST /streams/:id/tags { name: "Live Streaming" } * DELETE /streams/:id/tags/:tagId */ @@ -45,6 +52,27 @@ export class TagsListController { export class StreamTagsController { constructor(private readonly tagsService: TagsService) {} + /** + * Lists the tags attached to a stream (issue #517). The response + * uses the same `PagedTags` envelope as `GET /tags` so the dashboard's + * `useStreamTags` hook can parse it without a second shape. Ownership + * is enforced by {@link StreamOwnershipGuard} — non-owners get 403. + */ + @Get() + @ApiOperation({ summary: "List tags attached to a stream" }) + @ApiOkResponse({ + description: "The tags attached to the stream, in a PagedTags envelope", + }) + @ApiForbiddenResponse({ description: "You do not own this stream." }) + list( + @Param("id", ParseIntPipe) streamId: number, + @Query() query: ListTagsQueryDto, + ) { + const page = query.page ?? 1 + const limit = query.limit ?? 50 + return this.tagsService.listForStream(streamId, page, limit) + } + @Post() @HttpCode(HttpStatus.CREATED) attach( diff --git a/api/src/tags/tags.service.spec.ts b/api/src/tags/tags.service.spec.ts index cb48c8f..f2a10c4 100644 --- a/api/src/tags/tags.service.spec.ts +++ b/api/src/tags/tags.service.spec.ts @@ -163,6 +163,53 @@ describe("TagsService", () => { }) }) + // ── listForStream (issue #517) ────────────────────────────────────────── + + describe("listForStream", () => { + it("returns the stream's tags in the PagedTags envelope via the batch path", async () => { + const tag = makeTag() + repo.listForStreamIds.mockResolvedValue(new Map([[7, [tag]]])) + + const result = await service.listForStream(7) + + // Reuses the batch loader with a single-element array — no per-tag + // query is issued. + expect(repo.listForStreamIds).toHaveBeenCalledWith([7]) + expect(result).toEqual({ + data: [tag], + page: 1, + limit: 50, + total: 1, + hasMore: false, + }) + }) + + it("returns an empty list (not an error) for a stream with no tags", async () => { + repo.listForStreamIds.mockResolvedValue(new Map([[7, []]])) + + const result = await service.listForStream(7) + + expect(result.data).toEqual([]) + expect(result.total).toBe(0) + expect(result.hasMore).toBe(false) + }) + + it("paginates the page slice and sets hasMore", async () => { + const tags = Array.from({ length: 3 }, (_, i) => + makeTag({ id: i + 1, slug: `slug-${i + 1}` }), + ) + repo.listForStreamIds.mockResolvedValue(new Map([[7, tags]])) + + const page1 = await service.listForStream(7, 1, 2) + expect(page1.data).toHaveLength(2) + expect(page1.hasMore).toBe(true) + + const page2 = await service.listForStream(7, 2, 2) + expect(page2.data).toHaveLength(1) + expect(page2.hasMore).toBe(false) + }) + }) + // ── detachFromStream ───────────────────────────────────────────────────── describe("detachFromStream", () => { diff --git a/api/src/tags/tags.service.ts b/api/src/tags/tags.service.ts index 60261e8..7fa5d50 100644 --- a/api/src/tags/tags.service.ts +++ b/api/src/tags/tags.service.ts @@ -3,10 +3,11 @@ import { Injectable, NotFoundException, } from "@nestjs/common" -import { PaginatedResult } from "../common/dto/pagination.dto" + import { TagsRepository } from "./repository/tags.repository" import { slugify } from "./slugify" import { Tag } from "./tag.entity" +import { PaginatedResult } from "../common/dto/pagination.dto" export interface PagedTags extends PaginatedResult { hasMore: boolean @@ -42,6 +43,28 @@ export class TagsService { return this.tags.listForStreamIds(streamIds) } + /** + * Loads the tags attached to a single stream, reusing the batch + * `listForStreamIds` path (issue #330) so the DB cost stays at one + * round-trip instead of a per-tag query. Returns the `PagedTags` + * envelope the app's `useStreamTags` hook parses (issue #517). + */ + async listForStream( + streamId: number, + page = 1, + limit = 50, + ): Promise { + const tags = (await this.listForStreamIds([streamId])).get(streamId) ?? [] + const offset = (page - 1) * limit + return { + data: tags.slice(offset, offset + limit), + page, + limit, + total: tags.length, + hasMore: page * limit < tags.length, + } + } + /** * Create-or-fetch a tag from a raw name, then attach it to the stream. * Returns the canonical Tag row (existing or freshly created). diff --git a/app/hooks/useStreams.test.tsx b/app/hooks/useStreams.test.tsx index d9f63ea..87fc303 100644 --- a/app/hooks/useStreams.test.tsx +++ b/app/hooks/useStreams.test.tsx @@ -1,7 +1,8 @@ -import * as React from "react" import { QueryClient, QueryClientProvider } from "@tanstack/react-query" import { renderHook, waitFor } from "@testing-library/react" -import { useStreamList, streamKeys } from "@/hooks/useStreams" +import * as React from "react" + +import { useStreamList, useStreamTags, streamKeys } from "@/hooks/useStreams" function createWrapper() { const client = new QueryClient({ @@ -70,3 +71,40 @@ describe("useStreamList (issue #345 phase B)", () => { expect(result.current.data?.data[0]?.tags).toEqual([]) }) }) + +describe("useStreamTags (issue #517)", () => { + it("fetches GET /streams/:id/tags and parses the PagedTags envelope", async () => { + const mockResponse = { + ok: true, + status: 200, + headers: new Headers({ "content-type": "application/json" }), + json: async () => ({ + data: [ + { + id: 1, + name: "Live Streaming", + slug: "live-streaming", + createdAt: "2026-01-01T00:00:00.000Z", + }, + ], + page: 1, + limit: 50, + total: 1, + hasMore: false, + }), + } + const mock = jest.fn().mockResolvedValue(mockResponse) + global.fetch = mock as unknown as typeof fetch + + const { result } = renderHook(() => useStreamTags(42), { + wrapper: createWrapper(), + }) + + await waitFor(() => expect(result.current.isSuccess).toBe(true)) + expect(mock).toHaveBeenCalledTimes(1) + const calledUrl = (mock.mock.calls[0]?.[0] as URL | string).toString() + expect(calledUrl).toMatch(/\/streams\/42\/tags$/) + expect(result.current.data?.data[0]?.name).toBe("Live Streaming") + expect(result.current.data?.hasMore).toBe(false) + }) +}) diff --git a/tests/contracts/src/schemas.ts b/tests/contracts/src/schemas.ts index 7b9a6a0..96f7c6c 100644 --- a/tests/contracts/src/schemas.ts +++ b/tests/contracts/src/schemas.ts @@ -1,8 +1,10 @@ import { z, type ZodType } from "zod" + import type { ApiErrorResponse, PaginatedResponse, Stream, + Tag, User, } from "@xstreamroll/types" @@ -44,6 +46,29 @@ export const paginatedStreamsSchema = typed>()( }), ) +export const tagSchema = typed()( + z.object({ + id: z.number(), + name: z.string(), + slug: z.string(), + createdAt: z.string(), + }), +) + +/** + * Envelope returned by `GET /streams/:id/tags` (issue #517). Uses the + * `PagedTags` shape (with `hasMore`) that the app's `useStreamTags` + * hook parses — deliberately not pinned via `typed<>` because no + * shared `@xstreamroll/types` interface carries `hasMore` yet. + */ +export const pagedTagsSchema = z.object({ + data: z.array(tagSchema), + page: z.number(), + limit: z.number(), + total: z.number(), + hasMore: z.boolean(), +}) + export const userSchema = typed()( z.object({ id: z.string(), diff --git a/tests/contracts/src/streams.contract.ts b/tests/contracts/src/streams.contract.ts index 581bc79..d0c506d 100644 --- a/tests/contracts/src/streams.contract.ts +++ b/tests/contracts/src/streams.contract.ts @@ -1,6 +1,12 @@ -import type { CreateStreamDto, UpdateStreamDto } from "@xstreamroll/types" import { PLACEHOLDER, type Contract } from "./contract" -import { apiErrorSchema, paginatedStreamsSchema, streamSchema } from "./schemas" +import { + apiErrorSchema, + pagedTagsSchema, + paginatedStreamsSchema, + streamSchema, +} from "./schemas" + +import type { CreateStreamDto, UpdateStreamDto } from "@xstreamroll/types" const createBody: CreateStreamDto = { name: "My stream", @@ -82,6 +88,22 @@ export const streamsContracts: Contract[] = [ schema: apiErrorSchema, }, }, + { + name: "list-stream-tags", + description: "GET /streams/:id/tags returns the tags attached to an owned stream", + consumer: "xstreamroll-sdk", + provider: "api", + request: { + method: "GET", + path: "/streams/:id/tags", + pathParams: { id: PLACEHOLDER.EXISTING_STREAM_ID }, + authenticated: true, + }, + response: { + status: 200, + schema: pagedTagsSchema, + }, + }, { name: "update-stream", description: "PATCH /streams/:id updates a stream owned by the caller", diff --git a/xstreamroll-sdk/__tests__/contract.consumer.test.ts b/xstreamroll-sdk/__tests__/contract.consumer.test.ts index 5ecbcc8..72f2da3 100644 --- a/xstreamroll-sdk/__tests__/contract.consumer.test.ts +++ b/xstreamroll-sdk/__tests__/contract.consumer.test.ts @@ -20,10 +20,12 @@ import { allContracts, authResponseSchema, + pagedTagsSchema, streamSchema, type Contract, } from "@xstreamroll/contract-tests" import nock from "nock" + import { StreamingClient } from "../src/client" const BASE_URL = "http://api.test" @@ -71,6 +73,35 @@ describe("Consumer contract verification (xstreamroll-sdk)", () => { expect(result).toEqual(example) }) + it("getStreamTags() requests exactly what list-stream-tags expects and returns a contract-valid PagedTags", async () => { + const c = contract("list-stream-tags") + const example = { + data: [ + { + id: 1, + name: "Live Streaming", + slug: "live-streaming", + createdAt: "2026-01-01T00:00:00.000Z", + }, + ], + page: 1, + limit: 50, + total: 1, + hasMore: false, + } + // The example itself must be valid per the shared schema, or this + // test would be asserting nothing meaningful. + expect(() => pagedTagsSchema.parse(example)).not.toThrow() + + const scope = nock(BASE_URL).get("/streams/42/tags").reply(c.response.status, example) + + const result = await client.getStreamTags("42") + + expect(scope.isDone()).toBe(true) + expect(() => pagedTagsSchema.parse(result)).not.toThrow() + expect(result).toEqual(example) + }) + it("register() sends the request body the register contract expects", async () => { const c = contract("register") const example = { diff --git a/xstreamroll-sdk/src/client.ts b/xstreamroll-sdk/src/client.ts index c43063e..205d590 100644 --- a/xstreamroll-sdk/src/client.ts +++ b/xstreamroll-sdk/src/client.ts @@ -1,12 +1,12 @@ import { HttpClient, HttpRequestError } from "./http" import { paginateAll as createIterator, type PaginatedFetcher } from "./pagination" - import { ApiError, type ApiErrorResponse, type AuthTokens, type CreateUserDto, type CreateWebhookDto, + type PagedTags, type Stream, type StreamConfig, type StreamEvent, @@ -121,6 +121,16 @@ export class StreamingClient { } } + /** + * Lists the tags attached to a stream (issue #517). Requires the + * caller to own the stream — the API returns 403 otherwise. + */ + async getStreamTags(streamId: string): Promise { + return this.requestJson(`/streams/${streamId}/tags`, { + method: "GET", + }) + } + // ── Webhooks ────────────────────────────────────────────────────────────── /** diff --git a/xstreamroll-sdk/src/types.ts b/xstreamroll-sdk/src/types.ts index bd113a5..b9914eb 100644 --- a/xstreamroll-sdk/src/types.ts +++ b/xstreamroll-sdk/src/types.ts @@ -1,6 +1,9 @@ // ─── Generated types from OpenAPI spec ───────────────────────────────────── // Regenerate with `npm run generate:types` (requires API server running). import type { components } from "./generated/schema" +// Local type bindings for interfaces defined in this file (the +// `export type { … } from` blocks below re-export but do not bind). +import type { ApiErrorResponse, StreamEventType, Tag } from "@xstreamroll/types" export type { components } @@ -36,6 +39,7 @@ export type { StreamVisibility, CreateStreamDto, UpdateStreamDto, + Tag, StreamEventType, StreamEvent, StreamEventRecord, @@ -45,7 +49,17 @@ export type { ApiErrorResponse, } from "@xstreamroll/types" -import type { ApiErrorResponse, StreamEventType } from "@xstreamroll/types" +/** + * Paginated tags response — mirrors the API's `PagedTags` envelope + * returned by `GET /streams/:id/tags` and `GET /tags`. + */ +export interface PagedTags { + data: Tag[] + page: number + limit: number + total: number + hasMore: boolean +} // ─── Config ──────────────────────────────────────────────────────────────────