Bug 23767 - Spent and Ordered total values don't include child funds on acqui-home
Summary: Spent and Ordered total values don't include child funds on acqui-home
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on: 12987
Blocks: 28223
  Show dependency treegraph
 
Reported: 2019-10-07 17:16 UTC by Nick Clemens
Modified: 2021-04-26 12:43 UTC (History)
13 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.03,20.05.09


Attachments
Bug 23767: Include child funds in total spent and ordered on acqui-home (2.16 KB, patch)
2019-10-07 17:20 UTC, Nick Clemens
Details | Diff | Splinter Review
Total available sum incorrect (23.62 KB, image/png)
2019-10-28 14:36 UTC, Séverine Queune
Details
Bug 23767: Include child funds in total spent and ordered on acqui-home (2.22 KB, patch)
2019-11-12 16:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23767: (QA follow-up) Same treatment for available (1.19 KB, patch)
2019-11-12 16:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23767: Include child funds in total spent and ordered on acqui-home (2.22 KB, patch)
2019-11-26 19:46 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23767: Don't calculate 'Total available' (2.17 KB, patch)
2019-11-26 19:47 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23767: Include child funds in total spent and ordered on acqui-home (2.36 KB, patch)
2019-12-04 10:47 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 23767: Don't calculate 'Total available' (2.31 KB, patch)
2019-12-04 10:47 UTC, Séverine Queune
Details | Diff | Splinter Review
With patch applied (36.39 KB, image/png)
2020-02-09 12:00 UTC, Katrin Fischer
Details
Bug 23767: Total children only if parents not visible (5.84 KB, patch)
2021-02-03 14:53 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23767: Include child funds in total spent and ordered on acqui-home (2.43 KB, patch)
2021-02-03 16:14 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 23767: Total children only if parents not visible (5.91 KB, patch)
2021-02-03 16:14 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 23767: Include child funds in total spent and ordered on acqui-home (2.49 KB, patch)
2021-02-11 10:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23767: Total children only if parents not visible (5.95 KB, patch)
2021-02-11 10:39 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2019-10-07 17:16:51 UTC

    
Comment 1 Nick Clemens 2019-10-07 17:18:55 UTC
To recreate:
1 - Have three funds, two parents and one child
2 - Open a basket and add an order to each fund
3 - View acqui-home.pl
4 - Note that ordered values are only added from the two parent funds
5 - Complete these orders (close basket, receive)
6 - Note the spent only includes parent funds
7 - Place three more orders, one from each fund
8 - Now you can see both spent and ordered are incorrect
Comment 2 Nick Clemens 2019-10-07 17:20:50 UTC
Created attachment 93828 [details] [review]
Bug 23767: Include child funds in total spent and ordered on acqui-home

To test:
 1 - Have three funds, two parents and one child
 2 - Open a basket and add an order to each fund
 3 - View acqui-home.pl
 4 - Note that ordered values are only added from the two parent funds
 5 - Complete these orders (close basket, receive)
 6 - Note the spent only includes parent funds
 7 - Place three more orders, one from each fund
 8 - Now you can see both spent and ordered are incorrect
 9 - Apply patch
10 - Reload, all should add correctly
Comment 3 Jonathan Druart 2019-10-15 08:22:05 UTC
Hi Nick,
There is definitely something wrong on this screen but I am wondering if we should not fix something else as well.
First, do you remember why you linked this bug with bug 12987? I do not understand why they are directly in relation.

Then, take a look at those screenshots:
  https://snipboard.io/grClq9.jpg
  https://snipboard.io/EqYWxU.jpg
The first one is acqui-home, second is aqbudgets.
Cell fund11/Ordered => on one screen we display the ordered value for the fund, on the other one we sum the fund and the child funds.

