Add RecordArray#filterBy which contains a live, filtered subset

[pangratz]

This method takes the parameters key and an optional value and
returns an updating list of records of the RecordArray, which'
property value of key matches value.

---

This addresses #3171

[pangratz]

I'm very unsure about this implementation, since record on the InternalModel should be accessed via getRecord() I guess...

[igorT]

Ember.computed.filterBy('internalRecords') should be filterBy content

[pangratz]

I tried that, but then store.all('person').filterBy('name', xxx) is always an empty array. So I guess observing InternalModel's and properties on them doesn't work as expected yet ...

[pangratz]

@igorT I am not sure about naming the method filterBy, since the RecordArray#filterBy method (inherited by ArrayProxy) is hereby overwritten with a different implementation: an auto-updating filtered subset. Maybe it should be renamed into something like updatingFilterBy, liveFilterBy or so ...

[cibernox]

IMO the name it's ok as it is.
Seems reasoable to me that the filterBy method of a live collection returns another live collection.

[fivetanley]

Can we move the ArrayProxy class definition out of the function? It doesn't need to be in its own file (might be nice), but it seems like it could get expensive to create a new class every time a filter is created.

[pangratz]

@fivetanley done

[bmac]

Looks good. Should this also be implemented on a ManyArray?

[bmac]

Thanks @pangratz

[bendemboski]

This change seems problematic to me.

First of all, it's a breaking change. Previously a call to filterBy on a RecordArray would call ArrayProxy's filterBy method which returns a native array. So, the following code would behave as expected and would call doSomething:

let recordArray = getRecordArraySomehow(),
    newRecords = recordArray.filterBy('isNew');
if (newRecords.length > 0) {
    doSomething();
}

After this update, the same code will not run doSomething because newRecords is no longer a native array, so length is a computed property and newRecords.length is not an integer, so newRecords.length > 0 will always be false.

Now before updating to Ember Data 1.13.8 I have to audit all of my usages of RecordArrays to make sure I don't treat the return value like a native array anywhere :(

Secondly, it seems wrong to override filterBy but not override filter. This means that recordArray.filterBy('isNew') will return an ArrayProxy but recordArray.filter(function (record) { return record.get('isNew'); }) will return a native array. Same inconsistency applies to reject and to rejectBy.

Perhaps the decision to name this method filterBy could be re-visited? Or some kind of warning could be added to the release notes (I know it's already released...)?

[bmac]

Hi @bendemboski. Thanks for bringing up this issue. We talked about this issue in a recent team meeting and it sounds like this is a regression that we want to fix. The current fixing is we will back out the filterBy changes and replace it with a new name or a more limited API.

[cibernox]

I've also faced the same.
I was chaining this.get('manyRelation').filterBy('isNew', false).sort(compareFn) and that broke too, since there is no sort function in this new kind of liveArray.

I'm explicitly converting the relation to an array before filter/sort this.get('manyRelation').toArray().filt....