On our freshly updated 16.11 instance, we noticed that on pages /cgi-bin/koha/acqui/booksellers.pl and /cgi-bin/koha/acqui/supplier.pl, the acquisition toolbar always shows the "Delete vendor" button instead of the "Receive shipments", no matter the number of baskets attached to the vendor. When I looking on conditions on koha-tmpl/intranet-tmpl/prog/en/include/acquisitions-toolbar.inc, I saw that basketcount variable is not set. acqui/booksellers.pl and acqui/supplier.pl calling $suppliers[0]->{'basketcount'} and $supplier->{'basketcount'} from Koha::Acquisition::Bookseller while Bookseller.pm has a basket_count method to return it, explaining why the "Delete vendor" button is always shown. The problem is mainly cosmetic but with it, the test on vendor deletion in supplier.pl line 95... if ( $supplier->{'basketcount'} == 0 ) { ...always returns true so the only thing preventing deletion of vendor with baskets attached on is the database foreign key contraint.
I verified the problem in 16.11.x, but might need some help in order to fix it. The template expects a variable basketcount to contain the number of linked baskets. It's undefined in 16.11.x. The problem is in the API rewrite: Bug 12896: Move the bookseller-related code into Koha::Acquisition::Bookseller Changed the code from using GetBookSellerFromId which returned the count, to use the new API: $supplier = Koha::Acquisition::Bookseller->fetch({ id => $booksellerid }); This doesn't return the counts. The problem doesn't appear in 17.05, because in December after 16.11.x was released, another heavy change was made that fixed things in the 17.05 branch: Bug 13726 - Koha::Acquisition::Bookseller should use Koha::Object It added Koha::Acquisition::Booksellers and fixed the count variables for baskets and subscriptions. How to fix? GetBookSellerFromId is no longer there, but I am also missing the newer methods used in 17.05: + $supplier = Koha::Acquisition::Booksellers->find( $booksellerid ); basketcount => $supplier->baskets->count, Dependency trees from that far back are hard to unravel and trying to do so might cause new bad side effects. I think a 16.11.x specific fix for this problem only would be preferrable at this point in time. Can someone help?
Created attachment 69178 [details] [review] Rennes 2 local correction for basketcount and subscriptioncount calls Here's my local correction. A little bit nasty but it make its job and works on each calls of basketcount and subscriptioncount.
Created attachment 69708 [details] [review] Bug 19593 Basketcount correction on Bookseller.pm To test: 1/ go to /cgi-bin/koha/acqui/booksellers.pl or /cgi-bin/koha/acqui/supplier.pl 2/ Note that it displays the delete vendor button instead of the receive shipments button 3/ Apply patch 4/ Reload the page and notice the button is fixed Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Thx Gwendal and Chris!
Created attachment 69761 [details] [review] Bug 19593 Basketcount correction on Bookseller.pm To test: 1/ go to /cgi-bin/koha/acqui/booksellers.pl or /cgi-bin/koha/acqui/supplier.pl 2/ Note that it displays the delete vendor button instead of the receive shipments button 3/ Apply patch 4/ Reload the page and notice the button is fixed Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Tested: - vendor with no orders and subscriptions (button) - vendor with orders (no button) - vendor with subscriptions (no button) Just noticed something, author line reads: Author: KohaR2 <koha@listes.uhb.fr> Gwendal, do you want us to adjust that to your name and personal email?
Created attachment 69769 [details] [review] Bug 19593 Basketcount correction on Bookseller.pm Arf, default user/email on our preprod server. Here's the corrected version.
Pushed to 16.11.x will be in 16.11.15 Thats Gwendal
> Thats Gwendal That should be Thanks Gwendal :)
This change broke 3 tests: https://jenkins.koha-community.org/job/Koha_16.11_D8/lastCompletedBuild/testReport/
(In reply to Jonathan Druart from comment #10) > This change broke 3 tests: > https://jenkins.koha-community.org/job/Koha_16.11_D8/lastCompletedBuild/ > testReport/ Yup, the tests are wrong so they should break. I'll fix them this afternoon ;)
Interestingly the test fails without this patch # at t/db_dependent/Bookseller.t line 121. # Structures begin differing at: # $got->{discount} = '2' # $expected->{discount} = '2.0000' Just in a different way. Ill try to fix both issues
Ok, discount is a float(6,4) .. and tax_rate is a decimal(6,4), $VAR1 = { 'bookselleremail' => undef, 'notes' => 'notes2', 'address2' => 'address2-2', 'booksellerurl' => undef, 'booksellerfax' => undef, 'url' => 'url2', 'address1' => 'address1_2', 'phone' => 'phone2', 'listprice' => undef, 'fax' => 'fax2', 'id' => 44, 'invoiceincgst' => 1, 'currency' => '', 'accountnumber' => 'accountnumber2', 'name' => 'Name2', 'deliverytime' => 2, 'postal' => 'postal2', 'tax_rate' => '2.0000', 'gstreg' => 1, 'listincgst' => 1, 'invoiceprice' => undef, 'address4' => 'address4_2', 'discount' => '2', 'othersupplier' => undef, 'address3' => 'address3_2', 'active' => 1 }; It's handing back 2 instead of 2.0000 but fine for tax_rate, if I change the db to be decimal, it works Not to do with this bug, ill fix the basketcount thing, and push that, but this test needs looking into more