Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Escape comma #25

Merged
merged 12 commits into from
May 15, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ If you're returning a single resource from a call such as `GET /customers/1`, ma

`options` | Description
:------------- | :-------------
filter _object_ | Filters a result set based specific field. Example: `/pets?filter[name]=max` would only return pets named max. Keywords can be added to filters to give more control over the results. Example: `/pets?filterType[pet][like]=ax` would only return pets that have "ax" in their name. The supported types are "like", "not", "lt", "lte", "gt", and "gte". Both "like" and "not" support multiple values by comma separation. NOTE: This is not supported by JSON API spec.
filter _object_ | Filters a result set based specific field. Example: `/pets?filter[name]=max` would only return pets named max. Keywords can be added to filters to give more control over the results. Example: `/pets?filterType[like][pet]=ax` would only return pets that have "ax" in their name. The supported types are "like", "not", "lt", "lte", "gt", and "gte". Both "like" and "not" support multiple values by comma separation. Also, if your data has a string with a comma, you can filter for that comma by escaping the character with two backslashes. NOTE: This is not supported by JSON API spec.
fields _object_ | Limits the fields returned as part of the record. Example: `/pets?fields[pets]=name` would return pet records with only the name field rather than every field.
include _array_ | Returns relationships as part of the payload. Example: `/pets?include=owner` would return the pet record in addition to the full record of its owner. _Note:_ you may override an `include` parameter with your own Knex function rather than just a string representing the relationship name.
page _object/false_ | Paginates the result set. Example: `/pets?page[limit]=25&page[offset]=0` would return the first 25 records. If you've passed default pagination parameters to the plugin, but would like to disable paging on a specific call, just set `page` to `false`.
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
"dependencies": {
"bookshelf-page": "0.3.2",
"inflection": "^1.10.0",
"lodash": "4.13.1"
"lodash": "4.13.1",
"split-string": "^0.1.1"
}
}
30 changes: 25 additions & 5 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,15 @@ import {
isArray as _isArray,
isObject as _isObject,
isObjectLike as _isObjectLike,
isNull as _isNull,
forIn as _forIn,
keys as _keys,
map as _map,
zipObject as _zipObject
} from 'lodash';

import split from 'split-string';

import inflection from 'inflection';

import Paginator from 'bookshelf-page';
Expand Down Expand Up @@ -252,7 +255,10 @@ export default (Bookshelf, options = {}) => {
fieldNames[fieldKey] = _map(fieldNames[fieldKey], (value) => {

if (!fieldKey){
return value;
if (_includes(value, '.')){
return value;
}
return `${internals.modelName}.${value}`;
}
return `${fieldKey}.${value}`;
});
Expand Down Expand Up @@ -318,7 +324,13 @@ export default (Bookshelf, options = {}) => {
typeKey = internals.formatRelation(internals.formatColumnNames([typeKey])[0]);

// Determine if there are multiple filters to be applied
const valueArray = typeValue.toString().indexOf(',') !== -1 ? typeValue.split(',') : typeValue;
let valueArray = null;
if (!_isArray(typeValue)){
valueArray = split(typeValue.toString(), ',');
}
else {
valueArray = typeValue;
}

// Attach different query for each type
if (key === 'like'){
Expand Down Expand Up @@ -382,9 +394,17 @@ export default (Bookshelf, options = {}) => {
// Remove all but the last table name, need to get number of dots
key = internals.formatRelation(internals.formatColumnNames([key])[0]);

// Determine if there are multiple filters to be applied
value = value.toString().indexOf(',') !== -1 ? value.split(',') : value;
qb.whereIn(key, value);

if (_isNull(value)){
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the fix for filtering on null

qb.where(key, value);
}
else {
// Determine if there are multiple filters to be applied
if (!_isArray(value)){
value = split(value.toString(), ',');
}
qb.whereIn(key, value);
}
}
}
});
Expand Down
96 changes: 82 additions & 14 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,20 @@ describe('bookshelf-jsonapi-params', () => {
gender: 'm',
type: 'monster'
}),
PersonModel.forge().save({
id: 4,
firstName: 'Boo',
age: 28,
gender: 'f',
type: 'nothing, here'
}),
PersonModel.forge().save({
id: 5,
firstName: 'Elmo',
age: 3,
gender: 'm',
type: null
}),
PetModel.forge().save({
id: 1,
name: 'Big Bird',
Expand Down Expand Up @@ -168,7 +182,7 @@ describe('bookshelf-jsonapi-params', () => {
.fetchJsonApi()
.then((result) => {

expect(result.models).to.have.length(3);
expect(result.models).to.have.length(5);
done();
});
});
Expand Down Expand Up @@ -211,6 +225,23 @@ describe('bookshelf-jsonapi-params', () => {
done();
});
});

it('should return a single record with the matching type as null', (done) => {

PersonModel
.forge()
.fetchJsonApi({
filter: {
type: null
}
})
.then((result) => {

expect(result.models).to.have.length(1);
expect(result.models[0].get('firstName')).to.equal('Elmo');
done();
});
});
});

