Bug 11578

Summary: Improvement of the funds list view
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: AcquisitionsAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: gmcharlt, jean-manuel.broust, jsasse, katrin.fischer, kyle, m.de.rooy, marjorie.barry-vila, martin.renvoize, mtj, nengard, rkopaczka
Version: Main   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12984
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10577
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 11763, 12196, 12955    
Attachments: Bug 11578: Upgrade the treetable jquery plugin
Bug 11578: Improve the funds list view
Bug 11578: Add some css
Funds list view
screenshot after applying all patches
Bug 11578: Hide inactive budgets by default
Bug 11578: Align budget names
Bug 11578: Upgrade the treetable jquery plugin
Bug 11578: Improve the funds list view
Bug 11578: Add some css
Bug 11578: Hide inactive budgets by default
Bug 11578: Align budget names
Bug 11578: Hide filters if a budget (budgetperiod) is chosen
Bug 11578: Improve the funds list view
Bug 11578: Add some css
Bug 11578: Hide inactive budgets by default
Bug 11578: Align budget names
Bug 11578: Hide filters if a budget (budgetperiod) is chosen
Bug 11578: QA follow-up
Bug 11578: QA follow-up
Bug 11578: QA follow-up
[PASSED QA] Bug 11578: Upgrade the treetable jquery plugin
[PASSED QA] Bug 11578: Improve the funds list view
[PASSED QA] Bug 11578: Add some css
[PASSED QA] Bug 11578: Hide inactive budgets by default
[PASSED QA] Bug 11578: Align budget names
[PASSED QA] Bug 11578: Hide filters if a budget (budgetperiod) is chosen
[PASSED QA] Bug 11578: QA follow-up
Bug 11578: FIX show inactive link
Bug 11578: FIX funds grouping
Bug 11578: FIX funds grouping
Bug 11578: FIX budgets grouping

Description Jonathan Druart 2014-01-17 14:26:32 UTC
The funds list is not ergonomic and should be improved.
Comment 1 Jonathan Druart 2014-01-17 14:37:16 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2014-01-17 14:37:21 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2014-01-17 14:37:26 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2014-01-17 14:40:08 UTC
Created attachment 24508 [details]
Funds list view
Comment 5 Nicole C. Engard 2014-02-10 16:07:47 UTC
Created attachment 25175 [details]
screenshot after applying all patches

