Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#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)

dojox.grid._data.model_DojoData_20080110.patch (4.7 KB) - added by Jared Jurkiewicz 11 years ago.
Patch to fix various integration issues with dojo.data.
dojox.grid._data.model_20080114.patch (4.9 KB) - added by Jared Jurkiewicz 11 years ago.
Pass two. This includes a fix for a problem denoted by tracker:
dojox.grid_EnhancementsAndTests_20080115.patch (22.3 KB) - added by Jared Jurkiewicz 11 years ago.
Pass 3. A few more tests, plus a fix to CsvStore? error handling.
dojo_1.0.3_dojox.grid_EnhancementsUT_20080115.patch (29.2 KB) - added by Jared Jurkiewicz 11 years ago.
Backport of the same fixes to dojo 1.0.3. Had to include a fix to CsvStore? and XmlStore?.

Download all attachments as: .zip

Change History (18)

Changed 11 years ago by Jared Jurkiewicz

Patch to fix various integration issues with dojo.data.

comment:1 Changed 11 years ago by Adam Peller

Milestone: 1.11.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 11 years ago by Jared Jurkiewicz

Pass two. This includes a fix for a problem denoted by tracker:

Changed 11 years ago by Jared Jurkiewicz

Pass 3. A few more tests, plus a fix to CsvStore? error handling.

comment:2 Changed 11 years ago by Jared Jurkiewicz

(In [12038]) Fixes for the model for dojo.data + more tests. refs #5555 !strict

Changed 11 years ago by Jared Jurkiewicz

Backport of the same fixes to dojo 1.0.3. Had to include a fix to CsvStore? and XmlStore?.

comment:3 Changed 11 years ago by Jared Jurkiewicz

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 11 years ago by Jared Jurkiewicz

(In [12040]) Committing in fixes to grid + data stores refs #5555 !strict

comment:5 Changed 11 years ago by Jared Jurkiewicz

Resolution: fixed
Status: newclosed

comment:6 Changed 11 years ago by guest

Resolution: fixed
Status: closedreopened

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 11 years ago by Jared Jurkiewicz

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 11 years ago by Jared Jurkiewicz

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 11 years ago by Jared Jurkiewicz

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 11 years ago by Jared Jurkiewicz

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 11 years ago by guest

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 11 years ago by guest

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 11 years ago by Jared Jurkiewicz

Resolution: fixed
Status: reopenedclosed

comment:14 Changed 11 years ago by (none)

Milestone: 1.0.3

Milestone 1.0.3 deleted

Note: See TracTickets for help on using tickets.