C4::Acquisition::NewOrder needs the basketno, it is useless to return it.
Created attachment 31171 [details] [review] Bug 12827: NewOrder should not return basketno Since the basketno parameter is needed to insert an order, it is useless to return it. This patch changes the prototype for the C4::Acquisition::NewOrder subroutine. The return value is now a scalar containing the ordernumber created. Test plan: Verify there is no regression on an acquisition workflow: 1/ Create an order with several items 2/ Modify the order 3/ Receive some items 4/ Cancel the receipt 4/ Receive some items 5/ Receive all remaining items 6/ Cancel the receipt
Created attachment 31172 [details] [review] Bug 12827: NewOrder needs more unit tests NewOrder should be more tested! This patch moves the existing unit tests into a new file and adds some unit tests. Note that there is no DB field aqorders.subscription, so the test in NewOrder can be removed. Test plan: prove t/db_dependent/Acquisition/NewOrder.t and prove t/db_dependent/Acquisition.t should return green.
Created attachment 31235 [details] [review] Bug 12827: NewOrder should not return basketno Since the basketno parameter is needed to insert an order, it is useless to return it. This patch changes the prototype for the C4::Acquisition::NewOrder subroutine. The return value is now a scalar containing the ordernumber created. Test plan: Verify there is no regression on an acquisition workflow: 1/ Create an order with several items 2/ Modify the order 3/ Receive some items 4/ Cancel the receipt 4/ Receive some items 5/ Receive all remaining items 6/ Cancel the receipt
Created attachment 31236 [details] [review] Bug 12827: NewOrder needs more unit tests NewOrder should be more tested! This patch moves the existing unit tests into a new file and adds some unit tests. Note that there is no DB field aqorders.subscription, so the test in NewOrder can be removed. Test plan: prove t/db_dependent/Acquisition/NewOrder.t and prove t/db_dependent/Acquisition.t should return green.
patches rebased.
Testing the patch on sandbox 1 (http://pro.test1.biblibre.com/) I receive an error when a click on Acquisition tab, link: http://pro.test1.biblibre.com/cgi-bin/koha/acqui/acqui-home.pl The error: Global symbol "$rs" requires explicit package name at /home/koha/src/C4/Acquisition.pm line 1281. Compilation failed in require at /home/koha/src/acqui/acqui-home.pl line 36. BEGIN failed--compilation aborted at /home/koha/src/acqui/acqui-home.pl line 36.
Created attachment 31339 [details] [review] Bug 12827: NewOrder should not return basketno Since the basketno parameter is needed to insert an order, it is useless to return it. This patch changes the prototype for the C4::Acquisition::NewOrder subroutine. The return value is now a scalar containing the ordernumber created. Test plan: Verify there is no regression on an acquisition workflow: 1/ Create an order with several items 2/ Modify the order 3/ Receive some items 4/ Cancel the receipt 4/ Receive some items 5/ Receive all remaining items 6/ Cancel the receipt
Created attachment 31340 [details] [review] Bug 12827: NewOrder needs more unit tests NewOrder should be more tested! This patch moves the existing unit tests into a new file and adds some unit tests. Note that there is no DB field aqorders.subscription, so the test in NewOrder can be removed. Test plan: prove t/db_dependent/Acquisition/NewOrder.t and prove t/db_dependent/Acquisition.t should return green.
(In reply to Zeno Tajoli from comment #6) > The error: > > Global symbol "$rs" requires explicit package name at > /home/koha/src/C4/Acquisition.pm line 1281. > Compilation failed in require at /home/koha/src/acqui/acqui-home.pl line 36. > BEGIN failed--compilation aborted at /home/koha/src/acqui/acqui-home.pl line > 36. Sorry, I introduced this error on rebasing the patches. Should be fixed now.
Test with prove are OK. prove t/db_dependent/Acquisition/NewOrder.t OK prove t/db_dependent/Acquisition.t OK prove t/db_dependent/ OK prove t/ OK prove xt/ OK prove xt/author OK Tommorrow (04/09/2014) test about regression on acquisition workflow.
Patch tested with a sandbox, by Zeno Tajoli <z.tajoli@cineca.it>
Created attachment 31401 [details] [review] Bug 12827: NewOrder should not return basketno Since the basketno parameter is needed to insert an order, it is useless to return it. This patch changes the prototype for the C4::Acquisition::NewOrder subroutine. The return value is now a scalar containing the ordernumber created. Test plan: Verify there is no regression on an acquisition workflow: 1/ Create an order with several items 2/ Modify the order 3/ Receive some items 4/ Cancel the receipt 4/ Receive some items 5/ Receive all remaining items 6/ Cancel the receipt Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it>
Created attachment 31402 [details] [review] Bug 12827: NewOrder needs more unit tests NewOrder should be more tested! This patch moves the existing unit tests into a new file and adds some unit tests. Note that there is no DB field aqorders.subscription, so the test in NewOrder can be removed. Test plan: prove t/db_dependent/Acquisition/NewOrder.t and prove t/db_dependent/Acquisition.t should return green. Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it>
I've tried to apply against master 3.17.00.023 There is a conflict in the tests: Applying: Bug 12827: NewOrder should not return basketno Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/Budgets.t Auto-merging t/db_dependent/Acquisition/CancelReceipt.t CONFLICT (content): Merge conflict in t/db_dependent/Acquisition/CancelReceipt.t Auto-merging C4/Acquisition.pm Failed to merge in the changes. Patch failed at 0001 Bug 12827: NewOrder should not return basketno I pass the patch to "Patch doesn't apply" status.
Created attachment 31494 [details] [review] Bug 12827: NewOrder should not return basketno Since the basketno parameter is needed to insert an order, it is useless to return it. This patch changes the prototype for the C4::Acquisition::NewOrder subroutine. The return value is now a scalar containing the ordernumber created. Test plan: Verify there is no regression on an acquisition workflow: 1/ Create an order with several items 2/ Modify the order 3/ Receive some items 4/ Cancel the receipt 4/ Receive some items 5/ Receive all remaining items 6/ Cancel the receipt Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it>
Created attachment 31495 [details] [review] Bug 12827: NewOrder needs more unit tests NewOrder should be more tested! This patch moves the existing unit tests into a new file and adds some unit tests. Note that there is no DB field aqorders.subscription, so the test in NewOrder can be removed. Test plan: prove t/db_dependent/Acquisition/NewOrder.t and prove t/db_dependent/Acquisition.t should return green. Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it>
Created attachment 31519 [details] [review] Bug 12827: NewOrder should not return basketno Since the basketno parameter is needed to insert an order, it is useless to return it. This patch changes the prototype for the C4::Acquisition::NewOrder subroutine. The return value is now a scalar containing the ordernumber created. Test plan: Verify there is no regression on an acquisition workflow: 1/ Create an order with several items 2/ Modify the order 3/ Receive some items 4/ Cancel the receipt 4/ Receive some items 5/ Receive all remaining items 6/ Cancel the receipt Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it>
Created attachment 31520 [details] [review] Bug 12827: NewOrder needs more unit tests NewOrder should be more tested! This patch moves the existing unit tests into a new file and adds some unit tests. Note that there is no DB field aqorders.subscription, so the test in NewOrder can be removed. Test plan: prove t/db_dependent/Acquisition/NewOrder.t and prove t/db_dependent/Acquisition.t should return green. Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it>
Created attachment 31562 [details] [review] [PASSED QA] Bug 12827: NewOrder should not return basketno Since the basketno parameter is needed to insert an order, it is useless to return it. This patch changes the prototype for the C4::Acquisition::NewOrder subroutine. The return value is now a scalar containing the ordernumber created. Test plan: Verify there is no regression on an acquisition workflow: 1/ Create an order with several items 2/ Modify the order 3/ Receive some items 4/ Cancel the receipt 4/ Receive some items 5/ Receive all remaining items 6/ Cancel the receipt Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 31563 [details] [review] [PASSED QA] Bug 12827: NewOrder needs more unit tests NewOrder should be more tested! This patch moves the existing unit tests into a new file and adds some unit tests. Note that there is no DB field aqorders.subscription, so the test in NewOrder can be removed. Test plan: prove t/db_dependent/Acquisition/NewOrder.t and prove t/db_dependent/Acquisition.t should return green. Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Patches pushed to master. Thanks Jonathan!
This patch missed 1 occurrence of NewOrder (introduced in master after I wrote this patch). See bug 12953 for a fix.