Bug 12988 - Update table footer with the visible rows - budgets
Summary: Update table footer with the visible rows - budgets
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 12955 12987
Blocks:
  Show dependency treegraph
 
Reported: 2014-09-24 15:12 UTC by Jonathan Druart
Modified: 2015-06-04 23:32 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 12988: Update table footer with the visible rows - budgets (4.98 KB, patch)
2014-09-24 15:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
screen about the error case (82.22 KB, image/jpeg)
2014-10-06 16:20 UTC, Paola Rossi
Details
Bug 12988: Update table footer with the visible rows - budgets (4.45 KB, patch)
2014-10-10 14:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12988: Update table footer with the visible rows - budgets (4.62 KB, patch)
2014-10-15 15:09 UTC, Paola Rossi
Details | Diff | Splinter Review
[PASSED QA] Bug 12988: Update table footer with the visible rows - budgets (4.58 KB, patch)
2014-10-27 23:04 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 15:12:52 UTC

    
Comment 1 Jonathan Druart 2014-09-24 15:23:25 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2014-09-24 15:26:46 UTC
Note: If CurrencyFormat is set to FR, before this patch the totals were prefixed by the currency symbol.
With this patch, there are not anymore.
I don't think it is a big deal, since the symbol is displayed everywhere in the table.
This could be done later (when some others patches will be pushed).
Comment 3 Paola Rossi 2014-10-06 14:30:16 UTC
I've applied these patches against master 3.17.00.025:

12986, 12987 (2), 12955, 12988, 12984, 12989 (2)

Using both the browsers Mozilla FF version 32.0.3 and IExplorer version 11.0. everything is OK excluding this case:

currencyformat pref set to FR,
fund's (=aqbudgets.pl) totals are all zeros (ERROR).

So I pass the patch to "Failed QA" status.

NB. To test the patches it might be that the browser should be closed and reopened.
Comment 4 Jonathan Druart 2014-10-06 14:40:33 UTC
Paola, the issue only appears on the aqbudgets page (not the aqui-home page)?
Comment 5 Jonathan Druart 2014-10-06 15:05:34 UTC
I don't have this error. All looks good. Could you please attach a screenshot please?
Comment 6 Paola Rossi 2014-10-06 16:20:49 UTC
Created attachment 32016 [details]
screen about the error case

here is the image of the screen for the error case
Comment 7 Jonathan Druart 2014-10-06 18:20:26 UTC
Thanks for the screenshot, very useful!
I added a patch on bug 12987, it should fix your issue.
Comment 8 Paola Rossi 2014-10-07 13:49:09 UTC
(In reply to Jonathan Druart from comment #7)
> Thanks for the screenshot, very useful!
> I added a patch on bug 12987, it should fix your issue.

Thanks, Jonathan. I've applied the added patch, and now the error of comment 3 is fixed.

I've two further questions for you, and for now I pass the patch (this one) to "Failed QA" status (see A) point below).
[In case I've passed the erroneous patch, by your kind suggestion I'll soon rectify and make the right changes). 
In case of no problem, you can easily back 12988 to NSO, please, [and I'll sign off the patch; perhaps all the 12986, 12987, 12988, 12989].

---------------------------------------------------------------------------
On MASTER:

on Funds the "Library" filter on the left lists only two values:
- none [for ALL the libraries]
- the library which has been set as the current one (f.i. Centerville).

As you certainly know, the second option doesn't work: all the funds are shown the same (MASTER-ERROR-1), even if none of them is linked to the selected current library. 

I've selected exactly such a current library, to which none funds have been linked.
So all the funds have been shown. Moreover, their values have been formatted so (MASTER-ERROR-2 in showing):
2000
147.6
147.609
Totals are made of zeros here, as we know.
We're on master here, so nothing is worth until now.
----------------------------------------------------
After having applied the patches:

Now I've applied:
12986, 12987 (3), 12955, 12988, 12984, 12989 (2)

The Library's filter on the left of Funds page has listed all the libraries instead of only the current one.

A) Is this enhancement in "Library" filter desired on funds' page? Or is it an  ERROR-0?

Perhaps it is desired.
After applying the patches those MASTER-ERROR-1 and MASTER-ERROR-2 have been kept, as you can easily imagine. 
But moreover now the applied patches have shown the totals on funds' page, and I've seen this kind of ERROR-3 in summing (no matter US or FR):
in "Total ordered" column:
5 + 147.6 = 14.81 instead of 152.6
in "Base-level allocated" column:
100000.000000+100000.000000=2,000,000,000.00 instead of 200,000.00

just as no point, and then 2 decimal everywhere.

B) Should those summing errors have been prevented by the patches? 
Are those badly formatted values usual? are they possible, created by filtering or by some other way, and so must the patches take them into accout?
---------------------------------------------------------------------------
Comment 9 Jonathan Druart 2014-10-07 18:02:24 UTC
Arg, yes the last patch on bug 12987 does not fix correctly the problem.
I will try to provide a patch soon.
Comment 10 Jonathan Druart 2014-10-08 20:49:48 UTC
Solution proposed on bug 12987, I added a dependency.
Comment 11 Paola Rossi 2014-10-10 10:00:19 UTC
I've tried to apply the patches against master 3.17.00.025

