← Back to team overview

savoirfairelinux-openerp team mailing list archive

Re: lp:~savoirfairelinux-openerp/partner-contact-management/city-move into lp:partner-contact-management

 

Review: Disapprove

Hi all, sorry for the late replay, but I'm having hard days...

Although the convergence of both modules is desired, I have also to disagree technically with this MP because:

- 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).
- It is incompatible with other modules that expect to have a string on that field. For example, CRM.
- It doesn't provide a menu entry for managing cities.
- It is not adapted to v7 conventions.

So, please let me insist on trying to get base_location working for all. There are two requirements to be met for your approach:

- Removing the requirement of the zip field. City field is still required. We have to change a little 'name_get' method and add a 'search_name' method to fulfill complete ORM compatibility.
- As Raphaël, I also think that making city related to state is better than to country, but if you still need it, we can add a field country on city model that can be automatically filled when you enter the state (and also made readonly), and modify partner/company on_change accordingly.

What do you think about this proposal?

Regards.
-- 
https://code.launchpad.net/~savoirfairelinux-openerp/partner-contact-management/city-move/+merge/196023
Your team Savoir-faire Linux' OpenERP is subscribed to branch lp:~savoirfairelinux-openerp/partner-contact-management/city-move.


References