If one merges records A and B and A has holds in it then we get following error in /var/log/koha/kohadev/plack-intranet-error.log: > [WARN] DBD::mysql::st execute failed: called with 4 bind variables when 3 are needed [for Statement "SELECT * FROM reserves WHERE biblionumber = ? AND (found <> ? AND found <> ? OR found is NULL) ORDER BY reservedate ASC" with ParamValues: 0=Null!, 1=Null!, 2=Null!] at /kohadevbox/koha/C4/Reserves.pm line 2002.
Created attachment 113592 [details] [review] Bug 27012: Fix incorrect SQL syntax in hold merging If you merge two records with holds in them following error happens without this patch: [WARN] DBD::mysql::st execute failed: called with 4 bind variables when 3 are needed [for Statement "SELECT * FROM reserves WHERE biblionumber = ? AND (found <> ? AND found <> ? OR found is NULL) ORDER BY reservedate ASC" with ParamValues: 0=Null!, 1=Null!, 2=Null!] at /kohadevbox/koha/C4/Reserves.pm line 2002. To test: 1) Create a hold in record A 2) Use cataloging search to search for something that brings up record A and another record B. 3) Select A and B records in the search results and click Edit -> Merge records 4) Merge the records and notice that the above error is printed to plack-intranet-error.log 5) Apply patch and repeat steps 1-4 but notice the error is gone
Can you provide a test please?
Created attachment 113593 [details] [review] Bug 27012: Add test for C4::Reserves::MergeHolds This does a basic check whether a hold gets moved to another biblio when MergeHolds() is called.
Created attachment 113594 [details] [review] Bug 27012: Fix incorrect SQL syntax in hold merging If you merge two records with holds in them following error happens without this patch: [WARN] DBD::mysql::st execute failed: called with 4 bind variables when 3 are needed [for Statement "SELECT * FROM reserves WHERE biblionumber = ? AND (found <> ? AND found <> ? OR found is NULL) ORDER BY reservedate ASC" with ParamValues: 0=Null!, 1=Null!, 2=Null!] at /kohadevbox/koha/C4/Reserves.pm line 2002. To test: 1) Notice prove t/db_dependent/Reserves.t fails with above error 2) Apply patch 3) Notice prove t/db_dependent/Reserves.t passes
(In reply to Jonathan Druart from comment #2) > Can you provide a test please? I provided but I'm still against having those for old untested subroutines because those are mostly very long and hard to provide tests for due to having multiple functions combined in them and will have to go another round of changes when moving to Koha namespace. Also I think it as a separate matter that can be handled at another time and does not relate to the bug itself. This would allow the testers and QA team to work on smaller tasks therefore allowing higher chance of finding the time to review the code and pay more attention to every detail in the patch compared to long patch set with multiple goals (in this case: adding tests, fixing bug).
Created attachment 113632 [details] [review] Bug 27012: Add test for C4::Reserves::MergeHolds This does a basic check whether a hold gets moved to another biblio when MergeHolds() is called. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 113633 [details] [review] Bug 27012: Fix incorrect SQL syntax in hold merging If you merge two records with holds in them following error happens without this patch: [WARN] DBD::mysql::st execute failed: called with 4 bind variables when 3 are needed [for Statement "SELECT * FROM reserves WHERE biblionumber = ? AND (found <> ? AND found <> ? OR found is NULL) ORDER BY reservedate ASC" with ParamValues: 0=Null!, 1=Null!, 2=Null!] at /kohadevbox/koha/C4/Reserves.pm line 2002. To test: 1) Notice prove t/db_dependent/Reserves.t fails with above error 2) Apply patch 3) Notice prove t/db_dependent/Reserves.t passes Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Manual test and automated test ok! :)
Created attachment 113651 [details] [review] Bug 27012: Add test for C4::Reserves::MergeHolds This does a basic check whether a hold gets moved to another biblio when MergeHolds() is called. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 113652 [details] [review] Bug 27012: Fix incorrect SQL syntax in hold merging If you merge two records with holds in them following error happens without this patch: [WARN] DBD::mysql::st execute failed: called with 4 bind variables when 3 are needed [for Statement "SELECT * FROM reserves WHERE biblionumber = ? AND (found <> ? AND found <> ? OR found is NULL) ORDER BY reservedate ASC" with ParamValues: 0=Null!, 1=Null!, 2=Null!] at /kohadevbox/koha/C4/Reserves.pm line 2002. To test: 1) Notice prove t/db_dependent/Reserves.t fails with above error 2) Apply patch 3) Notice prove t/db_dependent/Reserves.t passes Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Pushed to master for 20.11, thanks to everybody involved!
Missing dependencies, no backport for 20.05.x