Ticket #6567 (closed defect: fixed)

Opened 15 months ago

Last modified 12 months ago

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

Reported by: guest Owned by: jaredj
Priority: normal Milestone: 1.2
Component: DojoX Grid Version: 1.1.0
Severity: normal Keywords: QueryReadStore Grid DojoData
Cc: awhite@…, jared.jurkiewicz@…

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

index.html (2.0 kB) - added by guest 15 months ago.
data.php (256 bytes) - added by guest 15 months ago.
6567.patch (3.5 kB) - added by benschell 15 months ago.
6567-2.patch (3.6 kB) - added by benschell 15 months ago.
6567-3.patch (3.6 kB) - added by benschell 12 months ago.

Change History

Changed 15 months ago by guest

Changed 15 months ago by guest

Changed 15 months ago by jaredj

  • owner changed from jaredj 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?

Changed 15 months ago by jaredj

  • component changed from DojoX Data to DojoX Grid

Changed 15 months ago by benschell

  • status changed from new to assigned

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 15 months ago by benschell

Changed 15 months 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.

Changed 15 months ago by benschell

Small typo. New patch uploaded.

Changed 15 months ago by benschell

Changed 15 months ago by benschell

  • cc jared.jurkiewicz@… added

Changed 15 months ago by guest

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

Changed 15 months ago by benschell

  • summary changed from QueryReadStore sends two requests when sorting on grid columns to [patch][cla] QueryReadStore sends two requests when sorting on grid columns

Changed 12 months ago by remi

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

Changed 12 months ago by benschell

  • status changed from assigned to new
  • owner deleted

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.

Changed 12 months 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 12 months ago by benschell

Changed 12 months ago by benschell

  • owner set to BryanForbes

Changed 12 months ago by peller

  • owner changed from BryanForbes to jaredj

Changed 12 months ago by peller

  • summary changed from [patch][cla] QueryReadStore sends two requests when sorting on grid columns to [patch][cla] (Compat grid) QueryReadStore sends two requests when sorting on grid columns

Changed 12 months ago by jaredj

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

Tested on:

IE 6

Firefox 2.0

Safari 3.1

Changed 12 months ago by jaredj

  • status changed from new to closed
  • resolution set to fixed

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

Note: See TracTickets for help on using tickets.