[Bug 11733] New: "gist" system preference inconsistently treated as single-value or multi-value
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11733 Bug ID: 11733 Summary: "gist" system preference inconsistently treated as single-value or multi-value Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Acquisitions Assignee: koha-bugs@lists.koha-community.org Reporter: gmcharlt@gmail.com QA Contact: testopia@bugs.koha-community.org The "gist" system preference, which specifies tax rates available for use in various places in acquisitions, lets the user specify multiple values separated by pipe symbols. The first value is taken to be the default tax rate to apply. However, there are some places in the code where it effectively assumes that gist contains only one value: acqui/basketgroup.pl: if ($bookseller->{invoiceincgst} && ! $bookseller->{listincgst} && ( $bookseller->{gstrate} // C4::Co acqui/basketgroup.pl: my $gst = $bookseller->{gstrate} // C4::Context->preference("gist"); acqui/basketgroup.pl: my $pdf = printpdf($basketgroup, $bookseller, $baskets, \%orders, $bookseller->{gstrate} // C4::Context-> acqui/invoice.pl:my $gist = $bookseller->{gstrate} // C4::Context->preference("gist") // 0; acqui/parcel.pl:my $gst = $bookseller->{gstrate} // C4::Context->preference("gist") // 0; and other places where it assumes (correctly) that it can have multiple values: acqui/neworderempty.pl:}, split( '\|', C4::Context->preference("gist") ); acqui/supplier.pl: }, split( '\|', C4::Context->preference("gist") ); Routines should be written for fetching either the list of tax rates or the default tax rate as needed. See also bug 11680. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11733 Jacek Ablewicz <abl@biblos.pk.edu.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |abl@biblos.pk.edu.pl --- Comment #1 from Jacek Ablewicz <abl@biblos.pk.edu.pl> --- (In reply to Galen Charlton from comment #0)
acqui/basketgroup.pl: my $pdf = printpdf($basketgroup, $bookseller, $baskets, \%orders, $bookseller->{gstrate} // C4::Context->
Looks like all existing pdfprint() variants are not using this gstrate argument any longer (that is probably good - they should use gst rates from invoice records instead).
acqui/invoice.pl:my $gist = $bookseller->{gstrate} // C4::Context->preference("gist") // 0;
This one is fixed by "Bug 10613 - Gst is not calculated correctly on the invoice page" - which we will really like to be pushed ;)
acqui/parcel.pl:my $gst = $bookseller->{gstrate} // C4::Context->preference("gist") // 0;
Dead code ($gst is set, but never used)? Yet another improper usage of gist is in the parcels.pl - this is an odd one GST => C4::Context->preference('gist'), I guess it's a part of (apparently never fully implemented) feature for choosing gst rate of the shipping costs (?). -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11733 Jacek Ablewicz <abl@biblos.pk.edu.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=10613 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11733 mathieu saby <mathsabypro@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mathsabypro@gmail.com --- Comment #2 from mathieu saby <mathsabypro@gmail.com> --- Hi By the way, the description of "gist" syspref is confusing, as gist does not contain "a default value" (see bz 11458). And the name "gist" is not very good (see last comment on bz 10327). I'm going to write a patch to amend the description of the syspref. Mathieu -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11733 --- Comment #3 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 25888 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25888&action=edit Bug 11733: gist can be multi-valued The gist syspref should always be treated as multi-value. In basketgroup.pl, parcel.pl and invoice.pl it is treated as single-value. The invoice.pl script will be fixed by bug 10613. If the pref contains several values, the first one is the default one (according to the syspref description). Test plan: No regression should be found on displaying total basketgroup for a vendor with 'list prices' don't include tax and 'invoice prices' include tax. No regression should be found on generating pdf. If no gstrate is defined for the vendor, the gstrate used should be the first one defined in the gist syspref. On parcel.pl, this patch only removes dead code. The $gst variable is not used. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11733 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff CC| |jonathan.druart@biblibre.co | |m Assignee|koha-bugs@lists.koha-commun |jonathan.druart@biblibre.co |ity.org |m -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11733 Koha Team Lyon 3 <koha@univ-lyon3.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |koha@univ-lyon3.fr See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=3980 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11733 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=11680 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11733 --- Comment #4 from Galen Charlton <gmcharlt@gmail.com> --- Comment on attachment 25888 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25888 Bug 11733: gist can be multi-valued Review of attachment 25888: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=11733&attachment=25888) ----------------------------------------------------------------- ::: acqui/basketgroup.pl @@ +82,5 @@
+ unless ( defined $gst ) { + ($gst) = split /\|/, C4::Context->preference("gist"); + } + $gst //= 0; + $total = $total * ( $gst / 100 + 1 );
I think there is another bug lurking here. The 'gist' contains a list of decimal values, so dividing the default one by 100 leads to the wrong result if the default tax rate isn't 0. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11733 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11733 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11733 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #25888|0 |1 is obsolete| | --- Comment #5 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 29623 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29623&action=edit Bug 11733: gist can be multi-valued The gist syspref should always be treated as multi-value. In basketgroup.pl, parcel.pl and invoice.pl it is treated as single-value. The invoice.pl script will be fixed by bug 10613. If the pref contains several values, the first one is the default one (according to the syspref description). Test plan: basketgroup.pl: create a vendor with List prices don't include tax and invoice prices include tax. add 1 basket with 1+ order(s) receive them and check that the total amount on the basket page is the same as on the basketgroup page Verify this is right for other vendor configurations. On parcel.pl, this patch only removes dead code. The $gst variable is not used. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11733 --- Comment #6 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Galen Charlton from comment #4)
I think there is another bug lurking here. The 'gist' contains a list of decimal values, so dividing the default one by 100 leads to the wrong result if the default tax rate isn't 0.
Actually this code does not work and should simply be removed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11733 sandboxes@biblibre.com <sandboxes@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |sandboxes@biblibre.com --- Comment #7 from sandboxes@biblibre.com <sandboxes@biblibre.com> --- Patch tested with a sandbox, by sonia.bouis@univ-lyon3.fr <2> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11733 sandboxes@biblibre.com <sandboxes@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #29623|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11733 --- Comment #8 from sandboxes@biblibre.com <sandboxes@biblibre.com> --- Created attachment 34198 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34198&action=edit Bug 12567: Make catalog statistics wizard publication year work for MARC21 Publication year was useless for MARC21, as it was using the biblioitems.publicationyear (UNIMARC) instead of biblio.copyrightdate (MARC21). This patch tries to fix the search options to work for both MARC flavours. To test: In Reports > Statistics wizards > Catalog: - Test the publication year search options on a UNIMARC and a MARC21 installation. - Make sure search results make sense for both. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> The changes are fine with me, but this script is certainly not bug free. Also note that all its SQL should be moved out according to our coding guidelines. When doing so, a rewrite would be appropriate. Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> Signed-off-by: sonia.bouis@univ-lyon3.fr <2> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11733 Koha Team Lyon 3 <koha@univ-lyon3.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11733 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #34198|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11733 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #29623|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11733 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de Status|Needs Signoff |RESOLVED Resolution|--- |MOVED --- Comment #9 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Will be taken care of with the tax rewrite. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org