Merge lp:~savoirfairelinux-openerp/partner-contact-management/partner_isp into lp:~partner-contact-core-editors/partner-contact-management/7.0

[Joao Alfredo Gama Batista]

Added the partner_isp module.

[Pedro Manuel Baeza (pedro.baeza)]

Hi, Joao, do you think this module is useful and generic enough to be in this community branch?

I tentatively dissaprove it, but let's wait another opinions.

Regards.

review: Disapprove

[Joao Alfredo Gama Batista (joao-gama)]

Hello Pedro,

I sincerely do believe that this module is useful and generic enough to be proposed for merge. That's why I proposed it ;-). I fail to see the point in not propose for merge a partner related module to a partner management related project. Maybe you took your decision based on parameters that I'm not aware of. If so I would be very thankful if you enlighten me on this matter.

Best regards.

Joao

> Hi, Joao, do you think this module is useful and generic enough to be in this
> community branch?
>
> I tentatively dissaprove it, but let's wait another opinions.
>
> Regards.

[Pedro Manuel Baeza (pedro.baeza)]

Sorry, I don't want to bother you. I talk this because previous days we have been talking in the community list about the convenience of increasing the number of hosted modules that makes very specific functions. That's why I ask for the opinion of the rest of the members.

Regards.

[Joao Alfredo Gama Batista (joao-gama)]

It's OK. Sorry if I sounded upset. It's not case. I'm aware of the discussions you mentioned and neither of us want to pollute this or any other project but on the other hand, every project needs a good set of modules to become the reference in a certain field of expertise. So the point in this contribution is to increase the possibilities of this project. It's a subjective matter but I think that we can be generic and (to a certain point) be domain specific at the same time.

Hope you get your community reviewer status soon!

Best regards.

[Stefan Rijnhart (Opener) (stefan-opener)]

I must agree that the features of this module, while useful, don't seem to necessarily belong together. Apart from that, the code does raise some questions with me:

- By ISP, do you mean internet service provider?
- What do you mean by 'representative'?
- How is code different from the regular partner reference 'ref'? Why not fill this field automatically instead of creating a new field?
- Can you explain, what onchange_name does (preferably in a docstring) and how the code is assembled (preferably by refactoring the code, it would be nice if this piece of the code became a little bit more readable)

You override the char field 'birthdate' from the base module by a date field. I would suspect that the installation of this module would make you 'lose' any existing birhdates in char format (orm moves the database column to birthdate_moved_nnn), and that an upgrade of the base module would move your date type column, and so on. Or am I missing something?

l.207: you catch just any exception, which you should generally avoid doing. I think the whole try/catch block can be substituted by checking if 'name' is in values, right?

review: Needs Information

[Holger Brunn (Therp) (hbrunn)]

Yes, changing types that don't convert to each other (like in the birthdate case) make you lose your data on every update.

