[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
--- 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;
--- 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;
--- 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:
--- 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)
--- 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()
--- 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
--- 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)