Koha::Acquisition::Bookseller has been introduced by bug 12896 but no consensus on the DBIx::Class implementation was reach. Now that we have Koha::Object (cf bug 13019), it should use it.
Created attachment 35992 [details] [review] Bug 13726: Make Koha::Acq::Bookseller using Koha::Object This patch create a Koha::Acquisition::Booksellers module and Koha::Acquisition::Bookseller::Contract[s] modules. All code in the acquisition module is adapted to use the CRUD methods of Koha::Object[s]. The former C4 routines are removed. Test plan: Since a lot of files are impacted by this patch, try a complete acquisition workflow and try to catch errors. Be focused on bookseller and bookseller's contracts data.
I did not test this patch deeply and there are certainly some bugs, but I would like to get feedback on it. I am waiting for confirmation before continuing with Koha::Acquisition::Order and Koha::Acquisition::Contract.
Note also that a lot of code could be removed (we should pass the object directly to the template instead of all values), but I would prefer to do that in another patch.
Auto-merging acqui/invoice.pl CONFLICT (content): Merge conflict in acqui/invoice.pl Auto-merging acqui/basket.pl CONFLICT (content): Merge conflict in acqui/basket.pl Auto-merging C4/Acquisition.pm CONFLICT (content): Merge conflict in C4/Acquisition.pm After applying 13719...
Created attachment 36977 [details] [review] Bug 13726: Make Koha::Acq::Bookseller using Koha::Object This patch create a Koha::Acquisition::Booksellers module and Koha::Acquisition::Bookseller::Contract[s] modules. All code in the acquisition module is adapted to use the CRUD methods of Koha::Object[s]. The former C4 routines are removed. Test plan: Since a lot of files are impacted by this patch, try a complete acquisition workflow and try to catch errors. Be focused on bookseller and bookseller's contracts data.
Could you please rebase?
Not yet :) This one depends on bug 13323.
Bug 13323 is in master, could you rebase now?
Created attachment 57253 [details] [review] Bug 13726: Make Koha::Acq::Bookseller using Koha::Object This patch create a Koha::Acquisition::Booksellers module and Koha::Acquisition::Bookseller::Contract[s] modules. All code in the acquisition module is adapted to use the CRUD methods of Koha::Object[s]. The former C4 routines are removed. Test plan: Since a lot of files are impacted by this patch, try a complete acquisition workflow and try to catch errors. Be focused on bookseller and bookseller's contracts data.
That was an awful rebase, I hope I did not loose too much things...
Created attachment 57263 [details] [review] Bug 13726: Make Koha::Acq::Bookseller using Koha::Object This patch create a Koha::Acquisition::Booksellers module and Koha::Acquisition::Bookseller::Contract[s] modules. All code in the acquisition module is adapted to use the CRUD methods of Koha::Object[s]. The former C4 routines are removed. Test plan: Since a lot of files are impacted by this patch, try a complete acquisition workflow and try to catch errors. Be focused on bookseller and bookseller's contracts data.
Thanks for rebasing! I started to test and here are first comments When receiving shipment (script parcel.pl) I got this error: "The method listinct is not covered by tests!" In tests: t/db_dependent/Bookseller.t fails with Can't locate object method "new" via package "Koha::Acquisition::Bookseller::Contact" (perhaps you forgot to load "Koha::Acquisition::Bookseller::Contact"?) at t/db_dependent/Bookseller.t line 686. t/db_dependent/Contract.t fails with Can't locate object method "new" via package "Koha::Acquisition::Bookseller" (perhaps you forgot to load "Koha::Acquisition::Bookseller"?) at t/db_dependent/Contract.t line 42. t/db_dependent/Letters.t fails with "my" variable $bookseller masks earlier declaration in same scope at t/db_dependent/Letters.t line 418. and Can't locate object method "new" via package "Koha::Acquisition::Bookseller::Contact" (perhaps you forgot to load "Koha::Acquisition::Bookseller::Contact"?) at t/db_dependent/Letters.t line 379. All other Acquisition test are passing - I am testing on top of 17562, 17563 and 17564 I tried all related (I hope) tests, and in ui I tested Vendor, Contracts, Baskets adding to order, and so on... I will continue after the error in parcel.pl will be fixed, thanks!
koha-qa comments: FAIL Koha/Acquisition/Bookseller/Contact.pm FAIL forbidden patterns forbidden pattern: Warning: The 'sub type' may be wrong is declared in a Koha::* package (see bug 15446) (line 9) FAIL Koha/Acquisition/Bookseller/Contacts.pm FAIL forbidden patterns forbidden pattern: Warning: The 'sub type' may be wrong is declared in a Koha::* package (see bug 15446) (line 9) FAIL t/db_dependent/Letters.t FAIL valid "my" variable $bookseller masks earlier declaration in same scope
Hi Josef, Yes it seems that the Contact code need a lot of work to be adapted. I give up for now.
Created attachment 57714 [details] [review] Bug 13726: Make Koha::Acq::Bookseller using Koha::Object This patch create a Koha::Acquisition::Booksellers module and Koha::Acquisition::Bookseller::Contract[s] modules. All code in the acquisition module is adapted to use the CRUD methods of Koha::Object[s]. The former C4 routines are removed. Test plan: Since a lot of files are impacted by this patch, try a complete acquisition workflow and try to catch errors. Be focused on bookseller and bookseller' contacts data.
Created attachment 57785 [details] [review] Bug 13726: Make Koha::Acq::Bookseller using Koha::Object This patch create a Koha::Acquisition::Booksellers module and Koha::Acquisition::Bookseller::Contract[s] modules. All code in the acquisition module is adapted to use the CRUD methods of Koha::Object[s]. The former C4 routines are removed. Test plan: Since a lot of files are impacted by this patch, try a complete acquisition workflow and try to catch errors. Be focused on bookseller and bookseller' contacts data.
I faced some ISE during testing (plack on): Supplier.pl: Can't locate object method "search" via package "Koha::Subscriptions" (perhaps you forgot to load "Koha::Subscriptions"?) at /home/vagrant/kohaclone/Koha/Acquisition/Bookseller.pm line 25. Invoices.pl op=do_search: Can't use string ("Koha::Acquisition::Bookseller") as a HASH ref while "strict refs" in use at /home/vagrant/kohaclone/Koha/Object.pm line 219.
Created attachment 57792 [details] [review] Bug 13726: Make Koha::Acq::Bookseller using Koha::Object This patch create a Koha::Acquisition::Booksellers module and Koha::Acquisition::Bookseller::Contract[s] modules. All code in the acquisition module is adapted to use the CRUD methods of Koha::Object[s]. The former C4 routines are removed. Test plan: Since a lot of files are impacted by this patch, try a complete acquisition workflow and try to catch errors. Be focused on bookseller and bookseller' contacts data.
Very close now I think. Just one issue I discovered now - when adding a serial subscription, I tried to search for a vendor: search is working, but no vendor name is shown in results table and vendor id is not transferred to new subscription form when clicking on Choose button.
Created attachment 57803 [details] [review] Bug 13726: Fix for serials/acqui-search-result.pl
Created attachment 57804 [details] [review] [SIGNED-OFF] Bug 13726: Make Koha::Acq::Bookseller using Koha::Object This patch create a Koha::Acquisition::Booksellers module and Koha::Acquisition::Bookseller::Contract[s] modules. All code in the acquisition module is adapted to use the CRUD methods of Koha::Object[s]. The former C4 routines are removed. Test plan: Since a lot of files are impacted by this patch, try a complete acquisition workflow and try to catch errors. Be focused on bookseller and bookseller' contacts data. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 57805 [details] [review] [SIGNED-OFF] Bug 13726: Fix for serials/acqui-search-result.pl Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 58473 [details] [review] Bug 13726: Make Koha::Acq::Bookseller using Koha::Object This patch create a Koha::Acquisition::Booksellers module and Koha::Acquisition::Bookseller::Contract[s] modules. All code in the acquisition module is adapted to use the CRUD methods of Koha::Object[s]. The former C4 routines are removed. Test plan: Since a lot of files are impacted by this patch, try a complete acquisition workflow and try to catch errors. Be focused on bookseller and bookseller' contacts data. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 58474 [details] [review] Bug 13726: Fix for serials/acqui-search-result.pl Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 58475 [details] [review] Bug 13726 - (QA followup) Fix vendor retrieval in invoices.pl Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Pushed to master for 17.05, thanks Jonathan, Nick!
This appears to dangerous for stable being part of the API rewrite - changing it to enh. As enh this won't be ported back to 16.11.