I would love to see a module that does *just* that (change birthdate's type), but this involves some trickery for the first installation and the update process. I once solved this problem this way:

    def _auto_init(self, cr, context=None):
        cr.execute("""
            SELECT typname FROM pg_class c,pg_attribute a, pg_type t
            WHERE c.relname=%s AND a.attname = %s
            AND c.oid=a.attrelid AND a.atttypid=t.oid
            AND c.oid=a.attrelid""", (self._table, 'birthdate'))
        type_before = cr.fetchone()[0]
        result = super(res_partner, self)._auto_init(cr, context=context)
        if type_before == 'varchar':
            self._copy_from_latest_of_type(cr, 'birthdate', 'date')
        return result

    def _copy_from_latest_of_type(self, cr, name, typename, drop_moved=True):
        values = {
                'table': self._table,
                'name': name,
                'name_moved': name + '_moved',
                'name_moved_like': name + '_moved%',
                'typename': typename
                }
        cr.execute("""
            SELECT attname
            FROM pg_class c,pg_attribute a, pg_type t
            WHERE c.relname=%(table)s AND a.attname like %(name_moved_like)s
            AND c.oid=a.attrelid AND a.atttypid=t.oid
            AND t.typname=%(typename)s
            order by
                substring(attname from length(%(name_moved)s) + 1)::int
                desc
            """, values)
        row = cr.fetchall()[0]
        if row:
            cr.execute('update %(table)s set %(name)s=%(name_moved)s' % dict(
                values, name_moved=row[0]))
        else:
            return
        if drop_moved:
            cr.execute("""
                SELECT attname
                FROM pg_class c,pg_attribute a, pg_type t
                WHERE c.relname=%(table)s
                AND a.attname like %(name_moved_like)s
                AND c.oid=a.attrelid AND a.atttypid=t.oid""", values)
            columns = [row[0] for row in cr.fetchall()]
            cr.execute(('alter table %(table)s ' + ','.join(
                ['drop column ' + column for column in columns])) % values)
            cr.commit()

Further, I also think this module binds together pretty unrelated properties and shouldn't be included as it is.

review: Disapprove

[Maxime Chambreuil (http://www.savoirfairelinux.com) (max3903)]

Thanks for your feedback.

Module was moved to a more specific project lp:openerp-isp.

Unmerged revisions

31. By Joao Alfredo Gama Batista on 2013-09-11

[FIX] Code clean-up

30. By Joao Alfredo Gama Batista on 2013-09-04

[FIX] Use always upper case

29. By Joao Alfredo Gama Batista on 2013-09-04

[FIX] Fix handling for unicode names

28. By Joao Alfredo Gama Batista on 2013-08-22

[FIX] Generate upper case codes

27. By Joao Alfredo Gama Batista on 2013-08-21

[ADD] Added mandatary information

26. By Joao Alfredo Gama Batista on 2013-08-20

[FIX] Fix error in std openerp test

25. By Joao Alfredo Gama Batista on 2013-08-19

[ADD] Added the mandatary field

24. By Joao Alfredo Gama Batista on 2013-08-19

[ADD] partner_isp initial version

=== added directory 'partner_isp'
=== added file 'partner_isp/__init__.py'
--- partner_isp/__init__.py	1970-01-01 00:00:00 +0000
+++ partner_isp/__init__.py	2013-09-11 16:19:34 +0000
@@ -0,0 +1,23 @@
+# -*- coding: utf-8 -*-
+
+##############################################################################
+#
+#    OpenERP, Open Source Management Solution
+#    Copyright (C) 2013 Savoir-faire Linux Inc. (<www.savoirfairelinux.com>).
+#
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU Affero General Public License as
+#    published by the Free Software Foundation, either version 3 of the
+#    License, or (at your option) any later version.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU Affero General Public License for more details.
+#
+#    You should have received a copy of the GNU Affero General Public License
+#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+##############################################################################
+
+import partner

=== added file 'partner_isp/__openerp__.py'
--- partner_isp/__openerp__.py	1970-01-01 00:00:00 +0000
+++ partner_isp/__openerp__.py	2013-09-11 16:19:34 +0000
@@ -0,0 +1,45 @@
+# -*- coding: utf-8 -*-
+
+##############################################################################
+#
+#    OpenERP, Open Source Management Solution
+#    Copyright (C) 2013 Savoir-faire Linux Inc. (<www.savoirfairelinux.com>).
+#
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU Affero General Public License as
+#    published by the Free Software Foundation, either version 3 of the
+#    License, or (at your option) any later version.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU Affero General Public License for more details.
+#
+#    You should have received a copy of the GNU Affero General Public License
+#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+##############################################################################
+
+{
+    'name': 'Partner for ISPs',
+    'version': '1.0',
+    'category': 'CRM',
+    'description': """
+A partner suitable for ISP companies
+====================================
+
+This module provides some minor modifications to the res.partner model including:
+
+* A customer code based on the customers name,
+* Required ZIP code and e-mail fields,
+* A representative field,
+* A birtday field for contacts and representatives.
+    """,
+    'author': 'Savoir-faire Linux Inc',
+    'website': 'www.savoirfairelinux.com',
+    'license': 'AGPL-3',
+    'depends': ['base'],
+    'data': ['partner_isp_data.xml'],
+    'active': False,
+    'installable': True,
+}

=== added directory 'partner_isp/i18n'
=== added file 'partner_isp/i18n/partner_isp.pot'
=== added file 'partner_isp/partner.py'
--- partner_isp/partner.py	1970-01-01 00:00:00 +0000
+++ partner_isp/partner.py	2013-09-11 16:19:34 +0000
@@ -0,0 +1,130 @@
+# -*- coding: utf-8 -*-
+
+##############################################################################
+#
+#    OpenERP, Open Source Management Solution
+#    Copyright (C) 2013 Savoir-faire Linux Inc. (<www.savoirfairelinux.com>).
+#
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU Affero General Public License as
+#    published by the Free Software Foundation, either version 3 of the
+#    License, or (at your option) any later version.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU Affero General Public License for more details.
+#
+#    You should have received a copy of the GNU Affero General Public License
+#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+##############################################################################
+
+from openerp.osv import orm, fields
+
+
+class res_partner(orm.Model):
+    _inherit = 'res.partner'
+
+    _columns = {
+        'birthdate': fields.date('Birth date'),
+        'representative': fields.char('Representative', size=64),
+        'code': fields.char('Code', size=16),
+        'representative_birthdate': fields.date('Representative birth date')
+    }
+
+    def onchange_name(self, cr, uid, ids, name, context=None):
+        words = name.split(u' ')
+        words_treated = []
+        code = u''
+        ret = {'value': None}
+        seq = 1
+        for w in words:
+            if w.find(u'-') != -1:
+                words_treated.append(w.split(u'-'))
+            else:
+                words_treated.append(w)
+
+        words = words_treated
+
+        if len(words) == 1:
+            if isinstance(words[-1], unicode):
+                code += words[0][0]
+            elif isinstance(words[0], list):
+                code += words[0][-1][0]
+                code += words[0][0][0]
+        elif len(words) == 2:
+            if isinstance(words[-1], unicode):
+                code += words[-1][0]
+            elif isinstance(words[-1], list):
+                code += words[-1][0][0]
+                code += words[-1][-1][0]
+            if isinstance(words[0], unicode):
+                code += words[0][0]
+            elif isinstance(words[0], list):
+                code += words[0][-1][0]
+                code += words[0][0][0]
+        elif len(words) == 3:
+            if isinstance(words[-1], unicode):
+                code += words[-1][0]
+            elif isinstance(words[-1], list):
+                code += words[-1][0][0]
+                code += words[-1][-1][0]
+            if isinstance(words[-2], unicode):
+                code += words[-2][0]
+            elif isinstance(words[-2], list):
+                code += words[-2][0][0]
+                code += words[-2][-1][0]
+            if isinstance(words[0], unicode):
+                code += words[0][0]
+            elif isinstance(words[0], list):
+                code += words[0][-1][0]
+                code += words[0][0][0]
+            code = code[:4]
+        elif len(words) >= 4:
+            if isinstance(words[-1], unicode):
+                code += words[-1][0]
+            elif isinstance(words[-1], list):
+                code += words[-1][-1][0]
+                code += words[-1][0][0]
+            if isinstance(words[-2], unicode):
+                code += words[-2][0]
+            elif isinstance(words[-2], list):
+                code += words[-2][0][0]
+                code += words[-2][-1][0]
+            if isinstance(words[0], unicode):
+                code += words[0][0]
+            elif isinstance(words[0], list):
+                code += words[0][0][0]
+                code += words[0][-1][0]
+            if isinstance(words[1], unicode):
+                code += words[1][0]
+            elif isinstance(words[1], list):
+                code += words[1][0][0]
+                code += words[1][-1][0]
+            code = code[:4]
+
+        code = code.upper()
+        while True:
+            query = [('code', '=', code + str(seq)), ('customer', '=', True)]
+            if not self.search(cr, uid, query, context=context):
+                code = code + str(seq)
+                break
+            seq += 1
+        ret['value'] = {'code': code.upper()}
+        return ret
+
+    def write(self, cr, uid, ids, values, context=None):
+        try:
+            values['code'] = self.onchange_name(cr, uid, ids,
+                                                name=values['name'],
+                                                context=context)['value']['code']
+        except:
+            pass
+        return super(res_partner, self).write(cr, uid, ids, values, context=context)
+
+    def create(self, cr, uid, values, context=None):
+        values['code'] = self.onchange_name(cr, uid, ids=None,
+                                            name=values['name'],
+                                            context=context)['value']['code']
+        return super(res_partner, self).create(cr, uid, values, context=context)

=== added file 'partner_isp/partner_isp_data.xml'
--- partner_isp/partner_isp_data.xml	1970-01-01 00:00:00 +0000
+++ partner_isp/partner_isp_data.xml	2013-09-11 16:19:34 +0000
@@ -0,0 +1,56 @@
+<?xml version="1.0" encoding="utf-8"?>
+<openerp>
+    <data>
+	<record id="view_partner_isp_form" model="ir.ui.view">
+	    <field name="name">res.partner.isp.form</field>
+	    <field name="model">res.partner</field>
+	    <field name="inherit_id" ref="base.view_partner_form" />
+            <field name="arch" type="xml">
+		<field name="name" position="before">
+		    <field name="code" readonly="1" />
+		</field>
+		<field name="name" position="attributes">
+		    <attribute name="on_change">onchange_name(name)</attribute>
+		</field>
+		<field name="website" position="after">
+		    <label for="representative" attrs="{'invisible': ['|', ('parent_id', '=', True), ('customer', '=', False)]}"/>
+		    <div attrs="{'invisible': ['|', ('parent_id', '=', True), ('customer', '=', False)]}">
+			<field 
+			    name="representative" 
+			    placeholder="Representative name"
+			    style="width: 63%%" />
+			<field
+			    name="representative_birthdate"
+			    placeholder="Birth date"
+			    style="width: 33%%" />
+		    </div>
+		</field>
+		<field name="image" position="replace" />
+		<field name="title" position="attributes">
+		    <attribute name="attrs">{'invisible': ['|', ('is_company', '=', True), ('customer', '=', True)]}</attribute>
+		</field>
+		<field name="title" position="before">
+		    <field name="birthdate" attrs="{'invisible': ['|', ('is_company', '=', True), ('customer', '=', False)]}" />
+		</field>
+		<field name="zip" position="attributes">
+		    <attribute name="attrs">{'required': ['|', ('parent_id', '=', False), ('customer', '=', True)]}</attribute>
+		</field>
+		<field name="email" position="attributes">
+		    <attribute name="attrs">{'required': ['|', ('parent_id', '=', False), ('customer', '=', True)]}</attribute>
+		</field>
+	    </field>
+	</record>
+
+	<record id="view_res_partner_isp_filter" model="ir.ui.view">
+            <field name="name">res.partner.isp.select</field>
+            <field name="model">res.partner</field>
+	    <field name="inherit_id" ref="base.view_res_partner_filter" />
+            <field name="arch" type="xml">
+		<field name="name" position="after">
+		    <field name="zip" />
+		    <field name="code" />
+		</field>
+	    </field>
+	</record>
+    </data>
+</openerp>