Bug 12984 - Improvement of the funds list view on acqui home
Summary: Improvement of the funds list view on acqui home
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
: 12360 (view as bug list)
Depends on:
Blocks: 12825 12989
  Show dependency treegraph
 
Reported: 2014-09-24 09:19 UTC by Jonathan Druart
Modified: 2015-06-15 17:59 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 12984: Improvement of the funds list view - acqui home (10.23 KB, patch)
2014-09-24 09:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12984: Improvement of the funds list view - acqui home (10.52 KB, patch)
2014-09-25 11:39 UTC, Paola Rossi
Details | Diff | Splinter Review
Bug 12984: Improvement of the funds list view - acqui home (10.32 KB, patch)
2014-10-10 14:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Child funds included in total (T2 in screenshot) (36.62 KB, image/png)
2014-10-14 15:22 UTC, Katrin Fischer
Details
[PASSED QA] Bug 12984: Improvement of the funds list view - acqui home (10.39 KB, patch)
2014-10-27 22:47 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2014-09-24 09:19:23 UTC
Bug 11578 improved the funds list view in the administration module.
It would be great to have the same improvement on the administration home page.
Comment 1 Jonathan Druart 2014-09-24 09:23:15 UTC Comment hidden (obsolete)
Comment 2 Paola Rossi 2014-09-24 15:32:48 UTC
I've applied the patch against master 3.17.00.025.

1) the title of the table is:
"All available funds for Centerville"

If I add a fund for another Library, this fund is shown too (and is well filtered, too).
If I set another Library, all the funds all listed the same.
If the logged user is the staff user of another library, all the funds all listed the same.

Perhaps the title could be:
"All available funds"

2) Some doubts about two totals at the bottom of the table: in the "Amount" and "Total Available" columns, totals sum the parent funds and the child funds amounts (ERROR).

Since the child fund amount can't exceed parent fund allocation, I think that the "Amount" and "Total Available" totals should only sum parent funds values instead.

On the contrary, in the "Ordered" and "Spent" columns, the totals sum both parent and child funds amounts (RIGHT).
--------------------------------------------------------------------------
On ACQ homepage, the master doesn't distinguish a child fund from a parent one, and so the second error occurs.
--------------------------------------------------------------------------
I pass the patch to "Failed QA" status.
Comment 3 Jonathan Druart 2014-09-24 15:48:05 UTC
Paola, are you sure what you listed are regression?
This patch should only display the funds in a hierarchy and add options to filter them, that's all.
Comment 4 Paola Rossi 2014-09-25 11:39:17 UTC
(In reply to Jonathan Druart from comment #3)
> 
> [...] This patch should only display the funds in a hierarchy and add options to
> filter them, that's all.

OK! 
And the related test is OK.

I pass the patch to "Signed off" status.
Comment 5 Paola Rossi 2014-09-25 11:39:57 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2014-10-10 14:20:15 UTC Comment hidden (obsolete)
Comment 7 Katrin Fischer 2014-10-14 15:19:24 UTC
Fist: I like the new filter options!

This patch applies cleanly on top of bug 12079, but I see problems with the behaviour of the 'summary' line at the bottom and the hide/display of inactive funds:

If I switch between hide/show, the formatting of the price on the last line gets broken for 'hide'. Example:
3431.4 (show)
3 431,40 (hide)

If you look at the amounts, they don't seem to add up correctly for the first (Amount) and last (Total available) columns. I will attach a screenshot. - I would think that the child funds should not be added to the total, as it's a partial of the parent? (if that makes any sense)

If you use the filter on top, the totals still seem to include the 'invisible' funds. 

It would be nice, if the descriptions 'toggled' so you see either 'hide' or 'show' links for the inactive funds and always know what is currently selected. Same for Collapse (no blocker).
Comment 8 Katrin Fischer 2014-10-14 15:22:19 UTC
Created attachment 32325 [details]
Child funds included in total (T2 in screenshot)
Comment 9 Jonathan Druart 2014-10-14 20:10:10 UTC
This patch does not depends on bug 12979.
Comment 10 Katrin Fischer 2014-10-14 20:35:25 UTC
Hi Jonathan, I think that's not true, it has lines referring to the plugin:
             <th class="data"><span class="bu_active">[% total %]</span><span class="bu_inactive" >[% total_active | $Price %]</span></th>
             <th class="data"><span class="bu_active">[% totordered %]</span><span class="bu_inactive" >[% totordered_active | $Price %]</span></th>
             <th class="data"><span class="bu_active">[% totspent %]</span><span class="bu_inactive" >[% totspent_active | $Price %]</span></th>
Comment 11 Katrin Fischer 2014-10-14 20:39:16 UTC
I think i had the bug number wrong - 12844?
Comment 12 Jonathan Druart 2014-10-14 22:53:45 UTC
Yes sorry, actually it does not block bug 12989, but depends on it (and consequently on bug 12844)
Comment 13 Jonathan Druart 2014-10-14 23:02:16 UTC
(In reply to Katrin Fischer from comment #7)
> If I switch between hide/show, the formatting of the price on the last line
> gets broken for 'hide'. Example:
> 3431.4 (show)
> 3 431,40 (hide)

Hum, weird, I don't reproduce.
What is the value of the CurrencyFormat pref (it seems to be US)?
Could you please detail click by click what you did?
Are you sure all deps are applied and you completely refreshed the page?

Actually I inverted the dependency in my last patch set, that's why this one is SO and the dependencies are not SO yet.
Comment 14 Katrin Fischer 2014-10-27 22:46:48 UTC
I retested, it was the dependencies. Ready to pass QA on this now. 

My branch on master as of today looks like this:

Bug 12989: Update table footer with the visible rows - acqui-home
Bug 12984: Improvement of the funds list view - acqui home
Bug 12987: The new format_price include file should be include o~
Bug 12987: Update table footer with the visible rows
Bug 12986: Upgrade the DataTables jQuery plugin to the latest ve~
Bug 12844: <Last patch on master today>
Comment 15 Katrin Fischer 2014-10-27 22:47:28 UTC
Created attachment 32816 [details] [review]
[PASSED QA] Bug 12984: Improvement of the funds list view - acqui home

Bug 11578 improved the funds list view in the administration module.
It would be great to have the same improvement on the acquisition
home page.

This improvement groups funds by budget and displays them with a
hierarchy.

Test plan:
0/ Create a budget and fund hierarchy, with active and inactive budgets.
1/ Go on the acquisition home page and verify the values are the same as
before
2/ Verify the funds are correctly listed
3/ Verify the links on top of table work (expand/collapse all, show/hide
inactive budgets).

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 16 Katrin Fischer 2014-11-10 22:45:59 UTC
*** Bug 12360 has been marked as a duplicate of this bug. ***
Comment 17 Tomás Cohen Arazi 2014-11-11 17:37:16 UTC
Patch pushed to master.

Thanks Jonathan!