Merge lp:~renatonlima/openerp-fiscal-rules/account_fiscal_position_rule_sale into lp:openerp-fiscal-rules

[Renato Lima - http://www.akretion.com]

- migrated Views
- Code formatting according to PEP-8
- Refactored stock.picking methods

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

In __openerp__.py you can replace the entry points 'init_xml' and 'update_xml' by 'data'.

in __openerp__.py still, the comment:
## WHEN MERGING PLEASE LEAVE THE DEPENDENCY ON delivery BECAUSE OTHERWISE THE ONCHANGE_PARTNER_ID WILL BE TRIGGERED SOMETIMES IN DELIVERY
speak about a dependency on delivery but there is no such dependency. Comment to remove, or dependency to add?

l.81
Are you sure that the key 'context' will always be present in kwargs?
I think that you should default it to {} when the key is not found in kwargs.

review: Needs Fixing (code review, no test)

[lp:~renatonlima/openerp-fiscal-rules/account_fiscal_position_rule_sale]

59. By Renato Lima - http://www.akretion.com on 2013-02-11

Added context default and use domain and removed comment in __openerp__.py

[Renato Lima - http://www.akretion.com (renatonlima)]

Hello Guewen,

I fixed conflict between account_fiscal_position_sale and delivery module, but I forgot remove comment in __openerp__.py

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

Quick note on:
    if not kwargs.get('context', False):

You can just write:
    if not kwargs.get('context'):

Because {}.get() returns None when the key is not found, so it is evaluated to False.

Thanks for the changes: LGTM

review: Approve (code review, no test)

[Niels Huylebroeck (red15)]

Also the big scary warning about leaving the dependency on 'delivery' when it's not even there anymore is a left over that would better be deleted I think ?

[Niels Huylebroeck (red15)]

Nvm, was looking at mail diff which is behind on revisions.

=== modified file 'account_fiscal_position_rule_sale/__init__.py'
--- account_fiscal_position_rule_sale/__init__.py	2012-10-29 13:41:30 +0000
+++ account_fiscal_position_rule_sale/__init__.py	2013-02-11 23:53:21 +0000
@@ -20,5 +20,3 @@
 ###############################################################################
 
 import sale
-
-# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:

=== modified file 'account_fiscal_position_rule_sale/__openerp__.py'
--- account_fiscal_position_rule_sale/__openerp__.py	2012-12-07 21:59:45 +0000
+++ account_fiscal_position_rule_sale/__openerp__.py	2013-02-11 23:53:21 +0000
@@ -25,22 +25,20 @@
     'name': 'Account Fiscal Position Rule Sale',
     'version': '1.1',
     'category': 'Generic Modules/Accounting',
-    'description': """Include a rule to decide the correct fiscal position for Sale""",
+    'description': """Include a rule to decide the
+    correct fiscal position for Sale""",
     'author': 'Akretion',
     'license': 'AGPL-3',
     'website': 'http://www.akretion.com',
-    ## WHEN MERGING PLEASE LEAVE THE DEPENDENCY ON delivery BECAUSE OTHERWISE THE ONCHANGE_PARTNER_ID WILL BE TRIGGERED SOMETIMES IN DELIVERY
-    'depends': ['account_fiscal_position_rule', 'sale'],
-    'init_xml': [],
-    'update_xml':
-                [
-                'sale_view.xml',
-                'security/account_fiscal_position_rule_sale_security.xml',
-                'security/ir.model.access.csv',
-
-                ],
-    'demo_xml': [],
+    'depends': [
+        'account_fiscal_position_rule',
+        'sale',
+    ],
+    'data': [
+        'sale_view.xml',
+        'security/account_fiscal_position_rule_sale_security.xml',
+        'security/ir.model.access.csv',
+    ],
+    'demo': [],
     'installable': True,
 }
-
-# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:

=== modified file 'account_fiscal_position_rule_sale/sale.py'
--- account_fiscal_position_rule_sale/sale.py	2012-12-07 22:05:13 +0000
+++ account_fiscal_position_rule_sale/sale.py	2013-02-11 23:53:21 +0000
@@ -23,82 +23,81 @@
 #
 ###############################################################################
 
-from osv import fields, osv
+from osv import osv
 
 
 class sale_order(osv.Model):
-    _inherit = "sale.order"
-
-    def onchange_partner_id(self, cr, uid, ids, partner_id,
-                            context=None, shop_id=False, **kwargs):
-
-        result = super(sale_order, self).onchange_partner_id(cr, uid, ids,
-                                                             partner_id,
-                                                             context)
-        if not shop_id:
-            return result
-
-        context.update({'use_domain': ('use_sale', '=', True)})
-        obj_shop = self.pool.get('sale.shop').browse(cr, uid, shop_id)
+    _inherit = 'sale.order'
+
+    def _fiscal_position_map(self, cr, uid, result, **kwargs):
+
+        if not kwargs.get('context', False):
+            kwargs['context'] = {}
+
+        kwargs['context'].update({'use_domain': ('use_sale', '=', True)})
+        obj_shop = self.pool.get('sale.shop').browse(
+            cr, uid, kwargs.get('shop_id'))
         company_id = obj_shop.company_id.id
-        partner_invoice_id = result['value'].get('partner_invoice_id', False)
-        partner_shipping_id = result['value'].get('partner_shipping_id', False)
-
-        kwargs.update({
-                       'partner_id': partner_id,
-                       'partner_invoice_id': partner_invoice_id,
-                       'partner_shipping_id': partner_shipping_id,
-                       'company_id': company_id,
-                       'context': context
-                       })
+        kwargs.update({'company_id': company_id})
         fp_rule_obj = self.pool.get('account.fiscal.position.rule')
         return fp_rule_obj.apply_fiscal_mapping(cr, uid, result, kwargs)
 
+    def onchange_partner_id(self, cr, uid, ids, partner_id, context=None):
+
+        if not context:
+            context = {}
+
+        result = super(sale_order, self).onchange_partner_id(
+            cr, uid, ids, partner_id, context)
+
+        if context.get('shop_id', False):
+            return result
+
+        kwargs = context.update({
+            'shop_id': context.get('shop_id', False),
+            'partner_id': partner_id,
+            'partner_invoice_id': result['value'].get('partner_invoice_id', False),
+            'partner_shipping_id': result['value'].get('partner_shipping_id', False),
+            'context': context
+        })
+        return self._fiscal_position_map(cr, uid, result, **kwargs)
+
     def onchange_address_id(self, cr, uid, ids, partner_invoice_id,
                             partner_shipping_id, partner_id,
                             shop_id=False, context=None, **kwargs):
 
+        if not context:
+            context = {}
+
         result = {'value': {}}
         if not shop_id or not partner_invoice_id:
             return result
 
-        obj_shop = self.pool.get('sale.shop').browse(cr, uid, shop_id)
-        company_id = obj_shop.company_id.id
-
-        context.update({'use_domain': ('use_sale', '=', True)})
         kwargs.update({
-                       'partner_id': partner_id,
-                       'partner_invoice_id': partner_invoice_id,
-                       'partner_shipping_id': partner_shipping_id,
-                       'company_id': company_id,
-                       'context': context
-                       })
-        fp_rule_obj = self.pool.get('account.fiscal.position.rule')
-        return fp_rule_obj.apply_fiscal_mapping(cr, uid, result, kwargs)
+            'shop_id': shop_id,
+            'partner_id': partner_id,
+            'partner_invoice_id': partner_invoice_id,
+            'partner_shipping_id': partner_shipping_id,
+            'context': context
+        })
+        return self._fiscal_position_map(cr, uid, result, **kwargs)
 
     def onchange_shop_id(self, cr, uid, ids, shop_id, context=None,
                          partner_id=None, partner_invoice_id=None,
                          partner_shipping_id=None, **kwargs):
 
