Bug 18329 - Batch record deletion broken
Summary: Batch record deletion broken
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Fridolin SOMERS
QA Contact: Martin Renvoize
URL:
Whiteboard:
Keywords:
Depends on: 14504 18242
Blocks:
  Show dependency treegraph
 
Reported: 2017-03-24 11:58 UTC by Fridolin SOMERS
Modified: 2017-12-07 22:24 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 18329 - Batch record deletion broken (4.56 KB, patch)
2017-03-24 15:40 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 18329 - Batch record deletion broken (4.63 KB, patch)
2017-03-24 19:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18329 - Batch record deletion broken (4.70 KB, patch)
2017-03-29 17:06 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin SOMERS 2017-03-24 11:58:41 UTC
Hie, Tools > Batch record deletion seems broken.
Any deletion returns error :
Bibliographic record YYY was not deleted. An error occurred. (The error was: {UNKNOWN}: DBD::mysql::db begin_work failed: Already in a transaction at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1560. at /home/koha/src/C4/Biblio.pm line 3468 , see the Koha log file for more information).

Looks like it is because of Bug 18242 which added a transaction  in C4::Biblio::_koha_delete_biblio_metadata : $schema->txn_do.
The script batch_delete_records created a transaction with $dbh->{AutoCommit} = 0;
Comment 1 Fridolin SOMERS 2017-03-24 15:40:59 UTC
Created attachment 61613 [details] [review]
Bug 18329 - Batch record deletion broken

Hie, Tools > Batch record deletion seems broken.
Any deletion returns error :
Bibliographic record YYY was not deleted. An error occurred. (The error was: {UNKNOWN}: DBD::mysql::db begin_work failed: Already in a transaction at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1560. at /home/koha/src/C4/Biblio.pm line 3468 , see the Koha log file for more information).

Looks like it is because of Bug 18242 which added a transaction  in C4::Biblio::_koha_delete_biblio_metadata : $schema->txn_do.
The script batch_delete_records created a transaction with $dbh->{AutoCommit} = 0;

This patch fixes by using also Koha::Schema in batch_delete_records to manage transaction.
It also removes "$dbh->{RaiseError} = 1", this behavior is managed in Koha::Database.

Test plan :
 - Go to Staff interface : Tools > Batch record deletion
 - Enter a few existing biblionumbers
 - Click on "Continue"
 - Click on "Delete selected records"
=> Without patch you get a DB error
=> With patch you get confirmation message
 - Try to get the biblios to confirm they are deleted : /cgi-bin/koha/catalogue/detail.pl?biblionumber=xxx
 - Test with and without Plack
Comment 2 Jonathan Druart 2017-03-24 19:07:05 UTC
Created attachment 61635 [details] [review]
Bug 18329 - Batch record deletion broken

Hie, Tools > Batch record deletion seems broken.
Any deletion returns error :
Bibliographic record YYY was not deleted. An error occurred. (The error was: {UNKNOWN}: DBD::mysql::db begin_work failed: Already in a transaction at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1560. at /home/koha/src/C4/Biblio.pm line 3468 , see the Koha log file for more information).

Looks like it is because of Bug 18242 which added a transaction  in C4::Biblio::_koha_delete_biblio_metadata : $schema->txn_do.
The script batch_delete_records created a transaction with $dbh->{AutoCommit} = 0;

This patch fixes by using also Koha::Schema in batch_delete_records to manage transaction.
It also removes "$dbh->{RaiseError} = 1", this behavior is managed in Koha::Database.

Test plan :
 - Go to Staff interface : Tools > Batch record deletion
 - Enter a few existing biblionumbers
 - Click on "Continue"
 - Click on "Delete selected records"
=> Without patch you get a DB error
=> With patch you get confirmation message
 - Try to get the biblios to confirm they are deleted : /cgi-bin/koha/catalogue/detail.pl?biblionumber=xxx
 - Test with and without Plack

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 3 Jonathan Druart 2017-03-24 19:07:30 UTC
Just wrote exactly the same patch, did not see yours!
Comment 4 Fridolin SOMERS 2017-03-27 13:48:19 UTC
(In reply to Jonathan Druart from comment #3)
> Just wrote exactly the same patch, did not see yours!

awsome ;)
Comment 5 Martin Renvoize 2017-03-29 17:06:04 UTC
Created attachment 61690 [details] [review]
Bug 18329 - Batch record deletion broken

Hie, Tools > Batch record deletion seems broken.
Any deletion returns error :
Bibliographic record YYY was not deleted. An error occurred. (The error was: {UNKNOWN}: DBD::mysql::db begin_work failed: Already in a transaction at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1560. at /home/koha/src/C4/Biblio.pm line 3468 , see the Koha log file for more information).

Looks like it is because of Bug 18242 which added a transaction  in C4::Biblio::_koha_delete_biblio_metadata : $schema->txn_do.
The script batch_delete_records created a transaction with $dbh->{AutoCommit} = 0;

This patch fixes by using also Koha::Schema in batch_delete_records to manage transaction.
It also removes "$dbh->{RaiseError} = 1", this behavior is managed in Koha::Database.

Test plan :
 - Go to Staff interface : Tools > Batch record deletion
 - Enter a few existing biblionumbers
 - Click on "Continue"
 - Click on "Delete selected records"
=> Without patch you get a DB error
=> With patch you get confirmation message
 - Try to get the biblios to confirm they are deleted : /cgi-bin/koha/catalogue/detail.pl?biblionumber=xxx
 - Test with and without Plack

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2017-03-29 17:06:32 UTC
Easy QA, works as expected, code changes are clear :)
Comment 7 Kyle M Hall 2017-03-31 13:14:12 UTC
Pushed to master for 17.05, thanks Frido!
Comment 8 Katrin Fischer 2017-04-02 17:04:08 UTC
Dependency indicates that we will need to resolve 18242 first before this can be pushed to 16.11.x
Comment 9 Katrin Fischer 2017-04-10 21:48:53 UTC
This patch has been pushed to 16.11.x and will be in 16.11.07.
Comment 10 Mason James 2017-05-03 04:08:50 UTC
Pushed to 16.05.x, for 16.05.12 release