#5555 closed enhancement (fixed)
[patch]Enhance dojox.gid.data.DojoData model to pass sort to the store, as well as fix dependency on identity.
Reported by: | Jared Jurkiewicz | Owned by: | sorvell |
---|---|---|---|
Priority: | high | Milestone: | |
Component: | DojoX Grid | Version: | 1.0 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description
Enhance dojox.gid.data.DojoData? model to pass sort to the store, as well as fix dependency on identity.
When I was trying to use the DojoData? model with grid, I found some problems. Namely, it didn't properly pass the sort data onto the store. Also, it was forcing identity to be implemented, regardless if the store actually supported notification (where it used it). So I created a patch to fix both of those issues. In addition, I added a hook so that people can monitor the model for errors reported by the datastore fetch. Lastly, I added support for initial sort details, as well as initial queryOptions for the store.
With these issues addressed, the dojox.grid.data.DojoData? model became a lot more user-friendly. Now it can even be used to update the view without recreating the model. All a user has to do is change the query object, then call 'refresh' on the model and the Grid automatically updates. The same is true for sort. Click a header, and sorting is deferred to the store and the grid view rebuilt.
It adds a few lines of code to the model, of course, but I feel it's a lot more usable and integrated with dojo.data with these changes. I'll be attaching a patch for review. If you are fine with the changes, I'll check the changes in. I think I'll even merge them into 1.0.3, as without them, the DojoData? integration is somewhat broken.
Attachments (4)
Change History (18)
Changed 13 years ago by
Attachment: | dojox.grid._data.model_DojoData_20080110.patch added |
---|
comment:1 Changed 13 years ago by
Milestone: | 1.1 → 1.0.3 |
---|---|
Summary: | Enhance dojox.gid.data.DojoData model to pass sort to the store, as well as fix dependency on identity. → [patch]Enhance dojox.gid.data.DojoData model to pass sort to the store, as well as fix dependency on identity. |
Changed 13 years ago by
Attachment: | dojox.grid._data.model_20080114.patch added |
---|
Pass two. This includes a fix for a problem denoted by tracker:
Changed 13 years ago by
Attachment: | dojox.grid_EnhancementsAndTests_20080115.patch added |
---|
Pass 3. A few more tests, plus a fix to CsvStore? error handling.
comment:2 Changed 13 years ago by
Changed 13 years ago by
Attachment: | dojo_1.0.3_dojox.grid_EnhancementsUT_20080115.patch added |
---|
comment:3 Changed 13 years ago by
Tested on:
Firefox 2.0.0.11 IE 6 IE 7 Safari B3 Opera 9.2 Seamonkey 1.1.2
Both dojo 1.0.3 and dojo 1.1 were tested.
comment:4 Changed 13 years ago by
comment:5 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:6 Changed 13 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
frankf:
From above:
Now it can even be used to update the view without recreating the model. All a user has to do is change the query object, then call 'refresh' on the model and the Grid automatically updates. The same is true for sort. Click a header, and sorting is deferred to the store and the grid view rebuilt.
Can this be reviewed? One benefit of the ItemFileReadStore? was that the model could be changed independent of the store. The above change forces the model (and any changes to it) to be destroyed and rebuilt from the store upon a sort. This makes methods such as setDatum, etc. that work on the model pretty useless, since the new values will be destroyed upon a sort (and maybe other actions I've not yet encountered). If the intent is to wipe out model data and make it match the store, should you just go ahead and remove "read" stores and only offer "write" stores? And change the model "change/set" methods to be store methods? Puzzled as to why these read capabilities were eliminated or compromised. I'm probably missing something here.
comment:7 Changed 13 years ago by
Sort is supposed to be handled by the store. The fatc that the DojoData? model didn't support it was a problem with the implementation. I also don't see how setDatum isn't useful. setDatum is supposed to write values back to the store, and in fact does. It called 'setValue'. So, how does a sort destroy this? If a value has been written back to the store, a sort doesn't affect it.
If your data store isn't writable, I'm not sure why allowing a write to data that was read-only according to the store makes any sense whatsoever.
comment:8 Changed 13 years ago by
If you don't go to the store for sorting and you're paging across the data, having the grid do the sort produces completely incorrect results. Grid only sorts on the current 'page' in the view. Theefore if say you were sorting it alphabetically, when you sorted it would only sort the page in view, not against the entire record set, and move up the data in say, the 10000th row to the top because it happened to be, in terms of a sort, the first entry.
comment:9 Changed 13 years ago by
And incidentally, I asked for a review several times from some of the key people in Dojo and they liked my proposed changes. I don't see the value, honestly, of being able to 'change' read-only data in a grid, if it's wired to pull data from a read-only store. The model shouldn't be changed independent of the store, IMHO, because then you get inconsistent representations of the data. The purpose of a data store was to allow a unified way to access data and have change events (if the store supports Notification).
comment:10 Changed 13 years ago by
The only thing I guess could be made an option is 'storeSort', which should default to true. If set to false, it would allow the grid to do sorting without asking the store and get you back the behavior mentioned earlier. I do consider that behavior to be fundamentally problem with respect to how things are supposed to interact with a data store, though.
comment:11 Changed 13 years ago by
frankf: jaredj, thanks for your replies. It helps me understand the thinking behind the changes. I had used setDatum for months without it writing to the store (in fact, it is grid.model.setDatum, which I believe checked to see if it was a read store or not, can_write, before attempting to write to the store). This worked great. Once the changes were made to have sort work from the store instead of from the model, the grid was not sorted correctly, since setDatum did not update the store...
But, I can see that this is a done deal. I don't mind finding a workaround.
I really like dojo, so don't take my disagreement with this grid store/model approach wrong.
Thanks, frankf.
comment:12 Changed 13 years ago by
Ok, after switching back to ItemFileWriteStore?, working with the changes above (which fixed Ticket 5275), I like it. Good work. Please re-close. Thanks, frankf.
comment:13 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Patch to fix various integration issues with dojo.data.