Adapt_framework: add ability to easily filter the results of findDescendantModels

[moloko]

With adaptModel.findDescendants, if you wanted to do something like get all available, mandatory components for a contentObject, you could do this:

Adapt.findById('co-05').findDescendants('components').where({'_isAvailable': true, '_isOptional': false});

However, following #1616 this function is deprecated in favour of findDescendantModels - but as this function returns an Array rather than a Backbone Collection it's necessary to do the following to achieve the same result:

_.filter(Adapt.findById('co-05').findDescendantModels('components'), function(comp) {
    return comp.get('_isAvailable') === true && comp.get('_isOptional') === false;
});

Which is obviously quite a bit wordier compared to the original.

I think it would therefore be useful to amend findDescendantModels to have an optional 2nd parameter filterBy that would give us similar functionality to the original version.

[moloko]

OK I'm obviously being a bit thick here and completely failing to realise that you could just do:

_.where(Adapt.findById('co-05').findDescendantModels('components'), {'_isAvailable': true, '_isOptional': false});

I blame the code in PLP's completionCalculations.js which uses _.filter even though you could achieve the same result in less code by using _.where...!

However, still might be useful to include this 2nd parameter in the function anyway just to make it even easier? i.e.

Adapt.findById('co-05').findDescendantModels('components', {'_isAvailable': true, '_isOptional': false});

[oliverfoster]

You can't do the _.where because findDescendantModels returns an array of models. You were right first time. Not being stoopid. _.where works on an array of objects.

[moloko]

ah of course it does! phew...

[oliverfoster]

You might find http://underscorejs.org/#isMatch useful for comparing the options.filterBy = { name: value } to the model.toJSON()

if (options && options.filterBy) {
  return _.filter(models, function(model) {
     return _.isMatch(model.toJSON(), options.filterBy);
  });
}

[moloko]

Oh that's interesting, I'll give that a try - definitely shorter than what I have at the moment:

return _.filter(returnedDescendants, function(descendant){
    for (var property in filterBy) {
        var value = filterBy[property];
        if (descendant.get(property) !== value) {
            return false;
        }
    }
    return true;
});

[oliverfoster]

It might also be slower than what you have.

[oliverfoster]

Oh, I keep meaning to say, if you make the second parameter an object with sub attributes, it leaves space for future behavioural extensions (like sortBy).

Adapt.findById('co-10').findDescendantModels('component', { filterBy: { _component: 'hotgraphic' } });

Adapt.findById('co-10').findDescendantModels('component', { 
  filterBy: { 
    _component: 'hotgraphic' 
  } 
});

It can also make it read more clearly despite being a bit odd syntactically.

[moloko]

It might also be slower than what you have.

It does seem to be... not the best test in the world but filtering the 30 components in OOTB Adapt using the _.isMatch method mostly seems to take around 1 - 3ms depending on browser whereas using a for...in loop is always < 0.5ms

I'll try and find some time to test against a larger amount of content tomorrow.

[moloko]

so, in a course with 75 components, using_.isMatch is anything from 1 x to 3 times slower in Firefox and 1x to 4x slower in IE11 - so I'm going to stick with the for..in version as although _.isMatch is a few lines shorter I don't think it's _that_ much more readable (kind of depends on your familiarity with the _.isMatch function I guess...)