What do you think? Should not we fix that as well on the same bug report?
Comment 4 Nick Clemens 2019-10-15 12:27:57 UTC
(In reply to Jonathan Druart from comment #3)
> First, do you remember why you linked this bug with bug 12987? I do not
> understand why they are directly in relation.

As far as I can tell, that bug changed footer_column_sum to only add columns with a span with class total_amount.

Prior to this I think the entire column was summed.


> 
> Then, take a look at those screenshots:
>   https://snipboard.io/grClq9.jpg
>   https://snipboard.io/EqYWxU.jpg
> The first one is acqui-home, second is aqbudgets.
> Cell fund11/Ordered => on one screen we display the ordered value for the
> fund, on the other one we sum the fund and the child funds.
> 
> What do you think? Should not we fix that as well on the same bug report?

I find it easier in Acq to handle on thing at a time - and I don't think I see the problem in your screen shot.  Base-level ordered is just from fund 11, Total ordered is fund 11 plus fund 111 (adult plus children) so the total ordered is correct. The total is different on the two screens because one totals two budgets and the other only one
Comment 5 Séverine Queune 2019-10-28 14:35:55 UTC
Hi Nick,

I think the issue also concerns the "Total available" amount.
I attach a screenshot with the patch applied, you can see the child amounts are not ignores in the last sum.
Comment 6 Séverine Queune 2019-10-28 14:36:33 UTC
Created attachment 94811 [details]
Total available sum incorrect
Comment 7 Andrew Fuerste-Henry 2019-11-08 13:12:10 UTC
*** Bug 22329 has been marked as a duplicate of this bug. ***
Comment 8 Martin Renvoize 2019-11-12 16:33:36 UTC
Created attachment 95334 [details] [review]
Bug 23767: Include child funds in total spent and ordered on acqui-home

To test:
 1 - Have three funds, two parents and one child
 2 - Open a basket and add an order to each fund
 3 - View acqui-home.pl
 4 - Note that ordered values are only added from the two parent funds
 5 - Complete these orders (close basket, receive)
 6 - Note the spent only includes parent funds
 7 - Place three more orders, one from each fund
 8 - Now you can see both spent and ordered are incorrect
 9 - Apply patch
10 - Reload, all should add correctly

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2019-11-12 16:33:40 UTC
Created attachment 95335 [details] [review]
Bug 23767: (QA follow-up) Same treatment for available
Comment 10 Séverine Queune 2019-11-13 14:13:43 UTC
Hi Martin,

Something is still bothering me with the 'total available'.
To me, this total should add the 'total available' for parent funds, and the remove the 'ordered' and 'spent' for child funds.
Without the patch, the child amount for 'ordered' and 'spent' are ignored.
With the patch, the child 'total available' is added to the global total.

Not sure to be enough, so you can find here the I got with the patch (problem with the last amount : I've got 10000€ in excess):
PAR1 	Parent 1 	10 000,00 	100,00 	 0,00 	 9 900,00
CHILD1 	Child 1 	 5 000,00 	  0,00 	10,00 	 4 990,00
CHILD2 	Child 2 	 5 000,00 	  1,00 	 0,00 	 4 999,00
PAR2 	Parent 2 	10 000,00 	  0,00 	 0,00 	10 000,00
Total			20 000,00	101,00	10,00	29 889,00
Comment 11 Nick Clemens 2019-11-26 19:46:59 UTC
Created attachment 95840 [details] [review]
Bug 23767: Include child funds in total spent and ordered on acqui-home

To test:
 1 - Have three funds, two parents and one child
 2 - Open a basket and add an order to each fund
 3 - View acqui-home.pl
 4 - Note that ordered values are only added from the two parent funds
 5 - Complete these orders (close basket, receive)
 6 - Note the spent only includes parent funds
 7 - Place three more orders, one from each fund
 8 - Now you can see both spent and ordered are incorrect
 9 - Apply patch
10 - Reload, all should add correctly

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Nick Clemens 2019-11-26 19:47:02 UTC
Created attachment 95841 [details] [review]
Bug 23767: Don't calculate 'Total available'

The acqui-home page is meant to be a simple view of the budgets. For details you should lok at aqbudgets.pl ( i.e. click on a budget)

The total available column isn't easy to calculate - it depends on child funds - trying to calculate what a top level fund has left depends on the values of the child funds. When searching the table and limiting rows you lose the values form the children.

On acqbudgets.pl we solve this with the Base-level/Total available columns, and we don't total base-level on that page at all. 'Total available' on acqui-home equals the 'Base level' on the other page.

Removing a recalculated total is the simplest answer here, otherwise we must add complexity to this view.
Comment 13 Séverine Queune 2019-11-28 17:12:04 UTC
Sorry Nick :(

M	koha-tmpl/intranet-tmpl/prog/en/modules/acqui/acqui-home.tt
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/acqui-home.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/acqui/acqui-home.tt
Failed to merge in the changes.
Patch failed at 0001 Bug 23767: Don't calculate 'Total available'
Comment 14 Nick Clemens 2019-12-02 11:54:43 UTC
Sorry, forgot to obsolete a patch
Comment 15 Séverine Queune 2019-12-04 10:47:53 UTC
Created attachment 95987 [details] [review]
Bug 23767: Include child funds in total spent and ordered on acqui-home

To test:
 1 - Have three funds, two parents and one child
 2 - Open a basket and add an order to each fund
 3 - View acqui-home.pl
 4 - Note that ordered values are only added from the two parent funds
 5 - Complete these orders (close basket, receive)
 6 - Note the spent only includes parent funds
 7 - Place three more orders, one from each fund
 8 - Now you can see both spent and ordered are incorrect
 9 - Apply patch
10 - Reload, all should add correctly

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 16 Séverine Queune 2019-12-04 10:47:56 UTC
Created attachment 95988 [details] [review]
Bug 23767: Don't calculate 'Total available'

The acqui-home page is meant to be a simple view of the budgets. For details you should lok at aqbudgets.pl ( i.e. click on a budget)

The total available column isn't easy to calculate - it depends on child funds - trying to calculate what a top level fund has left depends on the values of the child funds. When searching the table and limiting rows you lose the values form the children.

On acqbudgets.pl we solve this with the Base-level/Total available columns, and we don't total base-level on that page at all. 'Total available' on acqui-home equals the 'Base level' on the other page.

Removing a recalculated total is the simplest answer here, otherwise we must add complexity to this view.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 17 Katrin Fischer 2020-02-09 11:55:52 UTC
I am not totally persuaded by the removal of the 'Total available' column as it works well for libraries not using sub funds (we always recommend not to use them, tbh) and it removes valuable information for them. 

> When searching the table and limiting rows you lose the values form the children
I am not sure how this column is different in this regard to the others that we display the total amount for now?

Basically our main problem is the way that the fund hierarchy works. While we can divide the funds in sub funds, we can spent on all levels. And the parent fund has the full amount to spend (including what's in the sub funds), instead of only the remaining amount (total - sub funds).

At the moment, when you look at the "Totel spend" on the budget page, it will include the spent values of the sub funds. But on the acq hope page, it won't. Same for ordered. I believe, as the columns appear to be the same, this is quite confusing.

If the funds are collapsed, it might look like nothing was spent - even tho the child funds might be almost out of means and the total shows an amount. 

I believe we should fix this the other direction - make it match the 'Total' columns on the budgets page by adding the spent and ordered from the sub funds to the parent funds.

If we do that, I imagine calculating the totals also should get easier as well?
Comment 18 Katrin Fischer 2020-02-09 11:57:07 UTC
Not switching status... I think I could be persuaded with the first patch as a bug fix that is universal (for now), but I still believe there are fundamental issues left.
Comment 19 Katrin Fischer 2020-02-09 12:00:54 UTC
Created attachment 98618 [details]
With patch applied

The 2 in total is correct, but it's not visible/clear where it's coming from.
Comment 20 Katrin Fischer 2020-02-09 13:48:30 UTC
I've now found 16126 and would love if both could be merged/moved forward together. Both sadly lack the total amount, although it's available on the budget page.
Comment 21 Nick Clemens 2021-02-03 14:53:03 UTC
Created attachment 116258 [details] [review]
Bug 23767: Total children only if parents not visible

This patch changes the way to calculate the footer. It adds the budget and parent ids to the cell
as custom data elements. When totaling we grab a list of all the rows we are showing - if a
row has a parent and the parent is showing then we skip adding its value to the total.

As the function is used on both acqui-home and aqbudgets I adjusted both templates

To test:
1 - Follow the test plan on previous patch
2 - Try filtering the table so you see only the child funds
3 - Confirm the totals show the child alone when it is visible
4 - Confirm the child total is excluded when the parent is visible
Comment 22 Andrew Fuerste-Henry 2021-02-03 16:14:43 UTC
Created attachment 116284 [details] [review]
Bug 23767: Include child funds in total spent and ordered on acqui-home

To test:
 1 - Have three funds, two parents and one child
 2 - Open a basket and add an order to each fund
 3 - View acqui-home.pl
 4 - Note that ordered values are only added from the two parent funds
 5 - Complete these orders (close basket, receive)
 6 - Note the spent only includes parent funds
 7 - Place three more orders, one from each fund
 8 - Now you can see both spent and ordered are incorrect
 9 - Apply patch
10 - Reload, all should add correctly

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 23 Andrew Fuerste-Henry 2021-02-03 16:14:46 UTC
Created attachment 116285 [details] [review]
Bug 23767: Total children only if parents not visible

This patch changes the way to calculate the footer. It adds the budget and parent ids to the cell
as custom data elements. When totaling we grab a list of all the rows we are showing - if a
row has a parent and the parent is showing then we skip adding its value to the total.

As the function is used on both acqui-home and aqbudgets I adjusted both templates

To test:
1 - Follow the test plan on previous patch
2 - Try filtering the table so you see only the child funds
3 - Confirm the totals show the child alone when it is visible
4 - Confirm the child total is excluded when the parent is visible

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 24 Martin Renvoize 2021-02-11 10:39:12 UTC
Created attachment 116701 [details] [review]
Bug 23767: Include child funds in total spent and ordered on acqui-home

To test:
 1 - Have three funds, two parents and one child
 2 - Open a basket and add an order to each fund
 3 - View acqui-home.pl
 4 - Note that ordered values are only added from the two parent funds
 5 - Complete these orders (close basket, receive)
 6 - Note the spent only includes parent funds
 7 - Place three more orders, one from each fund
 8 - Now you can see both spent and ordered are incorrect
 9 - Apply patch
10 - Reload, all should add correctly

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 25 Martin Renvoize 2021-02-11 10:39:15 UTC
Created attachment 116702 [details] [review]
Bug 23767: Total children only if parents not visible

This patch changes the way to calculate the footer. It adds the budget and parent ids to the cell
as custom data elements. When totaling we grab a list of all the rows we are showing - if a
row has a parent and the parent is showing then we skip adding its value to the total.

As the function is used on both acqui-home and aqbudgets I adjusted both templates

To test:
1 - Follow the test plan on previous patch
2 - Try filtering the table so you see only the child funds
3 - Confirm the totals show the child alone when it is visible
4 - Confirm the child total is excluded when the parent is visible

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 26 Martin Renvoize 2021-02-11 10:40:23 UTC
This took me a while to get my head back into the game.. all appears to be correct now to me and the QA scripts etc are passing.

Passing QA
Comment 27 Jonathan Druart 2021-02-12 11:18:42 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 28 Fridolin Somers 2021-02-19 14:48:46 UTC
Pushed to 20.11.x for 20.11.03
Comment 29 Andrew Fuerste-Henry 2021-02-22 22:47:18 UTC
Pushed to 20.05.x for 20.05.09
Comment 30 Victor Grousset/tuxayo 2021-02-23 16:03:32 UTC
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.