| Summary: | SQLHelper replacement - C4::Acquisition | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Yohann Dufour <yohann.dufour> |
| Component: | Architecture, internals, and plumbing | Assignee: | Yohann Dufour <yohann.dufour> |
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | bgkriegel, dpavlin, jonathan.druart, tomascohen |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Small patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Bug Depends on: | |||
| Bug Blocks: | 11385, 12891 | ||
| Attachments: |
Bug 12626: SQLHelper replacement - C4::Acquisition
[SIGNED-OFF] Bug 12626: SQLHelper replacement - C4::Acquisition Bug 12626: SQLHelper replacement - C4::Acquisition [SIGNED-OFF] Bug 12626: SQLHelper replacement - C4::Acquisition [PASSED QA] Bug 12626: SQLHelper replacement - C4::Acquisition |
||
|
Description
Yohann Dufour
2014-07-22 10:14:33 UTC
Created attachment 29953 [details] [review] Bug 12626: SQLHelper replacement - C4::Acquisition With this patch, the subroutine NewOrder uses DBIx::Class instead of C4::SQLHelper. Test plan: 1) Apply the patch 2) Execute the unit tests by launching : prove t/db_dependent/Acquisition.t 3) The result has to be a success without error or warning : t/db_dependent/Acquisition.t .. ok All tests successful. Files=1, Tests=79, 2 wallclock secs ( 0.04 usr 0.01 sys + 1.80 cusr 0.09 csys = 1.94 CPU) Result: PASS Created attachment 30093 [details] [review] [SIGNED-OFF] Bug 12626: SQLHelper replacement - C4::Acquisition With this patch, the subroutine NewOrder uses DBIx::Class instead of C4::SQLHelper. Test plan: 1) Apply the patch 2) Execute the unit tests by launching : prove t/db_dependent/Acquisition.t 3) The result has to be a success without error or warning : t/db_dependent/Acquisition.t .. ok All tests successful. Files=1, Tests=79, 2 wallclock secs ( 0.04 usr 0.01 sys + 1.80 cusr 0.09 csys = 1.94 CPU) Result: PASS Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> I got: DBIx::Class::Row::new(): No such column 'origquantityrec' on Koha::Schema::Result::Aqorder at /home/koha/src/C4/Acquisition.pm line 1278 On creating an order via the acquisition module. Created attachment 30188 [details] [review] Bug 12626: SQLHelper replacement - C4::Acquisition With this patch, the subroutine NewOrder uses DBIx::Class instead of C4::SQLHelper. Test plan: 1) Apply the patch 2) Execute the unit tests by launching : prove t/db_dependent/Acquisition.t 3) The result has to be a success without error or warning : t/db_dependent/Acquisition.t .. ok All tests successful. Files=1, Tests=79, 2 wallclock secs ( 0.04 usr 0.01 sys + 1.80 cusr 0.09 csys = 1.94 CPU) Result: PASS 4) Log in the koha intranet and create a new order in the acquition module 5) The creation has to be a success Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 30602 [details] [review] [SIGNED-OFF] Bug 12626: SQLHelper replacement - C4::Acquisition With this patch, the subroutine NewOrder uses DBIx::Class instead of C4::SQLHelper. Test plan: 1) Apply the patch 2) Execute the unit tests by launching : prove t/db_dependent/Acquisition.t 3) The result has to be a success without error or warning : t/db_dependent/Acquisition.t .. ok All tests successful. Files=1, Tests=79, 2 wallclock secs ( 0.04 usr 0.01 sys + 1.80 cusr 0.09 csys = 1.94 CPU) Result: PASS 4) Log in the koha intranet and create a new order in the acquition module 5) The creation has to be a success Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Test pass, new order created without problem, no koha-qa errors Created attachment 30893 [details] [review] [PASSED QA] Bug 12626: SQLHelper replacement - C4::Acquisition With this patch, the subroutine NewOrder uses DBIx::Class instead of C4::SQLHelper. Test plan: 1) Apply the patch 2) Execute the unit tests by launching : prove t/db_dependent/Acquisition.t 3) The result has to be a success without error or warning : t/db_dependent/Acquisition.t .. ok All tests successful. Files=1, Tests=79, 2 wallclock secs ( 0.04 usr 0.01 sys + 1.80 cusr 0.09 csys = 1.94 CPU) Result: PASS 4) Log in the koha intranet and create a new order in the acquition module 5) The creation has to be a success Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Test pass, new order created without problem, no koha-qa errors Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Tested creating a new order from a subscription, no problems found. Passes tests and QA script. Patch pushed to master. Thanks Yohann! This bug introduced regression for acquisition in acqui/neworderempty.pl. In logs, I see error messages like: addorder.pl: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha`.`aqorders_items`, CONSTRAINT `aqorders_items_ibfk_1` FOREIGN KEY (`ordernumber`) REFERENCES `aqorders` (`ordernumber`) ON DELETE CASCADE ON UPDATE CASCADE) at /srv/koha_ffzg//C4/Acquisition.pm line 1317., referer: https://koha.ffzg.hr:8443/cgi-bin/koha/acqui/neworderempty.pl?booksellerid=54&basketno=3764&biblionumber=237202 And indeed, adding additional debug warns all over the place, it seems that code inside C4/Acquisition.pm sub NewOrder does: my $ordernumber = $rs->create($new_order)->id; which doesn't return ordernumber but instead empty string "" and all hell breaks loose (resulting in missing entries in aqorders_items. My DBIC foo isn't that good, but cookbook suggest that above code is correct: http://search.cpan.org/~danieltwc/DBIx-Class-0.07002/lib/DBIx/Class/Manual/Cookbook.pod#Getting_the_value_of_the_primary_key_for_the_last_database_insert I'm puzzled and confused how to fix this. It's quite weird, the unit tests pass! I got the same error when I create a new order with items. (In reply to Dobrica Pavlinusic from comment #8) This bug 12891 for a fix. |