Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8.0 account_invoice_merge fixes #220

Merged
merged 1 commit into from
Sep 12, 2017
Merged

Conversation

luc-demeyer
Copy link

This PR contains a couple of fixes and extra units tests for the account_invoice_merge modules:

  • fix python dump when running account_invoice_merge without account_invoice_merge_purchase
  • fix _dirty_check method
  • addition of unit tests to the account_invoice_merge module.
  • fix python dump in case 'sale_order_line_invoice_rel' points to an invoice line that is not present in Sale Order invoices
  • remove obsolete code in do_merge (the population of allinvoices list doesn't make any sense)
  • maintain invoice line sequence in merged invoice
  • extra unit tests in account_invoice_merge_purchase module

@luc-demeyer luc-demeyer changed the title invoice merge fixes 8.0 account_invoice_merge fixes Jan 23, 2017
Copy link

@antoniocanovas antoniocanovas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested in local.
LGTM!!

@kikopeiro
Copy link

This commit fixes a bug merging invoices that ocurrs when the invoices have been generated from a partial picking. I think this comit could be merged, what's the problem?

@kikopeiro
Copy link

👍

Copy link
Member

@pankk pankk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great Luc! Did a code review, installed it and checked the tests on a fresh db, and played around with the invoice merge on a db with real data.
LGTM

@pedrobaeza
Copy link
Member

Conflicts must be solved before merging.

Copy link
Member

@pankk pankk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed changes after resolving conflicts.

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging this after a while.

@pedrobaeza pedrobaeza merged commit e6926b0 into OCA:8.0 Sep 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants