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

Merge of #1899 causes problems loading existing invoices #1954

Closed
ehuelsmann opened this issue Aug 1, 2016 · 3 comments
Closed

Merge of #1899 causes problems loading existing invoices #1954

ehuelsmann opened this issue Aug 1, 2016 · 3 comments
Assignees
Labels
type:blocking Deemed "needs to be fixed before the assigned release"; unacceptable regressions or critical bugs type:bug-accounting Causes immediate accounting damage / warrants a release by itself
Milestone

Comments

@ehuelsmann
Copy link
Member

@einhverfr As of the merge of #1899, I can't load existing invoices anymore. I'm getting:

screenshot from 2016-08-01 23 09 08

Note that this is with the simple case of loading a single-line invoice (where the customer doesn't have customer-specific prices, but I think that doesn't matter).

The cause as I have been able to identify it, is that $form->{rowcount} is still '' or undef during the loading of the invoice, which means that the qtycache in the form doesn't get initialized for the specific part -- not even set to 0.

Now, my first reaction was to use // 0; however, when I do that, I ignore the fact that (as per my comment on the code in #1899) that will lead to incorrect calculation of the qty discount (calculation based on zero items) for invoices that should have qty discounts applied.

Is this a relatively easy fix? Or should we back out for the sake of stabilizing 1.5.0?

@ehuelsmann ehuelsmann added the type:regression Functionality that broke over the last release cycle (1.5.x -> 1.5.y or 1.5.x -> 1.6.0) label Aug 1, 2016
@ehuelsmann ehuelsmann added this to the 1.5-rc2 milestone Aug 1, 2016
@einhverfr
Copy link
Member

should be an easy fix. assigning to me.

@einhverfr einhverfr self-assigned this Aug 2, 2016
@einhverfr
Copy link
Member

Actually as I think about this, it is a symptom of a much more serious and deep problem only exposed by my patch.

If price matrix calculations are being applied to invoices when they are loaded from the database, that is bad. We need to make sure that doesnt happen.

@einhverfr einhverfr added type:blocking Deemed "needs to be fixed before the assigned release"; unacceptable regressions or critical bugs type:bug-accounting Causes immediate accounting damage / warrants a release by itself and removed type:regression Functionality that broke over the last release cycle (1.5.x -> 1.5.y or 1.5.x -> 1.6.0) labels Aug 2, 2016
@einhverfr
Copy link
Member

Might be worth making sure that changes to price matrix don't chance invoice prices on saved invoices in 1.4 too......

einhverfr added a commit to einhverfr/LedgerSMB that referenced this issue Aug 2, 2016
@einhverfr einhverfr modified the milestones: 1.4.32, 1.5-rc2 Aug 2, 2016
einhverfr added a commit that referenced this issue Aug 2, 2016
do not call pricematrix on existing order/invoice, fixes #1954, also …
einhverfr added a commit that referenced this issue Aug 2, 2016
do not call pricematrix on existing order/invoice, fixes #1954
ylavoie pushed a commit to ylavoie/LedgerSMB that referenced this issue Aug 4, 2016
ylavoie pushed a commit to ylavoie/LedgerSMB that referenced this issue Aug 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:blocking Deemed "needs to be fixed before the assigned release"; unacceptable regressions or critical bugs type:bug-accounting Causes immediate accounting damage / warrants a release by itself
Projects
None yet
Development

No branches or pull requests

2 participants