Merge lp:~akretion-team/banking-addons/bank-statement-reconcile-70-api-improvement into lp:banking-addons/bank-statement-reconcile-70

[Sébastien BEAU - http://www.akretion.com]

Hi
I just made some change so it's easier to inherit the actual method.

[Joël Grand-Guillaume @ camptocamp (jgrandguillaume-c2c)]

Hi,

Thanks for the contrib, LGTM

Regards

review: Approve (code review, no tests)

[Pedro Manuel Baeza (pedro.baeza)]

LGTM

review: Approve (code review)

[Stéphane Bidoul (Acsone) (sbi)]

LGTM.

Perhaps this MP should update the code everywhere _get_function is used (bank_statement_account_completion, account_statement_regex_account_completion, account_statement_so_completion, account_statement_transactionid_completion)?

review: Needs Information (code review)

[Sébastien BEAU - http://www.akretion.com (sebastien.beau)]

Hi Stéphane, I agree it will be great to always use this everywhere in banking addons and better in all OCA module! we can update the MP in this direction.

After thinking twice, and before we change it everywhere. I am not sure that using get_... and _get... is a good idea because the "get_..." method didn't have as signature the "cr, uid, ids...."

Maybe it will be better to use _get and __get ?

def _get_functions(self, cr, uid, context=None):
    """
    List of available methods for rules. Override this to add you own.
    """
    #Add you code here ....

def __get_functions(self, cr, uid, context=None):
        """
        This method can not be inherited call get_functions instead
        """
        return self._get_functions(cr, uid, context=context)

We can use this convention everywhere in OCA double underscore : "__" mean that you can not inherit the method because OpenERP call it directly.

What you you think?

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

Agreed with Sébastien on "__", selection fields and function field should be inheritable and that mean modules with this one as dependency should be changed as well.

review: Needs Fixing (code review)

[lp:~akretion-team/banking-addons/bank-statement-reconcile-70-api-improvement]

104. By Florian da Costa on 2014-05-14

[MERGE]

105. By Florian da Costa on 2014-05-14

[FIX] Change convention for selection fields and function : __function(...)

[Florian da Costa (florian-dacosta)]

I made these changes in all the modules of the branch for the functions get_import_type_selection (account_statement_base_import) and get_functions (account_statement_base_completion)

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

LGTM thanks for the changes

review: Approve (code review, no test)

[Pedro Manuel Baeza (pedro.baeza)]

This project is now hosted on https://github.com/OCA/bank-statement-reconcile. Please move your proposal there. This guide may help you https://github.com/OCA/maintainers-tools/wiki/How-to-move-a-Merge-Proposal-to-GitHub

Unmerged revisions

105. By Florian da Costa on 2014-05-14

[FIX] Change convention for selection fields and function : __function(...)

104. By Florian da Costa on 2014-05-14

[MERGE]

103. By Sébastien BEAU - http://www.akretion.com on 2013-12-04

[REF] add abstract function in order to inherit easily the function in other module.

=== modified file 'account_statement_bankaccount_completion/statement.py'
--- account_statement_bankaccount_completion/statement.py	2013-09-12 08:36:23 +0000
+++ account_statement_bankaccount_completion/statement.py	2014-05-14 11:50:22 +0000
@@ -38,9 +38,6 @@
                     'From bank account number (Normal or IBAN)'))
         return res
 
