Bug 17380: Do not allow Default template in merge form

This patch makes the following changes:
[1] Removes Default from the template list. We should not merge with the
    Default framework, since it does not have a reporting tag.
[2] Rearranges the error section in the template. It is confusing to have
    two error sections in this template. The error CANNOT_MOVE is not used.
    The error FRAMEWORK_NOT_SELECTED is replaced by WRONG_FRAMEWORK.
[3] Do not allow to merge a record with itself.
[4] Check if the merge reference record still contains any MARC tags.
[5] Additional polishing: Simplify passing frameworks to template. Remove
    an unused Koha::Authority::Types->search. Remove obsolete POD header
    for functions from the script.

Test plan:
[1] Select two authorities to merge. Verify that you cannot select Default
    anymore as framework for the reference record.
[2] Reproduce error WRONG_COUNT by adding another authid=999 in the URL
    after you selected two authority records for merging.
[3] Remove the third authid from the URL and change the first or second
    authid into an unexisting record id. You should generate an Internal
    Server Error. The log should show the exception message.
[4] Merge two authorities. Deselect all MARC tags. Should trigger the
    error EMPTY_MARC in the template.
[5] Select the same authority record twice for merging. Should trigger the
    error DESTRUCTIVE_MERGE in the template.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

diff --git a/authorities/merge.pl b/authorities/merge.pl
index 060db09..b6a7b27 100755 (executable)
--- a/authorities/merge.pl
+++ b/authorities/merge.pl
@@ -23,11 +23,12 @@ use CGI qw ( -utf8 );
 use C4::Output;
 use C4::Auth;
 use C4::AuthoritiesMarc;
-use Koha::MetadataRecord::Authority;
 use C4::Koha;
 use C4::Biblio;
 
+use Koha::Authority::Types;
 use Koha::Exceptions;
+use Koha::MetadataRecord::Authority;
 
 my $input  = new CGI;
 my @authid = $input->multi_param('authid');
