Merge lp:~initos.com/partner-contact-management/7.0 into lp:~partner-contact-core-editors/partner-contact-management/7.0

[Thomas Rehn]

I migrated 'base_partner_sequence' from lp:openobject-addons/extra-trunk to OpenERP v7.0. This module seems to fit well with the focus of your branch.

[Pedro Manuel Baeza (pedro.baeza)]

Hi, Thomas, indeed, this is the correct repository!

But we are following some code conventions to increase code quality. Please see this draft document for an explanation of all of them:

http://pad.openerp.com/p/community-review

Basically: PEP8, orm instead osv, not class initialisation, etc.

Regards.

review: Needs Fixing (code review, no test)

[lp:~initos.com/partner-contact-management/7.0]

29. By Thomas Rehn on 2013-11-13

improve adherance to coding conventions

[Thomas Rehn (thomas-rehn)]

Hi Pedro,

Thank you for your suggestions. I hope the new revision is more appropriate for merging.

Best,
 Thomas.

[Pedro Manuel Baeza (pedro.baeza)]

Hi, Thomas, thank you very much for the changes. You can also change on the manifest file (__openerp__.py) the word demo instead demo_xml, remove update_xml, and change 'data' instead 'init_xml'.

Functionally speaking, I see only one problem: when you duplicate a partner, the code is kept on both partners, so you must inherit copy method to fix this. This can be more or less (please check, because I haven't tested and write it very quickly):

    def copy(self, cr, uid, ids, default=None, context=None):
        if default is None:
            default = {}
        default['ref'] = self.pool.get('ir.sequence').get(cr, uid, 'res.partner', context=context)
        return super(res_partner, self).copy(cr, uid, ids, default, context=context)

Regards.

review: Needs Fixing (code review, no test)

[lp:~initos.com/partner-contact-management/7.0]

30. By Thomas Rehn on 2013-11-14

update keys in __openerp__ dictionary

31. By Thomas Rehn on 2013-11-14

implement 'copy' method, fix translation

[Thomas Rehn (thomas-rehn)]

Thank you for your suggestions. I have now added a 'copy' method and cleaned up the __openerp__.py.

[Pedro Manuel Baeza (pedro.baeza)]

Hi, Thomas, sorry for bothering you again, but there is other things you can improve:

- On __init__.py, instead of "import partner", please put "from . import partner".
- On __openerp__.py, you put on description that the sequence is for customer, but the code what makes is assign a sequence to customers, suppliers, users, and any partner that don't have a parent_id assigned. You should do two things:
   a) change description to put that the sequence is assigned to customers and supppliers.
   b) change _needsRef code to restrict the result to only customers and suppliers.
- When you call _needsRef method, you can use keyword arguments for better coding practises:

self._needsRef(cr, uid, id, None, context)

change by

self._needsRef(cr, uid, id=id, context=context)

Thank you for your effort.

Regards.

review: Needs Fixing

[lp:~initos.com/partner-contact-management/7.0]

32. By Thomas Rehn on 2013-11-15

change the scope of sequences to customers and suppliers

[Thomas Rehn (thomas-rehn)]

Thanks again for the hints. The new revision assigns codes to customers and suppliers only. I also clarified the documentation.

[lp:~initos.com/partner-contact-management/7.0]

33. By Thomas Rehn on 2013-11-15

whitespace cleanup

[Pedro Manuel Baeza (pedro.baeza)]

Hi, Thomas,

Thanks for all your effort. I'm going to approve the MP, but for the final merge, it would be welcome to refactorize the few lines that exceed the 79 cols limit for PEP8 compliance.

Regards.

review: Approve

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

Hi Thomas, thanks!

l.176 The get() method of ir.sequence is deprecated. I think 'next_by_code' should be used instead.

l.58 You claim authorship of the module, but I think 'Tiny' was the author. Maybe share the credits like you share the copyright?

l.195 I'm wondering if the condition should be in line with commercial partnership, i.e. maybe partners should qualify if there is no parent, or if the partner has is_company set (instead of being a supplier or a customer). What do think, Thomas and Pedro?

review: Needs Fixing

[Pedro Manuel Baeza (pedro.baeza)]

Hi, Stefan, good catch with next_by_code! I forgot this new call.

About is_company flag, it's very common that someone create a customer or a supplier and don't check this flag, but for the system, it will behave the same way as if you have ticked it, so I vote for stay as is.

Regards.

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

On 11/18/2013 11:26 PM, Pedro Manuel Baeza wrote:
> About is_company flag, it's very common that someone create a customer or a supplier and don't check this flag, but for the system, it will behave the same way as if you have ticked it, so I vote for stay as is.

