Summary: | [16.11.x] "Delete vendor" button is always set | ||
---|---|---|---|
Product: | Koha | Reporter: | Gwendal JONCOUR <gwendal.joncour> |
Component: | Acquisitions | Assignee: | Bugs List <koha-bugs> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | major | ||
Priority: | P5 - low | CC: | chris, jonathan.druart, katrin.fischer |
Version: | 16.11 | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Attachments: |
Rennes 2 local correction for basketcount and subscriptioncount calls
Bug 19593 Basketcount correction on Bookseller.pm Bug 19593 Basketcount correction on Bookseller.pm Bug 19593 Basketcount correction on Bookseller.pm |
Description
Gwendal JONCOUR
2017-11-08 15:58:42 UTC
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 |