Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #502 (closed enhancement: fixed)

Opened 13 years ago

Last modified 13 years ago

[PATCH] DataGrid migration to new widget style

Reported by: alberto@… Owned by: anonymous
Priority: normal Milestone: 0.9
Component: TG Widgets Version:
Severity: normal Keywords: datagrid new widgets threadsafety
Cc:

Description

Due to the new change introduced in the widgets API, DataGrid turns out to be non-threadsafe. It's create_dict method builds an auxiliary dict (dynfields) as an attribute of the widget for later retrieval of the field values from the template.

I've attached a patch that fixes it, though it changes the signature of getcol so it can receive dynfields as a parameter. I've taken the liberty of renaming: getcol -> getfield, dynfields -> table as I think they are more descriptive names.

P.S: Includes the changes I posted yesterday at #490 which adapts it to the new widget style (using create_dict instead of update_dict)

Attachments

datagrid.patch Download (3.2 KB) - added by anonymous 13 years ago.
datagrid.2.patch Download (5.8 KB) - added by alberto@… 13 years ago.
Update patch to last changes on widgets API
datagrid.3.patch Download (7.3 KB) - added by alberto@… 13 years ago.
How about this? Passing both unittests…
datawidgets.patch Download (861 bytes) - added by anonymous 13 years ago.

Change History

Changed 13 years ago by anonymous

Changed 13 years ago by alberto@…

Update patch to last changes on widgets API

comment:1 Changed 13 years ago by alberto@…

  • Type changed from defect to enhancement

I've updated and tested datagrid to the new widget's API (once again).

I've also added a "confirm deletion" javascript popup, as the widget (and my users) was (were) screaming for it ;)

comment:2 Changed 13 years ago by michele

  • Component changed from CherryPy to Widgets

Great work Albero, just two things:

  • Instead of removing widgets replace it with fields (where you use formmaker) just for the sake of clarity
  • I'm not sure a DataGrid deserves the SimpleWidget? as a base class, the reason I introduced this class is that attributes don't make sense for a so complex widget (you will be able to add them only to the extern tag) and that can lead to unpredicable behaviors (users asking why the can't set attributes for a <tr> for example).

comment:3 Changed 13 years ago by kevin

  • Summary changed from [PATCH] DataGrid migration to new widget style to DataGrid migration to new widget style

As Michele said, this patch looks almost there. The other problem, though, is that the tests in test_fastdata don't pass. I made a couple minor changes to bring test_fastdata in line with your changes, but I wasn't certain about the first test which didn't seem to have a table for get_field_getter. I wouldn't want to inadvertantly eliminate useful behavior from the class.

comment:4 Changed 13 years ago by anonymous

Ok, I'll try to get the patch in line with the new style and make sure test pass somewhen this afternoon... be posting back.

Changed 13 years ago by alberto@…

How about this? Passing both unittests...

comment:5 Changed 13 years ago by alberto@…

New patch attached. I'm now subclassing Widget and not setting any attrs at the <table> tag.

Fixed tests so they pass. grid.getcol becomes dget_field? (table is already set in update_data by the "currier" get_field_getter (better name, anyone?). This hack is so the table (former dynfields) is built at update_data time and not being bound to "self", for thread_safety issues. The currier func. is to avoid passing "table" to the template, i'm just passing the getter which knows what table to "get" from.

Alberto.

comment:6 Changed 13 years ago by anonymous

  • Summary changed from DataGrid migration to new widget style to [PATCH] DataGrid migration to new widget style

comment:7 Changed 13 years ago by kevin

The patch looks good! Thanks! Committed in [637].

comment:8 Changed 13 years ago by kevin

  • Status changed from new to closed
  • Resolution set to fixed

comment:9 Changed 13 years ago by alberto@…

  • Status changed from closed to reopened
  • Resolution fixed deleted

Just a copule of details regarding the rename of fastdata -> newfastdata.

This patch solves a problem as DataGrid was using the old template.

Also solves a typo in NewDataGrid?

Changed 13 years ago by anonymous

comment:10 Changed 13 years ago by kevin

committed in [644]. I'm leaving this open until the template name changes back.

comment:11 Changed 13 years ago by alberto

  • Status changed from reopened to closed
  • Resolution set to fixed

Template name changed back in r686

Note: See TracTickets for help on using tickets.