From ff13f1a4520cd806c2ba5b30e33b10c61951ecc1 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Wed, 22 Oct 2025 23:48:24 +0200 Subject: [PATCH] Update API responses to `204` (#10441) * shares.info * subscriptions and pins to 204 --- app/stores/PinsStore.ts | 3 ++ app/stores/SubscriptionsStore.ts | 3 ++ server/routes/api/pins/pins.test.ts | 4 +- server/routes/api/pins/pins.ts | 6 ++- server/routes/api/shares/shares.test.ts | 14 +++++++ server/routes/api/shares/shares.ts | 38 +++++++++++-------- .../api/subscriptions/subscriptions.test.ts | 4 +- .../routes/api/subscriptions/subscriptions.ts | 6 ++- 8 files changed, 56 insertions(+), 22 deletions(-) diff --git a/app/stores/PinsStore.ts b/app/stores/PinsStore.ts index b553a900ef..b92af41ed7 100644 --- a/app/stores/PinsStore.ts +++ b/app/stores/PinsStore.ts @@ -37,6 +37,9 @@ export default class PinsStore extends Store { documentId, collectionId, }); + if (!res) { + return; + } invariant(res?.data, "Data should be available"); return this.add(res.data); } catch (err) { diff --git a/app/stores/SubscriptionsStore.ts b/app/stores/SubscriptionsStore.ts index a7d9d23217..9932c48092 100644 --- a/app/stores/SubscriptionsStore.ts +++ b/app/stores/SubscriptionsStore.ts @@ -34,6 +34,9 @@ export default class SubscriptionsStore extends Store { try { const res = await client.post(`/${this.apiEndpoint}.info`, options); + if (!res) { + return; + } invariant(res?.data, "Data should be available"); return this.add(res.data); } catch (err) { diff --git a/server/routes/api/pins/pins.test.ts b/server/routes/api/pins/pins.test.ts index 0ebcf36295..c99c9bf351 100644 --- a/server/routes/api/pins/pins.test.ts +++ b/server/routes/api/pins/pins.test.ts @@ -227,7 +227,7 @@ describe("#pins.info", () => { expect(pin.data.collectionId).toEqual(document.collectionId); }); - it("should throw 404 if no pin found", async () => { + it("should return 204 if no pin for input", async () => { const user = await buildUser(); const document = await buildDocument({ userId: user.id, @@ -242,7 +242,7 @@ describe("#pins.info", () => { }, }); - expect(res.status).toEqual(404); + expect(res.status).toEqual(204); }); }); diff --git a/server/routes/api/pins/pins.ts b/server/routes/api/pins/pins.ts index 6e0cb61591..c6b60eec42 100644 --- a/server/routes/api/pins/pins.ts +++ b/server/routes/api/pins/pins.ts @@ -77,9 +77,13 @@ router.post( createdById: user.id, teamId: user.teamId, }, - rejectOnEmpty: true, }); + if (!pin) { + ctx.response.status = 204; + return; + } + ctx.body = { data: presentPin(pin), policies: presentPolicies(user, [pin]), diff --git a/server/routes/api/shares/shares.test.ts b/server/routes/api/shares/shares.test.ts index d6f2139f19..d76f69902c 100644 --- a/server/routes/api/shares/shares.test.ts +++ b/server/routes/api/shares/shares.test.ts @@ -655,6 +655,20 @@ describe("#shares.info", () => { expect(body.data.shares[0].id).toBe(share.id); expect(body.data.shares[0].published).toBe(true); }); + it("should allow reading share by documentId", async () => { + const user = await buildUser(); + const document = await buildDocument({ + userId: user.id, + teamId: user.teamId, + }); + const res = await server.post("/api/shares.info", { + body: { + token: user.getJwtToken(), + documentId: document.id, + }, + }); + expect(res.status).toEqual(204); + }); it("should return share for parent document with includeChildDocuments=true", async () => { const user = await buildUser(); const collection = await buildCollection({ diff --git a/server/routes/api/shares/shares.ts b/server/routes/api/shares/shares.ts index b17d9a454b..f0e39a5e1c 100644 --- a/server/routes/api/shares/shares.ts +++ b/server/routes/api/shares/shares.ts @@ -102,25 +102,31 @@ router.post( throw AuthenticationError("Authentication required"); } - const { share, parentShare } = await loadShareWithParent({ - collectionId, - documentId, - user, - }); + try { + const { share, parentShare } = await loadShareWithParent({ + collectionId, + documentId, + user, + }); - const shares = [share, parentShare].filter(Boolean) as Share[]; + const shares = [share, parentShare].filter(Boolean) as Share[]; + if (!shares.length) { + throw NotFoundError(); + } - if (!shares.length) { - ctx.response.status = 204; - return; + ctx.body = { + data: { + shares: shares.map((s) => presentShare(s, user.isAdmin ?? false)), + }, + policies: presentPolicies(user, shares), + }; + } catch (err) { + if (err.id === "not_found") { + ctx.response.status = 204; + return; + } + throw err; } - - ctx.body = { - data: { - shares: shares.map((s) => presentShare(s, user.isAdmin ?? false)), - }, - policies: presentPolicies(user, shares), - }; } ); diff --git a/server/routes/api/subscriptions/subscriptions.test.ts b/server/routes/api/subscriptions/subscriptions.test.ts index 96d2034323..7c3b612cdf 100644 --- a/server/routes/api/subscriptions/subscriptions.test.ts +++ b/server/routes/api/subscriptions/subscriptions.test.ts @@ -300,7 +300,7 @@ describe("#subscriptions.info", () => { ); }); - it("should throw 404 if no subscription found", async () => { + it("should return 204 if no subscription for input", async () => { const author = await buildUser(); const subscriber = await buildUser({ teamId: author.teamId }); const document = await buildDocument({ @@ -316,7 +316,7 @@ describe("#subscriptions.info", () => { }, }); - expect(res.status).toEqual(404); + expect(res.status).toEqual(204); }); it("should not allow outsiders to gain info about a subscription", async () => { diff --git a/server/routes/api/subscriptions/subscriptions.ts b/server/routes/api/subscriptions/subscriptions.ts index 4ad5d1334f..f1cd1ebe19 100644 --- a/server/routes/api/subscriptions/subscriptions.ts +++ b/server/routes/api/subscriptions/subscriptions.ts @@ -98,9 +98,13 @@ router.post( // There can be only one subscription with these props. const subscription = await Subscription.findOne({ where, - rejectOnEmpty: true, }); + if (!subscription) { + ctx.response.status = 204; + return; + } + ctx.body = { data: presentSubscription(subscription), };