Tapestry 5
  1. Tapestry 5
  2. TAP5-34

When using a Grid inside a Form, sorting the Grid may cause updates (when the form is submitted) to be applied to the wrong objects

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 5.0.15
    • Fix Version/s: 5.0.16
    • Component/s: None
    • Labels:
      None

      Description

      I could be doing something wrong, but it seems as though saving a Grid of objects which is included in a Form after sorting the Grid to a non-default sort order results in the rows getting saved under an incorrect ID.

      This should be reproducible in any form which includes an editable Grid, but here are the basic steps to reproduce what I'm seeing:
      1. Create an Item class which includes an ID column and a Name column (I have mine set up as a Hibernate object)
      2. Create a Page with a Grid of Items inside a Form including a Save button which causes the Page to persist the Items
      3. Create a couple Items (ID, name):
      (1, 'item one')
      (2, 'item two')
      4. Bring up the Page, Sort by ID (or Name) so they look like this:
      (2, 'item two')
      (1, 'item one')
      5. Click Save and if I'm not mistaken you will see:
      (2, 'item one') <-- Note that Item 2 is now associated with the name of Item 1...
      (1, 'item two')

      Please let me know if there is any difficulty in reproducing this issue and I will try to put together a minimal code example.

        Issue Links

          Activity

          Hide
          Howard M. Lewis Ship added a comment -

          At issue here is that the Grid stores row indexes into the form, not the primary key of the row. It might be nice to change that. Like Loop, Grid should be use a PrimaryKeyEncoder (optionally). However, this may also be related to TAPESTRY-2636, meaning that the Grid will work correctly as long as the underlying data has not shifted (had rows added or removed).

          Show
          Howard M. Lewis Ship added a comment - At issue here is that the Grid stores row indexes into the form, not the primary key of the row. It might be nice to change that. Like Loop, Grid should be use a PrimaryKeyEncoder (optionally). However, this may also be related to TAPESTRY-2636 , meaning that the Grid will work correctly as long as the underlying data has not shifted (had rows added or removed).
          Hide
          Howard M. Lewis Ship added a comment -

          I found the primary cause of this; CachingDataSource (inside Grid) was accessing the underlying source parameter directly; during a form submission, this value isn't cached, each access results in a fresh access of the underlying data source, which often means that the sort constraints (via prepare()) were lost, and the elements in the data source accessed and updated in a arbitrary order.

          I'm fixing that (CachingDataSource will be a static inner class and hold a direct reference to the underlying data source), but I'm also adding PrimaryKeyEncoder support to Grid and GridRows, as with Loop.

          Show
          Howard M. Lewis Ship added a comment - I found the primary cause of this; CachingDataSource (inside Grid) was accessing the underlying source parameter directly; during a form submission, this value isn't cached, each access results in a fresh access of the underlying data source, which often means that the sort constraints (via prepare()) were lost, and the elements in the data source accessed and updated in a arbitrary order. I'm fixing that (CachingDataSource will be a static inner class and hold a direct reference to the underlying data source), but I'm also adding PrimaryKeyEncoder support to Grid and GridRows, as with Loop.

            People

            • Assignee:
              Howard M. Lewis Ship
              Reporter:
              Skow
            • Votes:
              2 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development