Attachment 73662 Details for Bug 9302 – Bug 9302: (QA follow-up) Merge should be a transaction

[patch] Bug 9302: (QA follow-up) Merge should be a transaction

Bug-9302-QA-follow-up-Merge-should-be-a-transactio.patch (text/plain), 5.90 KB, created by Tomás Cohen Arazi (tcohen) on 2018-04-04 19:58:52 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2018-04-04 19:58:52 UTC

Size: 5.90 KB

patch

obsolete

>From f17161ad77eb7a7b7511e8d532f30710b51f8f91 Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>Date: Tue, 3 Apr 2018 03:51:51 +0000
>Subject: [PATCH] Bug 9302: (QA follow-up) Merge should be a transaction
>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>---
> Koha/Patrons.pm                               | 28 +++++++-------
> .../prog/en/modules/members/merge-patrons.tt  | 38 ++++++++++---------
> members/merge-patrons.pl                      |  9 ++++-
> 3 files changed, 43 insertions(+), 32 deletions(-)
>
>diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm
>index 1afb440e08..0ef5f4f0b9 100644
>--- a/Koha/Patrons.pm
>+++ b/Koha/Patrons.pm
>@@ -260,23 +260,25 @@ sub merge {
> 
>     my $results;
> 
>-    foreach my $borrowernumber (@borrowernumbers) {
>-        my $patron = Koha::Patrons->find( $borrowernumber );
>+    $self->_resultset->result_source->schema->txn_do( sub {
>+        foreach my $borrowernumber (@borrowernumbers) {
>+            my $patron = Koha::Patrons->find( $borrowernumber );
> 
>-        next unless $patron;
>+            next unless $patron;
> 
>-        # Unbless for safety, the patron will end up being deleted
>-        $results->{merged}->{$borrowernumber}->{patron} = $patron->unblessed;
>+            # Unbless for safety, the patron will end up being deleted
>+            $results->{merged}->{$borrowernumber}->{patron} = $patron->unblessed;
> 
>-        while (my ($r, $field) = each(%$RESULTSET_PATRON_ID_MAPPING)) {
>-            my $rs = $schema->resultset($r)->search({ $field => $borrowernumber} );
>-            $results->{merged}->{ $borrowernumber }->{updated}->{$r} = $rs->count();
>-            $rs->update( { $field => $keeper });
>-        }
>+            while (my ($r, $field) = each(%$RESULTSET_PATRON_ID_MAPPING)) {
>+                my $rs = $schema->resultset($r)->search({ $field => $borrowernumber} );
>+                $results->{merged}->{ $borrowernumber }->{updated}->{$r} = $rs->count();
>+                $rs->update( { $field => $keeper });
>+            }
> 
>-        $patron->move_to_deleted();
>-        $patron->delete();
>-    }
>+            $patron->move_to_deleted();
>+            $patron->delete();
>+        }
>+    });
> 
>     $results->{keeper} = $patron_to_keep;
> 
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/merge-patrons.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/merge-patrons.tt
>index 78a61d97c3..24ee5520dc 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/merge-patrons.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/merge-patrons.tt
>@@ -104,26 +104,30 @@ $(document).ready(function() {
>             [% ELSIF action == 'merge' %]
>                 <h4>Results</h4>
> 
>-                <p>
>-                    Patron records merged into <a href="moremember.pl?borrowernumber=[% results.keeper.id %]">[% results.keeper.firstname %] [% results.keeper.surname %] ([% results.keeper.cardnumber | html %])</a>
>-                </p>
>+                [% IF error %]
>+                    <div class="dialog alert">Merge failed! The following error was reported: [% error %].</div>
>+                [% ELSE %]
>+                    <p>
>+                        Patron records merged into <a href="moremember.pl?borrowernumber=[% results.keeper.id %]">[% results.keeper.firstname %] [% results.keeper.surname %] ([% results.keeper.cardnumber | html %])</a>
>+                    </p>
> 
>-                [% FOREACH pair IN results.merged.pairs %]
>-                    [% SET patron = pair.value.patron %]
>+                    [% FOREACH pair IN results.merged.pairs %]
>+                        [% SET patron = pair.value.patron %]
> 
>-                    <h5>[% patron.firstname %] [% patron.surname %] ([% patron.cardnumber %])</h5>
>+                        <h5>[% patron.firstname %] [% patron.surname %] ([% patron.cardnumber %])</h5>
> 
>-                    [% USE Dumper %]
>-                    [% FOREACH r IN pair.value.updated.pairs %]
>-                        [% SET name = r.key %]
>-                        [% SET count = r.value %]
>-                        [% IF count %]
>-                            <p>
>-                                [% count %] [% PROCESS display_names rs = name %] transferred.
>-                                [% IF name == 'Reserve' %]
>-                                    <strong>It is advisable to check for and resolve duplicate holds due to merging.</strong>
>-                                [% END %]
>-                            </p>
>+                        [% USE Dumper %]
>+                        [% FOREACH r IN pair.value.updated.pairs %]
>+                            [% SET name = r.key %]
>+                            [% SET count = r.value %]
>+                            [% IF count %]
>+                                <p>
>+                                    [% count %] [% PROCESS display_names rs = name %] transferred.
>+                                    [% IF name == 'Reserve' %]
>+                                        <strong>It is advisable to check for and resolve duplicate holds due to merging.</strong>
>+                                    [% END %]
>+                                </p>
>+                            [% END %]
>                         [% END %]
>                     [% END %]
>                 [% END %]
>diff --git a/members/merge-patrons.pl b/members/merge-patrons.pl
>index 8ad2cfbd9d..a35fe83f29 100755
>--- a/members/merge-patrons.pl
>+++ b/members/merge-patrons.pl
>@@ -47,8 +47,13 @@ if ( $action eq 'show' ) {
>     $template->param( patrons => $patrons );
> } elsif ( $action eq 'merge' ) {
>     my $keeper = $cgi->param('keeper');
>-    my $results = Koha::Patrons->merge( { keeper => $keeper, patrons => \@ids } );
>-    $template->param( results => $results );
>+    my $results;
>+    eval { $results = Koha::Patrons->merge( { keeper => $keeper, patrons => \@ids } ); };
>+    if ($@) {
>+        $template->param( results => $results );
>+    } else {
>+        $template->param( error => $@ );
>+    }
> }
> 
> $template->param( action => $action );
>-- 
>2.17.0

Actions: View | Diff | Splinter Review

Attachments on bug 9302: 66615 | 66617 | 68606 | 70474 | 70477 | 70478 | 70479 | 70482 | 70483 | 72082 | 72083 | 73645 | 73646 | 73647 | 73648 | 73649 | 73660 | 73661 | 73662 | 73663 | 73664 | 73668 | 73679 | 74439 | 74440 | 74441 | 74442 | 74443 | 74444 | 74445 | 74446 | 74447 | 74448 | 74449 | 74450 | 74451 | 74585 | 74653