Feat ordering by associated models now works #32

Merged
joeybaker merged 2 commits from more-order-integration into master 2016-10-31 21:23:43 +00:00
5 changed files with 131 additions and 33 deletions

View File

@ -172,6 +172,22 @@ Team.findAll({order: [['name', 'DESC']]})
Team.findAll({order: [['name'], ['city']]}) Team.findAll({order: [['name'], ['city']]})
``` ```
You can even order by associated models. Though there is a [sequelize bug](https://github.com/sequelize/sequelize/issues?utf8=%E2%9C%93&q=is%3Aissue%20is%3Aopen%20order%20join%20) that might prevent this from working properly. A workaround is to `&include` the model you're ordering by.
```js
// returns the players ordered by the team name
// GET /players?order[0]={"model": "Team"}&order[0]=name
// results in a Sequelize query:
Player.findAll({order: [[{model: Team}, 'name']]})
// if the above returns a Sequelize error: `No such column Team.name`,
// you can work around this by forcing the join into the query:
// GET /players?order[0]={"model": "Team"}&order[0]=name&include=team
// results in a Sequelize query:
Player.findAll({order: [[{model: Team}, 'name']], include: [Team]})
```
## Authorization and other hooks ## Authorization and other hooks
You can use Hapi's [`ext` option](http://hapijs.com/api#route-options) to interact with the request both before and after this module does. This is useful if you want to enforce authorization, or modify the request before or after this module does. Hapi [has a full list of hooks](http://hapijs.com/api#request-lifecycle) you can use. You can use Hapi's [`ext` option](http://hapijs.com/api#route-options) to interact with the request both before and after this module does. This is useful if you want to enforce authorization, or modify the request before or after this module does. Hapi [has a full list of hooks](http://hapijs.com/api#request-lifecycle) you can use.

View File

@ -9,7 +9,7 @@ setup(test);
test('/players?order=name', async (t) => { test('/players?order=name', async (t) => {
const { server, instances } = t.context; const { server, instances } = t.context;
const { player1, player2 } = instances; const { player1, player2, player3 } = instances;
const url = '/players?order=name'; const url = '/players?order=name';
const method = 'GET'; const method = 'GET';
@ -18,11 +18,12 @@ test('/players?order=name', async (t) => {
// this is the order we'd expect the names to be in // this is the order we'd expect the names to be in
t.is(result[0].name, player1.name); t.is(result[0].name, player1.name);
t.is(result[1].name, player2.name); t.is(result[1].name, player2.name);
t.is(result[2].name, player3.name);
}); });
test('/players?order=name%20ASC', async (t) => { test('/players?order=name%20ASC', async (t) => {
const { server, instances } = t.context; const { server, instances } = t.context;
const { player1, player2 } = instances; const { player1, player2, player3 } = instances;
const url = '/players?order=name%20ASC'; const url = '/players?order=name%20ASC';
const method = 'GET'; const method = 'GET';
@ -31,24 +32,26 @@ test('/players?order=name%20ASC', async (t) => {
// this is the order we'd expect the names to be in // this is the order we'd expect the names to be in
t.is(result[0].name, player1.name); t.is(result[0].name, player1.name);
t.is(result[1].name, player2.name); t.is(result[1].name, player2.name);
t.is(result[2].name, player3.name);
}); });
test('/players?order=name%20DESC', async (t) => { test('/players?order=name%20DESC', async (t) => {
const { server, instances } = t.context; const { server, instances } = t.context;
const { player1, player2 } = instances; const { player1, player2, player3 } = instances;
const url = '/players?order=name%20DESC'; const url = '/players?order=name%20DESC';
const method = 'GET'; const method = 'GET';
const { result, statusCode } = await server.inject({ url, method }); const { result, statusCode } = await server.inject({ url, method });
t.is(statusCode, STATUS_OK); t.is(statusCode, STATUS_OK);
// this is the order we'd expect the names to be in // this is the order we'd expect the names to be in
t.is(result[0].name, player2.name); t.is(result[0].name, player3.name);
t.is(result[1].name, player1.name); t.is(result[1].name, player2.name);
t.is(result[2].name, player1.name);
}); });
test('/players?order[]=name', async (t) => { test('/players?order[]=name', async (t) => {
const { server, instances } = t.context; const { server, instances } = t.context;
const { player1, player2 } = instances; const { player1, player2, player3 } = instances;
const url = '/players?order[]=name'; const url = '/players?order[]=name';
const method = 'GET'; const method = 'GET';
@ -57,19 +60,74 @@ test('/players?order[]=name', async (t) => {
// this is the order we'd expect the names to be in // this is the order we'd expect the names to be in
t.is(result[0].name, player1.name); t.is(result[0].name, player1.name);
t.is(result[1].name, player2.name); t.is(result[1].name, player2.name);
t.is(result[2].name, player3.name);
}); });
test('/players?order[0]=name&order[0]=DESC', async (t) => { test('/players?order[0]=name&order[0]=DESC', async (t) => {
const { server, instances } = t.context; const { server, instances } = t.context;
const { player1, player2 } = instances; const { player1, player2, player3 } = instances;
const url = '/players?order[0]=name&order[0]=DESC'; const url = '/players?order[0]=name&order[0]=DESC';
const method = 'GET'; const method = 'GET';
const { result, statusCode } = await server.inject({ url, method }); const { result, statusCode } = await server.inject({ url, method });
t.is(statusCode, STATUS_OK); t.is(statusCode, STATUS_OK);
// this is the order we'd expect the names to be in // this is the order we'd expect the names to be in
t.is(result[0].name, player2.name); t.is(result[0].name, player3.name);
t.is(result[1].name, player2.name);
t.is(result[2].name, player1.name);
});
// multiple sorts
test('/players?order[0]=active&order[0]=DESC&order[1]=name&order[1]=DESC', async (t) => {
const { server, instances } = t.context;
const { player1, player2, player3 } = instances;
const url = '/players?order[0]=name&order[0]=DESC&order[1]=teamId&order[1]=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, player3.name);
t.is(result[1].name, player2.name);
t.is(result[2].name, player1.name);
});
// this will fail b/c sequelize doesn't correctly do the join when you pass
// an order. There are many issues for this:
// eslint-disable-next-line
// https://github.com/sequelize/sequelize/issues?utf8=%E2%9C%93&q=is%3Aissue%20is%3Aopen%20order%20join%20
//
// https://github.com/sequelize/sequelize/issues/5353 is a good example
// if this test passes, that's great! Just remove the workaround note in the
// docs
// eslint-disable-next-line
test.failing('sequelize bug /players?order[0]={"model":"Team"}&order[0]=name&order[0]=DESC', async (t) => {
const { server, instances } = t.context;
const { player1, player2, player3 } = instances;
const url = '/players?order[0]={"model":"Team"}&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, player3.name);
t.is(result[1].name, player1.name); t.is(result[1].name, player1.name);
t.is(result[2].name, player2.name);
});
// b/c the above fails, this is a work-around
test('/players?order[0]={"model":"Team"}&order[0]=name&order[0]=DESC&include=team', async (t) => {
const { server, instances } = t.context;
const { player1, player2, player3 } = instances;
const url = '/players?order[0]={"model":"Team"}&order[0]=name&order[0]=DESC&include=team';
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, player3.name);
t.is(result[1].name, player1.name);
t.is(result[2].name, player2.name);
}); });
test('invalid column /players?order[0]=invalid', async (t) => { test('invalid column /players?order[0]=invalid', async (t) => {

View File

@ -3,21 +3,27 @@ import { notImplemented } from 'boom';
const sequelizeKeys = ['include', 'order', 'limit', 'offset']; const sequelizeKeys = ['include', 'order', 'limit', 'offset'];
const getModels = (request) => {
const noGetDb = typeof request.getDb !== 'function';
const noRequestModels = !request.models;
if (noGetDb && noRequestModels) {
return notImplemented('`request.getDb` or `request.models` are not defined.'
+ 'Be sure to load hapi-sequelize before hapi-sequelize-crud.');
}
const { models } = noGetDb ? request : request.getDb();
return models;
};
export const parseInclude = request => { export const parseInclude = request => {
const include = Array.isArray(request.query.include) const include = Array.isArray(request.query.include)
? request.query.include ? request.query.include
: [request.query.include] : [request.query.include]
; ;
const noGetDb = typeof request.getDb !== 'function'; const models = getModels(request);
const noRequestModels = !request.models; if (models.isBoom) return models;
if (noGetDb && noRequestModels) {
return notImplemented('`request.getDb` or `request.models` are not defined.'
+ 'Be sure to load hapi-sequelize before hapi-sequelize-crud.');
}
const { models } = noGetDb ? request : request.getDb();
return include.map(a => { return include.map(a => {
const singluarOrPluralMatch = Object.keys(models).find((modelName) => { const singluarOrPluralMatch = Object.keys(models).find((modelName) => {
@ -63,24 +69,40 @@ export const parseLimitAndOffset = (request) => {
return out; return out;
}; };
const parseOrderArray = (order, models) => {
return order.map((requestColumn) => {
if (Array.isArray(requestColumn)) {
return parseOrderArray(requestColumn, models);
}
let column;
try {
column = JSON.parse(requestColumn);
} catch (e) {
column = requestColumn;
}
if (column.model) column.model = models[column.model];
return column;
});
};
export const parseOrder = (request) => { export const parseOrder = (request) => {
const { order } = request.query; const { order } = request.query;
if (!order) return null; if (!order) return null;
const models = getModels(request);
if (models.isBoom) return models;
// transform to an array so sequelize will escape the input for us and // 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 // maintain security. See http://docs.sequelizejs.com/en/latest/docs/querying/#ordering
if (isString(order)) return [order.split(' ')]; const requestOrderColumns = isString(order) ? [order.split(' ')] : order;
for (const key of Object.keys(order)) { const parsedOrder = parseOrderArray(requestOrderColumns, models);
try {
order[key] = JSON.parse(order[key]);
} catch (e) {
//
}
}
return [order]; return parsedOrder;
}; };
export const getMethod = (model, association, plural = true, method = 'get') => { export const getMethod = (model, association, plural = true, method = 'get') => {

View File

@ -2,7 +2,8 @@ import test from 'ava';
import { parseLimitAndOffset, parseOrder, parseWhere } from './utils.js'; import { parseLimitAndOffset, parseOrder, parseWhere } from './utils.js';
test.beforeEach((t) => { test.beforeEach((t) => {
t.context.request = { query: {} }; const models = t.context.models = { User: {} };
t.context.request = { query: {}, models };
}); });
test('parseLimitAndOffset is a function', (t) => { test('parseLimitAndOffset is a function', (t) => {
@ -62,14 +63,13 @@ test('parseOrder returns order when a string', (t) => {
}); });
test('parseOrder returns order when json', (t) => { test('parseOrder returns order when json', (t) => {
const { request } = t.context; const { request,models } = t.context;
const order = [{ model: 'User' }, 'DESC'];
request.query.order = [JSON.stringify({ model: 'User' }), 'DESC']; request.query.order = [JSON.stringify({ model: 'User' }), 'DESC'];
request.query.thing = 'hi'; request.query.thing = 'hi';
t.deepEqual( t.deepEqual(
parseOrder(request) parseOrder(request)
, [order] , [{ model: models.User }, 'DESC']
); );
}); });

View File

@ -58,11 +58,13 @@ export default (test) => {
const { Player, Team, City } = t.context.sequelize.models; const { Player, Team, City } = t.context.sequelize.models;
const city1 = await City.create({ name: 'Healdsburg' }); const city1 = await City.create({ name: 'Healdsburg' });
const team1 = await Team.create({ name: 'Baseballs', cityId: city1.id }); const team1 = await Team.create({ name: 'Baseballs', cityId: city1.id });
const team2 = await Team.create({ name: 'Footballs', cityId: city1.id });
const player1 = await Player.create({ const player1 = await Player.create({
name: 'Pinot', teamId: team1.id, active: true, name: 'Cat', teamId: team1.id, active: true,
}); });
const player2 = await Player.create({ name: 'Syrah', teamId: team1.id }); const player2 = await Player.create({ name: 'Pinot', teamId: team1.id });
t.context.instances = { city1, team1, player1, player2 }; const player3 = await Player.create({ name: 'Syrah', teamId: team2.id });
t.context.instances = { city1, team1, team2, player1, player2, player3 };
}); });
// kill the server so that we can exit and don't leak memory // kill the server so that we can exit and don't leak memory