Bug 929

Summary: See details of a budget
Product: Koha Reporter: Chris Cormack <chris>
Component: AcquisitionsAssignee: Chris Cormack <chris>
Status: CLOSED FIXED QA Contact: Paul Poulain <paul.poulain>
Severity: enhancement    
Priority: P2 CC: henridamien, jcamins, katrin.fischer, koha.sekjal, m.de.rooy, magnus, nengard, paul.poulain, velu2k8
Version: 3.6   
Hardware: PC   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=3080
Change sponsored?: Sponsored Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 6504, 7359, 7602    
Bug Blocks: 7387, 7402    
Attachments: Bug 929 : Reinstating the breakdown of budgets
Bug 929 : Follow up, making some improvements to the spent page More to come
Bug 929 : Last follow up, implementing the last of Katrins suggestions
Bug 929 : Followup fixing date formatting
[SIGNED-OFF] Bug 929 : Reinstating the breakdown of budgets
[SIGNED-OFF] Bug 929 : Follow up, making some improvements to the spent page More to come
[SIGNED-OFF] Bug 929 : Last follow up, implementing the last of Katrins suggestions
[SIGNED-OFF] Bug 929 : Followup fixing date formatting
Bug 929: Follow up - showing fund code instead of internal number
Bug 929: Follow up - showing fund code instead of internal number
Bug 929: Follow up adding the Koha:: namespace to the list of modules to test
Bug 929 : Follow up adding unit tests, discovered C4::Dates cached the syspref with no way to clear it, fixed also.
Bug 929 : Follow up to allow tests to check plugins too
[SIGNED-OFF] Bug 929 : Follow up to allow tests to check plugins too
Bug 929 : Follow up, shifting test

Description Chris Cormack 2010-05-20 23:36:53 UTC


---- Reported by nathalie.fontaine@emn.fr 2005-02-08 05:03:36 ----

It would be interesting to see details of orders for each budget by a "click" on
budget line. The different staffs want to know what they order.



---- Additional Comments From jmf@liblime.com 2007-10-04 13:47:29 ----

bumping up to 3.0



---- Additional Comments From nengard@gmail.com 2009-12-28 00:28:24 ----

Does the new acq module take care of this?



--- Bug imported by chris@bigballofwax.co.nz 2010-05-20 23:36 UTC  ---

This bug was previously known as _bug_ 929 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=929

Actual time not defined. Setting to 0.0
The original reporter of this bug does not have
   an account here. Reassigning to the person who moved
   it here: chris@bigballofwax.co.nz.
   Previous reporter was nathalie.fontaine@emn.fr.

Comment 1 Katrin Fischer 2011-08-15 05:52:37 UTC
In current master there is no solution for this. There is no way to search by fund and no way to get a list of ordered or received items for a fund.
There used to be links for this in the budget table at the start page of acquisitions. This functionality should be brought back.
Comment 2 Katrin Fischer 2011-08-15 16:16:28 UTC
*** Bug 2145 has been marked as a duplicate of this bug. ***
Comment 3 Chris Cormack 2011-12-27 21:40:47 UTC Comment hidden (obsolete)
Comment 4 Katrin Fischer 2011-12-28 02:32:40 UTC
It's great to see this feature being brought back!

Some ideas for improvements that could be dealt with in a follow-up:

For ordered and spent:
- show itemtype description instead of code
- add the fund to the breadcrumbs
- add the fund to the heading, removing 'budgets' as the page is dealing with funds only
- add date ordered

For ordered:

- add the basket as additional column with a link
- the budget date column is always empty for me - I think that will not work with new acquisitions?

For spent:
- freight per item is not in use now - but I think bug 6504 could fix that
- make supplier = vendor and add a link to the vendor detail page
- fix typo: Receieved 
- format date received according to the dateformat system preference
Comment 5 Chris Cormack 2011-12-28 02:45:29 UTC Comment hidden (obsolete)
Comment 6 Chris Cormack 2011-12-28 20:12:53 UTC Comment hidden (obsolete)
Comment 7 Katrin Fischer 2011-12-29 08:11:28 UTC
Hi Chris, 

I tested and found 2 small problems:

1) Instead of the code or description of the fund only the number is shown. I am not sure if we should show code or description - most places in acq show the description, but the table with the links shows the code.