I like the idea but there are a few issues.  First the inactive Budgets are showing.  Next I have only 1 2011 budget but for some reason it shows 2 times... also I don't see the budget headings as clear dividers. They kind of blend in by having the white background.
Comment 6 Jonathan Druart 2014-02-14 09:41:53 UTC
(In reply to Nicole C. Engard from comment #5)

> I like the idea but there are a few issues.  First the inactive Budgets are
> showing.  Next I have only 1 2011 budget but for some reason it shows 2
> times... also I don't see the budget headings as clear dividers. They kind
> of blend in by having the white background.

I cannot reproduce your issues. Are you sure you clean your cache before testing?
I asked Julian to test and he didn't reproduce neither.
Comment 7 Jonathan Druart 2014-02-14 09:42:18 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2014-02-14 09:42:23 UTC Comment hidden (obsolete)
Comment 9 Owen Leonard 2014-02-14 14:09:43 UTC
(In reply to Nicole C. Engard from comment #5)

> also I don't see the budget headings as clear dividers. They kind
> of blend in by having the white background.

This is because of cached CSS. Shift-reload to get the correct view.
Comment 10 jmbroust 2014-03-12 08:37:11 UTC
<h1>Something went wrong !</h1>Applying: Bug 11578: Upgrade the treetable
jquery plugin
Applying: Bug 11578: Improve the funds list view
Using index info to reconstruct a base tree...
<stdin>:476: trailing whitespace.
[% ELSIF ( budget.remaining_neg ) %]
warning: 1 line adds whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt
CONFLICT (content): Merge conflict in
koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt
Failed to merge in the changes.
Patch failed at 0001 Bug 11578: Improve the funds list view
When you have resolved this problem run git bz apply --continue.
If you would prefer to skip this patch, instead run git bz apply --skip.
To restore the original branch and stop patching run git bz apply --abort.
Bug 11578 - Improvement of the funds list view

24505 - Bug 11578: Upgrade the treetable jquery plugin
24506 - Bug 11578: Improve the funds list view
24507 - Bug 11578: Add some css
25245 - Bug 11578: Hide inactive budgets by default
25246 - Bug 11578: Align budget names
Comment 11 Jonathan Druart 2014-03-13 10:29:26 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2014-03-13 10:29:34 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2014-03-13 10:29:40 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2014-03-13 10:29:45 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2014-03-13 10:29:51 UTC Comment hidden (obsolete)
Comment 16 Marcel de Rooy 2014-04-17 07:08:06 UTC
If this gets signed off, please remove the title marker from 11763.
Comment 17 Kyle M Hall 2014-04-18 13:41:58 UTC
Not sure what's going on, but I can see my 5 funds in master, but not with this patch. The datatable shows this: No entries to show (filtered from 5 total entries)
Comment 18 Jonathan Druart 2014-04-22 14:18:05 UTC
Kyle,
Thanks for testing.
With this patch, inactive funds are hidden by default. Which does not make sense if a budget is chosen.
I will provide a followup.
Comment 19 Jonathan Druart 2014-04-22 14:19:05 UTC Comment hidden (obsolete)
Comment 20 Katrin Fischer 2014-04-27 19:36:45 UTC
I am sorry, but there are a lot of conflicts in aqbudgets.tt:

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 11578: Upgrade the treetable jquery plugin
Applying: Bug 11578: Improve the funds list view
/home/katrin/kohaclone/.git/rebase-apply/patch:465: trailing whitespace.
    [% ELSIF ( budget.remaining_neg ) %] 
warning: 1 line adds whitespace errors.
Using index info to reconstruct a base tree...
M	C4/Budgets.pm
M	admin/aqbudgets.pl
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt
<stdin>:465: trailing whitespace.
    [% ELSIF ( budget.remaining_neg ) %] 
warning: 1 line applied after fixing whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt
Auto-merging admin/aqbudgets.pl
Auto-merging C4/Budgets.pm
Failed to merge in the changes.
Comment 21 Jonathan Druart 2014-04-28 09:03:09 UTC Comment hidden (obsolete)
Comment 22 Jonathan Druart 2014-04-28 09:03:14 UTC Comment hidden (obsolete)
Comment 23 Jonathan Druart 2014-04-28 09:03:19 UTC Comment hidden (obsolete)
Comment 24 Jonathan Druart 2014-04-28 09:03:24 UTC Comment hidden (obsolete)
Comment 25 Jonathan Druart 2014-04-28 09:03:28 UTC Comment hidden (obsolete)
Comment 26 Jonathan Druart 2014-04-28 09:04:01 UTC
Last patches fix conflict with bug 7308.
Comment 27 Martin Renvoize 2014-04-28 16:23:33 UTC
Sorry Jonathan, this is failing the QA script:

 FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt
   OK     forbidden patterns
   FAIL   tt_valid
                lines 310
   OK     valid_template
Comment 28 Jonathan Druart 2014-04-28 17:54:47 UTC Comment hidden (obsolete)
Comment 29 Katrin Fischer 2014-04-28 22:07:51 UTC
I have started reviewing this, just a first note:

1) Add new library to about page
+ * jQuery treetable Plugin 3.1.0
+ * http://ludo.cubicphuse.nl/jquery-treetable
Comment 30 Jonathan Druart 2014-04-29 07:21:55 UTC Comment hidden (obsolete)
Comment 31 Jonathan Druart 2014-04-29 08:40:52 UTC Comment hidden (obsolete)
Comment 32 Martin Renvoize 2014-04-29 09:02:03 UTC
Created attachment 27763 [details] [review]
[PASSED QA] Bug 11578: Upgrade the treetable jquery plugin

This patch upgrade the treetable jquery plugin to the last version
(3.1.0).
It is only used on the funds list view.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 33 Martin Renvoize 2014-04-29 09:02:10 UTC
Created attachment 27764 [details] [review]
[PASSED QA] Bug 11578: Improve the funds list view

The current funds list view does not allow to search in the table and
the ergonomics of the page is quite bad.

This patch add the datatables plugin combined to the treetable plugin in
order to offer a better view of the budgets/funds.

Test plan:
- Verify there is no regression on this page: try to add/modify/delete a
fund and a budget.
- Verify the funds hierarchie is correctly displayed.
- Filter the funds using the branch and the budget filters.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 34 Martin Renvoize 2014-04-29 09:02:17 UTC
Created attachment 27765 [details] [review]
[PASSED QA] Bug 11578: Add some css

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 35 Martin Renvoize 2014-04-29 09:02:24 UTC
Created attachment 27766 [details] [review]
[PASSED QA] Bug 11578: Hide inactive budgets by default

This patch hides inactive budgets by default.
They can be shown/hidden using 2 new links "Hide/Show inactive budgets".

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 36 Martin Renvoize 2014-04-29 09:02:32 UTC
Created attachment 27767 [details] [review]
[PASSED QA] Bug 11578: Align budget names

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 37 Martin Renvoize 2014-04-29 09:02:40 UTC
Created attachment 27768 [details] [review]
[PASSED QA] Bug 11578: Hide filters if a budget (budgetperiod) is chosen

If a budget_period_id is given, the filters should not be displayed.
Moreover, it does not make sense to display the links for
displaying/hiding inactive budgets in this case.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 38 Martin Renvoize 2014-04-29 09:02:48 UTC
Created attachment 27769 [details] [review]
[PASSED QA] Bug 11578: QA follow-up

This patch removes the error raised by tt_valid
 FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt
   FAIL   tt_valid
                lines 310

Actually, it fails because translate script does not match strings in
attribute value. In this case, we don't want to translate. This fix is
not necessary.

This patch also adds the licence for the new jQuery treetable plugin.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 39 Martin Renvoize 2014-04-29 09:06:02 UTC
I'm happy with this patch series now.  

We're passing the qa script and all tests.  It's a valuable enhancement to the ergonomics of the page.

Note for RM, the QA follow-up adds the library to the about page, curretly the patch claims MIT licence for the libraries, but it's available as either MIT or GPLv2. I'm no license guru, so thought I'de leave any thoughts on which we should claim to you ;)
Comment 40 Jonathan Druart 2014-04-30 10:16:07 UTC
Created attachment 27813 [details] [review]
Bug 11578: FIX show inactive link

Before this patch, the inactive funds were displayed by default.
The active funds should be displayed by default.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 41 Jonathan Druart 2014-04-30 10:16:11 UTC Comment hidden (obsolete)
Comment 42 Jonathan Druart 2014-04-30 10:17:09 UTC
I found 2 new (small but annoying) bugs. I fixed them in 2 last patches.
Comment 43 Jonathan Druart 2014-04-30 10:21:50 UTC
Created attachment 27815 [details] [review]
Bug 11578: FIX funds grouping

The datatables sort options were wrong.
The funds for a budget could be ungrouped.
To reproduce:
- Create 3 budgets A, B, C.
- Create 1 or more funds for A, B, C
- Mark B and C as inactive.
- Create another fund for A
- Go on the fund list view, click on "show inactive funds"
=> The last fund is not grouped without other funds of budget A.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 44 Jonathan Druart 2014-04-30 10:35:49 UTC
Created attachment 27816 [details] [review]
Bug 11578: FIX budgets grouping

If 2 budgets (budget periods) have the same description, the funds will
be grouped into the same "block".
This patch adds a distinction adding the id of the budget next to the
description.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 45 Galen Charlton 2014-05-03 18:35:34 UTC
Pushed to master.  Thanks, Jonathan!
Comment 46 Jonathan Druart 2015-02-23 15:57:59 UTC
*** Bug 9492 has been marked as a duplicate of this bug. ***
Comment 47 Marjorie Barry-Vila 2015-05-29 19:54:01 UTC
(In reply to Jonathan Druart from comment #44)
> Created attachment 27816 [details] [review] [review]
> Bug 11578: FIX budgets grouping
> 
> If 2 budgets (budget periods) have the same description, the funds will
> be grouped into the same "block".
> This patch adds a distinction adding the id of the budget next to the
> description.
> 
> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Hi,
I have a question about this patch: why column sorting was abandoned? 

Regards,
Marjorie
Comment 48 Katrin Fischer 2015-05-31 13:15:37 UTC
Hi Marjorie, I am not sure, it might have been by accident or because of the new grouping by budget, that makes sorting correctly too hard. If you look at the page with several active budgets you can see what I mean. 
Could you open a separate bug report please?
Comment 49 Jonathan Druart 2015-06-03 09:26:20 UTC
(In reply to Marjorie Barry-Vila from comment #47)
> Hi,
> I have a question about this patch: why column sorting was abandoned? 

It is expected, the sort option would be quite difficult to implement on this new view I think.
Comment 50 Jonathan Druart 2015-06-03 09:29:30 UTC
(In reply to Jonathan Druart from comment #49)
> (In reply to Marjorie Barry-Vila from comment #47)
> > Hi,
> > I have a question about this patch: why column sorting was abandoned? 
> 
> It is expected, the sort option would be quite difficult to implement on
> this new view I think.

... But not impossible ;)