Merge lp:~agilebg/account-consolidation/7.0-bug-1296740-elbati into lp:~account-core-editors/account-consolidation/7.0

Proposed by Lorenzo Battistini
Status: Merged
Approved by: Alexandre Fayolle - camptocamp
Approved revision: 21
Merged at revision: 22
Proposed branch: lp:~agilebg/account-consolidation/7.0-bug-1296740-elbati
Merge into: lp:~account-core-editors/account-consolidation/7.0
Diff against target: 29 lines (+0/-5)
1 file modified
account_parallel_currency/account.py (+0/-5)
To merge this branch: bzr merge lp:~agilebg/account-consolidation/7.0-bug-1296740-elbati
Reviewer Review Type Date Requested Status
Alexandre Fayolle - camptocamp code review, test Approve
Ana Juaristi Olalde (community) code review Approve
Yannick Vaucher @ Camptocamp Needs Fixing
Pedro Manuel Baeza code review Approve
Review via email: mp+212487@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

LGTM

Regards.

review: Approve (code review)
Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote :

centralized is defined in account-financial-report/account_financial_report_webkit/account.py

It seams there is no dependancy on it and I think no dependancy should be there.
But simply removing it will break compatibility with account_financial_report_webkit

However, a link module by extracting centralized can be added to ensure account_financial_report_webkit and account_parralel_currency are still compatible.

Can you extract centralized copying in a new module in auto-install when account_financial_report_webkit and account_parralel_currency are installed?

review: Needs Fixing
Revision history for this message
Lorenzo Battistini (elbati) wrote :

Dear Yannick,

I don't think this MP would break compatibility with account_financial_report_webkit.
I installed account_financial_report_webkit and account_parralel_currency and I can use them without problems.

It just doesn't copy the centralized field to the parallel accounts. For that, a link module can be surely done. But I think it has less priority than fixing the linked bug.

Revision history for this message
Ana Juaristi Olalde (ajuaristio) wrote :

LGTM, thanks

review: Approve (code review)
Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

I've reported a separate bug requestion the new module creation.

I'm approving this MP although the tests are failing because they were failing before too (lp:1334645)

Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) :
review: Approve (code review, test)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'account_parallel_currency/account.py'
2--- account_parallel_currency/account.py 2013-09-27 07:51:20 +0000
3+++ account_parallel_currency/account.py 2014-03-24 18:18:16 +0000
4@@ -85,8 +85,6 @@
5 vals['user_type'] = account_vals['user_type']
6 if 'active' in account_vals:
7 vals['active'] = account_vals['active']
8- if 'centralized' in account_vals:
9- vals['centralized'] = account_vals['centralized']
10 if 'parent_id' in account_vals:
11 parent_account = self.browse(cr, uid, account_vals['parent_id'], context)
12 parent_parallel_acc_id = self._search_parallel_account(
13@@ -112,7 +110,6 @@
14 'type': account.type,
15 'user_type': account.user_type and account.user_type.id or False,
16 'active': account.active,
17- 'centralized': account.centralized,
18 })
19 cr.execute("insert into parallel_account_rel(parent_id,child_id) values (%d,%d)"
20 % (account.id, new_id))
21@@ -443,8 +440,6 @@
22 vals['user_type'] = tax_code_vals['user_type']
23 if tax_code_vals.has_key('active'):
24 vals['active'] = tax_code_vals['active']
25- if tax_code_vals.has_key('centralized'):
26- vals['centralized'] = tax_code_vals['centralized']
27 if tax_code_vals.has_key('parent_id'):
28 parent_tax_code = self.browse(cr, uid, tax_code_vals['parent_id'], context)
29 parent_parallel_acc_id = self._search_parallel_tax_code(

Subscribers

People subscribed via source and target branches