2) The dateformat in the link to the order receive page is wrong now:
...cgi-bin/koha/acqui/parcel.pl?invoice=a12&supplierid=1&datereceived=29/12/2011
If the dateformat is not passed correctly, it's not displayed on the page behind 'On:'.
/cgi-bin/koha/acqui/parcel.pl?type=intra&supplierid=1&datereceived=2011-12-28&invoice=a12
Comment 8 Chris Cormack 2011-12-29 08:14:33 UTC
That'll learn me for following your suggestions!

Ill do another follow up tomorrow to undo the date formatting changes and use the code instead of the number.
Comment 9 Chris Cormack 2011-12-29 08:49:13 UTC Comment hidden (obsolete)
Comment 10 Katrin Fischer 2011-12-29 09:57:22 UTC Comment hidden (obsolete)
Comment 11 Katrin Fischer 2011-12-29 10:01:03 UTC Comment hidden (obsolete)
Comment 12 Katrin Fischer 2011-12-29 10:01:09 UTC Comment hidden (obsolete)
Comment 13 Katrin Fischer 2011-12-29 10:01:18 UTC Comment hidden (obsolete)
Comment 14 Katrin Fischer 2011-12-29 10:01:27 UTC Comment hidden (obsolete)
Comment 15 Chris Cormack 2011-12-29 19:11:47 UTC Comment hidden (obsolete)
Comment 16 Ian Walls 2012-01-04 13:28:38 UTC
The first 4 Signed off patches apply cleanly, but the last followup patch does not.  Therefore, my QA scope is limited to the first 4.

These patches introduce new pages and templates, and provide links to them from existing pages.  All above board and clear.

The introduction of the new KohaDates plugin for T:T will establish a new pattern for date formatting going forward, hopefully simplifying things and making more use of T:T's features.

I'm marking the first 4 patches Passed QA.  The last one needs rebasing, I believe.  More followups may be required, but so far, there are no regressions or negative consequences found by this work.

Oh, and neat feature!
Comment 17 Chris Cormack 2012-01-04 19:31:47 UTC
Hmm seems to work ok for me Ian

 git bz apply 929
bugs.koha-community.org
Bug 929 - See details of a budget

[SIGNED-OFF] Bug 929 : Reinstating the breakdown of budgets
Apply? [yn] y

Applying: Bug 929 : Reinstating the breakdown of budgets
[SIGNED-OFF] Bug 929 : Follow up, making some improvements to the spent page More to come
Apply? [yn] y

Applying: Bug 929 : Follow up, making some improvements to the spent page More to come
[SIGNED-OFF] Bug 929 : Last follow up, implementing the last of Katrins suggestions
Apply? [yn] y

Applying: Bug 929 : Last follow up, implementing the last of Katrins suggestions
[SIGNED-OFF] Bug 929 : Followup fixing date formatting
Apply? [yn] y

Applying: Bug 929 : Followup fixing date formatting
Bug 929: Follow up - showing fund code instead of internal number
Apply? [yn] y

Applying: Bug 929: Follow up - showing fund code instead of internal number
rorohiko:[git/929-2-]:~/git/koha%
Comment 18 Jared Camins-Esakov 2012-01-04 19:38:39 UTC
I just tried applying the five patches on latest master, and they all worked:

jcamins@cp-koha3:~/kohaclone$ git-bz apply 929
Bug 929 - See details of a budget

[SIGNED-OFF] Bug 929 : Reinstating the breakdown of budgets
Apply? [yn] y

Applying: Bug 929 : Reinstating the breakdown of budgets
[SIGNED-OFF] Bug 929 : Follow up, making some improvements to the spent page More to come
Apply? [yn]y

Applying: Bug 929 : Follow up, making some improvements to the spent page More to come
[SIGNED-OFF] Bug 929 : Last follow up, implementing the last of Katrins suggestions
Apply? [yn] y

Applying: Bug 929 : Last follow up, implementing the last of Katrins suggestions
[SIGNED-OFF] Bug 929 : Followup fixing date formatting
Apply? [yn] y

Applying: Bug 929 : Followup fixing date formatting
Bug 929: Follow up - showing fund code instead of internal number
Apply? [yn] y

