The funds list is not ergonomic and should be improved.
Created attachment 24505 [details] [review] 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.
Created attachment 24506 [details] [review] 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.
Created attachment 24507 [details] [review] Bug 11578: Add some css
Created attachment 24508 [details] Funds list view
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.
(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.
Created attachment 25245 [details] [review] 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".
Created attachment 25246 [details] [review] Bug 11578: Align budget names
(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.
<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
Created attachment 26243 [details] [review] 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.
Created attachment 26244 [details] [review] 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.
Created attachment 26245 [details] [review] Bug 11578: Add some css
Created attachment 26246 [details] [review] 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".
Created attachment 26247 [details] [review] Bug 11578: Align budget names
If this gets signed off, please remove the title marker from 11763.
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)
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.
Created attachment 27404 [details] [review] 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.
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.
Created attachment 27706 [details] [review] 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.
Created attachment 27707 [details] [review] Bug 11578: Add some css
Created attachment 27708 [details] [review] 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".
Created attachment 27709 [details] [review] Bug 11578: Align budget names
Created attachment 27710 [details] [review] 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.
Last patches fix conflict with bug 7308.
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
Created attachment 27736 [details] [review] 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.
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
Created attachment 27754 [details] [review] 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.
Created attachment 27755 [details] [review] 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.
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>
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>
Created attachment 27765 [details] [review] [PASSED QA] Bug 11578: Add some css Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
Created attachment 27767 [details] [review] [PASSED QA] Bug 11578: Align budget names Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
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>
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 ;)
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>
Created attachment 27814 [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>
I found 2 new (small but annoying) bugs. I fixed them in 2 last patches.
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>
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>
Pushed to master. Thanks, Jonathan!
*** Bug 9492 has been marked as a duplicate of this bug. ***
(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
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?
(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.
(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 ;)