View | Details | Raw Unified | Return to bug 16907
Collapse All | Expand All

(-)a/C4/Members.pm (-34 lines)
Lines 1347-1386 sub SetAge{ Link Here
1347
    return $borrower;
1347
    return $borrower;
1348
}    # sub SetAge
1348
}    # sub SetAge
1349
1349
1350
=head2 HandleDelBorrower
1351
1352
     HandleDelBorrower($borrower);
1353
1354
When a member is deleted, you should call me first.
1355
This routine deletes/moves lists and entries for the deleted member/borrower.
1356
Lists owned by the borrower are deleted, but entries from the borrower to
1357
other lists are kept.
1358
1359
=cut
1360
1361
sub HandleDelBorrower {
1362
    my ($borrower)= @_;
1363
    my $query;
1364
    my $dbh = C4::Context->dbh;
1365
1366
    #Delete all lists and all shares of this borrower
1367
    #Consistent with the approach Koha uses on deleting individual lists
1368
    #Note that entries in virtualshelfcontents added by this borrower to
1369
    #lists of others will be handled by a table constraint: the borrower
1370
    #is set to NULL in those entries.
1371
    $query="DELETE FROM virtualshelves WHERE owner=?";
1372
    $dbh->do($query,undef,($borrower));
1373
1374
    #NOTE:
1375
    #We could handle the above deletes via a constraint too.
1376
    #But a new BZ report 11889 has been opened to discuss another approach.
1377
    #Instead of deleting we could also disown lists (based on a pref).
1378
    #In that way we could save shared and public lists.
1379
    #The current table constraints support that idea now.
1380
    #This pref should then govern the results of other routines/methods such as
1381
    #Koha::Virtualshelf->new->delete too.
1382
}
1383
1384
=head2 GetHideLostItemsPreference
1350
=head2 GetHideLostItemsPreference
1385
1351
1386
  $hidelostitemspref = &GetHideLostItemsPreference($borrowernumber);
1352
  $hidelostitemspref = &GetHideLostItemsPreference($borrowernumber);
(-)a/Koha/Patron.pm (-1 / +21 lines)
Lines 32-37 use Koha::OldIssues; Link Here
32
use Koha::Patron::Categories;
32
use Koha::Patron::Categories;
33
use Koha::Patron::Images;
33
use Koha::Patron::Images;
34
use Koha::Patrons;
34
use Koha::Patrons;
35
use Koha::Virtualshelves;
35
36
36
use base qw(Koha::Object);
37
use base qw(Koha::Object);
37
38
Lines 49-55 Koha::Patron - Koha Patron Object class Link Here
49
50
50
$patron->delete
51
$patron->delete
51
52
52
Delete a patron.
53
Delete patron's holds, lists and finally the patron.
54
55
Lists owned by the borrower are deleted, but entries from the borrower to
56
other lists are kept.
53
57
54
=cut
58
=cut
55
59
Lines 63-68 sub delete { Link Here
63
            # FIXME Should be $patron->get_holds
67
            # FIXME Should be $patron->get_holds
64
            $_->delete for Koha::Holds->search( { borrowernumber => $self->borrowernumber } );
68
            $_->delete for Koha::Holds->search( { borrowernumber => $self->borrowernumber } );
65
69
70
            # Delete all lists and all shares of this borrower
71
            # Consistent with the approach Koha uses on deleting individual lists
72
            # Note that entries in virtualshelfcontents added by this borrower to
73
            # lists of others will be handled by a table constraint: the borrower
74
            # is set to NULL in those entries.
75
            # NOTE:
76
            # We could handle the above deletes via a constraint too.
77
            # But a new BZ report 11889 has been opened to discuss another approach.
78
            # Instead of deleting we could also disown lists (based on a pref).
79
            # In that way we could save shared and public lists.
80
            # The current table constraints support that idea now.
81
            # This pref should then govern the results of other routines/methods such as
82
            # Koha::Virtualshelf->new->delete too.
83
            # FIXME Could be $patron->get_lists
84
            $_->delete for Koha::Virtualshelves->search( { owner => $self->borrowernumber } );
85
66
            logaction( "MEMBERS", "DELETE", $self->borrowernumber, "" ) if C4::Context->preference("BorrowersLog");
86
            logaction( "MEMBERS", "DELETE", $self->borrowernumber, "" ) if C4::Context->preference("BorrowersLog");
67
            $deleted = $self->SUPER::delete;
87
            $deleted = $self->SUPER::delete;
68
        }
88
        }
(-)a/members/deletemem.pl (-1 lines)
Lines 163-169 if ( $op eq 'delete_confirm' or $countissues > 0 or $flags->{'CHARGES'} or $is_ Link Here
163
        });
163
        });
164
    my $patron = Koha::Patrons->find( $member );
164
    my $patron = Koha::Patrons->find( $member );
165
    $patron->move_to_deleted;
165
    $patron->move_to_deleted;
166
    C4::Members::HandleDelBorrower($member);
167
    $patron->delete;
166
    $patron->delete;
168
    # TODO Tell the user everything went ok
167
    # TODO Tell the user everything went ok
169
    print $input->redirect("/cgi-bin/koha/members/members-home.pl");
168
    print $input->redirect("/cgi-bin/koha/members/members-home.pl");
