Fix for Equals Expression and tolower OData function #11
Conversation
|
Thanks for the PR @marekkajda! Sorry for the delay. It seems like this would best be served with two PRs since the toLower seems much safer a change than the equality change though they both look good. What do you mean "recent changes implemented by another extensions broke it"? Sorry for being pedantic but I was only recently added to this project and am not sure of the scope. Overall these changes look good however so I hope to move this along this week. |
|
I added some hopefully simple requests for updates if you wish to merge these. Thanks again! |
| // $toLower function is implemented only for agregation in MongoDb | ||
| // using object with $eq instead of just value assigned to identifier we resolve an issue with OData: not (documentStatus eq 'OPEN') | ||
| context.query[context.identifier] = { $eq: context.literal }; | ||
| break; |
| @@ -288,6 +289,12 @@ export class Visitor{ | |||
| case "startswith": | |||
| context.query[context.identifier] = new RegExp("^" + context.literal, "gi"); | |||
| break; | |||
| case "tolower": | |||
There was a problem hiding this comment.
Do you have any plans to add a test case for this?
| @@ -288,6 +289,12 @@ export class Visitor{ | |||
| case "startswith": | |||
| context.query[context.identifier] = new RegExp("^" + context.literal, "gi"); | |||
| break; | |||
| case "tolower": | |||
| // mongoDb is case insensitive as default, so it should resolve 99% cases I hope - better than exception I think? | |||
There was a problem hiding this comment.
Remove "I hope - better than exception I think?" I think we agree 😃
| @@ -220,7 +220,8 @@ export class Visitor{ | |||
| this.Visit(node.value.left, context); | |||
| this.Visit(node.value.right, context); | |||
|
|
|||
| if (context.identifier) context.query[context.identifier] = context.literal; | |||
| // using object with $eq instead of just value assigned to identifier we resolve an issue with OData: not (documentStatus eq 'OPEN') | |||
There was a problem hiding this comment.
Adding the not (...) test case ensures we do not regress back and break your feature.
Hi,
We are using you module and it works great, but recent changes implemented by another extensions broke it and we have implemented 2 fixes: