Attachment 81252 Details for Bug 21337 – Bug 21337: (QA follow-up) On the road to a perfect delete

[patch] Bug 21337: (QA follow-up) On the road to a perfect delete

Bug-21337-QA-follow-up-On-the-road-to-a-perfect-de.patch (text/plain), 3.61 KB, created by Marcel de Rooy on 2018-10-26 07:58:59 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2018-10-26 07:58:59 UTC

Size: 3.61 KB

patch

obsolete

>From f245ae1e01cd117c9f2bd35aa16566ebf1f8b4d1 Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Date: Thu, 25 Oct 2018 14:17:16 +0200
>Subject: [PATCH] Bug 21337: (QA follow-up) On the road to a perfect delete
>Content-Type: text/plain; charset=utf-8
>
>We should not throw an exception if we get a -1 or undef from Patron->delete
>in Patrons->delete. Only 0's should trigger an exception.
>
>Test plan:
>Run t/db_dependent/Koha/Patrons.t
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>---
> Koha/Patrons.pm               | 17 ++++++++++++-----
> t/db_dependent/Koha/Patrons.t | 16 +++++++++++++---
> 2 files changed, 25 insertions(+), 8 deletions(-)
>
>diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm
>index dedf4c4..7404d35 100644
>--- a/Koha/Patrons.pm
>+++ b/Koha/Patrons.pm
>@@ -218,22 +218,29 @@ sub anonymise_issue_history {
>     Includes a move to deletedborrowers if move flag set.
> 
>     Just like DBIx, the delete will only succeed when all entries could be
>-    deleted. Returns true or throws an exception.
>+    deleted. Returns number of deletes or throws an exception. A number of
>+    zero deletes is represented as the true value '0E0'.
> 
> =cut
> 
> sub delete {
>     my ( $self, $params ) = @_;
>-    my $patrons_deleted;
>+    my $patrons_deleted = 0;
>     $self->_resultset->result_source->schema->txn_do( sub {
>         my ( $set, $params ) = @_;
>         while( my $patron = $set->next ) {
>             $patron->move_to_deleted if $params->{move};
>-            $patron->delete == 1 || Koha::Exceptions::Patron::FailedDelete->throw;
>-            $patrons_deleted++;
>+            my $delete = $patron->delete;
>+            # throw exception on zero only: if delete threw an exception, we do not come here and -1 is no reason to stop
>+            if( defined($delete) && $delete == 0 ) {
>+                Koha::Exceptions::Patron::FailedDelete->throw;
>+            } elsif( defined($delete) && $delete == 1 ) {
>+                $patrons_deleted++;
>+            }
>         }
>     }, $self, $params );
>-    return $patrons_deleted;
>+    return if $@;
>+    return $patrons_deleted || '0E0';
> }
> 
> =head3 _type
>diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t
>index c552220..abc91de 100644
>--- a/t/db_dependent/Koha/Patrons.t
>+++ b/t/db_dependent/Koha/Patrons.t
>@@ -424,7 +424,7 @@ subtest "delete" => sub {
> };
> 
> subtest 'Koha::Patrons->delete' => sub {
>-    plan tests => 4;
>+    plan tests => 6;
> 
>     my $mod_patron = Test::MockModule->new( 'Koha::Patron' );
>     my $moved_to_deleted = 0;
>@@ -438,11 +438,21 @@ subtest 'Koha::Patrons->delete' => sub {
>     is( $set->delete({ move => 1 }), 2, 'Two patrons deleted' );
>     is( $moved_to_deleted, 2, 'Patrons moved to deletedborrowers' );
> 
>-    # Add again, test if we can raise an exception
>-    $mod_patron->mock( 'delete', sub { return -1; } );
>+    # Check return value / rollback
>+    my $mod_object = Test::MockModule->new( 'Koha::Object' );
>+    my $mock_ret = -1; # no rollback expected
>+    $mod_object->mock( 'delete', sub { return $mock_ret; } );
>     $patron1 = $builder->build_object({ class => 'Koha::Patrons' });
>     $id1 = $patron1->borrowernumber;
>     $set = Koha::Patrons->search({ borrowernumber => { '>=' => $id1 }});
>+    is( $set->delete, '0E0', 'Try -1: no rollback' );
>+
>+    undef $mock_ret; # still no rollback expected
>+    $set->_resultset->reset; # needed for reiterate
>+    is( $set->delete, '0E0', 'Try undef: no rollback' );
>+
>+    $mock_ret = 0; # this should trigger rollback/exception
>+    $set->_resultset->reset;
>     throws_ok { $set->delete } 'Koha::Exceptions::Patron::FailedDelete',
>         'Exception raised for deleting patron';
> };
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 21337: 78586 | 78619 | 78621 | 79461 | 79462 | 79708 | 79709 | 79915 | 79916 | 79917 | 80624 | 81251 | 81252 | 81257