SitePen/dgrid — pull request #638 by kilink (2013-06-21 16:11:56+00:00)
Title: Fix OnDemandList issue with DataStore adapter

As noted in issue #440, there is a simple fix to support legacy stores that use the DataStore adapter.

The bug is in the OnDemandList's renderQuery method, on line 147, where it mixes in a "query" attribute into the query options:

```javascript
renderQuery: function(query, preloadNode, options){
/* ... */
// Establish query options, mixing in our own.
// (The getter returns a delegated object, so simply using mixin is safe.)
options = lang.mixin(this.get("queryOptions"), options,
{start: 0, count: this.minRowsPerPage, query: query});
```

Note that query in this context is a parameter to renderQuery, which is invoked in this refresh method like this:

```javascript

results = self.renderQuery(function(queryOptions){
return self.store.query(self.query, queryOptions);
});

```

So we end up with a reference to this closure that actually calls the store's query method with the correct query. Why would we ever need to pass this to a store in queryOptions? I suspect it is a mistake. Anyway, this ultimately interferes with the DataStore adapter, which mixes in the queryOptions object into the query object. The legacy stores expect the query object to look like {query: {name: "foo"}}.

I've looked at the new dojo/store Stores, and none of them expects a query function to be present in the options object (why would they?). Such a parameter is not listed in the dojo store API documentation, nor references in the SimpleQueryEngine implementation. Frankly, I don't see how it would be useful to pass such a function to any store, so I must conclude that it is a bug.

It would be great if we could make this trivial change in order to support legacy stores wrapped in the DataStore adapter.