# HG changeset patch
# User Sylvain Thénault <sylvain.thenault@logilab.fr>
# Date 1269606812 -3600
# Node ID 90493551b1eb9bb8060a2edec638258bcfde7106
# Parent  7778a2bbdf9d9cb4bb31582aaaf55a3f1e24057e
[form] fix validation error handling

* type when possible ValidationError raised by the edit controller

* don't rely on repository to check required field

* turn css class used for field error message to errorMsg to avoid
  confusion with the error class added to input

* fix css when errors are added by the form renderer

* fix form renderer to add the error message

diff -r 7778a2bbdf9d -r 90493551b1eb web/data/cubicweb.edition.js
--- a/web/data/cubicweb.edition.js	Fri Mar 26 13:23:25 2010 +0100
+++ b/web/data/cubicweb.edition.js	Fri Mar 26 13:33:32 2010 +0100
@@ -322,7 +322,7 @@
 
 function _clearPreviousErrors(formid) {
     jQuery('#' + formid + 'ErrorMessage').remove();
-    jQuery('#' + formid + ' span.error').remove();
+    jQuery('#' + formid + ' span.errorMsg').remove();
     jQuery('#' + formid + ' .error').removeClass('error');
 }
 
@@ -334,6 +334,7 @@
 	var fieldid = fieldname + ':' + eid;
 	var suffixes = ['', '-subject', '-object'];
 	var found = false;
+	// XXX remove suffixes at some point
 	for (var i=0, length=suffixes.length; i<length;i++) {
 	    var field = jqNode(fieldname + suffixes[i] + ':' + eid);
 	    if (field && getNodeAttribute(field, 'type') != 'hidden') {
@@ -341,7 +342,7 @@
 		    firsterrfield = 'err-' + fieldid;
 		}
 		addElementClass(field, 'error');
-		var span = SPAN({'id': 'err-' + fieldid, 'class': "error"}, errmsg);
+		var span = SPAN({'id': 'err-' + fieldid, 'class': "errorMsg"}, errmsg);
 		field.before(span);
 		found = true;
 		break;
diff -r 7778a2bbdf9d -r 90493551b1eb web/data/cubicweb.form.css
--- a/web/data/cubicweb.form.css	Fri Mar 26 13:23:25 2010 +0100
+++ b/web/data/cubicweb.form.css	Fri Mar 26 13:33:32 2010 +0100
@@ -192,11 +192,14 @@
   background-color: #eeedd9;
 }
 