(-)a/misc/cronjobs/delete_patrons.pl (-17 / +1 lines)
Lines 60-70 unless ($confirm) { Link Here
60
60
61
say scalar(@$members) . " patrons to delete";
61
say scalar(@$members) . " patrons to delete";
62
62
63
my $dbh = C4::Context->dbh;
64
$dbh->{RaiseError} = 1;
65
$dbh->{PrintError} = 0;
66
67
$dbh->{AutoCommit} = 0; # use transactions to avoid partial deletes
68
my $deleted = 0;
63
my $deleted = 0;
69
for my $member (@$members) {
64
for my $member (@$members) {
70
    print "Trying to delete patron $member->{borrowernumber}... "
65
    print "Trying to delete patron $member->{borrowernumber}... "
Lines 82-107 for my $member (@$members) { Link Here
82
        my $deleted = eval { $patron->move_to_deleted; };
77
        my $deleted = eval { $patron->move_to_deleted; };
83
        if ($@ or not $deleted) {
78
        if ($@ or not $deleted) {
84
            say "Failed to delete patron $borrowernumber, cannot move it" . ( $@ ? ": ($@)" : "" );
79
            say "Failed to delete patron $borrowernumber, cannot move it" . ( $@ ? ": ($@)" : "" );
85
            $dbh->rollback;
86
            next;
80
            next;
87
        }
81
        }
88
82
89
        eval {
83
        eval { $patron->delete };
90
            C4::Members::HandleDelBorrower( $borrowernumber );
91
        };
92
        if ($@) {
93
            say "Failed to delete patron $borrowernumber, error handling its lists: ($@)";
94
            $dbh->rollback;
95
            next;
96
        }
97
        eval { $patron->delete if $confirm; };
98
        if ($@) {
84
        if ($@) {
99
            say "Failed to delete patron $borrowernumber: $@)";
85
            say "Failed to delete patron $borrowernumber: $@)";
100
            $dbh->rollback;
101
            next;
86
            next;
102
        }
87
        }
103
    }
88
    }
104
    $dbh->commit;
105
    $deleted++;
89
    $deleted++;
106
    say "OK" if $verbose;
90
    say "OK" if $verbose;
107
}
91
}
(-)a/t/db_dependent/Koha/Patrons.t (-2 / +11 lines)
Lines 223-229 subtest "move_to_deleted" => sub { Link Here
223
};
223
};
224
224
225
subtest "delete" => sub {
225
subtest "delete" => sub {
226
    plan tests => 4;
226
    plan tests => 5;
227
    t::lib::Mocks::mock_preference( 'BorrowersLog', 1 );
227
    t::lib::Mocks::mock_preference( 'BorrowersLog', 1 );
228
    my $patron           = $builder->build( { source => 'Borrower' } );
228
    my $patron           = $builder->build( { source => 'Borrower' } );
229
    my $retrieved_patron = Koha::Patrons->find( $patron->{borrowernumber} );
229
    my $retrieved_patron = Koha::Patrons->find( $patron->{borrowernumber} );
Lines 232-243 subtest "delete" => sub { Link Here
232
            value  => { borrowernumber => $patron->{borrowernumber} }
232
            value  => { borrowernumber => $patron->{borrowernumber} }
233
        }
233
        }
234
    );
234
    );
235
    my $list = $builder->build(
236
        {   source => 'Virtualshelve',
237
            value  => { owner => $patron->{borrowernumber} }
238
        }
239
    );
240
235
    my $deleted = $retrieved_patron->delete;
241
    my $deleted = $retrieved_patron->delete;
236
    is( $deleted, 1, 'Koha::Patron->delete should return 1 if the patron has been correctly deleted' );
242
    is( $deleted, 1, 'Koha::Patron->delete should return 1 if the patron has been correctly deleted' );
237
    is( Koha::Patrons->find( $patron->{borrowernumber} ), undef, 'Koha::Patron->delete should have deleted the patron');
243
244
    is( Koha::Patrons->find( $patron->{borrowernumber} ), undef, 'Koha::Patron->delete should have deleted the patron' );
238
245
239
    is( Koha::Holds->search( { borrowernumber => $patron->{borrowernumber} } )->count, 0, q|Koha::Patron->delete should have deleted patron's holds| );
246
    is( Koha::Holds->search( { borrowernumber => $patron->{borrowernumber} } )->count, 0, q|Koha::Patron->delete should have deleted patron's holds| );
240
247
248
    is( Koha::Virtualshelves->search( { owner => $patron->{borrowernumber} } )->count, 0, q|Koha::Patron->delete should have deleted patron's lists| );
249
241
    my $number_of_logs = $schema->resultset('ActionLog')->search( { module => 'MEMBERS', action => 'DELETE', object => $retrieved_patron->borrowernumber } )->count;
250
    my $number_of_logs = $schema->resultset('ActionLog')->search( { module => 'MEMBERS', action => 'DELETE', object => $retrieved_patron->borrowernumber } )->count;
242
    is( $number_of_logs, 1, 'With BorrowerLogs, Koha::Patron->delete should have logged' );
251
    is( $number_of_logs, 1, 'With BorrowerLogs, Koha::Patron->delete should have logged' );
243
};
252
};
(-)a/tools/cleanborrowers.pl (-2 lines)
Lines 149-155 elsif ( $step == 3 ) { Link Here
149
            my $borrowernumber = $patrons_to_delete->[$i]->{'borrowernumber'};
149
            my $borrowernumber = $patrons_to_delete->[$i]->{'borrowernumber'};
150
            my $patron = Koha::Patrons->find($borrowernumber);
150
            my $patron = Koha::Patrons->find($borrowernumber);
151
            $radio eq 'trash' && $patron->move_to_deleted;
151
            $radio eq 'trash' && $patron->move_to_deleted;
152
            C4::Members::HandleDelBorrower($borrowernumber);
153
            $patron->delete;
152
            $patron->delete;
154
        }
153
        }
155
        $template->param(
154
        $template->param(
156
- 

Return to bug 16907