[Bug 12623] New: SQLHelper replacement - Koha::Borrower::Modifications
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 Bug ID: 12623 Summary: SQLHelper replacement - Koha::Borrower::Modifications Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: yohann.dufour@biblibre.com QA Contact: testopia@bugs.koha-community.org The module Koha::Borrower::Modifications depends on module C4::SQLHelper. However, the module SQLHelper is obsolete, so the calls for InsertInTable and UpdateInTable have to be replaced by the use of the module DBIx::Class. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 Yohann Dufour <yohann.dufour@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|gmcharlt@gmail.com |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=12623 Yohann Dufour <yohann.dufour@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |11385 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 Yohann Dufour <yohann.dufour@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 --- Comment #1 from Yohann Dufour <yohann.dufour@biblibre.com> --- Created attachment 29951 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29951&action=edit Bug 12623: SQLHelper replacement - Borrower::Modifications With this patch, the subroutines AddModification and ApproveModifications uses DBIx::Class instead of C4::SQLHelper. Test plan: 1) Apply the patch 2) Execute the unit tests by launching : prove t/db_dependent/Koha_borrower_modifications.t 3) The result has to be a success without error or warning : t/db_dependent/Koha_borrower_modifications.t .. ok All tests successful. Files=1, Tests=14, 2 wallclock secs ( 0.03 usr 0.01 sys + 1.60 cusr 0.08 csys = 1.72 CPU) Result: PASS -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 Yohann Dufour <yohann.dufour@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 Yohann Dufour <yohann.dufour@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 Yohann Dufour <yohann.dufour@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #29951|0 |1 is obsolete| | --- Comment #2 from Yohann Dufour <yohann.dufour@biblibre.com> --- Created attachment 29980 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29980&action=edit Bug 12623: SQLHelper replacement - Borrower::Modifications With this patch, the subroutines AddModification and ApproveModifications uses DBIx::Class instead of C4::SQLHelper. Moreover, the tests has been wrapped in a transaction. Test plan: 1) Apply the patch 2) Execute the unit tests by launching : prove t/db_dependent/Koha_borrower_modifications.t 3) The result has to be a success without error or warning : t/db_dependent/Koha_borrower_modifications.t .. ok All tests successful. Files=1, Tests=14, 2 wallclock secs ( 0.03 usr 0.01 sys + 1.60 cusr 0.08 csys = 1.72 CPU) Result: PASS -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #29980|0 |1 is obsolete| | --- Comment #3 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 30092 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30092&action=edit [SIGNED-OFF] Bug 12623: SQLHelper replacement - Borrower::Modifications With this patch, the subroutines AddModification and ApproveModifications uses DBIx::Class instead of C4::SQLHelper. Moreover, the tests has been wrapped in a transaction. Test plan: 1) Apply the patch 2) Execute the unit tests by launching : prove t/db_dependent/Koha_borrower_modifications.t 3) The result has to be a success without error or warning : t/db_dependent/Koha_borrower_modifications.t .. ok All tests successful. Files=1, Tests=14, 2 wallclock secs ( 0.03 usr 0.01 sys + 1.60 cusr 0.08 csys = 1.72 CPU) Result: PASS 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=12623 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |m.de.rooy@rijksmuseum.nl --- Comment #4 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- 1..14 ok 1 - Test AddModifications() and GetModifications() ok 2 - Test that Verify() succeeds with a valid token ok 3 - Test DelModifications() ok 4 - Test that Verify() method fails for a bad token ok 5 - Test GetPendingModificationsCount() ok 6 - Add a new pending modification and test GetPendingModificationsCount() again not ok 7 - Test GetPendingModifications() again # Failed test 'Test GetPendingModifications() again' # at t/db_dependent/Koha_borrower_modifications.t line 70. not ok 8 - Test GetPendingModifications() # Failed test 'Test GetPendingModifications()' # at t/db_dependent/Koha_borrower_modifications.t line 74. ok 9 - Test DenyModifications() ok 10 - Test ApproveModifications() removes pending modification from db ok 11 - Test ApproveModifications() applys modification to borrower ok 12 - Test GetPendingModificationsCount() ok 13 - Test ApproveModifications() removes pending modification from db, again ok 14 - Test ApproveModifications() applys modification to borrower, again # Looks like you failed 2 tests of 14. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 --- Comment #5 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- This part depends on the order of surnames in the borrowers table: ## Check GetPendingModifications my $pending = Koha::Borrower::Modifications->GetPendingModifications(); print "ZZZ".$pending->[0]->{firstname}."\n"; ok( $pending->[0]->{'firstname'} eq 'Sandy', 'Test GetPendingModifications() again' ); ok( $pending->[1]->{'firstname'} eq 'Kyle', 'Test GetPendingModifications()' ); $pending is sorted by surname, firstname My borrower 2 and 3 apparently had the order of surnames just in reverse as you had them while testing. Please make your test independent of this circumstance.. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|unspecified |master -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 --- Comment #6 from Yohann Dufour <yohann.dufour@biblibre.com> --- Did the tests pass before applying the patch ? Because my patch doesn't change the previous unit tests, so if the tests failed before applying the patch, I don't kow what I could do to solve it. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 --- Comment #7 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Yohann Dufour from comment #6)
Did the tests pass before applying the patch ? Because my patch doesn't change the previous unit tests, so if the tests failed before applying the patch, I don't kow what I could do to solve it.
In some cases it will pass, in other cases it won't. Depends on the surnames of your borrowers. Now that we stumble over it, I suggest you fix it. I am confident that you can do it in a few lines.. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 Yohann Dufour <yohann.dufour@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 --- Comment #8 from Yohann Dufour <yohann.dufour@biblibre.com> --- Created attachment 30628 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30628&action=edit Bug 12623: SQLHelper replacement - Borrower::Modifications With this patch, the subroutines AddModification and ApproveModifications uses DBIx::Class instead of C4::SQLHelper. Moreover, the tests has been wrapped in a transaction. Test plan: 1) Apply the patch 2) Execute the unit tests by launching : prove t/db_dependent/Koha_borrower_modifications.t 3) The result has to be a success without error or warning : t/db_dependent/Koha_borrower_modifications.t .. ok All tests successful. Files=1, Tests=14, 2 wallclock secs ( 0.03 usr 0.01 sys + 1.60 cusr 0.08 csys = 1.72 CPU) Result: PASS 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=12623 Yohann Dufour <yohann.dufour@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30092|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 --- Comment #9 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 30629 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30629&action=edit Bug 12623: SQLHelper replacement - Borrower::Modifications With this patch, the subroutines AddModification and ApproveModifications uses DBIx::Class instead of C4::SQLHelper. Moreover, the tests has been wrapped in a transaction. Test plan: 1) Apply the patch 2) Execute the unit tests by launching : prove t/db_dependent/Koha_borrower_modifications.t 3) The result has to be a success without error or warning : t/db_dependent/Koha_borrower_modifications.t .. ok All tests successful. Files=1, Tests=14, 2 wallclock secs ( 0.03 usr 0.01 sys + 1.60 cusr 0.08 csys = 1.72 CPU) Result: PASS Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30628|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #10 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- QA Comment: The signoff from Kyle is still valid for me. Test passes now (for the surnames). Code looks good to me. No qa tools complaints. Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |tomascohen@gmail.com --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patch pushed to master. Thanks Yohann! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- This broke the patron details update request from the OPAC: DBIx::Class::Storage::DBI::_dbh_execute(): Duplicate entry '1' for key 'PRIMARY' at /home/katrin/kohaclone/Koha/Borrower/Modifications.pm line 63 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |12222 --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- In another test I got: DBIx::Class::Row::new(): No such column 'B_email' on Koha::Schema::Result::BorrowerModification at /home/katrin/kohaclone/Koha/Borrower/Modifications.pm line 63 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 --- Comment #14 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Katrin Fischer from comment #13)
In another test I got: DBIx::Class::Row::new(): No such column 'B_email' on Koha::Schema::Result::BorrowerModification at /home/katrin/kohaclone/Koha/Borrower/Modifications.pm line 63
DBIx schema not up to date? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to M. de Rooy from comment #14)
(In reply to Katrin Fischer from comment #13)
In another test I got: DBIx::Class::Row::new(): No such column 'B_email' on Koha::Schema::Result::BorrowerModification at /home/katrin/kohaclone/Koha/Borrower/Modifications.pm line 63
DBIx schema not up to date?
This is broader than i first thought. Moved to its own bug 12781. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 Yohann Dufour <yohann.dufour@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=12784 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12623 --- Comment #16 from Yohann Dufour <yohann.dufour@biblibre.com> --- I've created bug 12784 for the bug about patron details update request from the OPAC. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org