-    _columns = {
-        'function_to_call': fields.selection(_get_functions, 'Method'),
-    }
 
     def get_from_bank_account(self, cr, uid, st_line, context=None):
         """
 
=== modified file 'account_statement_base_completion/statement.py'
--- account_statement_base_completion/statement.py	2014-05-06 12:22:55 +0000
+++ account_statement_base_completion/statement.py	2014-05-14 11:50:22 +0000
@@ -138,6 +138,12 @@
             ('get_from_label_and_partner_field', 'From line label (based on partner field)'),
             ('get_from_label_and_partner_name', 'From line label (based on partner name)')]
 
+    def __get_functions(self, cr, uid, context=None):
+        """
+        Call method which can be inherited
+        """
+        return self._get_functions(cr, uid, context=context)
+
     _columns = {
         'sequence': fields.integer('Sequence', help="Lower means parsed first."),
         'name': fields.char('Name', size=128),
@@ -145,7 +151,7 @@
             'account.statement.profile',
             rel='as_rul_st_prof_rel',
             string='Related statement profiles'),
-        'function_to_call': fields.selection(_get_functions, 'Method'),
+        'function_to_call': fields.selection(__get_functions, 'Method'),
     }
 
     def _find_invoice(self, cr, uid, st_line, inv_type, context=None):
 
=== modified file 'account_statement_base_import/statement.py'
--- account_statement_base_import/statement.py	2014-05-07 08:50:38 +0000
+++ account_statement_base_import/statement.py	2014-05-14 11:50:22 +0000
@@ -32,12 +32,15 @@
 class AccountStatementProfil(Model):
     _inherit = "account.statement.profile"
 
-    def get_import_type_selection(self, cr, uid, context=None):
+    def _get_import_type_selection(self, cr, uid, context=None):
         """This is the method to be inherited for adding the parser"""
         return [('generic_csvxls_so', 'Generic .csv/.xls based on SO Name')]
 
-    def _get_import_type_selection(self, cr, uid, context=None):
-        return self.get_import_type_selection(cr, uid, context=context)
+    def __get_import_type_selection(self, cr, uid, context=None):
+        """
+        Call method which can be inherited
+        """
+        return self._get_import_type_selection(cr, uid, context=context)
 
     _columns = {
         'launch_import_completion': fields.boolean(
@@ -48,7 +51,7 @@
         #  we remove deprecated as it floods logs in standard/warning level sob...
         'rec_log': fields.text('log', readonly=True),  # Deprecated
         'import_type': fields.selection(
-            _get_import_type_selection,
+            __get_import_type_selection,
             'Type of import',
             required=True,
             help="Choose here the method by which you want to import bank"
 
=== modified file 'account_statement_ofx_import/statement.py'
--- account_statement_ofx_import/statement.py	2014-01-21 01:04:59 +0000
+++ account_statement_ofx_import/statement.py	2014-05-14 11:50:22 +0000
@@ -24,12 +24,12 @@
 class AccountStatementProfil(orm.Model):
     _inherit = "account.statement.profile"
 
-    def get_import_type_selection(self, cr, uid, context=None):
+    def _get_import_type_selection(self, cr, uid, context=None):
         """
         Inherited from parent to add parser.
         """
         selection = super(AccountStatementProfil, self
-                          ).get_import_type_selection(cr, uid,
+                          )._get_import_type_selection(cr, uid,
                                                       context=context)
         selection.append(('ofx_so', _('OFX - Open Financial Exchange')))
         return selection
 
=== modified file 'account_statement_regex_account_completion/statement.py'
--- account_statement_regex_account_completion/statement.py	2014-01-20 09:06:39 +0000
+++ account_statement_regex_account_completion/statement.py	2014-05-14 11:50:22 +0000
@@ -47,7 +47,6 @@
         return res
 
     _columns = {
-        'function_to_call': fields.selection(_get_functions, 'Method'),
         'regex': fields.char('Regular Expression', size=128),
         'account_id': fields.many2one('account.account', string="Account to set"),
     }
 
=== modified file 'account_statement_so_completion/statement.py'
--- account_statement_so_completion/statement.py	2013-12-20 15:45:19 +0000
+++ account_statement_so_completion/statement.py	2014-05-14 11:50:22 +0000
@@ -89,6 +89,3 @@
                 res.update(st_vals)
         return res
 
-    _columns = {
-        'function_to_call': fields.selection(_get_functions, 'Method'),
-    }
 
=== modified file 'account_statement_transactionid_completion/statement.py'
--- account_statement_transactionid_completion/statement.py	2014-01-17 15:07:46 +0000
+++ account_statement_transactionid_completion/statement.py	2014-05-14 11:50:22 +0000
@@ -41,9 +41,6 @@
         ]
         return res
 
-    _columns = {
-        'function_to_call': fields.selection(_get_functions, 'Method'),
-    }
 
     def get_from_transaction_id_and_so(self, cr, uid, st_line, context=None):
         """
 
=== modified file 'account_statement_transactionid_import/statement.py'
--- account_statement_transactionid_import/statement.py	2012-12-20 13:37:01 +0000
+++ account_statement_transactionid_import/statement.py	2014-05-14 11:50:22 +0000
@@ -26,22 +26,13 @@
 class AccountStatementProfil(Model):
     _inherit = "account.statement.profile"
 
-    def get_import_type_selection(self, cr, uid, context=None):
+    def _get_import_type_selection(self, cr, uid, context=None):
         """
         Has to be inherited to add parser
         """
-        res = super(AccountStatementProfil, self).get_import_type_selection(
+        res = super(AccountStatementProfil, self)._get_import_type_selection(
                 cr, uid, context=context)
         res.append(('generic_csvxls_transaction',
                     'Generic .csv/.xls based on SO transaction ID'))
         return res
 
-    _columns = {
-        'import_type': fields.selection(
-            get_import_type_selection,
-            'Type of import',
-            required=True,
-            help="Choose here the method by which you want to import "
-                 "bank statement for this profile."),
-
-    }