-input.error {
+.error input { /* error added by the form renderer */
+  background: transparent url("error.png") 100% 50% no-repeat;
+}
+input.error { /* error added by javascript */
   background: transparent url("error.png") 100% 50% no-repeat;
 }
 
-span.error {
+span.errorMsg {
   display: block;
   font-weight: bold;
   color: #ed0d0d;
diff -r 7778a2bbdf9d -r 90493551b1eb web/formfields.py
--- a/web/formfields.py	Fri Mar 26 13:23:25 2010 +0100
+++ b/web/formfields.py	Fri Mar 26 13:33:32 2010 +0100
@@ -381,7 +381,10 @@
         for field in self.actual_fields(form):
             if field is self:
                 try:
-                    yield field, field.process_form_value(form)
+                    value = field.process_form_value(form)
+                    if value is None and field.required:
+                        raise ProcessFormError(form._cw._("required field"))
+                    yield field, value
                 except UnmodifiedField:
                     continue
             else:
diff -r 7778a2bbdf9d -r 90493551b1eb web/test/unittest_application.py
--- a/web/test/unittest_application.py	Fri Mar 26 13:23:25 2010 +0100
+++ b/web/test/unittest_application.py	Fri Mar 26 13:33:32 2010 +0100
@@ -186,37 +186,66 @@
         self.assertEquals(values['eid'], eid)
         error = forminfo['error']
         self.assertEquals(error.entity, user.eid)
-        self.assertEquals(error.errors['login-subject'], 'required attribute')
+        self.assertEquals(error.errors['login-subject'], 'required field')
 
 
-    def test_validation_error_dont_loose_subentity_data(self):
+    def test_validation_error_dont_loose_subentity_data_ctrl(self):
         """test creation of two linked entities
+
+        error occurs on the web controller
         """
         req = self.request()
-        form = {'eid': ['X', 'Y'], '__maineid': 'X',
-                '__type:X': 'CWUser', '_cw_edited_fields:X': 'login-subject,surname-subject',
-                # missing required field
-                'login-subject:X': u'',
-                'surname-subject:X': u'Mr Ouaoua',
-                # but email address is set
-                '__type:Y': 'EmailAddress', '_cw_edited_fields:Y': 'address-subject,alias-subject,use_email-object',
-                'address-subject:Y': u'bougloup@logilab.fr',
-                'alias-subject:Y': u'',
-                'use_email-object:Y': 'X',
-                # necessary to get validation error handling
-                '__errorurl': 'view?vid=edition...',
-                }
-        req.form = form
-        # monkey patch edited_eid to ensure both entities are edited, not only X
-        req.edited_eids = lambda : ('Y', 'X')
+        # set Y before X to ensure both entities are edited, not only X
+        req.form = {'eid': ['Y', 'X'], '__maineid': 'X',
+                    '__type:X': 'CWUser', '_cw_edited_fields:X': 'login-subject',
+                    # missing required field
+                    'login-subject:X': u'',
+                    # but email address is set
+                    '__type:Y': 'EmailAddress', '_cw_edited_fields:Y': 'address-subject',
+                    'address-subject:Y': u'bougloup@logilab.fr',
+                    'use_email-object:Y': 'X',
+                    # necessary to get validation error handling
+                    '__errorurl': 'view?vid=edition...',
+                    }
         path, params = self.expect_redirect(lambda x: self.app_publish(x, 'edit'), req)
         forminfo = req.get_session_data('view?vid=edition...')
         self.assertEquals(set(forminfo['eidmap']), set('XY'))
+        self.assertEquals(forminfo['eidmap']['X'], None)
+        self.assertIsInstance(forminfo['eidmap']['Y'], int)
+        self.assertEquals(forminfo['error'].entity, 'X')
+        self.assertEquals(forminfo['error'].errors,
+                          {'login-subject': 'required field'})
+        self.assertEquals(forminfo['values'], req.form)
+
+
+    def test_validation_error_dont_loose_subentity_data_repo(self):
+        """test creation of two linked entities
+
+        error occurs on the repository
+        """
+        req = self.request()
+        # set Y before X to ensure both entities are edited, not only X
+        req.form = {'eid': ['Y', 'X'], '__maineid': 'X',
+                    '__type:X': 'CWUser', '_cw_edited_fields:X': 'login-subject,upassword-subject',
+                    # already existent user
+                    'login-subject:X': u'admin',
+                    'upassword-subject:X': u'admin', 'upassword-subject-confirm:X': u'admin',
+                    '__type:Y': 'EmailAddress', '_cw_edited_fields:Y': 'address-subject',
+                    'address-subject:Y': u'bougloup@logilab.fr',
+                    'use_email-object:Y': 'X',
+                    # necessary to get validation error handling
+                    '__errorurl': 'view?vid=edition...',
+                    }
+        path, params = self.expect_redirect(lambda x: self.app_publish(x, 'edit'), req)
+        forminfo = req.get_session_data('view?vid=edition...')
+        self.assertEquals(set(forminfo['eidmap']), set('XY'))
+        self.assertIsInstance(forminfo['eidmap']['X'], int)
+        self.assertIsInstance(forminfo['eidmap']['Y'], int)
         self.assertEquals(forminfo['error'].entity, forminfo['eidmap']['X'])
         self.assertEquals(forminfo['error'].errors,
-                          {'login-subject': 'required attribute',
-                           'upassword-subject': 'required attribute'})
-        self.assertEquals(forminfo['values'], form)
+                          {'login-subject': u'the value "admin" is already used, use another one'})
+        self.assertEquals(forminfo['values'], req.form)
+
 
     def _test_cleaned(self, kwargs, injected, cleaned):
         req = self.request(**kwargs)
diff -r 7778a2bbdf9d -r 90493551b1eb web/test/unittest_views_basecontrollers.py
--- a/web/test/unittest_views_basecontrollers.py	Fri Mar 26 13:23:25 2010 +0100
+++ b/web/test/unittest_views_basecontrollers.py	Fri Mar 26 13:33:32 2010 +0100
@@ -430,7 +430,7 @@
                     'use_email-object:Y': 'X',
                     }
         ex = self.assertRaises(ValidationError, self.ctrl_publish, req)
-        self.assertEquals(ex.errors, {'address-subject': u'required attribute'})
+        self.assertEquals(ex.errors, {'address-subject': u'required field'})
 
     def test_nonregr_copy(self):
         user = self.user()
diff -r 7778a2bbdf9d -r 90493551b1eb web/views/editcontroller.py
--- a/web/views/editcontroller.py	Fri Mar 26 13:23:25 2010 +0100
+++ b/web/views/editcontroller.py	Fri Mar 26 13:33:32 2010 +0100
@@ -17,6 +17,11 @@
 from cubicweb.web import INTERNAL_FIELD_VALUE, RequestError, NothingToEdit, ProcessFormError
 from cubicweb.web.views import basecontrollers, autoform
 
+def valerror_eid(eid):
+    try:
+        return typed_eid(eid)
+    except (ValueError, TypeError):
+        return eid
 
 class RqlQuery(object):
     def __init__(self):
@@ -110,7 +115,7 @@
         self._cw.remove_pending_operations()
         if self.errors:
             errors = dict((f.name, unicode(ex)) for f, ex in self.errors)
-            raise ValidationError(form.get('__maineid'), errors)
+            raise ValidationError(valerror_eid(form.get('__maineid')), errors)
 
     def _insert_entity(self, etype, eid, rqlquery):
         rql = rqlquery.insert_query(etype)
@@ -166,7 +171,7 @@
                     self.handle_formfield(form, field, rqlquery)
         if self.errors:
             errors = dict((f.role_name(), unicode(ex)) for f, ex in self.errors)
-            raise ValidationError(entity.eid, errors)
+            raise ValidationError(valerror_eid(entity.eid), errors)
         if eid is None: # creation or copy
             entity.eid = self._insert_entity(etype, formparams['eid'], rqlquery)
         elif rqlquery.edited: # edition of an existant entity
diff -r 7778a2bbdf9d -r 90493551b1eb web/views/formrenderers.py
--- a/web/views/formrenderers.py	Fri Mar 26 13:23:25 2010 +0100
+++ b/web/views/formrenderers.py	Fri Mar 26 13:33:32 2010 +0100
@@ -224,6 +224,8 @@
                     w(u' class="error"')
                 w(u'>')
                 w(field.render(form, self))
+                if error:
+                    self.render_error(w, error)
                 if self.display_help:
                     w(self.render_help(form, field))
                 w(u'</td></tr>')
@@ -241,7 +243,7 @@
 
     def render_error(self, w, err):
         """return validation error for widget's field, if any"""
-        w(u'<span class="error">%s</span>' % err)
+        w(u'<span class="errorMsg">%s</span>' % err)