Applying: Bug 929: Follow up - showing fund code instead of internal number
jcamins@cp-koha3:~/kohaclone$ git status
# On branch bug_929_qa
# Your branch is ahead of 'kc/master' by 5 commits.
Comment 19 Paul Poulain 2012-01-06 13:00:45 UTC
I'm OK to put this patch, but it introduce a new (internal) feature i'd like to advertise before pushing.

Chris, could you explain the consequences for dates fields ? Does it mean we don't need to format them in pl anymore but just throw them "as they are", like:
[% order.entrydate | $KohaDates %] ?

(and :
-        my $entrydate = C4::Dates->new( $data->{'entrydate'}, 'iso' );
-        $data->{'entrydate'} = $entrydate->output("syspref");
-        my $datereceived = C4::Dates->new( $data->{'datereceived'}, 'iso' );
-        $data->{'datereceived'} = $datereceived->output("syspref");
to remove lines that are now useless ?

I like it. But if we want to adopt this tool (and I think we should), it must be validated & a bug entry must be open to clean all dates formattings in Koha !

Chris, please give me details, i'll take care of the bugzilla & mail to koha-devel
Comment 20 Katrin Fischer 2012-01-06 13:21:50 UTC
It's a new filter Chris wrote and I think advertising is good but should not keep these patches from being pushed. We can implement the use of this filter step by step in the other templates.

And yes, it makes it unnecessary to deal with date formatting in the .pl files - which is great!
Comment 21 Paul Poulain 2012-01-06 14:07:56 UTC
(In reply to comment #20)
> It's a new filter Chris wrote and I think advertising is good but should not
> keep these patches from being pushed. We can implement the use of this filter
> step by step in the other templates.

of course, and that's why i'm just asking for more info from chris.
Once i've them, i'll push, that won't be delayed a lot, don't worry
Comment 22 Chris Cormack 2012-01-06 17:41:59 UTC
(In reply to comment #19)
> I'm OK to put this patch, but it introduce a new (internal) feature i'd like to
> advertise before pushing.
> 
> Chris, could you explain the consequences for dates fields ? Does it mean we
> don't need to format them in pl anymore but just throw them "as they are",
> like:
> [% order.entrydate | $KohaDates %] ?
> 

Not as they are, but as they come from the database. It will format them in the style specified by the syspref. But you cant throw 20/2/2011 at it. Then need to be iso style dates.

> (and :
> -        my $entrydate = C4::Dates->new( $data->{'entrydate'}, 'iso' );
> -        $data->{'entrydate'} = $entrydate->output("syspref");
> -        my $datereceived = C4::Dates->new( $data->{'datereceived'}, 'iso' );
> -        $data->{'datereceived'} = $datereceived->output("syspref");
> to remove lines that are now useless ?
> 
> I like it. But if we want to adopt this tool (and I think we should), it must
> be validated & a bug entry must be open to clean all dates formattings in Koha
> !
> 
I think cleaning as people find them is fine, Also not all should be changed, only ones that are used for populating .tt files. 

But it does allow you to let the display be formatted in the template rather than the .pl or .pm


> Chris, please give me details, i'll take care of the bugzilla & mail to
> koha-devel
Comment 23 Paul Poulain 2012-01-09 20:00:10 UTC
(In reply to comment #22)
> Not as they are, but as they come from the database.
that's exactly what I was meaning by "as they are".

> > I like it. But if we want to adopt this tool (and I think we should), it must
> > be validated & a bug entry must be open to clean all dates formattings in Koha
> I think cleaning as people find them is fine, Also not all should be changed,
> only ones that are used for populating .tt files. 
I agree, i'll create the bug for it, it will be an omnibus for date display cleaning, nothing more, nothing less.

I forgot to add something about QA: there is no unit test. Even if it will be a trivial one, please follow-up with one & i'll push
Comment 24 Chris Cormack 2012-01-10 02:50:28 UTC
(In reply to comment #23)
> (In reply to comment #22)
> > Not as they are, but as they come from the database.
> that's exactly what I was meaning by "as they are".
> 
> > > I like it. But if we want to adopt this tool (and I think we should), it must
> > > be validated & a bug entry must be open to clean all dates formattings in Koha
> > I think cleaning as people find them is fine, Also not all should be changed,
> > only ones that are used for populating .tt files. 
> I agree, i'll create the bug for it, it will be an omnibus for date display
> cleaning, nothing more, nothing less.
> 
> I forgot to add something about QA: there is no unit test. Even if it will be a
> trivial one, please follow-up with one & i'll push

It is a display only change that can only be called in TT template, so it will indeed be a trivial one to test compilation which one of the 00 tests does already. But if the rule is no patches pushed without a unit test, then thats great and I hope it is applied for everybody. I look forward to seeing no patches pushed without unit tests.

Once I am back from holiday I will create a simple unit test.
Comment 25 Chris Cormack 2012-01-12 08:25:14 UTC Comment hidden (obsolete)
Comment 26 Chris Cormack 2012-01-12 08:25:25 UTC Comment hidden (obsolete)
Comment 27 Paul Poulain 2012-01-13 16:50:10 UTC
QA comment:

I was about to push this nice feature, but there is SQL in the .pl, and that's something we don't want.
For example, in ordered.pl :
+SELECT
+    aqorders.basketno, aqorders.ordernumber,
+    quantity-quantityreceived AS tleft,
+    ecost, budgetdate,
+    aqbasket.booksellerid,
+    itype,
+    title
+FROM (aqorders, aqbasket)
+LEFT JOIN biblio ON
+    biblio.biblionumber=aqorders.biblionumber
+LEFT JOIN aqorders_items ON
+    aqorders.ordernumber=aqorders_items.ordernumber
+LEFT JOIN items ON
+    items.itemnumber=aqorders_items.itemnumber
+WHERE
+    aqorders.basketno=aqbasket.basketno AND
+    budget_id=? AND
+    (datecancellationprinted IS NULL OR
+        datecancellationprinted='0000-00-00') AND
+    (quantity > quantityreceived OR quantityreceived IS NULL)

I also noticed that the POD is wrong:
+=head1 NAME
+
+committed.pl

I discussed of point #1 with kf on the channel, and she pointed this commit reinstate a script that was here, with SQL inside.
So I won't mark failed QA, will add a follow-up for the small committed.pl mistake in the POD.

BUT i'd like to have a follow-up to fix this SQL that should really not be here.

patch pushed, please test
Comment 28 Chris Cormack 2012-01-13 18:29:33 UTC Comment hidden (obsolete)
Comment 29 Chris Cormack 2012-01-13 18:30:29 UTC
To test run

perl xt/author/valid-templates.t before and after applying this patch
Comment 30 Katrin Fischer 2012-01-13 18:59:15 UTC Comment hidden (obsolete)
Comment 31 Paul Poulain 2012-01-16 17:20:35 UTC
patch pushed, jenkins should not complain anymore
Comment 32 Marcel de Rooy 2012-01-26 14:33:16 UTC
I see that the first file in Koha module dir,
Koha::Template::Plugin::KohaDates, contains a use C4::Dates !
See also report 7359.

Was introduced by commit 01875b565d7c04b3b00ba1986f824cae7d1fb2a9 under  Bug 929 : Followup fixing date formatting.
Comment 33 Jared Camins-Esakov 2012-01-30 01:25:02 UTC
This patch depends on bug 7359. As of right now, all non-dev installs ARE BROKEN. The patch for this bug added Koha/Template/Plugin/KohaDates.pm, but without the patch for bug 7359, that file will not be installed. This is true for 3.4.x, 3.6.x, and 3.7.x.
Comment 34 Chris Cormack 2012-01-30 23:30:12 UTC
Test for this is db dependent, so shifting it.
Comment 35 Chris Cormack 2012-01-30 23:40:25 UTC Comment hidden (obsolete)
Comment 36 Paul Poulain 2012-02-07 15:43:48 UTC
Comment on attachment 7393 [details] [review]
Bug 929 : Follow up, shifting test

this patch has been pushed
Comment 37 Paul Poulain 2012-03-13 15:02:45 UTC
(forgot to set to 3.8)
Comment 38 Katrin Fischer 2012-03-13 15:03:53 UTC
Hm, this feature is in 3.6 already - so shouldn't it be 3.6?
Comment 39 Chris Cormack 2012-04-08 07:18:03 UTC
Change is in 3.6.x and all follow ups