From fd8094a347e3514e6a7017ccff1db64f2934f418 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@biblibre.com>
Date: Thu, 19 Jun 2014 15:18:23 +0200
Subject: [PATCH] [SIGNED OFF] Bug 12874: A DB field without a default mapping
 is set to a default value on editing

If an item is edited and a field is not mapped to Koha, it is to 0 or
NULL (depending on the default value defined).

This patch adds a check on the mapping before editing the item. It there
is no mapping, the DB value is not erased.

Test plan:
1/ Edit an item and fill a value for a field
2/ Unmap this field
3/ Edit the item
4/ Verify that the value is not erased (using the MySQL CLI)

Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
---
 C4/Items.pm                             |   83 ++++++++++++++++++-------------
 C4/Koha.pm                              |   25 ++++++++++
 t/db_dependent/Koha/IsKohaFieldLinked.t |   44 ++++++++++++++++
 3 files changed, 118 insertions(+), 34 deletions(-)
 create mode 100644 t/db_dependent/Koha/IsKohaFieldLinked.t

diff --git a/C4/Items.pm b/C4/Items.pm
index acb2a9b..c23b272 100644
--- a/C4/Items.pm
+++ b/C4/Items.pm
@@ -442,38 +442,51 @@ Returns item record
 
 =cut
 
