[Bug 32595] New: Price format cost in ILL requests table
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 Bug ID: 32595 Summary: Price format cost in ILL requests table Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: ILL Assignee: koha-bugs@lists.koha-community.org Reporter: katrin.fischer@bsz-bw.de The table with all ILL requests on the start page of the ILL module shows the Cost as 4.00 indepent of the PriceFormat system preference. It needs to have price formatting applied. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #1 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Is this still the case? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 --- Comment #2 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 154717 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154717&action=edit Bug 32595: Add price formatting to cost and price paid fields in ILL list table Test plan: 1) Enable ILLmodule and install FreeForm, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.s...) 2) Create a new FreeForm request and edit its price on the manage page to something like '123' 3) Check back the ILL list table and confirm its shows as '123' 4) Apply patch 5) Verify the table again, see that it now shows as '123.00' -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #154717|0 |1 is obsolete| | --- Comment #3 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 154731 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154731&action=edit Bug 32595: Add price formatting to cost and price paid fields in ILL list table Test plan: 1) Enable ILLmodule and install FreeForm, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.s...) 2) Create a new FreeForm request and edit its price on the manage page to something like '123' 3) Check back the ILL list table and confirm its shows as '123' 4) Apply patch 5) Verify the table again, see that it now shows as '123.00' Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #154731|0 |1 is obsolete| | --- Comment #4 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 154780 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154780&action=edit Bug 32595: Add price formatting to cost and price paid fields in ILL list table Test plan: 1) Enable ILLmodule and install FreeForm, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.s...) 2) Create a new FreeForm request and edit its price on the manage page to something like '123' 3) Check back the ILL list table and confirm its shows as '123' 4) Apply patch 5) Verify the table again, see that it now shows as '123.00' Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff CC| |pedro.amorim@ptfs-europe.co | |m --- Comment #5 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Changed parseInt to Number, parseInt was ignoring float values (i.e. if price was 123,5 it would show as 123,00 on the table). However there is still a deeper issue here I think in the way the price is saved. If we add "123,5" (with a comma instead of a dot) the table shows 0. I'm revisiting this. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 --- Comment #6 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Changed parseInt to Number, parseInt was ignoring float values (i.e. if price was 123.5 it would show as 123,00 on the table). However there is still a deeper issue here I think in the way the price is saved. If we add "123,5" (with a comma instead of a dot) the table shows 0. I'm revisiting this. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #154780|0 |1 is obsolete| | --- Comment #7 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 154787 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154787&action=edit Bug 32595: Add price formatting to cost and price paid fields in ILL list table Test plan: 1) Enable ILLmodule and install FreeForm, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.s...) 2) Create a new FreeForm request and edit its price on the manage page to something like '123' 3) Check back the ILL list table and confirm its shows as '123' 4) Apply patch 5) Verify the table again, see that it now shows as '123.00' Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 --- Comment #8 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 154788 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154788&action=edit Bug 32595: Update price handling in UI edit form -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 --- Comment #9 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Updated original patch to consider possibility of price data having commas in it and replacing that with a dot to convert to the data format expected by format_price. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 --- Comment #10 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- (In reply to Owen Leonard from comment #3)
Created attachment 154731 [details] [review] Bug 32595: Add price formatting to cost and price paid fields in ILL list table
Test plan: 1) Enable ILLmodule and install FreeForm, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev. sh) 2) Create a new FreeForm request and edit its price on the manage page to something like '123' 3) Check back the ILL list table and confirm its shows as '123' 4) Apply patch 5) Verify the table again, see that it now shows as '123.00'
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Owen sorry would you please take a second look at this? I've updated the status to "Needs Signoff" because I've changed the code enough to justify it. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- The changes on the edit form are great, but I wonder if we should do the replace or not put this into the backends 'hands'. It seems that there is a mistake/flaw in the architecture being that price_paid and cost are both varchar(20) instead of a decimal? The changes to the edit form I fully agree with :) -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 --- Comment #12 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- (In reply to Katrin Fischer from comment #11) Correct, I noticed that too. The replacing of , -> . is visual only and to ensure compatibility with existing data, because there's no telling what may exist currently on live databases. We could come up with an atomic update like: 1) UPDATE illrequests SET price_paid = CAST(REPLACE(price_paid, ',', ',') as DECIMAL(28,6)) 2) ALTER TABLE illrequests MODIFY price_paid DECIMAL(28,6) But this may cause issues as there may be non-numeric data in current databases? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #154787|0 |1 is obsolete| | --- Comment #13 from David Nind <david@davidnind.com> --- Created attachment 157622 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157622&action=edit Bug 32595: Add price formatting to cost and price paid fields in ILL list table Test plan: 1) Enable ILLmodule and install FreeForm, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.s...) 2) Create a new FreeForm request and edit its price on the manage page to something like '123' 3) Check back the ILL list table and confirm its shows as '123' 4) Apply patch 5) Verify the table again, see that it now shows as '123.00' Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #154788|0 |1 is obsolete| | --- Comment #14 from David Nind <david@davidnind.com> --- Created attachment 157623 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157623&action=edit Bug 32595: Update price handling in UI edit form Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #15 from David Nind <david@davidnind.com> --- Have added my sign off, as things work as per the test plan. Feel free to change the status if anything more is required for comments #11 and #12 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |pedro.amorim@ptfs-europe.co |ity.org |m QA Contact| |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #16 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I still think we should not do this part here: let cost = row.cost && row.cost.replaceAll(',','.'); If the information wasn't correctly stored by the backend, we should not try to fix it like hat. It could possibly hide other issues, like calculations with those values would not work correctly. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|katrin.fischer@bsz-bw.de | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- How can we progress this one? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 --- Comment #18 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Can we perhaps fix incoming data to be a consistent form rather than fixing it as it comes out? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32595 --- Comment #19 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I think I was opposed to use this: let cost = row.cost && row.cost.replaceAll(',','.'); Don't we have a standard price formatting in JS now as well? I'd use that and assume the data is in the format Koha expects (decimal with dot) and that would also work for calculating in Perl. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org