Thank you for your response. I think if someone creates a customer
without checking the flag 'is_customer', it will usually be a partner
without a parent so it will get assigned a reference according to my
suggestion. So that should not be a problem.

Please be aware that new contacts on an existing partner are a
'customer' as per system default, and 'supplier' if their parent is a
supplier. And you cannot modify these default values in the simple
add-contact popup in the partner form, because this form does not show
the customer/supplier options. That means that at least every new
contact will get assigned a reference, which seems to defeat the purpose
of having the _needRef logic to me.

[Pedro Manuel Baeza (pedro.baeza)]

Hi, Stefan,

I think we are mixing 'is_company' and 'is_customer'/is_supplier meanings. The way it is now, one partner doesn't get sequence number if it's not a customer or a supplier and ** VERY IMPORTANT ** if the parent_id is not empty, so contacts are not assigned a reference for now.

One common scenario is to press on Create button from Customers list, fill data, but not check 'is_company' flag. In this scenario, partner must have a reference, but with your logic it never gets it.

Regards.

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

On 11/19/2013 10:31 AM, Pedro Manuel Baeza wrote:
> I think we are mixing 'is_company' and 'is_customer'/is_supplier
> meanings.

Yes, apparently my use case is for commercial partners, and your use
case is for partners marked as customer/suppliers. I must admit that my
use case is probably incomplete without some additional customization,
such as adding 'ref' to the fields that are copied to the non-commercial
contacts (and making it readonly on these contacts). Maybe this scenario
could be implemented in a separate module. I'll leave this point for now
then.

> One common scenario is to press on Create button from Customers list,
> fill data, but not check 'is_company' flag. In this scenario, partner
> must have a reference, but with your logic it never gets it.
>

What I was suggesting was that that such a partner gets a reference as
long as you do not assign a parent company, as per my commercial partner
use case.

[Pedro Manuel Baeza (pedro.baeza)]

> Yes, apparently my use case is for commercial partners, and your use
> case is for partners marked as customer/suppliers. I must admit that my
> use case is probably incomplete without some additional customization,
> such as adding 'ref' to the fields that are copied to the non-commercial
> contacts (and making it readonly on these contacts). Maybe this scenario
> could be implemented in a separate module. I'll leave this point for now
> then.

OK, I understand you, and yes, I think it's also important to get propagated the reference across children partners to mimic the current logic on partner structure. Maybe we should implement this together in this module to get a definitive module for v7.

> What I was suggesting was that that such a partner gets a reference as
> long as you do not assign a parent company, as per my commercial partner
> use case.

So you are proposing to remove is_customer/is_supplier check?

Regards.

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

Yes, I am suggesting to keep to the commercial partner concept from OpenERP 7.0 (commercial partners are partners without a parent, or partners with is_company set to true), instead of the customer/supplier criterium. While the latter makes sense conceptually, the defaults for these values in OpenERP render these criteria useless IMHO.

I'm tentatively proposing my suggestions into this branch here: https://code.launchpad.net/~therp-nl/partner-contact-management/7.0-base_partner_sequence_commercial_partner/+merge/195742

Sorry Thomas if you feel I am trying to get in the way. Consider it an illustration of the discussion. Feel free to comment.

[Pedro Manuel Baeza (pedro.baeza)]

I have approved the MP to this branch because I see the changes very correct. Thanks for proposing them.

Thomas, please take in consideration this proposition.

Regards.

[lp:~initos.com/partner-contact-management/7.0]

34. By Thomas Rehn on 2013-11-19

merged lp:~therp-nl/partner-contact-management/7.0-base_partner_sequence_commercial_partner

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

Thanks for reviewing and taking my suggestions!

review: Approve

[lp:~initos.com/partner-contact-management/7.0]

35. By Thomas Rehn on 2013-11-26

shortened long lines

[Thomas Rehn (thomas-rehn)]

I have finally managed to shorten the lines so that the code now is PEP-8 compliant.

[Pedro Manuel Baeza (pedro.baeza)]

Thanks for the PEP8! Checking again the code for the final inclusion, I have seen another potential problem: on the line 'if not default.get('ref')...' from the copy, this means that if the original record hasn't got ref, it is assigned a new one, but what we have to do is always assign a new sequence if requirements are met, independently from the old value.

Can you check it, please?

Regards.

review: Needs Fixing

[lp:~initos.com/partner-contact-management/7.0]

36. By Thomas Rehn on 2013-12-02

fix a bug in copying an object with a ref

[Thomas Rehn (thomas-rehn)]

Thanks for the hint, I address this issue in my last commit.

