|
Description
claire.hernandez@biblibre.com
2011-12-02 10:21:15 UTC
Created attachment 9167 [details] [review] Show ordered amount in aqbudgets.pl This patch add a column in admin/aqbudgets.pl to display the ordered amount I tested the patch. "Base level ordered" column is now visible. But there are few more things which maybe should be changed in this table: 1. Amount shown in "Base-level remaining" column should be same as "Avail" column in acqui/acqui-home.pl, i.e. remaining = allocated - ordered - spent. 2. We should have the same wording for this two columns for consistency reasons (available or remaining) 3. "Base-level remaining" column should also have sorting feature (Claire noticed that) I'm not sure what to do for 1.: In acqui/acqui-home.pl: Avail = allocated - (ordered + spent) (ordered and spent do not include child funds) In admin/aqbudgets.pl: Remaining = allocated - spent (here, spent include child funds, but column indicate "Base-level remaining") What should we have in these columns? Switching to 'in discussion' Sorry Maurice for me not being clear. I took another look to admin/aqbudgets.pl and I found more confusing stuff, so I will try to make it clear what I think this table should look like to be really useful. I am looking at my production data. admin/aqbudgets.pl should have the following columns: Base-level allocated (or just Allocated) Base-level ordered Total sub-levels ordered Base-level spent Total sub-levels spent Base-level available Total sub-levels available Base-level is always calculated for one level, without children. Total sub-levels should include child funds. Available is calculated as "allocated - (ordered + spent)". So, this means that: - "Total allocated" should be removed because currently it shows the same data as "Base-level alloceted". - three new columns should be added: "Base-level ordered", "Total sub-levels ordered" and "Total sub-levels available"; - wording should be changed from "remaining" to "available" (consistency with acqui/acqui-home.pl) This sounds ok(In reply to comment #5) > Sorry Maurice for me not being clear. I prefer my firstname: Julian ;-) > > I took another look to admin/aqbudgets.pl and I found more confusing stuff, > so I will try to make it clear what I think this table should look like to > be really useful. I am looking at my production data. > > admin/aqbudgets.pl should have the following columns: > > Base-level allocated (or just Allocated) > Base-level ordered > Total sub-levels ordered > Base-level spent > Total sub-levels spent > Base-level available > Total sub-levels available > > Base-level is always calculated for one level, without children. > Total sub-levels should include child funds. > Available is calculated as "allocated - (ordered + spent)". > > So, this means that: > - "Total allocated" should be removed because currently it shows the same > data as "Base-level alloceted". > - three new columns should be added: "Base-level ordered", "Total sub-levels > ordered" and "Total sub-levels available"; > - wording should be changed from "remaining" to "available" (consistency > with acqui/acqui-home.pl) This sounds ok for me. Can anyone else confirm that this is what we want in these tables? I'll start working on it. Created attachment 12769 [details] [review] Bug 7308: rework aqbudgets.pl table admin/aqbudgets.pl should have the following columns: Base-level allocated (or just Allocated) Base-level ordered Total sub-levels ordered Base-level spent Total sub-levels spent Base-level available Total sub-levels available Base-level is always calculated for one level, without children. Total sub-levels should include child funds. Available is calculated as "allocated - (ordered + spent)". Patch doesn't apply Applying: Bug 7308: Show ordered amount in aqbudgets.pl 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 Auto-merging admin/aqbudgets.pl CONFLICT (content): Merge conflict in admin/aqbudgets.pl Auto-merging C4/Budgets.pm Failed to merge in the changes. Patch failed at 0001 Bug 7308: Show ordered amount in aqbudgets.pl Created attachment 16011 [details] [review] Bug 7308: Show ordered amount in aqbudgets.pl Created attachment 16012 [details] [review] Bug 7308: rework aqbudgets.pl table admin/aqbudgets.pl should have the following columns: Base-level allocated (or just Allocated) Base-level ordered Total sub-levels ordered Base-level spent Total sub-levels spent Base-level available Total sub-levels available Base-level is always calculated for one level, without children. Total sub-levels should include child funds. Available is calculated as "allocated - (ordered + spent)". Patches rebased on master. Patch tested with a sandbox, by Cedric Vita <cedric.vita@dracenie.com> Created attachment 16381 [details] [review] Bug 7308: rework aqbudgets.pl table admin/aqbudgets.pl should have the following columns: Base-level allocated (or just Allocated) Base-level ordered Total sub-levels ordered Base-level spent Total sub-levels spent Base-level available Total sub-levels available Base-level is always calculated for one level, without children. Total sub-levels should include child funds. Available is calculated as "allocated - (ordered + spent)". Signed-off-by: Cedric Vita <cedric.vita@dracenie.com> Created attachment 16394 [details] [review] Bug 7308: Show ordered amount in aqbudgets.pl Patch signed-off (pb with sandbox) Created attachment 16395 [details] [review] Bug 7308: rework aqbudgets.pl table syntax error at /usr/share/koha/testclone/admin/aqbudgets.pl line 323, near "/) " syntax error at /usr/share/koha/testclone/admin/aqbudgets.pl line 327, near "$_}" Global symbol "@loop" requires explicit package name at /usr/share/koha/testclone/admin/aqbudgets.pl line 352. Unmatched right curly bracket at /usr/share/koha/testclone/admin/aqbudgets.pl line 357, at end of line syntax error at /usr/share/koha/testclone/admin/aqbudgets.pl line 357, near "}" Unmatched right curly bracket at /usr/share/koha/testclone/admin/aqbudgets.pl line 376, at end of line syntax error at /usr/share/koha/testclone/admin/aqbudgets.pl line 376, near "}" Execution of /usr/share/koha/testclone/admin/aqbudgets.pl aborted due to compilation errors. Failed QA I do not reproduce these errors. Here's what I've done: $ git checkout -b bug/7308 && git fetch origin && git reset --hard origin/master $ git bz apply 7308 Bug 7308 - new column in admin/aqbudgets Bug 7308: Show ordered amount in aqbudgets.pl Apply? [yn] y Applying: Bug 7308: Show ordered amount in aqbudgets.pl Bug 7308: rework aqbudgets.pl table Apply? [yn] y Applying: Bug 7308: rework aqbudgets.pl table $ perl -cw admin/aqbudgets.pl admin/aqbudgets.pl syntax OK $ git log --oneline -3 ebd30fe Bug 7308: rework aqbudgets.pl table 72e2c0a Bug 7308: Show ordered amount in aqbudgets.pl acd4b21 Bug 10311 - Holds queue ignores item-level holds where only one items exists $ perl -v | head -n2 This is perl, v5.10.1 (*) built for x86_64-linux-gnu-thread-multi Looking at the source file, the code looks good too. Could you retry please? I confirm I don't get any error. Reset to Signed off Created attachment 19824 [details] [review] [PASSED QA] Bug 7308: Show ordered amount in aqbudgets.pl Signed-off-by: Cédric Vita <cedric.vita@dracenie.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Created attachment 19825 [details] [review] [PASSED QA] Bug 7308: rework aqbudgets.pl table admin/aqbudgets.pl should have the following columns: Base-level allocated (or just Allocated) Base-level ordered Total sub-levels ordered Base-level spent Total sub-levels spent Base-level available Total sub-levels available Base-level is always calculated for one level, without children. Total sub-levels should include child funds. Available is calculated as "allocated - (ordered + spent)". Signed-off-by: Cedric Vita <cedric.vita@dracenie.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Seems to work alright for me. Passes QA script and tests, after I fixed 2 tabs in admin/aqbudgets.pl. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Created attachment 19826 [details] [review] Bug 7308 - Follow up: removing line breaks for better translatability Because of the lines breaks in the table headings the strings were separated in translation. Example: Base-level allocated - Base-level - allocated If you look at a smaller screen the table headings will still break without the additional <br />. Created attachment 19859 [details]
potential source of confusion if parent and child funds both have orders
Overall, I think the patch looks good, but there's a case where I think it leads to a confusing display.
In the screenshot, fund abc has $10.45 ordered against it, but it also has a child fund, def, that has $14.97 ordered against it.
The base-level ordered and total-ordered make sense, individually, but when you look at the whole picture, a librarian could be excused for asking why the total of the "total ordered" column is $33.97, not $48.94 (= $25.42 + 14.97 + $8.55).
Does anybody have any suggestions on how this could be made more clear? Setting to in discussion.
(In reply to Galen Charlton from comment #24) > Does anybody have any suggestions on how this could be made more clear? > Setting to in discussion. Pushed or in discussion ? status mistaken ? Galen, I don't see a good solution here. Maybe can we change the color for child cells? I think there is a deep ergonomic issue on this page and should be partially rewrite (always use DT and insert a way to always see funds as a tree). I will submit a followup in order to revive this bug. Created attachment 22462 [details] [review] Bug 7308: change color style for fund child Switch to "needs QA", waiting for a QAer point of view. Ok, it looks like Jonathan and me can't do QA here - could someone else take this on please? :) Created attachment 24936 [details] [review] [PASSED QA] Bug 7308 - Follow up: removing line breaks for better translatability Because of the lines breaks in the table headings the strings were separated in translation. Example: Base-level allocated - Base-level - allocated If you look at a smaller screen the table headings will still break without the additional <br />. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 24937 [details] [review] [PASSED QA] Bug 7308: change color style for fund child Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Pushed to master, along with a follow-up that introduces a descriptive CSS class (child_fund_amount) and uses italics rather than a subtle font color difference to indicate that child fund amounts do not participate in the totals. Thanks, Julian and Jonathan! *** Bug 11823 has been marked as a duplicate of this bug. *** Pushed to 3.14.x, will be in 3.14.11 |