openerp-community-reviewer team mailing list archive

[Raphaël Valyi - http://www.akretion.com <rvalyi@xxxxxxxxx>]

Review: Disapprove

@Pedro,

I agree with your 4 objections to the current merge:
1) It makes a direct replacing of the field city, which breaks automatically any existing installation with data in city field, because there are string in the DB on that field, and with this module, it is expected to exist an ID (integer).
2) It is incompatible with other modules that expect to have a string on that field. For example, CRM.
3) It doesn't provide a menu entry for managing cities.
4) It is not adapted to v7 conventions.

As for 1), notice that in our localization we opted for creating a new key instead https://github.com/openerpbrasil/l10n_br_core/blob/7.0/l10n_br_base/res_partner.py#L73

Also, for compatibility, we set the partner.city char field properly when we select a city in the m2o https://github.com/openerpbrasil/l10n_br_core/blob/7.0/l10n_br_base/res_partner.py#L173

As for incompatibilities, such as with the CRM module you mention, well yes indeed it requires extra extensions. This is what we do for instance in Brazil to get it working: https://github.com/openerpbrasil/l10n_br_core/blob/7.0/l10n_br_crm/crm_lead.py#L137

Not trivial to get it working across the codebase, but I wonder if there is anything simpler possible. And yes, in most countries getting OpenERP to work requires a ton of hacks, this is a sad reality.

As for having a city m2o country, well at least for us we don't need it. I think the problem might more be with Vauxoo and Savoir Faire who opted for a m2o to the country before I said I would need it to the state.

I'm waiting your proposal for base_location then. Do you have some existing MP? Because like the other said, in the current form, base_location doesn't seem to do the cut for us (no city object, and like Pierre said, we also have thousands of zips per city here).

@Maxime, right, France doesn't have states. But I guess that we could use department or regions here. In any case, with the current financial crisis, it's highly probably to see a total balkanization of the country collapsing under regional independentist forces just like in Spain or Belgium. Just saying... ;-p

Thank you all for your participation.
-- 
https://code.launchpad.net/~savoirfairelinux-openerp/partner-contact-management/city-move/+merge/196023
Your team Partner and Contact Core Editors is subscribed to branch lp:partner-contact-management.