refactor form error handling:
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Wed, 13 Jan 2010 15:54:07 +0100
changeset 4224 5998df006968
parent 4223 4fb00ccad3df
child 4225 c49bb6e3d343
refactor form error handling: * almost everything is in the base form class, with other form errors related code * deprecate form_field_error in favor of field_error and format_error on the renderer * fix pb with js localization of errors by properly using field.role_name() * rename 'errors' into 'error' in forminfo to avoid confusion
web/application.py
web/form.py
web/views/cwproperties.py
web/views/editcontroller.py
web/views/formrenderers.py
web/views/forms.py
--- a/web/application.py	Wed Jan 13 15:48:24 2010 +0100
+++ b/web/application.py	Wed Jan 13 15:54:07 2010 +0100
@@ -375,7 +375,7 @@
     def validation_error_handler(self, req, ex):
         ex.errors = dict((k, v) for k, v in ex.errors.items())
         if '__errorurl' in req.form:
-            forminfo = {'errors': ex,
+            forminfo = {'error': ex,
                         'values': req.form,
                         'eidmap': req.data.get('eidmap', {})
                         }
--- a/web/form.py	Wed Jan 13 15:48:24 2010 +0100
+++ b/web/form.py	Wed Jan 13 15:54:07 2010 +0100
@@ -7,7 +7,10 @@
 """
 __docformat__ = "restructuredtext en"
 
+from warnings import warn
+
 from logilab.common.decorators import iclassmethod
+from logilab.common.deprecation import deprecated
 
 from cubicweb.appobject import AppObject
 from cubicweb.view import NOINDEX, NOFOLLOW
@@ -81,17 +84,19 @@
         return self.parent_form.root_form
 
     @property
+    def form_valerror(self):
+        """the validation error exception if any"""
+        if self.parent_form is None:
+            return self._form_valerror
+        return self.parent_form.form_valerror
+
+    @property
     def form_previous_values(self):
+        """previously posted values (on validation error)"""
         if self.parent_form is None:
             return self._form_previous_values
         return self.parent_form.form_previous_values
 
-    @property
-    def form_valerror(self):
-        if self.parent_form is None:
-            return self._form_valerror
-        return self.parent_form.form_valerror
-
     @iclassmethod
     def _fieldsattr(cls_or_self):
         if isinstance(cls_or_self, type):
@@ -153,10 +158,8 @@
         # method on successful commit
         forminfo = self._cw.get_session_data(sessionkey, pop=True)
         if forminfo:
-            # XXX remove _cw.data assigment once cw.web.widget is killed
-            self._cw.data['formvalues'] = self._form_previous_values = forminfo['values']
-            self._cw.data['formerrors'] = self._form_valerror = forminfo['errors']
-            self._cw.data['displayederrors'] = self.form_displayed_errors = set()
+            self._form_previous_values = forminfo['values']
+            self._form_valerror = forminfo['error']
             # if some validation error occured on entity creation, we have to
             # get the original variable name from its attributed eid
             foreid = self.form_valerror.entity
@@ -169,3 +172,29 @@
         else:
             self._form_previous_values = {}
             self._form_valerror = None
+
+    def field_error(self, field):
+        """return field's error if specified in current validation exception"""
+        if self.form_valerror:
+            if field.eidparam and self.edited_entity.eid != self.form_valerror.eid:
+                return None
+            try:
+                return self.form_valerror.errors.pop(field.role_name())
+            except KeyError:
+                if field.role and field.name in self.form_valerror:
+                    warn('%s: errors key of attribute/relation should be suffixed by "-<role>"'
+                         % self.form_valerror.__class__, DeprecationWarning)
+                    return self.form_valerror.errors.pop(field.name)
+        return None
+
+    def remaining_errors(self):
+        return sorted(self.form_valerror.errors.items())
+
+    @deprecated('[3.6] use form.field_error and/or new renderer.render_error method')
+    def form_field_error(self, field):
+        """return validation error for widget's field, if any"""
+        err = self.field_error(field)
+        if err:
+            return u'<span class="error">%s</span>' % err
+        return u''
+
--- a/web/views/cwproperties.py	Wed Jan 13 15:48:24 2010 +0100
+++ b/web/views/cwproperties.py	Wed Jan 13 15:54:07 2010 +0100
@@ -371,7 +371,9 @@
             w(u'<div class="preffield">\n')
             if self.display_label:
                 w(u'%s' % self.render_label(form, field))
-            error = form.form_field_error(field)
+            error = form.field_error(field)
+            if error:
+                w(u'<span class="error">%s</span>' % err)
             w(u'%s' % self.render_help(form, field))
             w(u'<div class="prefinput">')
             w(field.render(form, self))
--- a/web/views/editcontroller.py	Wed Jan 13 15:48:24 2010 +0100
+++ b/web/views/editcontroller.py	Wed Jan 13 15:54:07 2010 +0100
@@ -162,7 +162,7 @@
             if field.has_been_modified(form):
                 self.handle_formfield(form, field, rqlquery)
         if self.errors:
-            errors = dict((f.name, unicode(ex)) for f, ex in self.errors)
+            errors = dict((f.role_name(), unicode(ex)) for f, ex in self.errors)
             raise ValidationError(entity.eid, errors)
         if eid is None: # creation or copy
             entity.eid = self._insert_entity(etype, formparams['eid'], rqlquery)
--- a/web/views/formrenderers.py	Wed Jan 13 15:48:24 2010 +0100
+++ b/web/views/formrenderers.py	Wed Jan 13 15:54:07 2010 +0100
@@ -127,9 +127,7 @@
         # get extra errors
         if errex is not None:
             errormsg = req._('please correct the following errors:')
-            displayed = form.form_displayed_errors
-            errors = sorted((field, err) for field, err in errex.errors.items()
-                            if not field in displayed)
+            errors = form.remaining_errors()
             if errors:
                 if len(errors) > 1:
                     templstr = '<li>%s</li>\n'
@@ -209,10 +207,10 @@
                 w(u'<tr class="%s_%s_row">' % (field.name, field.role))
                 if self.display_label:
                     w(u'<th class="labelCol">%s</th>' % self.render_label(form, field))
-                error = form.form_field_error(field)
+                error = form.field_error(field)
                 if error:
                     w(u'<td class="error">')
-                    w(error)
+                    self.render_error(w, error)
                 else:
                     w(u'<td>')
                 w(field.render(form, self))
@@ -231,6 +229,11 @@
             w(u'<td>%s</td>\n' % button.render(form))
         w(u'</tr></table>')
 
+    def render_error(self, w, err):
+        """return validation error for widget's field, if any"""
+        w(u'<span class="error">%s</span>' % err)
+
+
 
 class BaseFormRenderer(FormRenderer):
     """use form_renderer_id = 'base' if you want base FormRenderer layout even
@@ -265,10 +268,10 @@
         w(u'</tr>')
         w(u'<tr>')
         for field in fields:
-            error = form.form_field_error(field)
+            error = form.field_error(field)
             if error:
                 w(u'<td class="error">')
-                w(error)
+                self.render_error(w, error)
             else:
                 w(u'<td>')
             w(field.render(form, self))
@@ -324,10 +327,10 @@
             w(u'<td>%s</td>' % checkbox('eid', entity.eid,
                                         checked=qeid in values))
             for field in fields:
-                error = form.form_field_error(field)
+                error = form.field_error(field)
                 if error:
                     w(u'<td class="error">')
-                    w(error)
+                    self.render_error(w, error)
                 else:
                     w(u'<td>')
                 if isinstance(field.widget, (fwdgs.Select, fwdgs.CheckBox,
--- a/web/views/forms.py	Wed Jan 13 15:48:24 2010 +0100
+++ b/web/views/forms.py	Wed Jan 13 15:54:07 2010 +0100
@@ -162,18 +162,6 @@
             for field in field.actual_fields(self):
                 field.form_init(self)
 
-    def form_field_error(self, field):
-        """return validation error for widget's field, if any"""
-        if self._field_has_error(field):
-            self.form_displayed_errors.add(field.name)
-            return u'<span class="error">%s</span>' % self.form_valerror.errors[field.name]
-        return u''
-
-    def _field_has_error(self, field):
-        """return true if the field has some error in given validation exception
-        """
-        return self.form_valerror and field.name in self.form_valerror.errors
-
     @deprecated('[3.6] use .add_hidden(name, value, **kwargs)')
     def form_add_hidden(self, name, value=None, **kwargs):
         return self.add_hidden(name, value, **kwargs)
@@ -240,12 +228,6 @@
         self.add_hidden('_cw_edited_fields', u','.join(edited),
                         eidparam=True)
 
-    def _field_has_error(self, field):
-        """return true if the field has some error in given validation exception
-        """
-        return super(EntityFieldsForm, self)._field_has_error(field) \
-               and self.form_valerror.eid == self.edited_entity.eid
-
     def default_renderer(self):
         return self._cw.vreg['formrenderers'].select(
             self.form_renderer_id, self._cw, rset=self.cw_rset, row=self.cw_row,