[Pedro Manuel Baeza (pedro.baeza)]

Thank you very much for all your effort.

Regards.

review: Approve (code review and test)

[Holger Brunn (Therp) (hbrunn)]

review: Approve (code review)

=== added directory 'base_partner_sequence'
=== added file 'base_partner_sequence/__init__.py'
--- base_partner_sequence/__init__.py	1970-01-01 00:00:00 +0000
+++ base_partner_sequence/__init__.py	2013-12-02 16:16:05 +0000
@@ -0,0 +1,22 @@
+# -*- encoding: utf-8 -*-
+##############################################################################
+#
+#    OpenERP, Open Source Management Solution
+#    Copyright (C) 2004-2009 Tiny SPRL (<http://tiny.be>).
+#
+#    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 . import partner
+# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:

=== added file 'base_partner_sequence/__openerp__.py'
--- base_partner_sequence/__openerp__.py	1970-01-01 00:00:00 +0000
+++ base_partner_sequence/__openerp__.py	2013-12-02 16:16:05 +0000
@@ -0,0 +1,54 @@
+# -*- encoding: utf-8 -*-
+##############################################################################
+#
+#    OpenERP, Open Source Management Solution
+#    Copyright (C) 2004-2009 Tiny SPRL (<http://tiny.be>).
+#    Copyright (C) 2013 initOS GmbH & Co. KG (<http://www.initos.com>).
+#    Author Thomas Rehn <thomas.rehn at initos.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": "Add a sequence on customers' code",
+    "version": "1.1",
+    "author": "Tiny/initOS GmbH & Co. KG",
+    "category": "Generic Modules/Base",
+    "website": "http://www.initos.com",
+    "depends": ["base"],
+    "summary": "Sets customer's code from a sequence",
+    "description": """
+        This module adds the possibility to define a sequence for
+        the partner code. This code is then set as default when you
+        create a new commercial partner, using the defined sequence.
+
+        The reference field is treated as a commercial field, i.e. it
+        is managed from the commercial partner and then propagated to
+        the partner's contacts. The field is visible on the contacts,
+        but it can only be modified from the commercial partner.
+
+        No codes are assigned for contacts such as shipping and
+        invoice addresses.
+        This module is a migration of the original base_partner_sequence
+        addon to OpenERP version 7.0.
+    """,
+    "data": [
+        'partner_sequence.xml',
+        'partner_view.xml',
+        ],
+    "demo": [],
+    "active": False,
+    "installable": True
+}
+# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:

=== added directory 'base_partner_sequence/i18n'
=== added file 'base_partner_sequence/i18n/base_partner_sequence.pot'
--- base_partner_sequence/i18n/base_partner_sequence.pot	1970-01-01 00:00:00 +0000
+++ base_partner_sequence/i18n/base_partner_sequence.pot	2013-12-02 16:16:05 +0000
@@ -0,0 +1,21 @@
+# Translation of OpenERP Server.
+# This file contains the translation of the following modules:
+#
+msgid ""
+msgstr ""
+"Project-Id-Version: OpenERP Server 7.0-20131013-231025\n"
+"Report-Msgid-Bugs-To: \n"
+"POT-Creation-Date: 2013-11-13 13:47+0000\n"
+"PO-Revision-Date: 2013-11-13 13:47+0000\n"
+"Last-Translator: <>\n"
+"Language-Team: \n"
+"MIME-Version: 1.0\n"
+"Content-Type: text/plain; charset=UTF-8\n"
+"Content-Transfer-Encoding: \n"
+"Plural-Forms: \n"
+
+#. module: base_partner_sequence
+#: model:ir.model,name:base_partner_sequence.model_res_partner
+msgid "Partner"
+msgstr ""
+

=== added file 'base_partner_sequence/i18n/de.po'
--- base_partner_sequence/i18n/de.po	1970-01-01 00:00:00 +0000
+++ base_partner_sequence/i18n/de.po	2013-12-02 16:16:05 +0000
@@ -0,0 +1,21 @@
+# Translation of OpenERP Server.
+# This file contains the translation of the following modules:
+#
+msgid ""
+msgstr ""
+"Project-Id-Version: OpenERP Server 7.0-20131013-231025\n"
+"Report-Msgid-Bugs-To: \n"
+"POT-Creation-Date: 2013-11-13 13:47+0000\n"
+"PO-Revision-Date: 2013-11-13 13:47+0000\n"
+"Last-Translator: <>\n"
+"Language-Team: \n"
+"MIME-Version: 1.0\n"
+"Content-Type: text/plain; charset=UTF-8\n"
+"Content-Transfer-Encoding: \n"
+"Plural-Forms: \n"
+
+#. module: base_partner_sequence
+#: model:ir.model,name:base_partner_sequence.model_res_partner
+msgid "Partner"
+msgstr "Partner"
+

