From 01081db7a3002b81be0be80c222894c2a5a20086 Mon Sep 17 00:00:00 2001 From: Joey Baker Date: Thu, 27 Oct 2016 21:01:32 -0700 Subject: [PATCH 1/7] Test add destroyScope tests --- src/crud-destroy.integration.test.js | 63 ++++++++++++++++++++++++++++ test/fixtures/models/player.js | 13 ++++++ test/integration-setup.js | 4 +- 3 files changed, 79 insertions(+), 1 deletion(-) diff --git a/src/crud-destroy.integration.test.js b/src/crud-destroy.integration.test.js index 702f640..75aa965 100644 --- a/src/crud-destroy.integration.test.js +++ b/src/crud-destroy.integration.test.js @@ -4,6 +4,7 @@ import setup from '../test/integration-setup.js'; const STATUS_OK = 200; const STATUS_NOT_FOUND = 404; +const STATUS_BAD_REQUEST = 400; setup(test); @@ -112,3 +113,65 @@ test('not found /notamodel', async (t) => { const { statusCode } = await server.inject({ url, method }); t.is(statusCode, STATUS_NOT_FOUND); }); + +test('destroyScope /players/returnsOne', async (t) => { + const { server, instances, sequelize: { models: { Player } } } = t.context; + const { player1, player2 } = instances; + // this doesn't exist in our fixtures + const url = '/players/returnsOne'; + 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); + t.is(result.id, player1.id); + + const nonDeletedPlayers = await Player.findAll(); + t.is(nonDeletedPlayers.length, presentPlayers.length - 1); +}); + +test('destroyScope /players/returnsNone', async (t) => { + const { server, instances, sequelize: { models: { Player } } } = t.context; + const { player1, player2 } = instances; + // this doesn't exist in our fixtures + const url = '/players/returnsNone'; + 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(); + const nonDeletedPlayerIds = nonDeletedPlayers.map(({ id }) => id); + t.truthy(nonDeletedPlayerIds.includes(player1.id)); + t.truthy(nonDeletedPlayerIds.includes(player2.id)); +}); + +test('destroyScope invalid scope /players/invalid', async (t) => { + const { server, instances, sequelize: { models: { Player } } } = t.context; + const { player1, player2 } = instances; + // this doesn't exist in our fixtures + const url = '/players/invalid'; + 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_BAD_REQUEST); + + const nonDeletedPlayers = await Player.findAll(); + const nonDeletedPlayerIds = nonDeletedPlayers.map(({ id }) => id); + t.truthy(nonDeletedPlayerIds.includes(player1.id)); + t.truthy(nonDeletedPlayerIds.includes(player2.id)); +}); diff --git a/test/fixtures/models/player.js b/test/fixtures/models/player.js index 0f678b3..9753bb5 100644 --- a/test/fixtures/models/player.js +++ b/test/fixtures/models/player.js @@ -7,6 +7,7 @@ export default (sequelize, DataTypes) => { }, name: DataTypes.STRING, teamId: DataTypes.INTEGER, + active: DataTypes.BOOLEAN, }, { classMethods: { associate: (models) => { @@ -15,5 +16,17 @@ export default (sequelize, DataTypes) => { }); }, }, + scopes: { + returnsOne: { + where: { + active: true, + }, + }, + returnsNone: { + where: { + name: 'notaname', + }, + }, + }, }); }; diff --git a/test/integration-setup.js b/test/integration-setup.js index f5ae745..c895343 100644 --- a/test/integration-setup.js +++ b/test/integration-setup.js @@ -58,7 +58,9 @@ export default (test) => { const { Player, Team, City } = t.context.sequelize.models; const city1 = await City.create({ name: 'Healdsburg' }); const team1 = await Team.create({ name: 'Baseballs', cityId: city1.id }); - const player1 = await Player.create({ name: 'Pinot', teamId: team1.id }); + const player1 = await Player.create({ + name: 'Pinot', teamId: team1.id, active: true, + }); const player2 = await Player.create({ name: 'Syrah', teamId: team1.id }); t.context.instances = { city1, team1, player1, player2 }; }); -- 2.34.1 From 1daa68e03e57d57f1157defd5afb7a3569925a67 Mon Sep 17 00:00:00 2001 From: Joey Baker Date: Thu, 27 Oct 2016 21:03:57 -0700 Subject: [PATCH 2/7] Fix(crud) destroyScope sends 404 when not found --- src/crud.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/crud.js b/src/crud.js index be9bcd1..32cc61a 100644 --- a/src/crud.js +++ b/src/crud.js @@ -309,6 +309,8 @@ export const destroyScope = ({ server, model, prefix = '/', config }) => { const list = await model.scope(request.params.scope).findAll({ include, where }); + if (!list.length) return void reply(notFound('Nothing found.')); + await Promise.all(list.map(instance => instance.destroy())); const listAsJSON = list.map((item) => item.toJSON()); -- 2.34.1 From 6a2290f064bc707a2a39a69e2862e5d76ad0411a Mon Sep 17 00:00:00 2001 From: Joey Baker Date: Fri, 28 Oct 2016 11:20:12 -0700 Subject: [PATCH 3/7] Docs add docs for additional order option --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index a5cfdcc..f9e11ef 100644 --- a/README.md +++ b/README.md @@ -158,6 +158,7 @@ Team.findAll({order: ['name']}) ```js // returns the teams ordered by the name column, descending // GET /teams?order[0]=name&order[0]=DESC +// GET /teams?order=name%20DESC // results in a Sequelize query: Team.findAll({order: [['name', 'DESC']]}) -- 2.34.1 From 11e6ff596c07414b18bbd8884981bafb7d70c531 Mon Sep 17 00:00:00 2001 From: Joey Baker Date: Fri, 28 Oct 2016 11:20:59 -0700 Subject: [PATCH 4/7] Fix(crud) scope now 404s on no results --- src/crud.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/crud.js b/src/crud.js index 32cc61a..1f02710 100644 --- a/src/crud.js +++ b/src/crud.js @@ -214,6 +214,8 @@ export const scope = ({ server, model, prefix = '/', config }) => { include, where, limit, offset, order, }); + if (!list.length) return void reply(notFound('Nothing found.')); + reply(list.map((item) => item.toJSON())); }, config, -- 2.34.1 From 8fb3f2e849d5fb6eb6a3f24e9ba54a2a8f75d95c Mon Sep 17 00:00:00 2001 From: Joey Baker Date: Fri, 28 Oct 2016 11:22:05 -0700 Subject: [PATCH 5/7] Fix(crud) actually enable multiple orders This was supposed to work, but adding integration tests I realized it didn't. #oops --- src/crud.test.js | 2 +- src/get-config-for-method.js | 2 +- src/utils.js | 4 ++-- src/utils.test.js | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/crud.test.js b/src/crud.test.js index 0856451..362f65f 100644 --- a/src/crud.test.js +++ b/src/crud.test.js @@ -225,7 +225,7 @@ test('crud#list handler with order', async (t) => { t.deepEqual( findAllArgs.order, - [request.query.order], + [[request.query.order]], 'queries with the order as an array b/c that\'s what sequelize wants' ); }); diff --git a/src/get-config-for-method.js b/src/get-config-for-method.js index a87a43e..d6bc661 100644 --- a/src/get-config-for-method.js +++ b/src/get-config-for-method.js @@ -144,7 +144,7 @@ export default ({ .keys({ limit: joi.number().min(0).integer(), offset: joi.number().min(0).integer(), - order: joi.array(), + order: [joi.array(), joi.string()], }), get(methodConfig, 'validate.query') ); diff --git a/src/utils.js b/src/utils.js index bc19051..dae90c0 100644 --- a/src/utils.js +++ b/src/utils.js @@ -70,7 +70,7 @@ export const parseOrder = (request) => { // transform to an array so sequelize will escape the input for us and // maintain security. See http://docs.sequelizejs.com/en/latest/docs/querying/#ordering - if (isString(order)) return order.split(' '); + if (isString(order)) return [order.split(' ')]; for (const key of Object.keys(order)) { try { @@ -80,7 +80,7 @@ export const parseOrder = (request) => { } } - return order; + return [order]; }; export const getMethod = (model, association, plural = true, method = 'get') => { diff --git a/src/utils.test.js b/src/utils.test.js index 43eb29e..bd91198 100644 --- a/src/utils.test.js +++ b/src/utils.test.js @@ -57,7 +57,7 @@ test('parseOrder returns order when a string', (t) => { t.deepEqual( parseOrder(request) - , [order] + , [[order]] ); }); @@ -69,7 +69,7 @@ test('parseOrder returns order when json', (t) => { t.deepEqual( parseOrder(request) - , order + , [order] ); }); -- 2.34.1 From 5aec1242db940437a6cb55a4352533377ccf14b8 Mon Sep 17 00:00:00 2001 From: Joey Baker Date: Fri, 28 Oct 2016 11:22:38 -0700 Subject: [PATCH 6/7] Test add integration tests for ordering lists --- src/crud-list-order.integration.test.js | 83 +++++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 src/crud-list-order.integration.test.js diff --git a/src/crud-list-order.integration.test.js b/src/crud-list-order.integration.test.js new file mode 100644 index 0000000..daba5dc --- /dev/null +++ b/src/crud-list-order.integration.test.js @@ -0,0 +1,83 @@ +import test from 'ava'; +import 'sinon-bluebird'; +import setup from '../test/integration-setup.js'; + +const STATUS_OK = 200; +const STATUS_BAD_QUERY = 502; + +setup(test); + +test('/players?order=name', async (t) => { + const { server, instances } = t.context; + const { player1, player2 } = instances; + const url = '/players?order=name'; + const method = 'GET'; + + const { result, statusCode } = await server.inject({ url, method }); + t.is(statusCode, STATUS_OK); + // this is the order we'd expect the names to be in + t.is(result[0].name, player1.name); + t.is(result[1].name, player2.name); +}); + +test('/players?order=name%20ASC', async (t) => { + const { server, instances } = t.context; + const { player1, player2 } = instances; + const url = '/players?order=name%20ASC'; + const method = 'GET'; + + const { result, statusCode } = await server.inject({ url, method }); + t.is(statusCode, STATUS_OK); + // this is the order we'd expect the names to be in + t.is(result[0].name, player1.name); + t.is(result[1].name, player2.name); +}); + +test('/players?order=name%20DESC', async (t) => { + const { server, instances } = t.context; + const { player1, player2 } = instances; + const url = '/players?order=name%20DESC'; + const method = 'GET'; + + const { result, statusCode } = await server.inject({ url, method }); + t.is(statusCode, STATUS_OK); + // this is the order we'd expect the names to be in + t.is(result[0].name, player2.name); + t.is(result[1].name, player1.name); +}); + +test('/players?order[]=name', async (t) => { + const { server, instances } = t.context; + const { player1, player2 } = instances; + const url = '/players?order[]=name'; + const method = 'GET'; + + const { result, statusCode } = await server.inject({ url, method }); + t.is(statusCode, STATUS_OK); + // this is the order we'd expect the names to be in + t.is(result[0].name, player1.name); + t.is(result[1].name, player2.name); +}); + +test('/players?order[0]=name&order[0]=DESC', async (t) => { + const { server, instances } = t.context; + const { player1, player2 } = instances; + const url = '/players?order[0]=name&order[0]=DESC'; + const method = 'GET'; + + const { result, statusCode } = await server.inject({ url, method }); + t.is(statusCode, STATUS_OK); + // this is the order we'd expect the names to be in + t.is(result[0].name, player2.name); + t.is(result[1].name, player1.name); +}); + +test('invalid column /players?order[0]=invalid', async (t) => { + const { server } = t.context; + const url = '/players?order[]=invalid'; + const method = 'GET'; + + const { statusCode, result } = await server.inject({ url, method }); + t.is(statusCode, STATUS_BAD_QUERY); + t.truthy(result.message.includes('invalid')); +}); -- 2.34.1 From cb6ea518361700793d4bdd5f81650c5368c4669b Mon Sep 17 00:00:00 2001 From: Joey Baker Date: Fri, 28 Oct 2016 11:22:52 -0700 Subject: [PATCH 7/7] Test add integration tests for scope --- src/crud-scope.integration.test.js | 40 ++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 src/crud-scope.integration.test.js diff --git a/src/crud-scope.integration.test.js b/src/crud-scope.integration.test.js new file mode 100644 index 0000000..f8c575a --- /dev/null +++ b/src/crud-scope.integration.test.js @@ -0,0 +1,40 @@ +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('/players/returnsOne', async (t) => { + const { server, instances } = t.context; + const { player1 } = instances; + const url = '/players/returnsOne'; + const method = 'GET'; + + const { result, statusCode } = await server.inject({ url, method }); + t.is(statusCode, STATUS_OK); + t.is(result.length, 1); + t.truthy(result[0].id, player1.id); +}); + +test('/players/returnsNone', async (t) => { + const { server } = t.context; + const url = '/players/returnsNone'; + const method = 'GET'; + + const { statusCode } = await server.inject({ url, method }); + t.is(statusCode, STATUS_NOT_FOUND); +}); + +test('invalid scope /players/invalid', async (t) => { + const { server } = t.context; + // this doesn't exist in our fixtures + const url = '/players/invalid'; + const method = 'GET'; + + const { statusCode } = await server.inject({ url, method }); + t.is(statusCode, STATUS_BAD_REQUEST); +}); -- 2.34.1