Code review comment for lp:~sebastien.beau/magentoerpconnect/oerp6.1-cleanning

Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

Hi,

Some minor things:

Line 200:
The method ``map_magento_order`` is public (accessible from XMR/RPC) but may return a None value if they are zero or many invoices.

Line 1133
You've put an unused local variable ``first_install``.

Lines 1138-1149, I would rather use only one UPDATE with a CASE, it should something like:
UPDATE res_partner_address
SET name = CASE
  WHEN firstname IS NOT NULL AND lastname IS NOT NULL THEN (firstname || ' ' || lastname)
  WHEN firstname IS NOT NULL AND lastname IS NULL THEN firstname
  WHEN firstname IS NULL AND lastname IS NOT NULL THEN lastname
  ELSE '';

Otherwise, it seems good.
Thanks

review: Needs Fixing (no test, review)

« Back to merge proposal