Skip to content

findAll: format attributes filter#65

Open
chentsulin wants to merge 1 commit intobsiddiqui:masterfrom
chentsulin:findall-with-forge
Open

findAll: format attributes filter#65
chentsulin wants to merge 1 commit intobsiddiqui:masterfrom
chentsulin:findall-with-forge

Conversation

@chentsulin
Copy link
Copy Markdown
Contributor

.forge() enables case-converter(https://github.com/bookshelf/bookshelf/blob/master/lib/plugins/case-converter.js), but .where() does not do case convert like forge().

https://github.com/bookshelf/bookshelf/blob/b6abda9486b3f06d1ee58aff063c14836f0a2e1d/lib/model.js#L1317-L1319

(.where just call .where on query builder)

We should let findAll and findOne have the same case converting behavior

@bsiddiqui
Copy link
Copy Markdown
Owner

That makes sense. It'll be a breaking change so I'll need work it into a major relase

@chentsulin
Copy link
Copy Markdown
Contributor Author

chentsulin commented Sep 21, 2018

I found that this is not working:

this.forge(extend({}, filter)).fetchAll(options);

(got select command with no where condition)

But this works:

const model = this.forge();
return model.where(model.format(extend({}, filter))).fetchAll(options)

Just format attributes for where.

@chentsulin chentsulin changed the title findAll: put filter into forge instead of where findAll: format attributes filter Sep 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants