Merge lp:~pedro.baeza/banking-addons/bank-statement-reconcile-70-import_imp into lp:banking-addons/bank-statement-reconcile-70

[Pedro Manuel Baeza]

Improved system to inherit selection of the different import types, because as it was, you have to redefine the field on each extension module, due to OpenERP doesn't allow direct inheritance on selection methods.

Now, the selection method calls another private method that can be inherited without problems.

[lp:~pedro.baeza/banking-addons/bank-statement-reconcile-70-import_imp]

109. By Pedro Manuel Baeza on 2014-01-21

[IMP] account_statement_base_import: Inheritable method with public name.

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

Nice, thanks

review: Approve

[Christophe Combelles (ccomb)]

It looks like a good idea, but it is broken :

return get_import_type selection(cr, uid, context=context)

review: Needs Fixing

[Pedro Manuel Baeza (pedro.baeza)]

Hi, Cristophe, now it's working. Sorry for the mistake.

Regards.

[Christophe Combelles (ccomb)]

*sigh*

review: Disapprove

[lp:~pedro.baeza/banking-addons/bank-statement-reconcile-70-import_imp]

110. By Pedro Manuel Baeza on 2014-02-24

[FIX] account_statement_base_import: self reference calling method.

[Pedro Manuel Baeza (pedro.baeza)]

Please don't disapprove it, because that action is only used when the entire MP can't be merged. This only needed fixing that it's already done.

Regards.

[Christophe Combelles (ccomb)]

I've tried with only one filter module, and it allows to have it appear in the list.
Didn't have time to try with 2 filter modules though.
Renaming the selection field function is clever as it avoid to change all the filter modules around.
However there is no added unit test :'( This should be proposed in another MP maybe.

review: Approve

[Pedro Manuel Baeza (pedro.baeza)]

On each module that adds a selection type, you only have to make this:

        selection = super(AccountStatementProfil, self).get_import_type_selection(cr, uid, context=context)
        selection.append(('-', _('-')))

What is not assured is the order of the appended values.

Regards.

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

LGTM, thanks !

review: Approve (code review, no tests)

=== modified file 'account_statement_base_import/statement.py'
--- account_statement_base_import/statement.py	2014-02-21 18:29:07 +0000
+++ account_statement_base_import/statement.py	2014-02-24 22:45:51 +0000
@@ -32,11 +32,12 @@
     _inherit = "account.statement.profile"
 
     def get_import_type_selection(self, cr, uid, context=None):
-        """
-        Has to be inherited to add parser
-        """
+        """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)
+
     _columns = {
         'launch_import_completion': fields.boolean(
             "Launch completion after import",
@@ -46,12 +47,11 @@
         #  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"
                  "statement for this profile."),
-
     }
 
     def _write_extra_statement_lines(

=== modified file 'account_statement_ofx_import/statement.py'
--- account_statement_ofx_import/statement.py	2013-11-04 12:22:39 +0000
+++ account_statement_ofx_import/statement.py	2014-02-24 22:45:51 +0000
@@ -33,14 +33,3 @@
                                                       context=context)
         selection.append(('ofx_so', _('OFX - Open Financial Exchange')))
         return selection
-
-    _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."),
-
-    }
-