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

Opened 13 years ago

Last modified 12 years ago

cherrypy.request_form.errors only unroll top level

Reported by: anonymous Owned by: anonymous
Priority: normal Milestone: 0.9
Component: TurboGears Version:
Severity: normal Keywords:
Cc:

Description

cherrypy.request.form_errors uses error_dict from an Invalid instance, however that only unrolls the first level, and puts "Invalid" instances into the values. That's no good if you are using embedded schemas, so unpack_errors() is a much better idea.

This included one line patch replaces error_dict with unpack_errors(). Highly recommend.

Attachments

controllers.diff Download (513 bytes) - added by anonymous 13 years ago.
controllers.2.diff Download (685 bytes) - added by downtown1@… 13 years ago.
patch on controllers.py to unpack_errors recursively instead of using error_dict
controllers.2.2.diff Download (693 bytes) - added by downtown1@… 13 years ago.
had a slight error in controllers.2.diff, tested this patch on my box though!

Change History

Changed 13 years ago by anonymous

comment:1 Changed 13 years ago by kevin

  • Summary changed from [PATCH] cherrypy.request_form.errors only unroll top level to cherrypy.request_form.errors only unroll top level

There was something else that was choking when I used unpack_errors(). I wouldn't do this change without tests being run (and maybe a new test case that shows the problem.)

comment:2 Changed 13 years ago by anonymous

The patch could test if there was an unpack_errors function present instead of just error_dict. If unpack_errors is choking, then something else is wrong somewhere else. However, if errors aren't unpacked initially, then you have to waste code unpacking errors within the controller.

If you have a schema like:

class CompanySchema(formencode.Schema):
    name = validators.String(not_empty=True, max=100)
    address1 = validators.String(not_empty=True)
    address2 = validators.String(not_empty=False)
    city = validators.String(not_empty=True, max=100)
    state = validators.StateProvince(not_empty=True)
    zip = validators.PostalCode(not_empty=True)
    phone = validators.PhoneNumber(not_empty=True)
    county = validators.String(not_empty=True, max=100)
    
class AttendeeSchema(formencode.Schema):
    first_name = validators.String(not_empty=True, max=255)
    last_name = validators.String(not_empty=True, max=255)
    training = validators.Int(not_empty=True)
    
class RegisterSchema(formencode.Schema):
    
    payment_type = validators.OneOf(["Credit Card", "Check", "Purchase Order"], not_empty=True)
    
    company = formencode.ForEach(CompanySchema())
    attendees = formencode.ForEach(AttendeeSchema()

Then unless errors are unpacked you can't do form_errorsattendees?[i] to access an error.. you'd have to call form_errorsattendees?.unpack_errors() which is pretty absurd.

Though I've been using this patch myself and nothing has been choking, maybe if you said what was exactly the problem?

comment:3 Changed 13 years ago by downtown1@…

New patch that will use unpack_errors if an unpack_errors method exists, will use error_dict if the former doesn't exist and error_dict does, and the regular tg_error error will still show up if neither exist.

If unpack_errors() is causing errors then it needs to show up because errors shouldn't be implicit.

Changed 13 years ago by downtown1@…

patch on controllers.py to unpack_errors recursively instead of using error_dict

Changed 13 years ago by downtown1@…

had a slight error in controllers.2.diff, tested this patch on my box though!

comment:4 Changed 13 years ago by kevin

  • Component changed from CherryPy to TurboGears

hmm... this patch doesn't seem like it's against the current trunk. this is at line 140 in controllers.py right now:

                    except turbogearsvalid.Invalid, error:
                        if error.error_dict:
                            errors.update(error.error_dict)
                        else:
                            errors["tg_general"] = error

comment:5 Changed 13 years ago by simon

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

Commited in r725. Thanks!

Note: See TracTickets for help on using tickets.