Should balance sheet fixes be added to 22.01 release branch?

[Daniel Watford <da...@foomoo.co.uk>]

Hello,

While reading through BalanceSheet.groovy - the script used to prepare data
used during the rendering of the Accounting Balance Sheet report - I found
that some refactoring would make the script easier to read and therefore
created OFBIZ-12753.

While testing the refactored implementation I found OFBIZ-12754, a
previously existing bug, that in some circumstances caused contra account
values to be added to the asset total rather than subtracted. Since this
was causing incorrect results in reporting I thought it would be considered
important enough to fix in release 22.01 and created a patch (PR586) for
review. This patch is not built on top of the refactoring of OFBIZ-12753
since I wasn't sure a bug fix would justify bringing in refactored code
while we are in a feature freeze for 22.01.

However, another pre-existing bug in the balance sheet has since been found
- OFBIZ-12755 - which also causes misreporting of account values in some
circumstances. I have created a fix for trunk (PR 588), built on top of the
refactoring from OFBIZ-12753, but have not yet created a separate patch for
release 22.01 since it will be a completely different implementation.

My questions:-

- Should we fix balance sheet bugs in release 22.01 as it seems these bugs
may have existed for quite some time and were previously unnoticed? I
haven't tested older releases yet, but the code has not changed for a long
time. There is also the possibility that I have just interpreted something
incorrectly and that the bugs I have reported are invalid.

- If we decide to fix the bugs in release 22.01, is it acceptable to bring
in the refactored code to do so? In my opinion the refactoring has made it
easier to reason about the correctness of the resultant balance sheet so I
think bringing the refactoring into 22.01 will result in quicker bug fixes.

Side question:
If the bug reports are valid, why haven't we heard about them already from
the users? I didn't find anything similar in Jira. Could it be the case
that the Accounting Balance Sheet Report is not really used? If so, how are
users preparing their financial statements? Perhaps users export accounting
entries to another accounting tool?

Thanks,

Dan.
-- 
Daniel Watford

Re: Should balance sheet fixes be added to 22.01 release branch?

[Jacques Le Roux <ja...@les7arts.com>]

+1 for 22.01

Jacques

Le 09/02/2023 à 15:55, Jacopo Cappellato a écrit :
> Hello Dan,
>
> On Tue, Feb 7, 2023 at 11:36 PM Daniel Watford <da...@foomoo.co.uk> wrote:
>> [...]
>> - If we decide to fix the bugs in release 22.01, is it acceptable to bring
>> in the refactored code to do so? In my opinion the refactoring has made it
>> easier to reason about the correctness of the resultant balance sheet so I
>> think bringing the refactoring into 22.01 will result in quicker bug fixes.
> Yes, I think it is totally acceptable, considering that the risk of
> regressions in this particular case is rather small.
>
>> Side question:
>> If the bug reports are valid, why haven't we heard about them already from
>> the users? I didn't find anything similar in Jira. Could it be the case
>> that the Accounting Balance Sheet Report is not really used? If so, how are
>> users preparing their financial statements? Perhaps users export accounting
>> entries to another accounting tool?
> It is possible that most users export transactions to external tools;
> others may instead use the script, that is quite basic, as a template
> for a customized version (in which the issues are resolved).
>
> Regards,
>
> Jacopo

Re: Should balance sheet fixes be added to 22.01 release branch?

[Jacopo Cappellato <ja...@gmail.com>]

Hello Dan,

On Tue, Feb 7, 2023 at 11:36 PM Daniel Watford <da...@foomoo.co.uk> wrote:
>
> [...]
> - If we decide to fix the bugs in release 22.01, is it acceptable to bring
> in the refactored code to do so? In my opinion the refactoring has made it
> easier to reason about the correctness of the resultant balance sheet so I
> think bringing the refactoring into 22.01 will result in quicker bug fixes.

Yes, I think it is totally acceptable, considering that the risk of
regressions in this particular case is rather small.

>
> Side question:
> If the bug reports are valid, why haven't we heard about them already from
> the users? I didn't find anything similar in Jira. Could it be the case
> that the Accounting Balance Sheet Report is not really used? If so, how are
> users preparing their financial statements? Perhaps users export accounting
> entries to another accounting tool?

It is possible that most users export transactions to external tools;
others may instead use the script, that is quite basic, as a template
for a customized version (in which the issues are resolved).

Regards,

Jacopo

Re: Should balance sheet fixes be added to 22.01 release branch?

["michael.brohl@ecomify.de" <mi...@ecomify.de>]

+1

Thanks Daniel,
Michael 


%PERSON%
%MARKETING%
%COMPANY%
%LEGAL%

