Bug 8117 - Divide budget periods into two tabs
Summary: Divide budget periods into two tabs
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: 3.10
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Jonathan Druart
QA Contact: Katrin Fischer
URL:
Whiteboard:
Keywords:
: 8654 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-05-18 09:57 UTC by Jonathan Druart
Modified: 2013-05-23 06:23 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: String patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 5849: Divide budget periods into two tabs (12.58 KB, patch)
2012-05-18 10:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8117: Divide budget periods into two tabs (12.58 KB, patch)
2012-05-18 10:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
screenshot of budget page (31.33 KB, image/png)
2012-06-10 16:02 UTC, Katrin Fischer
Details
Bug 8117: Followup: FIX compatibility for the new version of jQuery tabs (1.37 KB, patch)
2012-06-11 09:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8117: Followup: Use the KohaDates plugin in aqbudgetperiods.tt (6.84 KB, patch)
2012-06-11 09:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8117: Divide budget periods into two tabs (12.65 KB, patch)
2012-06-11 11:09 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8117: Followup: FIX compatibility for the new version of jQuery tabs (1.44 KB, patch)
2012-06-11 11:09 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8117: Followup: Use the KohaDates plugin in aqbudgetperiods.tt (6.91 KB, patch)
2012-06-11 11:09 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 8117: Followup Move the "no budget" string outside the table (8.06 KB, patch)
2012-06-11 15:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8117: Followup Move the "no budget" string outside the table (8.12 KB, patch)
2012-06-11 16:17 UTC, Michael Davis
Details | Diff | Splinter Review
Bug 8117: Divide budget periods into two tabs (15.09 KB, patch)
2012-07-20 14:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8117: Divide budget periods into two tabs (15.18 KB, patch)
2012-07-20 14:49 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 8117: Followup: remove h3 title (duplicate infos) (1.09 KB, patch)
2012-08-03 12:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8117 [Follow-up] Divide budget periods into two tabs (1.73 KB, patch)
2012-11-27 21:00 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 8117 [Follow-up] Divide budget periods into two tabs (1.76 KB, patch)
2012-11-28 08:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8117 [Follow-up] Divide budget periods into two tabs (1.85 KB, patch)
2012-12-02 13:22 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 2012-05-18 09:57:12 UTC
Budget periods are now divided into two tabs : active and inactive
budget periods.
Comment 1 Jonathan Druart 2012-05-18 10:03:22 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2012-05-18 10:40:30 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2012-06-10 16:02:58 UTC
Created attachment 10138 [details]
screenshot of budget page

Hi Jonathan, 

the javasript page on the page seems to be broken for me. I tried refreshing the cache and closing the browser completely. I think it should not be separate tables but tabs?

Perhaps patch could also make use of the Koha dates plugin? :)
Comment 4 Jonathan Druart 2012-06-11 09:13:29 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2012-06-11 09:14:28 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2012-06-11 11:09:32 UTC Comment hidden (obsolete)
Comment 7 Katrin Fischer 2012-06-11 11:09:40 UTC Comment hidden (obsolete)
Comment 8 Katrin Fischer 2012-06-11 11:09:47 UTC Comment hidden (obsolete)
Comment 9 Michael Davis 2012-06-11 11:14:40 UTC
Looks really good, and I think this is a really good feature. However, it fails a javascript validation error when the patch is applied. To recreate, add two active budgets, apply patch, refresh budgets page.

DataTables warning (table id = 'inactiveperiodst'): Requested unknown parameter '1' from the data source for row 0
Comment 10 Jonathan Druart 2012-06-11 15:29:28 UTC Comment hidden (obsolete)
Comment 11 Michael Davis 2012-06-11 16:17:38 UTC Comment hidden (obsolete)
Comment 12 Michael Davis 2012-06-11 16:20:24 UTC
This works for me, thanks, but I may have messed up the patches, so someone else should sign off.
Comment 13 Owen Leonard 2012-07-20 14:09:13 UTC
Applying the patches fails for me at this step:

Applying: Bug 8117: Followup: Use the KohaDates plugin in aqbudgetperiods.tt
fatal: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgetperiods.tt).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 8117: Followup: Use the KohaDates plugin in aqbudgetperiods.tt
Comment 14 Jonathan Druart 2012-07-20 14:22:28 UTC Comment hidden (obsolete)
Comment 15 Owen Leonard 2012-07-20 14:49:29 UTC Comment hidden (obsolete)
Comment 16 Paul Poulain 2012-08-03 10:41:25 UTC
QA comment:
 * small patch, just for a better display
 * passes test-qa.pl
 * A fix and a question:
    - you weren't respecting [HTML4] coding guideline (Active Budget should have been Active budget). I've fixed them
    - why do you repeat (as <h3> the title of the tab at the beginning ot the tab ?)
 * is it me or the toolbar does not display nice buttons but basic links New budget, New, Budgets, Budget planning are in blue, not styled. I tried shift-reload, switching to another page, switching to another language, the problem is still here

pls check my last point
Comment 17 Jonathan Druart 2012-08-03 12:19:51 UTC
(In reply to comment #16)
> QA comment:
>     - you weren't respecting [HTML4] coding guideline (Active Budget should
> have been Active budget). I've fixed them

Thanks!


>     - why do you repeat (as <h3> the title of the tab at the beginning ot
> the tab ?)

You're right. It's useless, I will submit a followup


>  * is it me or the toolbar does not display nice buttons but basic links New
> budget, New, Budgets, Budget planning are in blue, not styled. I tried
> shift-reload, switching to another page, switching to another language, the
> problem is still here

I don't reproduce this behaviour. Buttons display correctly for me.
Comment 18 Jonathan Druart 2012-08-03 12:20:03 UTC Comment hidden (obsolete)
Comment 19 Jonathan Druart 2012-08-20 10:10:25 UTC
*** Bug 8654 has been marked as a duplicate of this bug. ***
Comment 20 Paul Poulain 2012-09-19 15:01:19 UTC
QA comment:

 this time it's OK ! passed QA

A follow-up will be added though, because we understood why I had some javascript error and jonathan didn't. It's because I've plack + debug

debug panel in plack adds a lot of <table> (without any id)

thus, when you call 
$("table").dataTable(...)
there's a javacscript error because datatable can't handle the plack <table> !

The solution has been to explicitely name the tables to dataTable (which is a good thing anyway)
        $("#activeperiodst,#inactiveperiodst").dataTable($.extend(true, {}, dataTablesDefaults, {
Comment 21 Paul Poulain 2012-09-19 15:02:30 UTC
Patch pushed to master
Comment 22 Owen Leonard 2012-11-27 21:00:18 UTC Comment hidden (obsolete)
Comment 23 Jonathan Druart 2012-11-28 08:25:29 UTC Comment hidden (obsolete)
Comment 24 Katrin Fischer 2012-12-02 13:22:24 UTC
Created attachment 13821 [details] [review]
[SIGNED-OFF] Bug 8117 [Follow-up] Divide budget periods into two tabs

This patch corrects some capitalization errors.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
String changes, all tests pass.
Comment 25 Jared Camins-Esakov 2012-12-02 14:22:34 UTC
This patch has been pushed to master.
Comment 26 Chris Cormack 2012-12-03 06:53:09 UTC
Pushed to 3.10.x, will be in 3.10.1