@@ -52,12 +53,28 @@ if ($merge) {
 
     # Creating a new record from the html code
     my $record   = TransformHtmlToMarc($input, 0);
-    my $recordid1   = $input->param('recordid1');
-    my $recordid2   = $input->param('recordid2');
+    my $recordid1   = $input->param('recordid1') // q{};
+    my $recordid2   = $input->param('recordid2') // q{};
     my $typecode = $input->param('frameworkcode');
 
+    # Some error checking
+    if( $recordid1 eq $recordid2 ) {
+        push @errors, { code => 'DESTRUCTIVE_MERGE' };
+    } elsif( !$typecode || !Koha::Authority::Types->find($typecode) ) {
+        push @errors, { code => 'WRONG_FRAMEWORK' };
+    } elsif( scalar $record->fields == 0 ) {
+        push @errors, { code => 'EMPTY_MARC' };
+    }
+    if( @errors ) {
+        $template->param( errors => \@errors );
+        output_html_with_http_headers $input, $cookie, $template->output;
+        exit;
+    }
+
     # Rewriting the leader
-    $record->leader( GetAuthority($recordid1)->leader() );
+    if( my $authrec = GetAuthority($recordid1) ) {
+        $record->leader( $authrec->leader() );
+    }
 
     # Modifying the reference record
     # This triggers a merge for the biblios attached to $recordid1
@@ -87,8 +104,9 @@ else {
 
     if ( scalar(@authid) != 2 ) {
         push @errors, { code => "WRONG_COUNT", value => scalar(@authid) };
-    }
-    else {
+    } elsif( $authid[0] eq $authid[1] ) {
+        push @errors, { code => 'DESTRUCTIVE_MERGE' };
+    } else {
         my $recordObj1 = Koha::MetadataRecord::Authority->get_from_authid($authid[0]);
         Koha::Exceptions::ObjectNotFound->throw( "No authority record found for authid $authid[0]\n" ) if !$recordObj1;
 
@@ -104,8 +122,7 @@ else {
 
             my $framework;
             if ( $recordObj1->authtypecode ne $recordObj2->authtypecode && $mergereference ne 'breeding' ) {
-                $framework = $input->param('frameworkcode')
-                  or push @errors, { code => 'FRAMEWORK_NOT_SELECTED' };
+                $framework = $input->param('frameworkcode');
             }
             else {
                 $framework = $recordObj1->authtypecode;
@@ -167,17 +184,9 @@ else {
                 title2          => $recordObj2->authorized_heading,
             );
             if ( $recordObj1->authtypecode ne $recordObj2->authtypecode ) {
-                my $authority_types = Koha::Authority::Types->search( {}, { order_by => ['authtypecode'] } );
-                my @frameworkselect;
-                while ( my $authority_type = $authority_types->next ) {
-                    my %row = (
-                        value => $authority_type->authtypecode,
-                        frameworktext => $authority_type->authtypetext,
-                    );
-                    push @frameworkselect, \%row;
-                }
+                my $authority_types = Koha::Authority::Types->search( { authtypecode => { '!=' => '' } }, { order_by => ['authtypecode'] } );
                 $template->param(
-                    frameworkselect => \@frameworkselect,
+                    frameworkselect => $authority_types->unblessed,
                     frameworkcode1  => $recordObj1->authtypecode,
                     frameworkcode2  => $recordObj2->authtypecode,
                 );
@@ -186,22 +195,7 @@ else {
     }
 }
 
-my $authority_types = Koha::Authority::Types->search({}, { order_by => ['authtypetext']});
-$template->param( authority_types => $authority_types );
-
 if (@errors) {
-
-    # Errors
     $template->param( errors => \@errors );
 }
-
 output_html_with_http_headers $input, $cookie, $template->output;
-exit;
-
-=head1 FUNCTIONS
-
-=cut
-
-# ------------------------
-# Functions
-# ------------------------

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/authorities/merge.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/authorities/merge.tt
index 57b1c16..493bcbd 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/authorities/merge.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/authorities/merge.tt
@@ -54,31 +54,32 @@ function changeFramework(fw) {
 
 
 <h1>Merging records</h1>
-[% IF ( result ) %]
-    [% IF ( errors ) %]
+
+[% IF ( errors ) %]
 
     [% FOREACH error IN errors %]
         <div class="dialog alert">
-
-                [% IF error.code == 'CANNOT_MOVE' %]
-                    The following items could not be moved from the old record to the new one: [% error.value %]
-                [% ELSIF error.code == 'FRAMEWORK_NOT_SELECTED' %]
-                    No framework has been selected. Please select a framework for merging.
-                [% ELSE %]
-                    [% error %]
-                [% END %]
-
-            <br />Therefore, the record to be merged has not been deleted.</div>
+            [% IF error.code == 'WRONG_COUNT' %]
+                Number of records provided for merging: [% error.value %]. Currently only 2 records can be merged at a time.
+            [% ELSIF error.code == 'DESTRUCTIVE_MERGE' %]
+                You cannot merge a record with itself. Please select two different authorities.
+            [% ELSIF error.code == 'WRONG_FRAMEWORK' %]
+                The Default framework cannot be used, or the framework does not exist. Please select another framework for merging.
+            [% ELSIF error.code == 'EMPTY_MARC' %]
+                Sorry, but we did not find any MARC tags in the reference record.
+            [% ELSE %]
+                [% error %]
+            [% END %]
+        </div>
     [% END %]
 
-    [% ELSE %]
+[% ELSIF ( result ) %]
+
         <script type="text/javascript">window.location.href="/cgi-bin/koha/authorities/detail.pl?authid=[% recordid1 %]"</script>
         <p>The merging was successful. <a href="/cgi-bin/koha/authorities/detail.pl?authid=[% recordid1 %]">Click here to see the merged record.</a></p>
-    [% END %]
 
-[% ELSE %]
+[% ELSIF ( choosereference ) %]
 
-[% IF ( choosereference ) %]
 <p>Please choose which record will be the reference for the merge. The record chosen as reference will be kept, and the other will be deleted.</p>
 <form id="mergeform" action="/cgi-bin/koha/authorities/merge.pl" method="post">
     <fieldset class="rows">
@@ -92,11 +93,11 @@ function changeFramework(fw) {
                       <select name="frameworkcode" id="frameworkcode">
                                       [% FOREACH frameworkcodeloo IN frameworkselect %]
                                           [% IF frameworkcodeloo.authtypecode == frameworkcode1 %]
-                                              <option value="[% frameworkcodeloo.value %]" selected="selected">
+                                              <option value="[% frameworkcodeloo.authtypecode %]" selected="selected">
                                           [% ELSE %]
-                                              <option value="[% frameworkcodeloo.value %]">
+                                              <option value="[% frameworkcodeloo.authtypecode %]">
                                           [% END %]
-                                           [% frameworkcodeloo.frameworktext %]
+                                           [% frameworkcodeloo.authtypetext %]
                                            </option>
                                       [% END %]
                       </select></li>
@@ -108,21 +109,9 @@ function changeFramework(fw) {
     <fieldset class="action"><input type="submit" value="Next" /></fieldset>
     </fieldset>
 </form>
+
 [% ELSE %]
-[% IF ( errors ) %]
-    <div class="dialog alert">
-    [% FOREACH error IN errors %]
-        <p>
-                [% IF error.code == 'WRONG_COUNT' %]
-                    Number of records provided for merging: [% error.value %]. Currently only 2 records can be merged at a time.
-                [% ELSE %]
-                    [% error %]
-                [% END %]
-
-            </p>
-    [% END %]
-    </div>
-[% ELSE %]
+
 <form id="mergeform" action="/cgi-bin/koha/authorities/merge.pl" method="post" onsubmit="return mergeformsubmit()">
 
 <div class="yui-g">
@@ -141,8 +130,7 @@ function changeFramework(fw) {
 <fieldset class="action"><input type="submit" name="merge" value="Merge" /></fieldset>
 </div>
 </form>
-[% END %]
-[% END %]
+
 [% END %]
 
 </div>