I've applied:
12844 (4), 12986, 12987 (2), 12955

Applying: Bug 12988: Update table footer with the visible rows - budgets
Using index info to reconstruct a base tree...
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 12988: Update table footer with the visible rows - budgets

So I pass the patch to "Patch doesn't apply" status.
Comment 12 Jonathan Druart 2014-10-10 14:16:06 UTC Comment hidden (obsolete)
Comment 13 Paola Rossi 2014-10-10 15:01:43 UTC
Thanks, Jonathan.

I've tried to apply against master 3.17.00.025

Now I've applied:

12844 (4), 12986, 12987 (2), 12955, 12988.

The patches 12984 and 12989 remain yet to be applied.

Applying: Bug 12984: Improvement of the funds list view - acqui home
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/acqui-home.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/acqui/acqui-home.tt
Failed to merge in the changes.
Patch failed at 0001 Bug 12984: Improvement of the funds list view - acqui home

So I pass again the patch to "Patch doesn't apply" status. [I beg your pardon if this is not the right bug to be passed]
Comment 14 Jonathan Druart 2014-10-10 15:10:11 UTC
Looks good here:

 Bug 12989: Fix JS error
 Bug 12989: Update table footer with the visible rows - acqui-home
 Bug 12984: Improvement of the funds list view - acqui home
 Bug 12988: Update table footer with the visible rows - budgets
 Bug 12987: The new format_price include file should be include on using datatables
 Bug 12987: Update table footer with the visible rows
 Bug 12986: Upgrade the DataTables jQuery plugin to the latest version (1.10.2)
 Bug 12955: The "show my funds only" should be take into account
 Bug 12844: FIX import package missing
 Bug 12844: Use Koha::Number::Price where it can be useful
 Bug 12844: Remove the C4::Output::FormatNumber subroutine
 Bug 12844: New module to manage prices into Koha
 Bug 12979: Fix error on layout3pagesfr
 Bug 12979: Price formatting should only be defined at one place
Comment 15 Paola Rossi 2014-10-15 15:09:17 UTC Comment hidden (obsolete)
Comment 16 Katrin Fischer 2014-10-27 23:04:13 UTC
Created attachment 32819 [details] [review]
[PASSED QA] Bug 12988: Update table footer with the visible rows - budgets

On the budget list view, the total was not updated.
With this patch, the footer (totals) will be updated on filtering rows.

Test plan:
To test with both CurrencyFormat pref values.
1/ Go on the budget list view
2/ Verify the totals are correct.
3/ Filter the table using the filter input and verify the totals are
updated with the rows shown
4/ Hide/Show inactive budgets and verify the totals are still corrects.

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described.
Comment 17 Katrin Fischer 2014-10-27 23:21:46 UTC
My testing branch looked like this:
Bug 12988: Update table footer with the...
Bug 12955: The "show my funds only" should be take into account
Bug 12989: Fix JS error
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 on using...
Bug 12987: Update table footer with the visible rows
Bug 12986: Upgrade the DataTables jQuery plugin to the latest version (1.10.2)
<master> Bug 12844: Force the symbol place in the tests
Comment 18 Tomás Cohen Arazi 2014-11-11 17:12:18 UTC
Patch pushed to master.

Thanks Jonathan!