Attachment 60253 Details for Bug 18070 – Bug 18070: [Follow-up] What should DelAuthority return?

[patch] Bug 18070: [Follow-up] What should DelAuthority return?

Bug-18070-Follow-up-What-should-DelAuthority-retur.patch (text/plain), 2.71 KB, created by Marcel de Rooy on 2017-02-15 13:05:53 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2017-02-15 13:05:53 UTC

Size: 2.71 KB

patch

obsolete

>From d739b84aacb0eefa0db6b77bcebe8cf4d56410d6 Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Date: Tue, 14 Feb 2017 16:41:26 +0100
>Subject: [PATCH] Bug 18070: [Follow-up] What should DelAuthority return?
>Content-Type: text/plain; charset=utf-8
>
>Script tools/batch_delete_records.pl only checks the return value of
>DelAuthority. The return value depends on DBI rows, which is not always
>reliable. It may return -1 when it 'does not know'.
>
>Testing $@ in tools/batch_delete_records.pl should actually be enough.
>The return value was discarded in C4/ImportBatch.pm. Removing an unused
>variable.
>
>Test plan:
>[1] Pick an authority record with a few linked biblios.
>    Delete this authority record via tools/batch_delete_records.pl.
>    Check if the linked biblio records are cleaned up.
>[2] Bonus: Make a typo in the SQL statement of DelAuthority. Check
>    if batch_delete_records shows you the error message.
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>---
> C4/AuthoritiesMarc.pm         | 2 +-
> C4/ImportBatch.pm             | 2 +-
> tools/batch_delete_records.pl | 4 ++--
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/C4/AuthoritiesMarc.pm b/C4/AuthoritiesMarc.pm
>index afef9c8..7e367c8 100644
>--- a/C4/AuthoritiesMarc.pm
>+++ b/C4/AuthoritiesMarc.pm
>@@ -682,7 +682,7 @@ sub AddAuthority {
> 
> =head2 DelAuthority
> 
>-  $authid = DelAuthority( $authid )
>+    DelAuthority( $authid )
> 
> Deletes $authid and calls merge to cleanup in linked biblio records
> 
>diff --git a/C4/ImportBatch.pm b/C4/ImportBatch.pm
>index 88d670d..14364f4 100644
>--- a/C4/ImportBatch.pm
>+++ b/C4/ImportBatch.pm
>@@ -852,7 +852,7 @@ sub BatchRevertRecords {
>                 $num_items_deleted += BatchRevertItems($rowref->{'import_record_id'}, $rowref->{'matched_biblionumber'});
>                 $error = DelBiblio($rowref->{'matched_biblionumber'});
>             } else {
>-                my $deletedauthid = DelAuthority($rowref->{'matched_authid'});
>+                DelAuthority( $rowref->{'matched_authid'} );
>             }
>             if (defined $error) {
>                 $num_errors++;
>diff --git a/tools/batch_delete_records.pl b/tools/batch_delete_records.pl
>index 2c2d32e..2b82796 100755
>--- a/tools/batch_delete_records.pl
>+++ b/tools/batch_delete_records.pl
>@@ -198,8 +198,8 @@ if ( $op eq 'form' ) {
>         } else {
>             # Authorities
>             my $authid = $record_id;
>-            my $r = eval { C4::AuthoritiesMarc::DelAuthority( $authid ) };
>-            if ( $r eq '0E0' or $@ ) {
>+            eval { C4::AuthoritiesMarc::DelAuthority( $authid ) };
>+            if ( $@ ) {
>                 push @messages, {
>                     type => 'error',
>                     code => 'authority_not_deleted',
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 18070: 59972 | 59973 | 59977 | 59978 | 60251 | 60252 | 60253 | 60254 | 60392 | 60393 | 60394 | 60395 | 60557 | 60558 | 60559 | 60560