> Am 07.02.2023 um 23:36 schrieb Daniel Watford <da...@foomoo.co.uk>:
> 
> Hello,
> 
> While reading through BalanceSheet.groovy - the script used to prepare data
> used during the rendering of the Accounting Balance Sheet report - I found
> that some refactoring would make the script easier to read and therefore
> created OFBIZ-12753.
> 
> While testing the refactored implementation I found OFBIZ-12754, a
> previously existing bug, that in some circumstances caused contra account
> values to be added to the asset total rather than subtracted. Since this
> was causing incorrect results in reporting I thought it would be considered
> important enough to fix in release 22.01 and created a patch (PR586) for
> review. This patch is not built on top of the refactoring of OFBIZ-12753
> since I wasn't sure a bug fix would justify bringing in refactored code
> while we are in a feature freeze for 22.01.
> 
> However, another pre-existing bug in the balance sheet has since been found
> - OFBIZ-12755 - which also causes misreporting of account values in some
> circumstances. I have created a fix for trunk (PR 588), built on top of the
> refactoring from OFBIZ-12753, but have not yet created a separate patch for
> release 22.01 since it will be a completely different implementation.
> 
> My questions:-
> 
> - Should we fix balance sheet bugs in release 22.01 as it seems these bugs
> may have existed for quite some time and were previously unnoticed? I
> haven't tested older releases yet, but the code has not changed for a long
> time. There is also the possibility that I have just interpreted something
> incorrectly and that the bugs I have reported are invalid.
> 
> - If we decide to fix the bugs in release 22.01, is it acceptable to bring
> in the refactored code to do so? In my opinion the refactoring has made it
> easier to reason about the correctness of the resultant balance sheet so I
> think bringing the refactoring into 22.01 will result in quicker bug fixes.
> 
> Side question:
> If the bug reports are valid, why haven't we heard about them already from
> the users? I didn't find anything similar in Jira. Could it be the case
> that the Accounting Balance Sheet Report is not really used? If so, how are
> users preparing their financial statements? Perhaps users export accounting
> entries to another accounting tool?
> 
> Thanks,
> 
> Dan.
> -- 
> Daniel Watford

Re: Should balance sheet fixes be added to 22.01 release branch?

[Nicolas Malin <ni...@nereide.fr>]

No problem to fix the 22.01 with refactoring.

To use the accounting component on ootb it's hard because you need had 
exactly the same schemes or I never the this configuration.
The potential user that can use it directly is on my view, few, and 
appear more an help to how I will do the mine by the example.

So good reason to improve the code by good refactoring :)

Nicolas

On 07/02/2023 23:34, Daniel Watford wrote:
> Hello,
>
> While reading through BalanceSheet.groovy - the script used to prepare data
> used during the rendering of the Accounting Balance Sheet report - I found
> that some refactoring would make the script easier to read and therefore
> created OFBIZ-12753.
>
> While testing the refactored implementation I found OFBIZ-12754, a
> previously existing bug, that in some circumstances caused contra account
> values to be added to the asset total rather than subtracted. Since this
> was causing incorrect results in reporting I thought it would be considered
> important enough to fix in release 22.01 and created a patch (PR586) for
> review. This patch is not built on top of the refactoring of OFBIZ-12753
> since I wasn't sure a bug fix would justify bringing in refactored code
> while we are in a feature freeze for 22.01.
>
> However, another pre-existing bug in the balance sheet has since been found
> - OFBIZ-12755 - which also causes misreporting of account values in some
> circumstances. I have created a fix for trunk (PR 588), built on top of the
> refactoring from OFBIZ-12753, but have not yet created a separate patch for
> release 22.01 since it will be a completely different implementation.
>
> My questions:-
>
> - Should we fix balance sheet bugs in release 22.01 as it seems these bugs
> may have existed for quite some time and were previously unnoticed? I
> haven't tested older releases yet, but the code has not changed for a long
> time. There is also the possibility that I have just interpreted something
> incorrectly and that the bugs I have reported are invalid.
>
> - If we decide to fix the bugs in release 22.01, is it acceptable to bring
> in the refactored code to do so? In my opinion the refactoring has made it
> easier to reason about the correctness of the resultant balance sheet so I
> think bringing the refactoring into 22.01 will result in quicker bug fixes.
>
> Side question:
> If the bug reports are valid, why haven't we heard about them already from
> the users? I didn't find anything similar in Jira. Could it be the case
> that the Accounting Balance Sheet Report is not really used? If so, how are
> users preparing their financial statements? Perhaps users export accounting
> entries to another accounting tool?
>
> Thanks,
>
> Dan.