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

Check if the lines can be reconciled before to reconcile them #12

Merged
merged 2 commits into from
Feb 6, 2015

Conversation

guewen
Copy link
Member

@guewen guewen commented Sep 15, 2014

Prevents a lot of errors in the logs with reconciles that cannot be done.

Fixes #11

Prevents a lot of errors in the logs with reconciles that cannot be
done.

Fixes OCA#11
Because the 'reconcile' method will refuse to reconcile them anyway
guewen added a commit to guewen/e-commerce that referenced this pull request Sep 15, 2014
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.72%) when pulling 4b4107c on guewen:check-before-reconcile into d5063a5 on OCA:7.0.

@florian-dacosta
Copy link

I think it is good.
But, for the use case explained in the PR #18 , it won't be enought.

Because the method _lines_can_be_reconciled will check the account of 2 account move line coming from the invoice. It will see the account is different and won't try to reconcile.
But in this use case, we do not try to reconcile the account_move_line created for the refund.

I guess it would be great to change _get_sum_invoice_move_line.
We should pass the account of the invoice in the parameters, and then we check the move_lines list.
If the account of the account.move.line differ from the one of the invoice, we delete it from the list.

What do you think?

@guewen
Copy link
Member Author

guewen commented Nov 10, 2014

Your reasoning seems good to me.

@bwrsandman
Copy link

👍

@yvaucher
Copy link
Member

yvaucher commented Feb 6, 2015

👍

yvaucher added a commit that referenced this pull request Feb 6, 2015
Check if the lines can be reconciled before to reconcile them
@yvaucher yvaucher merged commit 29b60ca into OCA:7.0 Feb 6, 2015
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