feat(filter): allow filtering of an object #5451
Conversation
|
I'm sorry, but I wasn't able to verify your CLA signature. CLA signature is required for any code contributions to AngularJS. Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match. If you signed the CLA as a corporation, please let me know the company's name. Thanks a bunch! PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR. |
|
Hi Igor, My company just signed. We are Unity Marketing, Inc, dba Cloudspace. |
ghost
assigned
|
@IgorMinar, Could you confirm whether Cloudspace's CLA has been received? |
petebacondarwin
force-pushed
the
master
branch
from
02dc2aa to
fd2d6c0
Compare
jeffbcross
force-pushed
the
master
branch
2 times, most recently
from
cad9560 to
f294244
Compare
jeffbcross
force-pushed
the
master
branch
4 times, most recently
from
e8dc429 to
e83fab9
Compare
Will check on this --- In the mean time, could you rebase this fix? |
Allow ng.filter to filter an object as well as an array. Before, an object would need to be converted into an array before filtering. Closes #2694.
|
@caitp: rebased. |
| it('should filter an object', function() { | ||
| var items = {0: {name: 'a'}, 1: {name: 'abc', done: true}}; | ||
| expect(filter(items, function(i) {return i.done;}).length).toBe(1); | ||
| expect(filter(items, 'a').length).toBe(2); |
There was a problem hiding this comment.
I think you should add a proper test for what the data should look like --- just making assertions about the length isn't really that useful
|
Thanks for rebasing --- I think the tests could use a bit of work --- I'm kinda thinking it might make sense to have a separate path for arrays and array-like objects, compared with non-arraylike objects. It would make the code bigger, but it might give us the ability to return a more meaningful collection when passing in a non-array-like object. I'm still not positive how we feel about including this, @IgorMinar what do you think? |
|
Agreed on the tests. I actually thought about returning an object when one is passed in, but decided against it so the PR would be as frictionless as possible. I think it'd be much more useful if we did return an object, though. |
|
lets make the tests expect what the actual value will be --- and then we can use that to help decide whether we want it or not. I think we will want to return an array actually, but we might want to change the way the array is created |
| @@ -19,7 +19,7 @@ | |||
| * will be returned. The predicate can be negated by prefixing the string with `!`. | |||
There was a problem hiding this comment.
All occurrences of array should be replaced with collection.
It is also not clear if you are matching against the keys or the values in case collection is an object.
petebacondarwin
force-pushed
the
master
branch
from
4dd5a20 to
998c61c
Compare
|
I've attempted to verify this CLA, but there is no CLA for Cloudspace on file, apparently. If you work with us, we should be able to correct that. |
|
Closing with #2694. |
Allow ng.filter to filter an object as well as an array. Before, an
object would need to be converted into an array before filtering.
Closes #2694.