Created attachment 31855 [details] [review] 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.
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).
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.
Paola, the issue only appears on the aqbudgets page (not the aqui-home page)?
I don't have this error. All looks good. Could you please attach a screenshot please?
Created attachment 32016 [details] screen about the error case here is the image of the screen for the error case
Thanks for the screenshot, very useful! I added a patch on bug 12987, it should fix your issue.
(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? ---------------------------------------------------------------------------
Arg, yes the last patch on bug 12987 does not fix correctly the problem. I will try to provide a patch soon.
Solution proposed on bug 12987, I added a dependency.
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.
Created attachment 32145 [details] [review] 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.
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]
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
Created attachment 32346 [details] [review] Bug 12988: Update table footer with the visible rows - budgets Thanks, Jonathan. On today Oct, the 15-th I've applied the patches against master 3.17.00.027 The patch is about "Funds" (=aqbudgets.pl) (on ACQ and ADMIN). Today I've applied: 0001-Bug-12979-Price-formatting-should-only-be-defined-at.patch 0002-Bug-12979-Fix-error-on-layout3pagesfr.patch 0003-Bug-12844-New-module-to-manage-prices-into-Koha.patch 0004-Bug-12844-Remove-the-C4-Output-FormatNumber-subrouti.patch 0005-Bug-12844-Use-Koha-Number-Price-where-it-can-be-usef.patch 0006-Bug-12844-FIX-import-package-missing.patch 0007-Bug-12844-Force-the-symbol-place-in-the-tests.patch 0008-Bug-12955-The-show-my-funds-only-should-be-take-into.patch 0009-Bug-12986-Upgrade-the-DataTables-jQuery-plugin-to-th.patch 0010-Bug-12987-Update-table-footer-with-the-visible-rows.patch 0011-Bug-12987-The-new-format_price-include-file-should-b.patch 0012-Bug-12988-Update-table-footer-with-the-visible-rows-.patch 0013-Bug-12984-Improvement-of-the-funds-list-view-acqui-h.patch 0014-Bug-12989-Update-table-footer-with-the-visible-rows-.patch 0015-Bug-12989-Fix-JS-error.patch I've tested setting either FR or US currencyformat syspref. Everything is OK. So I pass the patch to "Signed Off" status.
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.
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
Patch pushed to master. Thanks Jonathan!