setting a DbSingleSelectField to None

37 views
Skip to first unread message

Wayne

unread,
Sep 17, 2012, 11:58:43 PM9/17/12
to toscawidge...@googlegroups.com
I'm using tw2 with TurboGears 2.  When I create a DbFormPage with a DbSingleSelectField, and if the select field has a value associated with it, I cannot change the value to None in the database.  This happens because in the from_dict method of tw2.sqla.utils None values are ignored when setting attributes on an entity.

The same thing happens for e.g. a text field: once the field is set to a value, you can't set it to the empty string from a DbFormPage.

[Example: Department table with Employees, DbFormPage for an employee, DbSingleSelectField is uses a sqlalchemy relation to display dept. name by the dept id.  You won't be able to set an employee's dept to null in the db.  You also can't set the employee's name to the empty string.]

There's a possible simple fix, which didn't seem to break any (non-skipped) tests.  Since the comment below doesn't explain why None and empty string are ignored, the purpose of the conditional is unclear to me.  Is the change below likely to cause problems?  If so, what is a better approach to fix the problem?

--- a/tw2/sqla/utils.py
+++ b/tw2/sqla/utils.py
@@ -34,12 +34,7 @@ def from_dict(obj, data, protect_prm_tamp=True):
                 protect_prm_tamp=protect_prm_tamp
             )
         elif key not in pk_props:
-            if value or type(value) in (bool, int):
-                # Ignore None and '', but we do want to explicitly
-                # set 'False' or '0' if its a boolean/integer.
-                setattr(obj, key, value)
-            else:
-                pass
+            setattr(obj, key, value)
     return obj

Wayne

Paul Johnston

unread,
Sep 18, 2012, 5:05:10 AM9/18/12
to toscawidge...@googlegroups.com
Hi,

I noticed the same issue recently, and I'm not quite sure when this bug was introduced. Your fix looks good to me and I will shortly apply it to my branch. I think we can pretty rapidly apply this to mainline too. A unit test would be good, so the bug doesn't get reintroduced in future.

Paul



Wayne

--
You received this message because you are subscribed to the Google Groups "ToscaWidgets-discuss" group.
To view this discussion on the web visit https://groups.google.com/d/msg/toscawidgets-discuss/-/HZ7RdrnPP9UJ.
To post to this group, send email to toscawidge...@googlegroups.com.
To unsubscribe from this group, send email to toscawidgets-dis...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/toscawidgets-discuss?hl=en.

Ralph Bean

unread,
Sep 18, 2012, 9:42:49 AM9/18/12
to toscawidge...@googlegroups.com
Thanks, Wayne. Just for the record, that original change was made
here: http://bit.ly/SYpxc6 Based on the other commits around it, it
looks like I was working on tw2.captcha at the time.

I just introduced your patch to the develop branch of the main tw2
repo here http://bit.ly/OAJLRU with an included test.

-Ralph
Reply all
Reply to author
Forward
0 new messages