Filter path fix #6009
Filter path fix #6009
Conversation
ghost
assigned 
| @@ -60,6 +60,16 @@ describe('Filter: filter', function() { | |||
| expect(filter(items, {first:'misko', last:'hevery'})[0]).toEqual(items[0]); | |||
| }); | |||
|
|
|||
|
|
|||
| iit('should support predicat object with dots in the name', function() { | |||
There was a problem hiding this comment.
iit tests should not be checked in IMO
|
sorry.. I pushed this as I was getting off the shuttle so it contained the iit and extra commits from previous work. it should be all sorted out now. #sloppyMe |
|
Not a problem =) LGTM, but is this a breaking change? Looks like it impacts people who depend on crawling through nested objects, if people were using it that way. |
|
I realized that it's a breaking change, however this behavior was not intentional, we had no tests for it and it was never documented. for these reasons I think it's fine to make it. |
|
Is there any alternative? Can filterFilter support nested objects instead? |
|
@benol the filterFilter no longer uses the expression parser, but what you could do is write your own comparator function, or alternatively just put together a new filter from scratch. |
|
Can't this do both? Nested objects seem like a more frequent use case than dotted property names, and the principle of least surprise would argue for at least trying both of them. |
|
We'll have to discuss how that would work, it would be a problem to have an API which is sometimes an expression and sometimes a string. Consistency is important here. |
|
The filterFilter already takes strings, objects, or expressions. I agree On Mon, Feb 3, 2014 at 10:28 AM, Caitlin Potter notifications@github.comwrote:
|
|
Just to clarify, I meant that filterFilter could support something like this: instead of the old |
|
This commit broke our filter in one place in our app. Even if it was not tested, I feel that sufficiently enough people could depend on that it should be placed in the Breaking Changes section, it took me a while of selectively applying patches between 1.2.10 & 1.2.12 to figure out what's happening. I did look into the Breaking Changes section before manually testing it, if the info was there, it would save me a lot of time. |
I like this proposal though I imagine it would increase implementation complexity & might slow parsing down. Is that why it doesn't work that way? |
|
maybe some kind of config on filterProvider to decide what kind of behavior one want filters to adopt? |
I don't think it's a good idea to add up configuration for such cases; it's practically global state and if some modules start depending on one value and others on the other one, you're effectively not able to use both. |
|
then @benol's suggestion is the best that could be done in this case to regain nesting capabilities. Or maybe could be treated in different way than with quotes it's a name, without - nested property (or some other notation dedicated to name-with-dots case) |
|
In looking at @kueblboe's issue reproduction, it seems that deeply nested expression objects don't actually work, only the top level object is used. This should be easy to fix, but it's probably a bit of a user pain |
|
Hey @caitp . Not sure what you mean with "a bit of a user pain". Are you going to support nested expression objects again or do I have to fix my application that uses it? |
|
@kueblboe what I mean is, the suggested This is not intended, so yeah we need to fix that, this fix will make it into the library for Valentines Day, I'm sure =) |
|
OK, I'll wait for Valentines Day then. Thanks, @caitp for your quick replies. |
Due to 339a165, it became impossible to filter nested properties of an object using the filterFilter. A proposed solution to this was to enable the use of nested predicate objects. This change enables the use of these nested predicate objects. Example: ```html <div ng-repeat="it in items | filter:{ address: { country: 'Canuckistan'}}"></div> ``` Or ```js $filter('filter')(items, { address: { country: 'Canuckistan' } }); ``` Closes angular#6215 Related to angular#6009
|
Merged~ It is possible/likely that there are still some situations where it matches 100% of the time but shouldn't, so please file a bug if you run into a related problem. |
Due to 339a165, it became impossible to filter nested properties of an object using the filterFilter. A proposed solution to this was to enable the use of nested predicate objects. This change enables the use of these nested predicate objects. Example: ```html <div ng-repeat="it in items | filter:{ address: { country: 'Canuckistan'}}"></div> ``` Or ```js $filter('filter')(items, { address: { country: 'Canuckistan' } }); ``` Closes angular#6215 Related to angular#6009
|
For us it was a breaking change as well. But how would you identify an undocumented feature as broken... thanks anyway! #6215 fixes this in the same second. |
Closes #6005