Merge lp:~acsone-openerp/banking-addons/bank-statement-reconcile-70-import-statement-hook into lp:banking-addons/bank-statement-reconcile-70
- bank-statement-reconcile-70-import-statement-hook
- Merge into bank-statement-reconcile-70
Status: | Merged |
---|---|
Approved by: | Pedro Manuel Baeza |
Approved revision: | 113 |
Merged at revision: | 116 |
Proposed branch: | lp:~acsone-openerp/banking-addons/bank-statement-reconcile-70-import-statement-hook |
Merge into: | lp:banking-addons/bank-statement-reconcile-70 |
Diff against target: |
202 lines (+36/-49) 4 files modified
account_statement_base_completion/statement.py (+4/-5) account_statement_base_import/parser/parser.py (+13/-32) account_statement_base_import/statement.py (+14/-11) account_statement_commission/commission.py (+5/-1) |
To merge this branch: | bzr merge lp:~acsone-openerp/banking-addons/bank-statement-reconcile-70-import-statement-hook |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Pedro Manuel Baeza | Approve | ||
Review via email: mp+203127@code.launchpad.net |
This proposal supersedes a proposal from 2014-01-15.
Commit message
Description of the change
Hi,
In some cases, the imported file may contains information about the account.
The proposal add a new method 'get_st_vals' on the 'BankStatementI
Pedro Manuel Baeza (pedro.baeza) wrote : Posted in a previous version of this proposal | # |
Pedro Manuel Baeza (pedro.baeza) wrote : Posted in a previous version of this proposal | # |
Hi, Laurent,
Checking pending MPs, I have found that there is already an MP that issues this problem in a similar way:
So I'm going to put this as rejected and let the other MP continues. Please review it to contribute with your comments.
Regards.
Laurent Mignon (Acsone) (lmi) wrote : Posted in a previous version of this proposal | # |
> Hi, Laurent,
>
> Checking pending MPs, I have found that there is already an MP that issues
> this problem in a similar way:
>
> https:/
> reconcile-
>
> So I'm going to put this as rejected and let the other MP continues. Please
> review it to contribute with your comments.
>
> Regards.
Hi,
Thanks for the review and the link to the other proposal. I'm a little bit disappointed by your decision. I've the feeling that the proposed approach is more generic that the one proposed by akretion-team.
In the other proposal, it's not the responsibility of the parser to provide additional values to the statement creation. So if I want to provide an other value from the parser to statement (ex the date), I need to override the statement profile. My point of view is that's the responsibility of the parser to provide values for statement and statement line creation. The profile is just there to make the glue between the parser and the logic. What do you think?
Regards
lmi
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote : Posted in a previous version of this proposal | # |
In a way other MP was created and Approved before this one. So disapproving this one seems fair.
However, your changes and improvements are welcome.
Sorry that it wasn't noticed by anybody before. BTW good catch Pedro
Pedro Manuel Baeza (pedro.baeza) wrote : Posted in a previous version of this proposal | # |
Sorry Laurent if I bother you. Please contribute now with a new MP over the merge changes with your improvements, that indeed are good ones! I want to make a new MP tonight, but I think it's better that you make it and get credited.
Regards.
Pedro Manuel Baeza (pedro.baeza) wrote : | # |
Now I see redundant to add two methods for the same purpose: get_st_vals and prepare_
Regards.
Laurent Mignon (Acsone) (lmi) wrote : | # |
Hi Pedro and once again thanks for the review.
> Now I see redundant to add two methods for the same purpose: get_st_vals and
> prepare_
The change mirrors the approach for statement lines with the same approach for the statement.
* on the parser we have 'get_st_line_vals' vs 'get_st_vals'
* on the account.
The motivation behind the 'prepare_
Regards,
Laurent Mignon
Pedro Manuel Baeza (pedro.baeza) wrote : | # |
Hi, I proceed then with the merge. Sorry for the past inconveniences.
Regards.
Preview Diff
1 | === modified file 'account_statement_base_completion/statement.py' | |||
2 | --- account_statement_base_completion/statement.py 2014-01-23 08:44:24 +0000 | |||
3 | +++ account_statement_base_completion/statement.py 2014-01-24 17:16:24 +0000 | |||
4 | @@ -184,7 +184,7 @@ | |||
5 | 184 | inv = self._find_invoice(cr, uid, line, inv_type, context=context) | 184 | inv = self._find_invoice(cr, uid, line, inv_type, context=context) |
6 | 185 | if inv: | 185 | if inv: |
7 | 186 | # FIXME use only commercial_partner_id of invoice in 7.1 | 186 | # FIXME use only commercial_partner_id of invoice in 7.1 |
9 | 187 | # this is for backward compatibility in 7.0 before | 187 | # this is for backward compatibility in 7.0 before |
10 | 188 | # the refactoring of res.partner | 188 | # the refactoring of res.partner |
11 | 189 | if hasattr(inv, 'commercial_partner_id'): | 189 | if hasattr(inv, 'commercial_partner_id'): |
12 | 190 | partner_id = inv.commercial_partner_id.id | 190 | partner_id = inv.commercial_partner_id.id |
13 | @@ -418,7 +418,7 @@ | |||
14 | 418 | """ | 418 | """ |
15 | 419 | statement_line_obj = self.pool['account.bank.statement.line'] | 419 | statement_line_obj = self.pool['account.bank.statement.line'] |
16 | 420 | model_cols = statement_line_obj._columns | 420 | model_cols = statement_line_obj._columns |
18 | 421 | sparse_fields = dict([(k , col) for k, col in model_cols.iteritems() if isinstance(col, fields.sparse) and col._type == 'char']) | 421 | sparse_fields = dict([(k, col) for k, col in model_cols.iteritems() if isinstance(col, fields.sparse) and col._type == 'char']) |
19 | 422 | values = [] | 422 | values = [] |
20 | 423 | for statement in statement_store: | 423 | for statement in statement_store: |
21 | 424 | to_json_k = set() | 424 | to_json_k = set() |
22 | @@ -429,10 +429,9 @@ | |||
23 | 429 | serialized = st_copy.setdefault(col.serialization_field, {}) | 429 | serialized = st_copy.setdefault(col.serialization_field, {}) |
24 | 430 | serialized[k] = st_copy[k] | 430 | serialized[k] = st_copy[k] |
25 | 431 | for k in to_json_k: | 431 | for k in to_json_k: |
27 | 432 | st_copy[k] = simplejson.dumps(st_copy[k]) | 432 | st_copy[k] = simplejson.dumps(st_copy[k]) |
28 | 433 | values.append(st_copy) | 433 | values.append(st_copy) |
29 | 434 | return values | 434 | return values |
30 | 435 | |||
31 | 436 | 435 | ||
32 | 437 | def _insert_lines(self, cr, uid, statement_store, context=None): | 436 | def _insert_lines(self, cr, uid, statement_store, context=None): |
33 | 438 | """ Do raw insert into database because ORM is awfully slow | 437 | """ Do raw insert into database because ORM is awfully slow |
34 | @@ -456,7 +455,7 @@ | |||
35 | 456 | when cheking security. | 455 | when cheking security. |
36 | 457 | TODO / WARM: sparse fields are skipped by the method. IOW, if your | 456 | TODO / WARM: sparse fields are skipped by the method. IOW, if your |
37 | 458 | completion rule update an sparse field, the updated value will never | 457 | completion rule update an sparse field, the updated value will never |
39 | 459 | be stored in the database. It would be safer to call the update method | 458 | be stored in the database. It would be safer to call the update method |
40 | 460 | from the ORM for records updating this kind of fields. | 459 | from the ORM for records updating this kind of fields. |
41 | 461 | """ | 460 | """ |
42 | 462 | cols = self._get_available_columns([vals]) | 461 | cols = self._get_available_columns([vals]) |
43 | 463 | 462 | ||
44 | === modified file 'account_statement_base_import/parser/parser.py' | |||
45 | --- account_statement_base_import/parser/parser.py 2013-12-11 19:11:44 +0000 | |||
46 | +++ account_statement_base_import/parser/parser.py 2014-01-24 17:16:24 +0000 | |||
47 | @@ -20,7 +20,6 @@ | |||
48 | 20 | ############################################################################## | 20 | ############################################################################## |
49 | 21 | import base64 | 21 | import base64 |
50 | 22 | import csv | 22 | import csv |
51 | 23 | from datetime import datetime | ||
52 | 24 | 23 | ||
53 | 25 | 24 | ||
54 | 26 | def UnicodeDictReader(utf8_data, **kwargs): | 25 | def UnicodeDictReader(utf8_data, **kwargs): |
55 | @@ -115,6 +114,19 @@ | |||
56 | 115 | """ | 114 | """ |
57 | 116 | return NotImplementedError | 115 | return NotImplementedError |
58 | 117 | 116 | ||
59 | 117 | def get_st_vals(self): | ||
60 | 118 | """ | ||
61 | 119 | This method return a dict of vals that ca be passed to | ||
62 | 120 | create method of statement. | ||
63 | 121 | :return: dict of vals that represent additional infos for the statement | ||
64 | 122 | """ | ||
65 | 123 | return { | ||
66 | 124 | 'name': self.statement_name, | ||
67 | 125 | 'balance_start': self.balance_start, | ||
68 | 126 | 'balance_end_real': self.balance_end, | ||
69 | 127 | 'date': self.statement_date | ||
70 | 128 | } | ||
71 | 129 | |||
72 | 118 | def get_st_line_vals(self, line, *args, **kwargs): | 130 | def get_st_line_vals(self, line, *args, **kwargs): |
73 | 119 | """ | 131 | """ |
74 | 120 | Implement a method in your parser that must return a dict of vals that can be | 132 | Implement a method in your parser that must return a dict of vals that can be |
75 | @@ -155,37 +167,6 @@ | |||
76 | 155 | self._post(*args, **kwargs) | 167 | self._post(*args, **kwargs) |
77 | 156 | return self.result_row_list | 168 | return self.result_row_list |
78 | 157 | 169 | ||
79 | 158 | def get_start_balance(self, *args, **kwargs): | ||
80 | 159 | """ | ||
81 | 160 | This is called by the importation method to set the balance start | ||
82 | 161 | amount in the bank statement. | ||
83 | 162 | return: float of the balance start (self.balance_start) | ||
84 | 163 | """ | ||
85 | 164 | return self.balance_start | ||
86 | 165 | |||
87 | 166 | def get_end_balance(self, *args, **kwargs): | ||
88 | 167 | """ | ||
89 | 168 | This is called by the importation method to set the balance end | ||
90 | 169 | amount in the bank statement. | ||
91 | 170 | return: float of the balance end (self.balance_end) | ||
92 | 171 | """ | ||
93 | 172 | return self.balance_end | ||
94 | 173 | |||
95 | 174 | def get_statement_name(self, *args, **kwargs): | ||
96 | 175 | """ | ||
97 | 176 | This is called by the importation method to set the statement | ||
98 | 177 | name in the bank statement. | ||
99 | 178 | return: string of the statement name (self.statement_name) | ||
100 | 179 | """ | ||
101 | 180 | return self.statement_name or '/' | ||
102 | 181 | |||
103 | 182 | def get_statement_date(self, *args, **kwargs): | ||
104 | 183 | """ | ||
105 | 184 | This is called by the importation method to set the statement | ||
106 | 185 | date in the bank statement. | ||
107 | 186 | return: datetime of the statement date (self.statement_date) | ||
108 | 187 | """ | ||
109 | 188 | return self.statement_date or datetime.now() | ||
110 | 189 | 170 | ||
111 | 190 | def itersubclasses(cls, _seen=None): | 171 | def itersubclasses(cls, _seen=None): |
112 | 191 | """ | 172 | """ |
113 | 192 | 173 | ||
114 | === modified file 'account_statement_base_import/statement.py' | |||
115 | --- account_statement_base_import/statement.py 2013-12-04 17:39:04 +0000 | |||
116 | +++ account_statement_base_import/statement.py 2014-01-24 17:16:24 +0000 | |||
117 | @@ -90,7 +90,7 @@ | |||
118 | 90 | statement_id, context): | 90 | statement_id, context): |
119 | 91 | """ | 91 | """ |
120 | 92 | Hook to build the values of a line from the parser returned values. At | 92 | Hook to build the values of a line from the parser returned values. At |
122 | 93 | least it fullfill the statement_id and account_id. Overide it to add your | 93 | least it fullfill the statement_id and account_id. Override it to add your |
123 | 94 | own completion if needed. | 94 | own completion if needed. |
124 | 95 | 95 | ||
125 | 96 | :param dict of vals from parser for account.bank.statement.line (called by | 96 | :param dict of vals from parser for account.bank.statement.line (called by |
126 | @@ -126,14 +126,14 @@ | |||
127 | 126 | values['type'] = 'general' | 126 | values['type'] = 'general' |
128 | 127 | return values | 127 | return values |
129 | 128 | 128 | ||
138 | 129 | 129 | def prepare_statement_vals(self, cr, uid, profile_id, result_row_list, parser, context): | |
139 | 130 | def _prepare_statement_vals(self, cr, uid, prof, parser, context=None): | 130 | """ |
140 | 131 | return { | 131 | Hook to build the values of the statement from the parser and |
141 | 132 | 'profile_id': prof.id, | 132 | the profile. |
142 | 133 | 'name': parser.get_statement_name(), | 133 | """ |
143 | 134 | 'balance_start': parser.get_start_balance(), | 134 | vals = {'profile_id': profile_id} |
144 | 135 | 'balance_end_real': parser.get_end_balance(), | 135 | vals.update(parser.get_st_vals()) |
145 | 136 | } | 136 | return vals |
146 | 137 | 137 | ||
147 | 138 | def statement_import(self, cr, uid, ids, profile_id, file_stream, ftype="csv", context=None): | 138 | def statement_import(self, cr, uid, ids, profile_id, file_stream, ftype="csv", context=None): |
148 | 139 | """ | 139 | """ |
149 | @@ -168,8 +168,11 @@ | |||
150 | 168 | _("Column %s you try to import is not " | 168 | _("Column %s you try to import is not " |
151 | 169 | "present in the bank statement line!") % col) | 169 | "present in the bank statement line!") % col) |
152 | 170 | 170 | ||
155 | 171 | st_vals = self._prepare_statement_vals(cr, uid, prof, parser, context=context) | 171 | statement_vals = self.prepare_statement_vals(cr, uid, prof.id, result_row_list, parser, context) |
156 | 172 | statement_id = statement_obj.create(cr, uid, st_vals, context=context) | 172 | statement_id = statement_obj.create(cr, uid, |
157 | 173 | statement_vals, | ||
158 | 174 | context=context) | ||
159 | 175 | |||
160 | 173 | if prof.receivable_account_id: | 176 | if prof.receivable_account_id: |
161 | 174 | account_receivable = account_payable = prof.receivable_account_id.id | 177 | account_receivable = account_payable = prof.receivable_account_id.id |
162 | 175 | else: | 178 | else: |
163 | 176 | 179 | ||
164 | === modified file 'account_statement_commission/commission.py' | |||
165 | --- account_statement_commission/commission.py 2013-12-04 14:14:56 +0000 | |||
166 | +++ account_statement_commission/commission.py 2014-01-24 17:16:24 +0000 | |||
167 | @@ -2,9 +2,11 @@ | |||
168 | 2 | import datetime | 2 | import datetime |
169 | 3 | from openerp.osv import orm, fields | 3 | from openerp.osv import orm, fields |
170 | 4 | 4 | ||
171 | 5 | |||
172 | 5 | def float_or_zero(val): | 6 | def float_or_zero(val): |
173 | 6 | return float(val) if val else 0.0 | 7 | return float(val) if val else 0.0 |
174 | 7 | 8 | ||
175 | 9 | |||
176 | 8 | class AccountStatementProfil(orm.Model): | 10 | class AccountStatementProfil(orm.Model): |
177 | 9 | _inherit = "account.statement.profile" | 11 | _inherit = "account.statement.profile" |
178 | 10 | 12 | ||
179 | @@ -22,7 +24,7 @@ | |||
180 | 22 | commission_analytic_id = profile.commission_analytic_id and profile.commission_analytic_id.id or False | 24 | commission_analytic_id = profile.commission_analytic_id and profile.commission_analytic_id.id or False |
181 | 23 | comm_values = { | 25 | comm_values = { |
182 | 24 | 'name': 'IN ' + _('Commission line'), | 26 | 'name': 'IN ' + _('Commission line'), |
184 | 25 | 'date': parser.get_statement_date(), | 27 | 'date': parser.get_st_vals().get('date') or datetime.datetime.now(), |
185 | 26 | 'amount': global_commission_amount, | 28 | 'amount': global_commission_amount, |
186 | 27 | 'partner_id': partner_id, | 29 | 'partner_id': partner_id, |
187 | 28 | 'type': 'general', | 30 | 'type': 'general', |
188 | @@ -36,6 +38,7 @@ | |||
189 | 36 | statement_line_obj = self.pool.get('account.bank.statement.line') | 38 | statement_line_obj = self.pool.get('account.bank.statement.line') |
190 | 37 | statement_line_obj.create(cr, uid, comm_values, context=context) | 39 | statement_line_obj.create(cr, uid, comm_values, context=context) |
191 | 38 | 40 | ||
192 | 41 | |||
193 | 39 | class AccountStatementLineWithCommission(orm.Model): | 42 | class AccountStatementLineWithCommission(orm.Model): |
194 | 40 | _inherit = "account.bank.statement.line" | 43 | _inherit = "account.bank.statement.line" |
195 | 41 | _columns = { | 44 | _columns = { |
196 | @@ -45,6 +48,7 @@ | |||
197 | 45 | serialization_field='additionnal_bank_fields'), | 48 | serialization_field='additionnal_bank_fields'), |
198 | 46 | } | 49 | } |
199 | 47 | 50 | ||
200 | 51 | |||
201 | 48 | class CreditPartnerStatementImporter(orm.TransientModel): | 52 | class CreditPartnerStatementImporter(orm.TransientModel): |
202 | 49 | _inherit = "credit.statement.import" | 53 | _inherit = "credit.statement.import" |
203 | 50 | 54 |
Hi, Laurent, very interesting and needed changes!
I was to say that you have made the MP to the wrong branch, but you corrected at the moment!
I have checked the code, and the solution seems perfect for Spain, where I'm adopting national standard statement format in 7.0 to these addons, and there is information about start and end date of the statement in the file that must be dumped.
Regards.