Hi there,

I've started to play with this great module and I've found some minor errors and warnings while trying to configure it. Attached is a patch with some fixes.

Note I've changed lines as form_set_error('erp-passwd', '...') to form_error($form['user']['erp-passwd'], '..');.

form_set_error was not working (first param should be some as 'user][erp-passwd' (sic). form_error() is a wrapper to set this errors without a headache :)

There're some other things in the code to improve but I didn't want to spam you with a big patch. I can continue providing patches if you're receptive.

CommentFileSizeAuthor
openerp_fixes.patch6.17 KBjonhattan

Comments

BoogieBug’s picture

BoogieBug commented
Assigned: Unassigned » BoogieBug
Status: Needs review » Fixed

Hi!

Thanks for your contribution. I've partially applied your patch into the HEAD version. The exceptions are the form_set_error related issues which are seemingly not really about the form_set_error() function itself but the mis-validation in the code.

Please feel free to submit me any patch you see fit.

rgds.

jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
commented
Status: Fixed » Needs review

Let me explain it:

when you group form items as in

$form['openerp']['erp-passwd'] = array(...)

to set an error on this form item, you need form_set_error('openerp][erp-passwd', 'error msg'). This way the item with a validation error will be highlighted in red.
An alternative to this unfriendly syntax is to use form_error($form['openerp']['erp-passwd'], 'error msg')

http://api.drupal.org/api/drupal/includes--form.inc/function/form_error/6

BoogieBug’s picture

BoogieBug commented

According to the form_set_error documentation, I understand that the form_set_error does not need the name parameter in the format you mentioned, unless the #parents property is set.

$name The name of the form element. If the #parents property of your form element is array('foo', 'bar', 'baz') then you may set an error on 'foo' or 'foo][bar][baz'. Setting an error on 'foo' sets an error for every element where the #parents array starts with 'foo'.

In our case the #parents of the $form is not set and the form element name is unique. In addition, it works correctly on my test environment. Unless you found any additional error on this, would you mind if I'd like to keep the current code, due to lack of additional effort, required by the testing.