-my %default_values_for_mod_from_marc = (
-# DO NOT include (internal) item fields here.
-# Only fields that are related to the MARC structure used in additem.pl
-# Bug 7817 removed permanent_location.
-    barcode              => undef, 
-    booksellerid         => undef, 
-    ccode                => undef, 
-    'items.cn_source'    => undef, 
-    coded_location_qualifier => undef,
-    copynumber           => undef, 
-    damaged              => 0,
-#    dateaccessioned      => undef,
-    enumchron            => undef, 
-    holdingbranch        => undef, 
-    homebranch           => undef, 
-    itemcallnumber       => undef, 
-    itemlost             => 0,
-    itemnotes            => undef, 
-    itype                => undef, 
-    location             => undef, 
-    materials            => undef, 
-    notforloan           => 0,
-    paidfor              => undef,  # should not be here: see BZ 12817
-    price                => undef, 
-    replacementprice     => undef, 
-    replacementpricedate => undef, 
-    restricted           => undef, 
-    stack                => undef, 
-    stocknumber          => undef, 
-    uri                  => undef, 
-    withdrawn             => 0,
-);
+our %default_values_for_mod_from_marc;
+
+sub _build_default_values_for_mod_marc {
+    my ($frameworkcode) = @_;
+    return $default_values_for_mod_from_marc{$frameworkcode}
+      if exists $default_values_for_mod_from_marc{$frameworkcode};
+    my $marc_structure = C4::Biblio::GetMarcStructure( 1, $frameworkcode );
+    my $default_values = {
+        barcode                  => undef,
+        booksellerid             => undef,
+        ccode                    => undef,
+        'items.cn_source'        => undef,
+        coded_location_qualifier => undef,
+        copynumber               => undef,
+        damaged                  => 0,
+        enumchron                => undef,
+        holdingbranch            => undef,
+        homebranch               => undef,
+        itemcallnumber           => undef,
+        itemlost                 => 0,
+        itemnotes                => undef,
+        itype                    => undef,
+        location                 => undef,
+        permanent_location       => undef,
+        materials                => undef,
+        notforloan               => 0,
+        # paidfor => undef, # commented, see bug 12817
+        price                    => undef,
+        replacementprice         => undef,
+        replacementpricedate     => undef,
+        restricted               => undef,
+        stack                    => undef,
+        stocknumber              => undef,
+        uri                      => undef,
+        withdrawn                => 0,
+    };
+    while ( my ( $field, $default_value ) = each %$default_values ) {
+        $field =~ s|[^\.]*\.?(.*)|items.$1|;
+        $default_values_for_mod_from_marc{$frameworkcode}{$field} =
+          $default_value
+          if C4::Koha::IsKohaFieldLinked(
+            { kohafield => $field, frameworkcode => $frameworkcode } );
+    }
+    return $default_values_for_mod_from_marc{$frameworkcode};
+}
 
 sub ModItemFromMarc {
     my $item_marc = shift;
@@ -487,8 +500,10 @@ sub ModItemFromMarc {
     my $localitemmarc = MARC::Record->new;
     $localitemmarc->append_fields( $item_marc->field($itemtag) );
     my $item = &TransformMarcToKoha( $dbh, $localitemmarc, $frameworkcode, 'items' );
-    foreach my $item_field ( keys %default_values_for_mod_from_marc ) {
-        $item->{$item_field} = $default_values_for_mod_from_marc{$item_field} unless (exists $item->{$item_field});
+    my $default_values = _build_default_values_for_mod_marc();
+    foreach my $item_field ( keys %$default_values ) {
+        $item->{$item_field} = $default_values->{$item_field}
+          unless exists $item->{$item_field};
     }
     my $unlinked_item_subfields = _get_unlinked_item_subfields( $localitemmarc, $frameworkcode );
 
diff --git a/C4/Koha.pm b/C4/Koha.pm
index ee4bd67..7b40c67 100644
--- a/C4/Koha.pm
+++ b/C4/Koha.pm
@@ -1722,6 +1722,31 @@ sub GetVariationsOfISBNs {
     return wantarray ? @isbns : join( " | ", @isbns );
 }
 
+=head2 IsKohaFieldLinked
+
+    my $is_linked = IsKohaFieldLinked({
+        kohafield => $kohafield,
+        frameworkcode => $frameworkcode,
+    });
+
+    Return 1 if the field is linked
+
+=cut
+
+sub IsKohaFieldLinked {
+    my ( $params ) = @_;
+    my $kohafield = $params->{kohafield};
+    my $frameworkcode = $params->{frameworkcode} || '';
+    my $dbh = C4::Context->dbh;
+    my $is_linked = $dbh->selectcol_arrayref( q|
+        SELECT COUNT(*)
+        FROM marc_subfield_structure
+        WHERE frameworkcode = ?
+        AND kohafield = ?
+    |,{}, $frameworkcode, $kohafield );
+    return $is_linked->[0];
+}
+
 1;
 
 __END__
diff --git a/t/db_dependent/Koha/IsKohaFieldLinked.t b/t/db_dependent/Koha/IsKohaFieldLinked.t
new file mode 100644
index 0000000..90a5704
--- /dev/null
+++ b/t/db_dependent/Koha/IsKohaFieldLinked.t
@@ -0,0 +1,44 @@
+use Modern::Perl;
+use Test::More;
+
+use C4::Context;
+use C4::Koha;
+
+my $dbh = C4::Context->dbh;
+$dbh->{RaiseError} = 1;
+$dbh->{AutoCommit} = 0;
+
+my $frameworkcode = 'FCUT';
+$dbh->do( qq|
+    INSERT INTO marc_subfield_structure(
+        tagfield, tagsubfield, liblibrarian, kohafield, frameworkcode
+    ) VALUES
+        ('952', 'p', 'Barcode', 'items.barcode', '$frameworkcode'),
+        ('952', '8', 'Collection code', 'items.ccode', '$frameworkcode'),
+        ('952', '7', 'Not for loan', 'items.notforloan', '$frameworkcode'),
+        ('952', 'y', 'Koha item type', 'items.itype', '$frameworkcode'),
+        ('952', 'c', 'Permanent location', '', '$frameworkcode')
+|);
+
+is ( C4::Koha::IsKohaFieldLinked( {
+    kohafield => 'items.barcode',
+    frameworkcode => $frameworkcode,
+}), 1, 'items.barcode is linked' );
+
+is ( C4::Koha::IsKohaFieldLinked( {
+    kohafield => 'items.notforloan',
+    frameworkcode => $frameworkcode,
+}), 1, 'items.notforloan is linked' );
+
+is ( C4::Koha::IsKohaFieldLinked( {
+    kohafield => 'notforloan',
+    frameworkcode => $frameworkcode,
+}), 0, 'notforloan is not linked' );
+
+is ( C4::Koha::IsKohaFieldLinked( {
+    kohafield => 'items.permanent_location',
+    frameworkcode => $frameworkcode,
+}), 0, 'items.permanent_location is not linked' );
+
+
+done_testing;
-- 
1.7.10.4