[10.0][mig] account_invoice_merge #238

LoisRForgeFlow · 2017-03-16T15:21:53Z

Task done in this migration:

Replace openerp tags and import by odoo.
Update README.
Adapt header of the files to new template.
Correct typo error in wizard view.
Remove the use of workflow in the invoice.
Squash commit history.

adrienpeiffer · 2017-04-03T09:46:26Z

account_invoice_merge/models/account_invoice.py

            'partner_bank_id': invoice.partner_bank_id.id,
        }

    @api.multi
-    def do_merge(self, keep_references=True, date_invoice=False):
+    def do_merge(self, keep_references=True, date_invoice=False,
+                 remove_empty_invoice_lines=True):


Where this new option is used

That feature was added by @alexis-via

A fact is that it's by default 'True' so the corresponding part of code is always executed. But my guess is that it is added as an option for more customization possibilities in other modules.

adrienpeiffer

Just one question. Otherwise 👍 (Code review and functional testing)

chienandalu

tested functionality 👍

pedrobaeza · 2017-04-25T09:18:22Z

I think tests for this module are very needed.

JordiBForgeFlow · 2017-04-25T10:09:58Z

@mreficent can you add unit tests to this one?

LoisRForgeFlow · 2017-04-26T16:50:31Z

Thank you @mreficent! This is ready to merge, right?

cc @pedrobaeza @jbeficent

pedrobaeza · 2017-04-26T16:55:31Z

Can you make a bit more asserts on the tests (like checking the number of lines in the resulting invoice, check that 2 invoices with different partner are not merged...)? If it's too much, I understand, and as a first test version can serve.

sergio-teruel · 2017-04-26T19:50:00Z

@lreficent This is not ready for merge...
Can you rebase to review please??

LoisRForgeFlow · 2017-04-27T08:11:15Z

@pedrobaeza We'll try to improve a bit. 👍

@sergio-teruel well, all test are green, there are no conflicts, runbot is working... I don't see the problem to review it. Anyway, it was for sure rebased yesterday since @mreficent squashed the commits (for what you need to do a rebase).

sergio-teruel · 2017-04-27T09:19:24Z

@lreficent This module not works well. When I merge a two invoices from one sale the sale line qty_invoiced field is set to zero, so i can invoice this sale order.
I try do a reviw but the code is not in your diffs

sergio-teruel · 2017-04-27T09:19:41Z

@chienandalu Can test??

carlosdauden · 2017-04-27T09:41:27Z

@lreficent ¿Can you include this modification?:
#262
Thanks

MiquelRForgeFlow · 2017-04-27T11:19:19Z

Added the suggested tests by @pedrobaeza and some more.
Solves sales object issue.
Added OCA icon and summary info.

sergio-teruel · 2017-04-27T16:08:48Z

@mreficent You can not marge invoice lines thatn come in from different order_lines, if you do that the qty_invoiced field is wrong.
I attach images for better explanation:
I think that if invoice has a sale order you do not merge lines

MiquelRForgeFlow · 2017-04-28T13:27:20Z

@sergio-teruel Ok, we will work in this issue.

sergio-teruel · 2017-04-28T15:30:12Z

@mreficent perfect... thanks!! 😄

MiquelRForgeFlow · 2017-05-05T16:20:34Z

account_invoice_merge/models/account_invoice.py

            'product_id', 'account_id', 'account_analytic_id',
+            'uom_id'
        ]
        for field in ['analytics_id']:


The 'analytics_id' field was added by account_analytic_plans base module. But this base module was removed in v9. So this field is not anymore in versions 9.0 and 10.0

MiquelRForgeFlow

LGTM! Please, review and then I will squash.
@lreficent @sergio-teruel @pedrobaeza

LoisRForgeFlow · 2017-05-11T13:48:13Z

LGTM 👍

sergio-teruel · 2017-05-23T09:00:11Z

@lreficent Can we add the context key "is_merge" whan cancel the old invoices??

Move variable to argument of method Update version number to 9.0 Update to recent OCA standards

rafaelbn · 2017-05-24T14:23:52Z

Hi @mreficent last commit 9cc6a1c introduce a travis error. Do you know why?

MiquelRForgeFlow · 2017-05-24T14:29:59Z

It's not from last commit. It's from #254 and their tests.

rafaelbn · 2017-05-24T15:21:43Z

@luismontalba could you please review #238 (comment) thanks!

