Merge lp:~agilebg/account-consolidation/7.0-fix-1334645-elbati into lp:~account-core-editors/account-consolidation/7.0

[Lorenzo Battistini]

[Leonardo Pistone (lepistone)]

thanks for the fix Lorenzo.

Can you confirm that the test is correct and production code was actually broken?

review: Needs Information

[Lorenzo Battistini (elbati)]

I confirm.
Note that the bug only occurs when you create a new tax code without parent.

[Leonardo Pistone (lepistone)]

Hi Lorenzo,

I am still non convinced :)

This way the test is not actually testing a useful workflow, but just a "return True" workaround.

I'd be happier if the yaml test actually tested a realistic case. (or at least, clarify in the yaml file what's going on)

Thanks!

review: Needs Fixing

[lp:~agilebg/account-consolidation/7.0-fix-1334645-elbati]

24. By Lorenzo Battistini on 2014-07-07

[IMP] when parent_id is missing, just don't fill the parent_id field of the parallel account

[Lorenzo Battistini (elbati)]

lep, right. I looked at it better and saw that if parent_id is missing, I should just not fill the parent_id field of the parallel account.

[Leonardo Pistone (lepistone)]

Thanks Lorenzo.

maybe you could refactor a bit putting

parent_parallel_tax_code_id = False

in an else: block a few lines down? otherwise, lgtm

review: Needs Fixing

[lp:~agilebg/account-consolidation/7.0-fix-1334645-elbati]

25. By Lorenzo Battistini on 2014-07-07

[IMP] else block

[Lorenzo Battistini (elbati)]

On 07/07/2014 03:00 PM, Leonardo Pistone - camptocamp wrote:
> maybe you could refactor a bit putting
>
> parent_parallel_tax_code_id = False
>
> in an else: block a few lines down? otherwise, lgtm

Done

[Leonardo Pistone (lepistone)]

thanks

review: Approve

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

This project is now hosted on https://github.com/OCA/account-consolidation. 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

[Lorenzo Battistini (elbati)]

Moved to https://github.com/OCA/account-consolidation/pull/3

Unmerged revisions

25. By Lorenzo Battistini on 2014-07-07

[IMP] else block

24. By Lorenzo Battistini on 2014-07-07

[IMP] when parent_id is missing, just don't fill the parent_id field of the parallel account

23. By Lorenzo Battistini on 2014-07-02

[FIX] account_parallel_currency: test failure test/mapping_parallel_accounts.yml

=== modified file 'account_parallel_currency/account.py'
--- account_parallel_currency/account.py	2014-03-24 18:16:13 +0000
+++ account_parallel_currency/account.py	2014-07-07 13:07:55 +0000
@@ -454,9 +454,6 @@
     def create_parallel_tax_codes(self, cr, uid, ids, context=None):
         for tax_code in self.browse(cr, SUPERUSER_ID, ids, context):
             for parallel_company in tax_code.company_id.parallel_company_ids:
-                if not tax_code.parent_id:
-                    raise orm.except_orm(_('Error'),_('Tax code %s does not have parent')
-                        % tax_code.code)
                 existing_ids = self.search(cr, SUPERUSER_ID, [
                     ('code', '=', tax_code.code),
                     ('company_id', '=', parallel_company.id),
@@ -465,9 +462,14 @@
                     raise orm.except_orm(_('Error'),
                     _('Tax code %s already exists for company %s')
                     % (tax_code.code, parallel_company.name))
-                parent_parallel_tax_code_id = self._search_parallel_tax_code(
-                    cr, SUPERUSER_ID, tax_code.parent_id.code, parallel_company,
-                    context=context)
+                if tax_code.parent_id:
+                    parent_parallel_tax_code_id = (
+                        self._search_parallel_tax_code(
+                            cr, SUPERUSER_ID, tax_code.parent_id.code,
+                            parallel_company, context=context)
+                        )
+                else:
+                    parent_parallel_tax_code_id = False
                 new_id = self.create(cr, SUPERUSER_ID,{
                     'company_id': parallel_company.id,
                     'parent_id': parent_parallel_tax_code_id,