-        result = super(sale_order, self).onchange_shop_id(cr, uid, ids,
-                                                          shop_id, context)
+        if not context:
+            context = {}
+
+        result = {'value': {}}
         if not shop_id or not partner_id:
             return result
 
-        obj_shop = self.pool.get('sale.shop').browse(cr, uid, shop_id)
-        company_id = obj_shop.company_id.id
-
-        context.update({'use_domain': ('use_sale', '=', True)})
         kwargs.update({
-                       'partner_id': partner_id,
-                       'partner_invoice_id': partner_invoice_id,
-                       'partner_shipping_id': partner_shipping_id,
-                       'company_id': company_id,
-                       'context': context
-                       })
-        fp_rule_obj = self.pool.get('account.fiscal.position.rule')
-        return fp_rule_obj.apply_fiscal_mapping(cr, uid, result, kwargs)
-
-sale_order()
-
-# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:
+            'shop_id': shop_id,
+            'partner_id': partner_id,
+            'partner_invoice_id': partner_invoice_id,
+            'partner_shipping_id': partner_shipping_id,
+            'context': context
+        })
+        return self._fiscal_position_map(cr, uid, result, **kwargs)

=== modified file 'account_fiscal_position_rule_sale/sale_view.xml'
--- account_fiscal_position_rule_sale/sale_view.xml	2012-12-07 21:59:45 +0000
+++ account_fiscal_position_rule_sale/sale_view.xml	2013-02-11 23:53:21 +0000
@@ -25,7 +25,10 @@
                 </field>
                 <field name="partner_id" position="attributes">
                     <attribute name="on_change">
-                        onchange_partner_id(partner_id, context, shop_id)
+                        onchange_partner_id(partner_id, context)
+                    </attribute>
+                    <attribute name="context">
+                        {'shop_id': shop_id}
                     </attribute>
                 </field>
             </field>