[Bug 12493] New: The module C4::Acquisition.pm contains subroutines which belong to C4::Contract.pm
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 Bug ID: 12493 Summary: The module C4::Acquisition.pm contains subroutines which belong to C4::Contract.pm Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Test Suite Assignee: chris@bigballofwax.co.nz Reporter: yohann.dufour@biblibre.com QA Contact: gmcharlt@gmail.com The module C4::Acquisition.pm has two subroutines : GetContract and GetContracts which normally belong to C4::Contract.pm. Moreover, the module C4::Contract.pm needs unit tests. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 Yohann Dufour <yohann.dufour@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|chris@bigballofwax.co.nz |yohann.dufour@biblibre.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 Yohann Dufour <yohann.dufour@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Medium patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 --- Comment #1 from Yohann Dufour <yohann.dufour@biblibre.com> --- Created attachment 29329 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29329&action=edit Bug 12493: moving the subroutines GetContract and GetContracts from C4::Acquisition.pm to C4::Contract.pm This patch includes: - the subroutines GetContract and GetContracts has been moved from C4::Acquisition.pm to C4::Contract.pm and adapted for a general use - adaptation of acqui/basket.pl, acqui/basketheader.pl, acqui/neworderempty.pl, acqui/supplier.pl and admin/aqcontract.pl - the unit tests for the module C4::Contract.pm Test plan: 1) Apply the patch 2) Execute the unit tests by launching: prove t/db_dependent/Contract.t t/Acquisition/ t/db_dependent/Acquisition/ t/db_dependent/Acquisition.t 3) The command has to be a success : t/db_dependent/Contract.t ................................. ok t/Acquisition/CanUserManageBasket.t ....................... ok t/Acquisition/Invoice.t ................................... ok t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t .. ok t/db_dependent/Acquisition/GetOrdersByBiblionumber.t ...... ok t/db_dependent/Acquisition/Invoices.t ..................... ok t/db_dependent/Acquisition/OrderFromSubscription.t ........ ok t/db_dependent/Acquisition/TransferOrder.t ................ 1/11 # Transfering order to basket2 t/db_dependent/Acquisition/TransferOrder.t ................ ok t/db_dependent/Acquisition/close_reopen_basket.t .......... ok t/db_dependent/Acquisition.t .............................. ok All tests successful. Files=10, Tests=284, 15 wallclock secs ( 0.11 usr 0.02 sys + 12.88 cusr 0.77 csys = 13.78 CPU) Result: PASS 4) Log on with a superlibrarian permission 5) Go on the page acqui/supplier.pl (Acquisitions > Button "New vendor") 6) Record a vendor with a nonzero "name" 7) Go on the page admin/aqcontract.pl (click on the "Contracts" item in the menu) 8) Click on the button "New" > "Contract" and record a new one 9) Verify the displayed data are correct about the contract 10) "Edit" the contract with different values and verify the data are updated 11) Click on "Delete" in order to delete the contract, verify the displayed data are correct but cancel the operation 12) Click on "New" > "Basket" and verify there is the created contract in field "Contract", then record a basket by selectioning the created contract 13) Verify the contract name displayed is correct 14) Record an active budget and a fund linked to this budget 15) Go on the new basket (Home > Acquisitions > Search the created vendor) 16) Click on "Add to basket" then "From a new (empty) record" and verify the displayed contract name is correct -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 Yohann Dufour <yohann.dufour@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |12487 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 --- Comment #2 from Yohann Dufour <yohann.dufour@biblibre.com> --- Created attachment 29353 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29353&action=edit Bug 12493: unit tests for the module C4::Contract.pm This patch are the unit tests of the previous patch Test plan : Same as the previous patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 Yohann Dufour <yohann.dufour@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |12508 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #29329|0 |1 is obsolete| | --- Comment #3 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 29494 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29494&action=edit [SIGNED-OFF] Bug 12493: moving the subroutines GetContract and GetContracts from C4::Acquisition.pm to C4::Contract.pm This patch includes: - the subroutines GetContract and GetContracts has been moved from C4::Acquisition.pm to C4::Contract.pm and adapted for a general use - adaptation of acqui/basket.pl, acqui/basketheader.pl, acqui/neworderempty.pl, acqui/supplier.pl and admin/aqcontract.pl - the unit tests for the module C4::Contract.pm Test plan: 1) Apply the patch 2) Execute the unit tests by launching: prove t/db_dependent/Contract.t t/Acquisition/ t/db_dependent/Acquisition/ t/db_dependent/Acquisition.t 3) The command has to be a success : t/db_dependent/Contract.t ................................. ok t/Acquisition/CanUserManageBasket.t ....................... ok t/Acquisition/Invoice.t ................................... ok t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t .. ok t/db_dependent/Acquisition/GetOrdersByBiblionumber.t ...... ok t/db_dependent/Acquisition/Invoices.t ..................... ok t/db_dependent/Acquisition/OrderFromSubscription.t ........ ok t/db_dependent/Acquisition/TransferOrder.t ................ 1/11 # Transfering order to basket2 t/db_dependent/Acquisition/TransferOrder.t ................ ok t/db_dependent/Acquisition/close_reopen_basket.t .......... ok t/db_dependent/Acquisition.t .............................. ok All tests successful. Files=10, Tests=284, 15 wallclock secs ( 0.11 usr 0.02 sys + 12.88 cusr 0.77 csys = 13.78 CPU) Result: PASS 4) Log on with a superlibrarian permission 5) Go on the page acqui/supplier.pl (Acquisitions > Button "New vendor") 6) Record a vendor with a nonzero "name" 7) Go on the page admin/aqcontract.pl (click on the "Contracts" item in the menu) 8) Click on the button "New" > "Contract" and record a new one 9) Verify the displayed data are correct about the contract 10) "Edit" the contract with different values and verify the data are updated 11) Click on "Delete" in order to delete the contract, verify the displayed data are correct but cancel the operation 12) Click on "New" > "Basket" and verify there is the created contract in field "Contract", then record a basket by selectioning the created contract 13) Verify the contract name displayed is correct 14) Record an active budget and a fund linked to this budget 15) Go on the new basket (Home > Acquisitions > Search the created vendor) 16) Click on "Add to basket" then "From a new (empty) record" and verify the displayed contract name is correct Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Tested with both patches applied. Works as described following test plan, all points (I did 14 first) All test pass No koha-qa errors -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #29353|0 |1 is obsolete| | --- Comment #4 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 29495 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29495&action=edit [SIGNED-OFF] Bug 12493: unit tests for the module C4::Contract.pm This patch are the unit tests of the previous patch Test plan : Same as the previous patch Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Comments on previous patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |bgkriegel@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Comment on attachment 29494 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29494 [SIGNED-OFF] Bug 12493: moving the subroutines GetContract and GetContracts from C4::Acquisition.pm to C4::Contract.pm Review of attachment 29494: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=12493&attachment=29494) ----------------------------------------------------------------- ::: C4/Acquisition.pm @@ +363,5 @@
my @rows; for my $basket (@$baskets) { my @orders = GetOrders( $$basket{basketno} ); + my $contract = GetContract({ + contractnumber => $$basket{contractnumber}
I know this line existed before, but see here for future reference: http://wiki.koha-community.org/wiki/Coding_Guidelines#PERL5:_Disagreements And maybe fix in a follow up? ::: C4/Contract.pm @@ +21,1 @@
use strict;
When using Modern::Perl you can delete the use strict; and use warnings; lines here. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #6 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Creating a new file like Contract.pm is a good time to perltidy it - there is some mixup between tabs and spaces right at the beginning. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I started fixing this, but don't want to touch too many lines, so maybe it's better you do a follow up? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 Yohann Dufour <yohann.dufour@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 --- Comment #8 from Yohann Dufour <yohann.dufour@biblibre.com> --- Created attachment 29568 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29568&action=edit [FOLLOW UP] Bug 12493 : moving the subroutines GetContract and GetContracts from C4::Acquisition.pm to C4::Contract.pm Fix in order to respect the coding guidelines -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 --- Comment #9 from Yohann Dufour <yohann.dufour@biblibre.com> --- Katrin, I've tested my commit with the tools koha-qa.pl but it doesn't detect tabs or spaces, and I've not found the mixup you said. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Yohann, you are right, I thought Contract.pm was new, but it isn't - sorry for that and thx for your follow-up! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #29494|0 |1 is obsolete| | Attachment #29495|0 |1 is obsolete| | Attachment #29568|0 |1 is obsolete| | --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 29831 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29831&action=edit [PASSED QA] Bug 12493: moving the subroutines GetContract and GetContracts from C4::Acquisition.pm to C4::Contract.pm This patch includes: - the subroutines GetContract and GetContracts has been moved from C4::Acquisition.pm to C4::Contract.pm and adapted for a general use - adaptation of acqui/basket.pl, acqui/basketheader.pl, acqui/neworderempty.pl, acqui/supplier.pl and admin/aqcontract.pl - the unit tests for the module C4::Contract.pm Test plan: 1) Apply the patch 2) Execute the unit tests by launching: prove t/db_dependent/Contract.t t/Acquisition/ t/db_dependent/Acquisition/ t/db_dependent/Acquisition.t 3) The command has to be a success : t/db_dependent/Contract.t ................................. ok t/Acquisition/CanUserManageBasket.t ....................... ok t/Acquisition/Invoice.t ................................... ok t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t .. ok t/db_dependent/Acquisition/GetOrdersByBiblionumber.t ...... ok t/db_dependent/Acquisition/Invoices.t ..................... ok t/db_dependent/Acquisition/OrderFromSubscription.t ........ ok t/db_dependent/Acquisition/TransferOrder.t ................ 1/11 # Transfering order to basket2 t/db_dependent/Acquisition/TransferOrder.t ................ ok t/db_dependent/Acquisition/close_reopen_basket.t .......... ok t/db_dependent/Acquisition.t .............................. ok All tests successful. Files=10, Tests=284, 15 wallclock secs ( 0.11 usr 0.02 sys + 12.88 cusr 0.77 csys = 13.78 CPU) Result: PASS 4) Log on with a superlibrarian permission 5) Go on the page acqui/supplier.pl (Acquisitions > Button "New vendor") 6) Record a vendor with a nonzero "name" 7) Go on the page admin/aqcontract.pl (click on the "Contracts" item in the menu) 8) Click on the button "New" > "Contract" and record a new one 9) Verify the displayed data are correct about the contract 10) "Edit" the contract with different values and verify the data are updated 11) Click on "Delete" in order to delete the contract, verify the displayed data are correct but cancel the operation 12) Click on "New" > "Basket" and verify there is the created contract in field "Contract", then record a basket by selectioning the created contract 13) Verify the contract name displayed is correct 14) Record an active budget and a fund linked to this budget 15) Go on the new basket (Home > Acquisitions > Search the created vendor) 16) Click on "Add to basket" then "From a new (empty) record" and verify the displayed contract name is correct Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Tested with both patches applied. Works as described following test plan, all points (I did 14 first) All test pass No koha-qa errors Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 --- Comment #12 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 29832 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29832&action=edit [PASSED QA] Bug 12493: unit tests for the module C4::Contract.pm This patch are the unit tests of the previous patch Test plan : Same as the previous patch Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Comments on previous patch Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 --- Comment #13 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 29833 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29833&action=edit [PASSED QA] Bug 12493 : moving the subroutines GetContract and GetContracts from C4::Acquisition.pm to C4::Contract.pm Fix in order to respect the coding guidelines Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Unit tests pass but *ONLY* with but 12487 applied on top. These two bugs must be pushed simultaneously. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12493 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |tomascohen@gmail.com --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master. Thanks Yohann! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org