clarkie / dynogels

DynamoDB data mapper for node.js. Originally forked from https://github.com/ryanfitz/vogels
Other
490 stars 110 forks source link

Add support for custom bindings #147

Closed sp90 closed 5 years ago

sp90 commented 6 years ago

So it has annoyed me for a while that I could only use the AND binding between filters & key conditions so I made this pull request to support custom bindings

So with this update, it's easy to just to change the operator to 'OR'

Example 1:

query
  .filter('age').gt(5)
  .filter('name', 'OR').equals('lars')
  .filter('age').ne(15);

// This results in following behavior
// '(#age > :age) OR (#name = :name) AND (#age <> :age_2)'

Example 2:

query
  .filter('age').gt(5)
  .filter('age', 'OR').lt(20)
  .filter('age').ne(15);

// This results in following behavior
// '(#age > :age) OR (#age < :age_2) AND (#age <> :age_3)'

Please pitch in if you like changes @clarkie 👍

sp90 commented 6 years ago

Added support for handling parentheses with and/or behavior

Example:

query
  .filter('age', null, 'start').gt(5)
  .filter('name', 'OR', 'end').equals('lars')
  .filter('age').ne(15);

// This results in the following behavior
// '((#age > :age) OR (#name = :name)) AND (#age <> :age_2)'
cdhowie commented 6 years ago

There's something about passing start/end around that makes this feel pretty unclean to me. If we're going to support arbitrary nesting, I'd suggest going the route of creating a separate "condition" object that can be nested, and passing that to the query object somehow.

Honestly, as soon as a filter gets fairly complex, I think using .filterExpression() actually increases readability.

sp90 commented 6 years ago

The problem with filterExpression cannot be mixed with other filters.

But I do agree that is not super pretty but it does work I use it in production today with my team its really neat 😀

If you have a better syntax suggestion I'm open to it

sp90 commented 5 years ago

Closing this because it doesn't seem to be a wanted feature 👍