pedrobaeza · 2017-05-24T15:35:51Z

@mreficent I think the tests are failing because of your additions in this PR. Just run Travis again on 10.0 branch, and if it's green, then your module is breaking correct inheritance and it's reflected in any of the modules tests.

rafaelbn

Functionally tested in runbot 👍
But travis should be fixed

MiquelRForgeFlow · 2017-05-25T08:20:55Z

@pedrobaeza The account_invoice_refund_link tests are not failing by this branch. The Travis on 10.0 branch is failing already. https://travis-ci.org/OCA/account-invoicing/builds/234196352

gurneyalex · 2017-05-30T07:45:39Z

@mreficent @rafaelbn #269 fixes Travis for the 10.0 branch. Please review 😸

gurneyalex · 2017-05-31T07:37:58Z

triggering a rebuild

adrienpeiffer · 2017-05-31T09:05:06Z

account_invoice_merge/models/account_invoice.py

        ]
-        for field in ['analytics_id']:


Why do you replace 'analytics_id' instead of adding the new field in this list ?

The 'analytics_id' field was added by account_analytic_plans base module. But this base module was removed in v9.

Thanks for the explanation @mreficent

adrienpeiffer · 2017-05-31T10:04:36Z

@mreficent is it make sense to squash your last commits ?

LoisRForgeFlow force-pushed the 10.0-mig-account_invoice_merge branch 2 times, most recently from 81c19d4 to dc4b273 Compare March 16, 2017 15:24

pedrobaeza mentioned this pull request Mar 17, 2017

Migration to version 10.0 #194

Closed

34 tasks

adrienpeiffer reviewed Apr 3, 2017

View reviewed changes

adrienpeiffer approved these changes Apr 3, 2017

View reviewed changes

adrienpeiffer mentioned this pull request Apr 3, 2017

[MIG] Migration of account_invoice_merge_purchase to 10.0 #251

Merged

1 task

chienandalu approved these changes Apr 25, 2017

View reviewed changes

MiquelRForgeFlow force-pushed the 10.0-mig-account_invoice_merge branch 2 times, most recently from fc7530f to 7b83526 Compare April 26, 2017 16:06

MiquelRForgeFlow force-pushed the 10.0-mig-account_invoice_merge branch from a96deaf to 0f4d0a5 Compare April 27, 2017 11:21

MiquelRForgeFlow suggested changes May 5, 2017

View reviewed changes

MiquelRForgeFlow approved these changes May 5, 2017

View reviewed changes

benwillig mentioned this pull request May 8, 2017

[10.0][MIG] account_invoice_merge_payment to 10.0 #264

Merged

MiquelRForgeFlow mentioned this pull request May 19, 2017

[9.0][FIX] account_invoice_merge: Invoices not related with sale order #262

Merged

account_invoice_merge: Migration to 9.0

0bece87

alexis-via and others added 2 commits May 24, 2017 12:01

Use float_is_zero instead of != 0

eadf07e

Move variable to argument of method Update version number to 9.0 Update to recent OCA standards

OCA Transbot updated translations from Transifex

394d8cd

MiquelRForgeFlow force-pushed the 10.0-mig-account_invoice_merge branch from 4e5896c to 9cc6a1c Compare May 24, 2017 10:02

MiquelRForgeFlow mentioned this pull request May 24, 2017

[10.0][MIG] account_invoice_refund_link #254

Merged

LoisRForgeFlow mentioned this pull request May 24, 2017

[10.0][IMP] account_invoice_merge: Add context key is_merge when cancelling old invoices ForgeFlow/account-invoicing#2

Closed

rafaelbn approved these changes May 24, 2017

View reviewed changes

gurneyalex closed this May 31, 2017

gurneyalex reopened this May 31, 2017

adrienpeiffer reviewed May 31, 2017

View reviewed changes

adrienpeiffer added the approved label May 31, 2017

adrienpeiffer added the question label May 31, 2017

MiquelRForgeFlow force-pushed the 10.0-mig-account_invoice_merge branch 2 times, most recently from d167354 to 3265d99 Compare May 31, 2017 14:10

adrienpeiffer removed the question label May 31, 2017

MiquelRForgeFlow force-pushed the 10.0-mig-account_invoice_merge branch from 3265d99 to 6d1baae Compare May 31, 2017 14:30

[MIG] account_invoice_merge to v10

e356799

