ledgersmb / LedgerSMB

Double-entry accounting & ERP for the web
https://ledgersmb.org
Other
413 stars 150 forks source link

Fix cogs calculation for assemblies #8356

Closed ehuelsmann closed 6 days ago

ehuelsmann commented 1 month ago

The root cause of all this is the fact that the sellprice of the assemblies is calculated and stored as a negative number. To make things work going forward, all we had to do was to make the number positive. However, to fix all the historic data, a largish migration script was required which in turn required a new batch and transaction type because none of the existing ones looked particularly useful for this purpose.

Fixes #8351

einhverfr commented 3 weeks ago

Looking through this, it looks like books might have to be open for data to be corrected. Otherwise it might fail or worse.

i think there are two choices here:

  1. announce that books MUST be open and any imbalanced adjustments to compensate in the past might need to be reversed. The reopening and closing could be automated.
  2. Instead only correct to the last closing point of the books.

If it were up to me I would select the second.

It's possible I might be missing something though.

ehuelsmann commented 3 weeks ago

@einhverfr I chose for (2).

Because the correction is created as a batch with a transaction per (affected) month, users will be able to reopen their books and change the date on the transaction and re-save. Then, from there, the batch could be approved.

... If it were not for #8355 ...

ehuelsmann commented 3 weeks ago

For now, I'm not going to address #8351: Doing that will incorrectly loose the COGS relationship with the original posting items, because re-saving deletes the acc_trans lines, without retaining the "invoice_id" values.

ehuelsmann commented 3 weeks ago

Even though there's still community support for 1.10, it's simply too complex to backport all the required commits to fix this in 1.10 (as there's not just this commit, but also the entire infrastructure to report "upgrade actions" to the upgrading admin.

einhverfr commented 3 weeks ago

Looks reasonable to me.