Code review comment for lp:~openerp-dev/openobject-addons/6.1-opw-573096-ira

Revision history for this message
Julien Thewys (julien-thewys) wrote :

Hello,

Thanks for the patch but it does not work for me:
1. the contact is still not created
2. it breaks a test:
openerp.tools.yaml_import: Assertion "NONAME" FAILED
test: len(job_ids) == 2
values: ! 0 == 2
3. the title cannot be saved on the crm.lead form (so it cannot be converted either)
4. base_contact uses both firstname and lastname instead of name, so converting to partner should also copy the firstname. It means that the crm.lead form view must be adapted to display a firstname and a lastname field.
5. the email and the phone should not be saved on the contact but rather on the address (= on the job).
Could you fix that also please?

Also, as a good practice for readability, it is recommended to write
    if 'contact_id' in data:
instead of
    if not data.get('contact_id', False):

I just also saw that the create method you modified had an error prone signature:
    context={}
instead of the idiom
    context=None

You can fix all that in a raw.

Thank you.

review: Needs Fixing

« Back to merge proposal