MiquelRForgeFlow force-pushed the 10.0-mig-account_invoice_merge branch from 6d1baae to e356799 Compare May 31, 2017 14:31

adrienpeiffer merged commit ce9b888 into OCA:10.0 May 31, 2017

LoisRForgeFlow deleted the 10.0-mig-account_invoice_merge branch June 13, 2017 15:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[10.0][mig] account_invoice_merge #238

[10.0][mig] account_invoice_merge #238

LoisRForgeFlow commented Mar 16, 2017 •

edited

Loading

adrienpeiffer Apr 3, 2017

LoisRForgeFlow Apr 3, 2017

adrienpeiffer left a comment •

edited

Loading

chienandalu left a comment

pedrobaeza commented Apr 25, 2017

JordiBForgeFlow commented Apr 25, 2017

LoisRForgeFlow commented Apr 26, 2017

pedrobaeza commented Apr 26, 2017

sergio-teruel commented Apr 26, 2017 •

edited

Loading

LoisRForgeFlow commented Apr 27, 2017

sergio-teruel commented Apr 27, 2017

sergio-teruel commented Apr 27, 2017

carlosdauden commented Apr 27, 2017

MiquelRForgeFlow commented Apr 27, 2017

sergio-teruel commented Apr 27, 2017

MiquelRForgeFlow commented Apr 28, 2017

sergio-teruel commented Apr 28, 2017

MiquelRForgeFlow May 5, 2017

MiquelRForgeFlow left a comment

LoisRForgeFlow commented May 11, 2017

sergio-teruel commented May 23, 2017

rafaelbn commented May 24, 2017

MiquelRForgeFlow commented May 24, 2017

rafaelbn commented May 24, 2017

pedrobaeza commented May 24, 2017

rafaelbn left a comment

MiquelRForgeFlow commented May 25, 2017 •

edited

Loading

gurneyalex commented May 30, 2017

gurneyalex commented May 31, 2017

adrienpeiffer May 31, 2017

MiquelRForgeFlow May 31, 2017

adrienpeiffer May 31, 2017

adrienpeiffer commented May 31, 2017

[10.0][mig] account_invoice_merge #238

[10.0][mig] account_invoice_merge #238

Conversation

LoisRForgeFlow commented Mar 16, 2017 • edited Loading

adrienpeiffer Apr 3, 2017

Choose a reason for hiding this comment

LoisRForgeFlow Apr 3, 2017

Choose a reason for hiding this comment

adrienpeiffer left a comment • edited Loading

Choose a reason for hiding this comment

chienandalu left a comment

Choose a reason for hiding this comment

pedrobaeza commented Apr 25, 2017

JordiBForgeFlow commented Apr 25, 2017

LoisRForgeFlow commented Apr 26, 2017

pedrobaeza commented Apr 26, 2017

sergio-teruel commented Apr 26, 2017 • edited Loading

LoisRForgeFlow commented Apr 27, 2017

sergio-teruel commented Apr 27, 2017

sergio-teruel commented Apr 27, 2017

carlosdauden commented Apr 27, 2017

MiquelRForgeFlow commented Apr 27, 2017

sergio-teruel commented Apr 27, 2017

MiquelRForgeFlow commented Apr 28, 2017

sergio-teruel commented Apr 28, 2017

MiquelRForgeFlow May 5, 2017

Choose a reason for hiding this comment

MiquelRForgeFlow left a comment

Choose a reason for hiding this comment

LoisRForgeFlow commented May 11, 2017

sergio-teruel commented May 23, 2017

rafaelbn commented May 24, 2017

MiquelRForgeFlow commented May 24, 2017

rafaelbn commented May 24, 2017

pedrobaeza commented May 24, 2017

rafaelbn left a comment

Choose a reason for hiding this comment

MiquelRForgeFlow commented May 25, 2017 • edited Loading

gurneyalex commented May 30, 2017

gurneyalex commented May 31, 2017

adrienpeiffer May 31, 2017

Choose a reason for hiding this comment

MiquelRForgeFlow May 31, 2017

Choose a reason for hiding this comment

adrienpeiffer May 31, 2017

Choose a reason for hiding this comment

adrienpeiffer commented May 31, 2017

LoisRForgeFlow commented Mar 16, 2017 •

edited

Loading

adrienpeiffer left a comment •

edited

Loading

sergio-teruel commented Apr 26, 2017 •

edited

Loading

MiquelRForgeFlow commented May 25, 2017 •

edited

Loading