From f49e4daf792e0d5ef73762a9a09abec37c550927 Mon Sep 17 00:00:00 2001 From: Joey Baker Date: Thu, 27 Oct 2016 12:32:36 -0700 Subject: [PATCH 1/3] Test fix error checking in include tests #oops --- src/crud-include.integration.test.js | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/crud-include.integration.test.js b/src/crud-include.integration.test.js index e436f9b..169781f 100644 --- a/src/crud-include.integration.test.js +++ b/src/crud-include.integration.test.js @@ -2,6 +2,8 @@ import test from 'ava'; import 'sinon-bluebird'; import setup from '../test/integration-setup.js'; +const STATUS_OK = 200; + setup(test); test('belongsTo /team?include=city', async (t) => { @@ -9,8 +11,8 @@ test('belongsTo /team?include=city', async (t) => { const { team1, city1 } = instances; const path = `/team/${team1.id}?include=city`; - const { result, response } = await server.inject(path); - t.falsy(response instanceof Error); + const { result, statusCode } = await server.inject(path); + t.is(statusCode, STATUS_OK); t.is(result.id, team1.id); t.is(result.City.id, city1.id); }); @@ -20,8 +22,8 @@ test('belongsTo /team?include=cities', async (t) => { const { team1, city1 } = instances; const path = `/team/${team1.id}?include=cities`; - const { result, response } = await server.inject(path); - t.falsy(response instanceof Error); + const { result, statusCode } = await server.inject(path); + t.is(statusCode, STATUS_OK); t.is(result.id, team1.id); t.is(result.City.id, city1.id); }); @@ -31,8 +33,8 @@ test('hasMany /team?include=player', async (t) => { const { team1, player1, player2 } = instances; const path = `/team/${team1.id}?include=player`; - const { result, response } = await server.inject(path); - t.falsy(response instanceof Error); + const { result, statusCode } = await server.inject(path); + t.is(statusCode, STATUS_OK); t.is(result.id, team1.id); const playerIds = result.Players.map(({ id }) => id); @@ -45,8 +47,8 @@ test('hasMany /team?include=players', async (t) => { const { team1, player1, player2 } = instances; const path = `/team/${team1.id}?include=players`; - const { result, response } = await server.inject(path); - t.falsy(response instanceof Error); + const { result, statusCode } = await server.inject(path); + t.is(statusCode, STATUS_OK); t.is(result.id, team1.id); const playerIds = result.Players.map(({ id }) => id); @@ -59,8 +61,8 @@ test('multiple includes /team?include=players&include=city', async (t) => { const { team1, player1, player2, city1 } = instances; const path = `/team/${team1.id}?include=players&include=city`; - const { result, response } = await server.inject(path); - t.falsy(response instanceof Error); + const { result, statusCode } = await server.inject(path); + t.is(statusCode, STATUS_OK); t.is(result.id, team1.id); const playerIds = result.Players.map(({ id }) => id); @@ -74,8 +76,8 @@ test('multiple includes /team?include[]=players&include[]=city', async (t) => { const { team1, player1, player2, city1 } = instances; const path = `/team/${team1.id}?include[]=players&include[]=city`; - const { result, response } = await server.inject(path); - t.falsy(response instanceof Error); + const { result, statusCode } = await server.inject(path); + t.is(statusCode, STATUS_OK); t.is(result.id, team1.id); const playerIds = result.Players.map(({ id }) => id); From 0713f813011a594e547afd918a91412055d07428 Mon Sep 17 00:00:00 2001 From: Joey Baker Date: Thu, 27 Oct 2016 12:33:02 -0700 Subject: [PATCH 2/3] Test add CRUD tests boosting our test coverage --- src/crud-create.integration.test.js | 44 +++++++++++ src/crud-destroy.integration.test.js | 114 +++++++++++++++++++++++++++ src/crud-update.integration.test.js | 54 +++++++++++++ src/crud-where.integration.test.js | 53 +++++++++++++ 4 files changed, 265 insertions(+) create mode 100644 src/crud-create.integration.test.js create mode 100644 src/crud-destroy.integration.test.js create mode 100644 src/crud-update.integration.test.js create mode 100644 src/crud-where.integration.test.js diff --git a/src/crud-create.integration.test.js b/src/crud-create.integration.test.js new file mode 100644 index 0000000..18e8cd8 --- /dev/null +++ b/src/crud-create.integration.test.js @@ -0,0 +1,44 @@ +import test from 'ava'; +import 'sinon-bluebird'; +import setup from '../test/integration-setup.js'; + +const STATUS_OK = 200; +const STATUS_NOT_FOUND = 404; +const STATUS_BAD_REQUEST = 400; + +setup(test); + +test('where /player {name: "Chard"}', async (t) => { + const { server, sequelize: { models: { Player } } } = t.context; + const url = '/player'; + const method = 'POST'; + const payload = { name: 'Chard' }; + + const notPresentPlayer = await Player.findOne({ where: payload }); + t.falsy(notPresentPlayer); + + const { result, statusCode } = await server.inject({ url, method, payload }); + t.is(statusCode, STATUS_OK); + t.truthy(result.id); + t.is(result.name, payload.name); +}); + +test('not found /notamodel {name: "Chard"}', async (t) => { + const { server } = t.context; + const url = '/notamodel'; + const method = 'POST'; + const payload = { name: 'Chard' }; + + const { statusCode } = await server.inject({ url, method, payload }); + t.is(statusCode, STATUS_NOT_FOUND); +}); + + +test('no payload /player/1', async (t) => { + const { server } = t.context; + const url = '/player'; + const method = 'POST'; + + const { statusCode } = await server.inject({ url, method }); + t.is(statusCode, STATUS_BAD_REQUEST); +}); diff --git a/src/crud-destroy.integration.test.js b/src/crud-destroy.integration.test.js new file mode 100644 index 0000000..702f640 --- /dev/null +++ b/src/crud-destroy.integration.test.js @@ -0,0 +1,114 @@ +import test from 'ava'; +import 'sinon-bluebird'; +import setup from '../test/integration-setup.js'; + +const STATUS_OK = 200; +const STATUS_NOT_FOUND = 404; + +setup(test); + +test('destroy where /player?name=Baseball', async (t) => { + const { server, instances, sequelize: { models: { Player } } } = t.context; + const { player1, player2 } = instances; + const url = `/player?name=${player1.name}`; + const method = 'DELETE'; + + const presentPlayer = await Player.findById(player1.id); + t.truthy(presentPlayer); + + const { result, statusCode } = await server.inject({ url, method }); + t.is(statusCode, STATUS_OK); + t.is(result.id, player1.id); + + const deletedPlayer = await Player.findById(player1.id); + t.falsy(deletedPlayer); + const stillTherePlayer = await Player.findById(player2.id); + t.truthy(stillTherePlayer); +}); + +test('destroyAll where /players?name=Baseball', async (t) => { + const { server, instances, sequelize: { models: { Player } } } = t.context; + const { player1, player2 } = instances; + const url = `/players?name=${player1.name}`; + const method = 'DELETE'; + + const presentPlayer = await Player.findById(player1.id); + t.truthy(presentPlayer); + + const { result, statusCode } = await server.inject({ url, method }); + t.is(statusCode, STATUS_OK); + t.is(result.id, player1.id); + + const deletedPlayer = await Player.findById(player1.id); + t.falsy(deletedPlayer); + const stillTherePlayer = await Player.findById(player2.id); + t.truthy(stillTherePlayer); +}); + +test('destroyAll /players', async (t) => { + const { server, instances, sequelize: { models: { Player } } } = t.context; + const { player1, player2 } = instances; + const url = '/players'; + const method = 'DELETE'; + + const presentPlayers = await Player.findAll(); + const playerIds = presentPlayers.map(({ id }) => id); + t.truthy(playerIds.includes(player1.id)); + t.truthy(playerIds.includes(player2.id)); + + const { result, statusCode } = await server.inject({ url, method }); + t.is(statusCode, STATUS_OK); + const resultPlayerIds = result.map(({ id }) => id); + t.truthy(resultPlayerIds.includes(player1.id)); + t.truthy(resultPlayerIds.includes(player2.id)); + + const deletedPlayers = await Player.findAll(); + t.is(deletedPlayers.length, 0); +}); + +test('destroy not found /player/10', async (t) => { + const { server, instances, sequelize: { models: { Player } } } = t.context; + const { player1, player2 } = instances; + // this doesn't exist in our fixtures + const url = '/player/10'; + const method = 'DELETE'; + + const presentPlayers = await Player.findAll(); + const playerIds = presentPlayers.map(({ id }) => id); + t.truthy(playerIds.includes(player1.id)); + t.truthy(playerIds.includes(player2.id)); + + const { statusCode } = await server.inject({ url, method }); + t.is(statusCode, STATUS_NOT_FOUND); + + const nonDeletedPlayers = await Player.findAll(); + t.is(nonDeletedPlayers.length, presentPlayers.length); +}); + +test('destroyAll not found /players?name=no', async (t) => { + const { server, instances, sequelize: { models: { Player } } } = t.context; + const { player1, player2 } = instances; + // this doesn't exist in our fixtures + const url = '/players?name=no'; + const method = 'DELETE'; + + const presentPlayers = await Player.findAll(); + const playerIds = presentPlayers.map(({ id }) => id); + t.truthy(playerIds.includes(player1.id)); + t.truthy(playerIds.includes(player2.id)); + + const { statusCode } = await server.inject({ url, method }); + t.is(statusCode, STATUS_NOT_FOUND); + + const nonDeletedPlayers = await Player.findAll(); + t.is(nonDeletedPlayers.length, presentPlayers.length); +}); + +test('not found /notamodel', async (t) => { + const { server } = t.context; + const url = '/notamodel'; + const method = 'DELETE'; + + const { statusCode } = await server.inject({ url, method }); + t.is(statusCode, STATUS_NOT_FOUND); +}); diff --git a/src/crud-update.integration.test.js b/src/crud-update.integration.test.js new file mode 100644 index 0000000..50bed45 --- /dev/null +++ b/src/crud-update.integration.test.js @@ -0,0 +1,54 @@ +import test from 'ava'; +import 'sinon-bluebird'; +import setup from '../test/integration-setup.js'; + +const STATUS_OK = 200; +const STATUS_NOT_FOUND = 404; +const STATUS_BAD_REQUEST = 400; + +setup(test); + +test('where /player/1 {name: "Chard"}', async (t) => { + const { server, instances } = t.context; + const { player1 } = instances; + const url = `/player/${player1.id}`; + const method = 'PUT'; + const payload = { name: 'Chard' }; + + const { result, statusCode } = await server.inject({ url, method, payload }); + t.is(statusCode, STATUS_OK); + t.is(result.id, player1.id); + t.is(result.name, payload.name); +}); + +test('not found /player/10 {name: "Chard"}', async (t) => { + const { server } = t.context; + // this doesn't exist in our fixtures + const url = '/player/10'; + const method = 'PUT'; + const payload = { name: 'Chard' }; + + const { statusCode } = await server.inject({ url, method, payload }); + t.is(statusCode, STATUS_NOT_FOUND); +}); + + +test('no payload /player/1', async (t) => { + const { server, instances } = t.context; + const { player1 } = instances; + const url = `/player/${player1.id}`; + const method = 'PUT'; + + const { statusCode } = await server.inject({ url, method }); + t.is(statusCode, STATUS_BAD_REQUEST); +}); + +test('not found /notamodel {name: "Chard"}', async (t) => { + const { server } = t.context; + const url = '/notamodel'; + const method = 'PUT'; + const payload = { name: 'Chard' }; + + const { statusCode } = await server.inject({ url, method, payload }); + t.is(statusCode, STATUS_NOT_FOUND); +}); diff --git a/src/crud-where.integration.test.js b/src/crud-where.integration.test.js new file mode 100644 index 0000000..9ac1900 --- /dev/null +++ b/src/crud-where.integration.test.js @@ -0,0 +1,53 @@ +import test from 'ava'; +import 'sinon-bluebird'; +import setup from '../test/integration-setup.js'; + +const STATUS_OK = 200; +const STATUS_NOT_FOUND = 404; + +setup(test); + +test('single result /team?name=Baseball', async (t) => { + const { server, instances } = t.context; + const { team1 } = instances; + const path = `/team?name=${team1.name}`; + + const { result, statusCode } = await server.inject(path); + t.is(statusCode, STATUS_OK); + t.is(result.id, team1.id); + t.is(result.name, team1.name); +}); + +test('no results /team?name=Baseball&id=2', async (t) => { + const { server, instances } = t.context; + const { team1 } = instances; + // this doesn't exist in our fixtures + const path = `/team?name=${team1.name}&id=2`; + + const { statusCode } = await server.inject(path); + t.is(statusCode, STATUS_NOT_FOUND); +}); + +test('single result from list query /teams?name=Baseball', async (t) => { + const { server, instances } = t.context; + const { team1 } = instances; + const path = `/team?name=${team1.name}`; + + const { result, statusCode } = await server.inject(path); + t.is(statusCode, STATUS_OK); + t.is(result.id, team1.id); + t.is(result.name, team1.name); +}); + +test('multiple results from list query /players?teamId=1', async (t) => { + const { server, instances } = t.context; + const { team1, player1, player2 } = instances; + const path = `/players?teamId=${team1.id}`; + + const { result, statusCode } = await server.inject(path); + t.is(statusCode, STATUS_OK); + const playerIds = result.map(({ id }) => id); + t.truthy(playerIds.includes(player1.id)); + t.truthy(playerIds.includes(player2.id)); +}); + From 94e987013396a884e03378358273daeea76fbec1 Mon Sep 17 00:00:00 2001 From: Joey Baker Date: Thu, 27 Oct 2016 12:33:31 -0700 Subject: [PATCH 3/3] Fix(crud) 404 errors for destroy and destroyAll --- src/crud.js | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/crud.js b/src/crud.js index da0b756..be9bcd1 100644 --- a/src/crud.js +++ b/src/crud.js @@ -244,10 +244,18 @@ export const destroy = ({ server, model, prefix = '/', config }) => { @error async handler(request, reply) { const where = parseWhere(request); - if (request.params.id) where[model.primaryKeyField] = request.params.id; + const { id } = request.params; + if (id) where[model.primaryKeyField] = id; const list = await model.findAll({ where }); + if (!list.length) { + return void reply(id + ? notFound(`${id} not found.`) + : notFound('Nothing found.') + ); + } + await Promise.all(list.map(instance => instance.destroy())); const listAsJSON = list.map((item) => item.toJSON()); @@ -266,9 +274,17 @@ export const destroyAll = ({ server, model, prefix = '/', config }) => { @error async handler(request, reply) { const where = parseWhere(request); + const { id } = request.params; const list = await model.findAll({ where }); + if (!list.length) { + return void reply(id + ? notFound(`${id} not found.`) + : notFound('Nothing found.') + ); + } + await Promise.all(list.map(instance => instance.destroy())); const listAsJSON = list.map((item) => item.toJSON());