Opened 12 years ago

Closed 11 years ago

#6567 closed defect (fixed)

[patch][cla] (Compat grid) QueryReadStore sends two requests when sorting on grid columns

Reported by: guest Owned by: Jared Jurkiewicz
Priority: high Milestone: 1.2
Component: DojoX Grid Version: 1.1.0
Keywords: QueryReadStore Grid DojoData Cc: awhite@…, jared.jurkiewicz@…
Blocked By: Blocking:

Description

If you hook QueryReadStore? to a grid via DojoData? and click a grid column to sort it makes a double request, each with the same data.

See the attached files for a simple demo.

Attachments (5)

index.html (2.0 KB) - added by guest 12 years ago.
data.php (256 bytes) - added by guest 12 years ago.
6567.patch (3.5 KB) - added by benschell 12 years ago.
6567-2.patch (3.6 KB) - added by benschell 11 years ago.
6567-3.patch (3.6 KB) - added by benschell 11 years ago.

Download all attachments as: .zip

Change History (21)

Changed 12 years ago by guest

Attachment: index.html added

Changed 12 years ago by guest

Attachment: data.php added

comment:1 Changed 12 years ago by Jared Jurkiewicz

Owner: changed from Jared Jurkiewicz to benschell

I believe this is a grid problem, not a store problem. Ben, I know you made changes to grid to try and solve the double-fetch issue. Can you verify those changes didn't get regressed out?

comment:2 Changed 12 years ago by Jared Jurkiewicz

Component: DojoX DataDojoX Grid

comment:3 Changed 12 years ago by benschell

Status: newassigned

This bug isn't specific to QueryReadStore?. In fact, any dojo data store used will have the fetch() function called twice for a sort. The problem lies in the Grid receiving a 'change' event when the row count is set as a result of a normal 'onBegin' call from the store.

My solution proposed in the patch is to add two new listeners to the Grid for a 'BeginUpdate?' and 'EndUpdate?' event from the model. The functionality for delaying updates until all updates are complete is already built into the Grid, so simply notifying the Grid that the model is in flux should be sufficient.

Part of this patch also adjusts the Grid for the note left in the comments, that the beginUpdate and endUpdate functions do not take into account the possibility of multiple beginUpdate calls (aka, nested updating calls). Now, endUpdate checks that it is the final endUpdate call and handles all updates (aka, if/if rather than if/else as previous).

Feel free to tear this one up, it's kindof a big patch for such a small problem, but it should help out for other issues in the future, as it ensures a way for the Model to notify the grid of an in-progress update.

Changed 12 years ago by benschell

Attachment: 6567.patch added

comment:4 Changed 12 years ago by guest

Unfortunately this patch leads to the grid not showing all rows as expected. In a grid with 115 entries, only about 60 of rows are fine, all the other rows just show three dots in each cell. Quick tests have shown, that always the same rows and subrequests are affected. Firebug does not display any errors. Firefox and IE make no difference here. I am using plain dojox.grid and dojox.data.QueryReadStore? (1.1.0). Your patch was applied with all chunks succeeding.

All rows show valid data after undoing the patch.

Will post a copy of the JSON data and dojo markup if you like.

comment:5 Changed 11 years ago by benschell

Small typo. New patch uploaded.

Changed 11 years ago by benschell

Attachment: 6567-2.patch added

comment:6 Changed 11 years ago by benschell

Cc: jared.jurkiewicz@… added

comment:7 Changed 11 years ago by guest

Thank you very much, 6567-2 works as expected. No more cells showing up as dots. Excellent. Cheers, Sascha.

comment:8 Changed 11 years ago by benschell

Summary: QueryReadStore sends two requests when sorting on grid columns[patch][cla] QueryReadStore sends two requests when sorting on grid columns

comment:9 Changed 11 years ago by remi

How is the current status with this Patch? Will we see it in 1.2?

comment:10 Changed 11 years ago by benschell

Owner: benschell deleted
Status: assignednew

I'm not sure if this bug persists or not, but the patch most certainly isn't still applicable given the rewrite in most of the Grid code. I'm not prepared to re-investigate before 1.2, so un-owning this bug.

comment:11 Changed 11 years ago by benschell

After some discussion with Jared J., this bug apparently does not exist in the new version of the Grid. However, it (obviously) persists in the compat version. Adding a new patch (with no changes other than directory structure) which fixes this issue.

Changed 11 years ago by benschell

Attachment: 6567-3.patch added

comment:12 Changed 11 years ago by benschell

Owner: set to Bryan Forbes

comment:13 Changed 11 years ago by Adam Peller

Owner: changed from Bryan Forbes to Jared Jurkiewicz

comment:14 Changed 11 years ago by Adam Peller

Summary: [patch][cla] QueryReadStore sends two requests when sorting on grid columns[patch][cla] (Compat grid) QueryReadStore sends two requests when sorting on grid columns

comment:15 Changed 11 years ago by Jared Jurkiewicz

Applied to compat, used BackwardsCompatibilityTest?, verified double-fetch no longer occurs.

Tested on:

IE 6

Firefox 2.0

Safari 3.1

comment:16 Changed 11 years ago by Jared Jurkiewicz

Resolution: fixed
Status: newclosed

(In [14502]) Fixing double-fetch issue. \!strict fixes #6567

Note: See TracTickets for help on using tickets.