describe('passing a `filters` parameter with multiple filters', () => {
Expand Down Expand Up @@ -293,9 +324,11 @@ describe('bookshelf-jsonapi-params', () => {
})
.then((result) => {

expect(result.models).to.have.length(2);
expect(result.models).to.have.length(4);
expect(result.models[0].get('firstName')).to.equal('Baby Bop');
expect(result.models[1].get('firstName')).to.equal('Cookie Monster');
expect(result.models[2].get('firstName')).to.equal('Boo');
expect(result.models[3].get('firstName')).to.equal('Elmo');
done();
});
});
Expand All @@ -310,7 +343,7 @@ describe('bookshelf-jsonapi-params', () => {
.fetchJsonApi({
filter: {
not: {
first_name: 'Barney,Baby Bop'
first_name: 'Barney,Baby Bop,Boo,Elmo'
}
}
})
Expand Down Expand Up @@ -338,8 +371,9 @@ describe('bookshelf-jsonapi-params', () => {
})
.then((result) => {

expect(result.models).to.have.length(1);
expect(result.models).to.have.length(2);
expect(result.models[0].get('firstName')).to.equal('Barney');
expect(result.models[1].get('firstName')).to.equal('Elmo');
done();
});
});
Expand All @@ -360,9 +394,10 @@ describe('bookshelf-jsonapi-params', () => {
})
.then((result) => {

expect(result.models).to.have.length(2);
expect(result.models).to.have.length(3);
expect(result.models[0].get('firstName')).to.equal('Barney');
expect(result.models[1].get('firstName')).to.equal('Baby Bop');
expect(result.models[2].get('firstName')).to.equal('Elmo');
done();
});
});
Expand All @@ -383,8 +418,9 @@ describe('bookshelf-jsonapi-params', () => {
})
.then((result) => {

expect(result.models).to.have.length(1);
expect(result.models).to.have.length(2);
expect(result.models[0].get('firstName')).to.equal('Cookie Monster');
expect(result.models[1].get('firstName')).to.equal('Boo');
done();
});
});
Expand All @@ -405,9 +441,10 @@ describe('bookshelf-jsonapi-params', () => {
})
.then((result) => {

expect(result.models).to.have.length(2);
expect(result.models).to.have.length(3);
expect(result.models[0].get('firstName')).to.equal('Baby Bop');
expect(result.models[1].get('firstName')).to.equal('Cookie Monster');
expect(result.models[2].get('firstName')).to.equal('Boo');
done();
});
});
Expand Down Expand Up @@ -470,8 +507,9 @@ describe('bookshelf-jsonapi-params', () => {
})
.then((result) => {

expect(result.models).to.have.length(3);
expect(result.models[0].get('type')).to.equal('monster');
expect(result.models).to.have.length(5);
expect(result.models[0].get('type')).to.equal(null);
expect(result.models[1].get('type')).to.equal('monster');
done();
});
});
Expand All @@ -485,7 +523,7 @@ describe('bookshelf-jsonapi-params', () => {
})
.then((result) => {

expect(result.models).to.have.length(3);
expect(result.models).to.have.length(5);
expect(result.models[0].get('type')).to.equal('triceratops');
done();
});
Expand All @@ -500,7 +538,7 @@ describe('bookshelf-jsonapi-params', () => {
})
.then((result) => {

expect(result.models).to.have.length(3);
expect(result.models).to.have.length(5);
expect(result.models[0].get('firstName')).to.equal('Baby Bop');
done();
});
Expand All @@ -515,8 +553,8 @@ describe('bookshelf-jsonapi-params', () => {
})
.then((result) => {

expect(result.models).to.have.length(3);
expect(result.models[0].get('firstName')).to.equal('Cookie Monster');
expect(result.models).to.have.length(5);
expect(result.models[0].get('firstName')).to.equal('Elmo');
done();
});
});
Expand Down Expand Up @@ -565,6 +603,36 @@ describe('bookshelf-jsonapi-params', () => {
});
});

describe('escape commas in filter', () => {
it('should escape the comma and find a result', (done) => {
PersonModel
.fetchJsonApi({
filter: {
type: 'nothing\\, here'
}
}, false)
.then((result) => {

expect(result).to.be.an('object');
expect(result.get('firstName')).to.equal('Boo');
done();
});
});
it('should find no results if comma is not escaped', (done) => {
PersonModel
.fetchJsonApi({
filter: {
type: 'nothing, here'
}
}, false)
.then((result) => {

expect(result).to.equal(null);
done();
});
});
});

describe('passing default paging parameters to the plugin', () => {

before((done) => {
Expand All @@ -584,7 +652,7 @@ describe('bookshelf-jsonapi-params', () => {

expect(result.models).to.have.length(1);
expect(result.models[0].get('id')).to.equal(1);
expect(result.pagination.pageCount).to.equal(3);
expect(result.pagination.pageCount).to.equal(5);
done();
});
});
Expand Down