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 #848 (closed defect: fixed)

Opened 12 years ago

Last modified 10 years ago

[PATCH] DateTimeConvertor fails to throw empty exception

Reported by: andy@… Owned by: anonymous
Priority: normal Milestone: 1.0b1
Component: TurboGears Version: 0.9a5
Severity: normal Keywords:
Cc:

Description

import turbogears.validators as v

v = v.DateTimeConverter(allow_empty=False)
print v.to_python("")

This should throw an exception, but prints "None".

I traced this a little and it looks like it's calling FancyValidator?.to_python() instead of DateTimeConverter?._to_python(). Of course FV has no knowledge of allow_empty and does the wrong thing. I don't understand the _to vs. to so thats as far as I got.

Attachments

valid.diff Download (2.5 KB) - added by andy@… 12 years ago.
untested patch that removes usage of allow_empty and calls super for DateTimeConverter?(thus allowing not_empty to work)
valid2.diff Download (4.2 KB) - added by andy@… 12 years ago.
updated patch

Change History

comment:1 Changed 12 years ago by michele

I don't even understand why DateTimeConverter? is using allow_empty when the *right* way of doing this is using not_empty that now is working right in FE.

All validators should use not_empty and not allow_empty.

not_empty works automatically no need to take care of it, this means that we can safely remove any use of allow_empty...

comment:2 Changed 12 years ago by andy@…

Looks like all the inits in validator.py need to call base class init.

comment:3 Changed 12 years ago by andy@…

I believe bug #713 is also this issue.

I'm attaching a patch that calls super, and removes usage of allow_empty. Completely untested, but hopefully this will give someone with more knowledge (and checkin privs) a jumpstart in fixing this.

Changed 12 years ago by andy@…

untested patch that removes usage of allow_empty and calls super for DateTimeConverter?(thus allowing not_empty to work)

comment:4 Changed 12 years ago by alberto

Haven't looked at the patch so what I say might not apply... You're not removing the "allow_empty" possibility, aren't you? There are a couple of use cases for this... like when you'd like "no due date" to be stored as a NULL in the db backend.

Alberto

comment:5 Changed 12 years ago by michele

  • Summary changed from DateTimeConvertor fails to throw empty exception to [PATCH] DateTimeConvertor fails to throw empty exception

Hi Alberto,

I can't see the need to duplicate the use of not_empty by using a orthogonal attribute (allow_empty) that in the end does the exact same thing if you look at the code (it even raises the empty message if needed), if you need to store NULL you simply keep not_empty=False as by default.

Or am I missing something here?

comment:6 Changed 12 years ago by alberto

mmm, you're probably right. If not_empty=False and if_empty=None works then I guess I'm happy. Unfortunately I haven't got the time right now to look deeper into this. :( Alberto

Changed 12 years ago by andy@…

updated patch

comment:7 Changed 12 years ago by andy@…

Uploaded a new patch (valid2.diff) that is working right for me. it removes allow_empty in validators.py as well as widgets/big_widgets.py in favor of using FancyValidator?'s not_empty. This requires some logic reversals in CalendarDatePicker?.

In addition it removes the _to_python function in DateTimeConverter? altogether. I am not sure why but until I removed it, if I had the CalendarDatePicker? in a form and something else failed validation, then I got an error, something about strftime not existing for a unicode object.

comment:8 Changed 11 years ago by kevin

  • Status changed from new to closed
  • Resolution set to fixed
  • Milestone set to 1.0b1

committed in [1568] and [1569] with deprecationwarnings. I also did not remove the from_python... I just made it more tolerant of strings heading out from Python.

Note: See TracTickets for help on using tickets.