From 0c840a92e31c42776a3afafdd927e792b27bc2e8 Mon Sep 17 00:00:00 2001 From: Aurelio Ogliari Date: Mon, 18 Nov 2019 14:02:01 +0100 Subject: [PATCH 1/4] remove unused "it" --- test/unit/associations.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/associations.test.ts b/test/unit/associations.test.ts index 6603d06..a95c0e5 100644 --- a/test/unit/associations.test.ts +++ b/test/unit/associations.test.ts @@ -55,7 +55,6 @@ describe("Associations", () => { const jsonapiTypeAssoc = assoc({ type: "type_strings" }) expect(jsonapiTypeAssoc.jsonapiType).to.eq("type_strings") - it }) it("accepts a bare spraypaint type string", () => { From 5f53db4690ec6f7ec803af97eeced023ac88aa1f Mon Sep 17 00:00:00 2001 From: Aurelio Ogliari Date: Mon, 18 Nov 2019 14:13:48 +0100 Subject: [PATCH 2/4] Add failing test for handling POST -> 204 status code see #51 --- test/integration/persistence.test.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/test/integration/persistence.test.ts b/test/integration/persistence.test.ts index 8257f94..e05f477 100644 --- a/test/integration/persistence.test.ts +++ b/test/integration/persistence.test.ts @@ -343,7 +343,7 @@ describe("Model persistence", () => { expect(instance.isPersisted).to.eq(false) }) - describe("when the server returns 204 no content", () => { + describe.only("when the server returns 204 no content", () => { beforeEach(() => { fetchMock.restore() @@ -357,11 +357,18 @@ describe("Model persistence", () => { resetMocks() }) - it("does not blow up", async () => { - expect(instance.isPersisted).to.eq(true) - await instance.destroy() + describe("does not blow up", () => { + it("when deleting", async () => { + expect(instance.isPersisted).to.eq(true) + await instance.destroy() - expect(instance.isPersisted).to.eq(false) + expect(instance.isPersisted).to.eq(false) + }) + + it("when creating", async () => { + instance.lastName = "Richards" + await instance.save() + }) }) }) From b9812455e8f4557efe27f256ce5b32aa378e75ec Mon Sep 17 00:00:00 2001 From: Aurelio Ogliari Date: Mon, 18 Nov 2019 16:20:52 +0100 Subject: [PATCH 3/4] Handle 204 responses (reponse with no body) fix #51 --- src/model.ts | 2 +- src/request.ts | 7 ++++--- test/integration/persistence.test.ts | 18 ++++++++++++------ 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/model.ts b/src/model.ts index a36cca9..635e895 100644 --- a/src/model.ts +++ b/src/model.ts @@ -988,7 +988,7 @@ export class SpraypaintBase { throw err } - if (response.status === 202) { + if (response.status === 202 || response.status === 204) { return await this._handleAcceptedResponse(response, this.onDeferredUpdate) } diff --git a/src/request.ts b/src/request.ts index cc35599..a74b56b 100644 --- a/src/request.ts +++ b/src/request.ts @@ -118,16 +118,17 @@ export class Request { response: Response, requestOptions: RequestInit ) { - let wasDelete = + const emptyResponseStatuses = [202, 204] + const wasDelete = requestOptions.method === "DELETE" && - [204, 200].indexOf(response.status) > -1 + emptyResponseStatuses.indexOf(response.status) > -1 if (wasDelete) return let json try { json = await response.clone().json() } catch (e) { - if (response.status === 202) return + if (emptyResponseStatuses.indexOf(response.status) > -1) return throw new ResponseError(response, "invalid json", e) } diff --git a/test/integration/persistence.test.ts b/test/integration/persistence.test.ts index e05f477..1d7d28b 100644 --- a/test/integration/persistence.test.ts +++ b/test/integration/persistence.test.ts @@ -349,7 +349,12 @@ describe("Model persistence", () => { fetchMock.mock({ matcher: "http://example.com/api/v1/people/1", - response: new Response({ status: 204 } as any) + response: new Response(null, { status: 204 } as any) + }) + + fetchMock.mock({ + matcher: "http://example.com/api/v1/people", + response: new Response(null, { status: 204 } as any) }) }) @@ -358,17 +363,18 @@ describe("Model persistence", () => { }) describe("does not blow up", () => { + it("when creating", async () => { + instance.isPersisted = false + instance.lastName = "Richards" + await instance.save() + }) + it("when deleting", async () => { expect(instance.isPersisted).to.eq(true) await instance.destroy() expect(instance.isPersisted).to.eq(false) }) - - it("when creating", async () => { - instance.lastName = "Richards" - await instance.save() - }) }) }) From 87d1c579430049f7a953322b3d4393163748ec0f Mon Sep 17 00:00:00 2001 From: Aurelio Ogliari Date: Mon, 18 Nov 2019 17:04:10 +0100 Subject: [PATCH 4/4] Make sure post and patch work when 204 is returned fix #51 --- src/request.ts | 6 +-- test/integration/persistence.test.ts | 60 +++++++++++++++++----------- 2 files changed, 39 insertions(+), 27 deletions(-) diff --git a/src/request.ts b/src/request.ts index a74b56b..907e34e 100644 --- a/src/request.ts +++ b/src/request.ts @@ -118,17 +118,17 @@ export class Request { response: Response, requestOptions: RequestInit ) { - const emptyResponseStatuses = [202, 204] const wasDelete = requestOptions.method === "DELETE" && - emptyResponseStatuses.indexOf(response.status) > -1 + [204, 200].indexOf(response.status) > -1 if (wasDelete) return let json try { json = await response.clone().json() } catch (e) { - if (emptyResponseStatuses.indexOf(response.status) > -1) return + const isEmptyResponse = [202, 204].indexOf(response.status) > -1 + if (isEmptyResponse) return throw new ResponseError(response, "invalid json", e) } diff --git a/test/integration/persistence.test.ts b/test/integration/persistence.test.ts index 1d7d28b..8985142 100644 --- a/test/integration/persistence.test.ts +++ b/test/integration/persistence.test.ts @@ -322,6 +322,34 @@ describe("Model persistence", () => { expect(job.klass).to.eq(BackgroundJob) }) }) + + describe("when the server returns 204 (no content)", () => { + const lastName = "Richards" + beforeEach(() => { + fetchMock.restore() + + fetchMock.mock("http://example.com/api/v1/people", { + status: 204, + body: null + }) + }) + + afterEach(resetMocks) + + it("doesn't blow up when posting", async () => { + instance.isPersisted = false + instance.lastName = lastName + await instance.save() + expect(instance.lastName).to.eq(lastName) + }) + + it("doesn't blow up when patching", async () => { + instance.isPersisted = true + instance.lastName = lastName + await instance.save() + expect(instance.lastName).to.eq(lastName) + }) + }) }) describe("#destroy", () => { @@ -343,38 +371,22 @@ describe("Model persistence", () => { expect(instance.isPersisted).to.eq(false) }) - describe.only("when the server returns 204 no content", () => { + describe("when the server returns 204 no content", () => { beforeEach(() => { fetchMock.restore() - fetchMock.mock({ - matcher: "http://example.com/api/v1/people/1", - response: new Response(null, { status: 204 } as any) - }) - - fetchMock.mock({ - matcher: "http://example.com/api/v1/people", - response: new Response(null, { status: 204 } as any) + fetchMock.mock("http://example.com/api/v1/people/1", { + status: 204 }) }) - afterEach(() => { - resetMocks() - }) - - describe("does not blow up", () => { - it("when creating", async () => { - instance.isPersisted = false - instance.lastName = "Richards" - await instance.save() - }) + afterEach(resetMocks) - it("when deleting", async () => { - expect(instance.isPersisted).to.eq(true) - await instance.destroy() + it("does not blow up", async () => { + expect(instance.isPersisted).to.eq(true) + await instance.destroy() - expect(instance.isPersisted).to.eq(false) - }) + expect(instance.isPersisted).to.eq(false) }) })