=== added file 'base_partner_sequence/partner.py'
--- base_partner_sequence/partner.py	1970-01-01 00:00:00 +0000
+++ base_partner_sequence/partner.py	2013-12-02 16:16:05 +0000
@@ -0,0 +1,80 @@
+# -*- encoding: utf-8 -*-
+##############################################################################
+#
+#    OpenERP, Open Source Management Solution
+#    Copyright (C) 2004-2009 Tiny SPRL (<http://tiny.be>).
+#    Copyright (C) 2013 initOS GmbH & Co. KG (<http://www.initos.com>).
+#    Author Thomas Rehn <thomas.rehn at initos.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 ResPartner(orm.Model):
+    """Assigns 'ref' from a sequence on creation and copying"""
+
+    _inherit = 'res.partner'
+
+    def create(self, cr, uid, vals, context=None):
+        context = context or {}
+        if not vals.get('ref') and self._needsRef(cr, uid, vals=vals,
+                                                  context=context):
+            vals['ref'] = self.pool.get('ir.sequence')\
+                                   .next_by_code(cr, uid, 'res.partner')
+        return super(ResPartner, self).create(cr, uid, vals, context)
+
+    def copy(self, cr, uid, id, default=None, context=None):
+        default = default or {}
+        if self._needsRef(cr, uid, id=id, context=context):
+            default['ref'] = self.pool.get('ir.sequence')\
+                                      .next_by_code(cr, uid, 'res.partner',
+                                                    context=context)
+        return super(ResPartner, self).copy(cr, uid, id, default,
+                                            context=context)
+
+    def _needsRef(self, cr, uid, id=None, vals=None, context=None):
+        """
+        Checks whether a sequence value should be assigned to a partner's 'ref'
+
+        :param cr: database cursor
+        :param uid: current user id
+        :param id: id of the partner object
+        :param vals: known field values of the partner object
+        :return: true iff a sequence value should be assigned to the\
+                      partner's 'ref'
+        """
+        if not vals and not id:
+            raise Exception('Either field values or an id must be provided.')
+        # only assign a 'ref' to commercial partners
+        if id:
+            vals = self.read(cr, uid, id, ['parent_id', 'is_company'],
+                             context=context)
+        return vals.get('is_company') or not vals.get('parent_id')
+
+    _columns = {
+        'ref': fields.char('Reference', size=64, readonly=True),
+    }
+
+    def _commercial_fields(self, cr, uid, context=None):
+        """
+        Make the partner reference a field that is propagated
+        to the partner's contacts
+        """
+        return super(ResPartner, self)._commercial_fields(
+            cr, uid, context=context) + ['ref']
+
+# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:

=== added file 'base_partner_sequence/partner_sequence.xml'
--- base_partner_sequence/partner_sequence.xml	1970-01-01 00:00:00 +0000
+++ base_partner_sequence/partner_sequence.xml	2013-12-02 16:16:05 +0000
@@ -0,0 +1,21 @@
+<?xml version="1.0"?>
+<openerp>
+<data noupdate="1">
+
+	#
+	# Sequences for res.partner
+	#
+
+	<record model="ir.sequence.type" id="seq_type_res_partner">
+		<field name="name">Partner code</field>
+		<field name="code">res.partner</field>
+	</record>
+	<record model="ir.sequence" id="seq_res_partner">
+		<field name="name">Partner code</field>
+		<field name="code">res.partner</field>
+		<field name="prefix">P/</field>
+		<field name="padding">5</field>
+	</record>
+
+</data>
+</openerp>

=== added file 'base_partner_sequence/partner_view.xml'
--- base_partner_sequence/partner_view.xml	1970-01-01 00:00:00 +0000
+++ base_partner_sequence/partner_view.xml	2013-12-02 16:16:05 +0000
@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<openerp>
+    <data>
+
+        <record id="view_partner_form" model="ir.ui.view">
+            <field name="name">Make partner reference readonly when non-commercial</field>
+            <field name="model">res.partner</field>
+            <field name="inherit_id" ref="base.view_partner_form"/>
+            <field name="arch" type="xml">
+                <field name="ref" position="attributes">
+                    <attribute name="attrs">{
+                    'readonly': [('is_company', '=', False),
+                                 ('parent_id', '!=', False)]}</attribute>
+                </field>
+            </field>
+
+        </record>
+    </data>
+</openerp>
+