Attachment 56613 Details for Bug 14367 – Bug 14367: Add MARC record history

[patch] Bug 14367: Add MARC record history

Bug-14367-Add-MARC-record-history.patch (text/plain), 37.70 KB, created by Aleisha Amohia on 2016-10-18 02:21:05 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Aleisha Amohia

Created: 2016-10-18 02:21:05 UTC

Size: 37.70 KB

patch

obsolete

>From 00d521d3aaf9b25036a95cc472f0e1bb1690c771 Mon Sep 17 00:00:00 2001
>From: Martin Persson <xarragon@gmail.com>
>Date: Fri, 19 Jun 2015 21:36:27 +0200
>Subject: [PATCH] Bug 14367: Add MARC record history
>
>This is a proof-of-concept implementation for adding history
>support to MARC records. Every time a change is made a complete
>copy of the record is stored along with the date/time and user
>identity. The changes are listed under each field in the MARC
>record editor and can be reverted with a click.
>
>The changes are stored as a JSON array in a new column named
>'history' in the database. The array is re-read from the database
>before updating the record to prevent old data lingering in
>the session from overwriting newer changes made by other users.
>
>If we decide to implement this feature it might be better to
>simply create a new table altogether and link it rather than
>the clumsy JSON solution. That would eliminate a lot of bulky
>code that transforms MARC-KOHA-JSON and back while ensuring
>data integrity.
>
>Also, there are plans to add permissions to the MARC records;
>this likely requires more complex interactions that will scale
>badly with the current JSON solution.
>
>At present, the history is hardcoded to 10 entries.
>This can easily be made into a syspref.
>
>The current implementation should probably be refactored
>into a 'BiblioHistory' class before deploying.
>
>Documentation of the functions/methods are also needed.
>
>Icon is ugly and needs to be improved.
>
>Sponsored-By: Halland County Library & Catalyst IT
>
>Test plan:
>* Run updatedatabase.pl
>* Log into Staff interface, search for a title, choose to edit it's MARC record.
>* Chose a MARC field and modify it, press Save.
>* Open the MARC editor again for the same title.
>* Next to the edited field a new icon should appear, looking like
>  a clock face with a encircling arrow; the history icon.
>* Clicking the icon should open a table showing all changes done
>  to the record, including value change, date/time and user name.
>* Clicking a record in the history log should revert that field to the
>  clicked value (a previous value)
>* Run prove t/db_dependent/BiblioHistory.t and confirm all tests pass
>---
> C4/Biblio.pm                                       |  31 ++-
> Koha/MetadataRecord/History.pm                     | 262 +++++++++++++++++++++
> cataloguing/addbiblio.pl                           |  43 ++--
> installer/data/mysql/atomicupdate/add_history.sql  |   1 +
> koha-tmpl/intranet-tmpl/prog/css/addbiblio.css     |  22 +-
> .../prog/en/modules/cataloguing/addbiblio.tt       |  29 ++-
> koha-tmpl/intranet-tmpl/prog/img/icon-history.png  | Bin 0 -> 513 bytes
> koha-tmpl/intranet-tmpl/prog/js/cataloging.js      |  10 +
> t/db_dependent/BiblioHistory.t                     | 256 ++++++++++++++++++++
> 9 files changed, 625 insertions(+), 29 deletions(-)
> create mode 100644 Koha/MetadataRecord/History.pm
> create mode 100644 installer/data/mysql/atomicupdate/add_history.sql
> create mode 100644 koha-tmpl/intranet-tmpl/prog/img/icon-history.png
> create mode 100644 t/db_dependent/BiblioHistory.t
>
>diff --git a/C4/Biblio.pm b/C4/Biblio.pm
>index f846e7d..9991dfa 100644
>--- a/C4/Biblio.pm
>+++ b/C4/Biblio.pm
>@@ -28,6 +28,7 @@ use MARC::File::USMARC;
> use MARC::File::XML;
> use POSIX qw(strftime);
> use Module::Load::Conditional qw(can_load);
>+use JSON qw(encode_json decode_json);
> 
> use C4::Koha;
> use C4::Log;    # logaction
>@@ -43,6 +44,7 @@ use Koha::Authority::Types;
> use Koha::Acquisition::Currencies;
> use Koha::SearchEngine;
> use Koha::Libraries;
>+use Koha::MetadataRecord::History;
> 
> use vars qw(@ISA @EXPORT);
> use vars qw($debug $cgi_debug);
>@@ -81,6 +83,7 @@ BEGIN {
>       &GetMarcISBN
>       &GetMarcISSN
>       &GetMarcSubjects
>+      &GetMarcBiblio
>       &GetMarcAuthors
>       &GetMarcSeries
>       &GetMarcHosts
>@@ -270,8 +273,8 @@ sub AddBiblio {
>     # update MARC subfield that stores biblioitems.cn_sort
>     _koha_marc_update_biblioitem_cn_sort( $record, $olddata, $frameworkcode );
> 
>-    # now add the record
>-    ModBiblioMarc( $record, $biblionumber, $frameworkcode ) unless $defer_marc_save;
>+    # now add the record (history parameter is undef: no history yet])
>+    ModBiblioMarc( $record, $biblionumber, $frameworkcode, undef ) unless $defer_marc_save;
> 
>     # update OAI-PMH sets
>     if(C4::Context->preference("OAI-PMH:AutoUpdateSets")) {
>@@ -336,9 +339,9 @@ sub ModBiblio {
>     # update biblionumber and biblioitemnumber in MARC
>     # FIXME - this is assuming a 1 to 1 relationship between
>     # biblios and biblioitems
>-    my $sth = $dbh->prepare("select biblioitemnumber from biblioitems where biblionumber=?");
>+    my $sth = $dbh->prepare("select biblioitemnumber,history from biblioitems where biblionumber=?");
>     $sth->execute($biblionumber);
>-    my ($biblioitemnumber) = $sth->fetchrow;
>+    my ($biblioitemnumber, $history) = $sth->fetchrow;
>     $sth->finish();
>     _koha_marc_update_bib_ids( $record, $frameworkcode, $biblionumber, $biblioitemnumber );
> 
>@@ -349,7 +352,7 @@ sub ModBiblio {
>     _koha_marc_update_biblioitem_cn_sort( $record, $oldbiblio, $frameworkcode );
> 
>     # update the MARC record (that now contains biblio and items) with the new record data
>-    &ModBiblioMarc( $record, $biblionumber, $frameworkcode );
>+    &ModBiblioMarc( $record, $biblionumber, $frameworkcode, $history );
> 
>     # modify the other koha tables
>     _koha_modify_biblio( $dbh, $oldbiblio, $frameworkcode );
>@@ -3478,7 +3481,7 @@ Function exported, but should NOT be used, unless you really know what you're do
> sub ModBiblioMarc {
>     # pass the MARC::Record to this function, and it will create the records in
>     # the marc field
>-    my ( $record, $biblionumber, $frameworkcode ) = @_;
>+    my ( $record, $biblionumber, $frameworkcode, $history ) = @_;
>     if ( !$record ) {
>         carp 'ModBiblioMarc passed an undefined record';
>         return;
>@@ -3524,8 +3527,20 @@ sub ModBiblioMarc {
>       $f005->update(sprintf("%4d%02d%02d%02d%02d%04.1f",@a)) if $f005;
>     }
> 
>-    $sth = $dbh->prepare("UPDATE biblioitems SET marc=?,marcxml=? WHERE biblionumber=?");
>-    $sth->execute( $record->as_usmarc(), $record->as_xml_record($encoding), $biblionumber );
>+    # Decode JSON history, create new record, update and re-encode to JSON.
>+    my $historylength = 10;
>+	eval {
>+		$history = decode_json($history);
>+		1;
>+	} or do {
>+		$history = undef;
>+	};
>+    my $newrecord = HistoryRecordNew($record);
>+    my $newhistory = HistoryUpdate($newrecord, $historylength, $history);
>+    $newhistory = encode_json($newhistory);
>+
>+    $sth = $dbh->prepare("UPDATE biblioitems SET marc=?,marcxml=?,history=? WHERE biblionumber=?");
>+    $sth->execute( $record->as_usmarc(), $record->as_xml_record($encoding), $newhistory, $biblionumber );
>     $sth->finish;
>     ModZebra( $biblionumber, "specialUpdate", "biblioserver", $record );
>     return $biblionumber;
>diff --git a/Koha/MetadataRecord/History.pm b/Koha/MetadataRecord/History.pm
>new file mode 100644
>index 0000000..e1ba484
>--- /dev/null
>+++ b/Koha/MetadataRecord/History.pm
>@@ -0,0 +1,262 @@
>+package Koha::MetadataRecord::History;
>+
>+# Copyright 2016 Aleisha Amohia <aleisha@catalyst.net.nz>
>+#
>+# This file is part of Koha.
>+#
>+# Koha is free software; you can redistribute it and/or modify it
>+# under the terms of the GNU General Public License as published by
>+# the Free Software Foundation; either version 3 of the License, or
>+# (at your option) any later version.
>+#
>+# Koha is distributed in the hope that it will be useful, but
>+# WITHOUT ANY WARRANTY; without even the implied warranty of
>+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>+# GNU General Public License for more details.
>+#
>+# You should have received a copy of the GNU General Public License
>+# along with Koha; if not, see <http://www.gnu.org/licenses>.
>+
>+use strict;
>+use warnings;
>+
>+use C4::Charset;
>+use JSON qw(decode_json);
>+
>+use vars qw(@ISA @EXPORT);
>+
>+BEGIN {
>+    require Exporter;
>+    @ISA = qw( Exporter );
>+
>+    push @EXPORT, qw(
>+        &GetMarcBiblioHistory
>+        &HistoryUpdate
>+        &HistoryParse
>+        &HistoryMapUsers
>+        &HistoryBorrowerDetails
>+        &HistoryRecordNew
>+    );
>+ }
>+
>+=head1 NAME
>+
>+Koha::MetadataRecord::History - tracking changes to MARC records
>+
>+=head1 DESCRIPTION
>+
>+Koha::MetadataRecord::History not only tracks changes to MARC records and presents them in a log, but also allows users to
>+roll them back, either in batch to a previous point in the history or separately for each field (so you can roll back changes
>+to a field without changing fields that may have been changed after this one).
>+
>+=head2 GetMarcBiblioHistory
>+
>+    my ($record, $history) = GetMarcBiblio($biblionumber, [$embeditems]);
>+
>+Returns MARC::Record representing bib identified by C<$biblionumber>.  If no bib exists, returns undef.
>+C<$biblionumber>.  If no bib exists, returns undef.
>+C<$embeditems>.  If set to true, items data are included.
>+The MARC record contains biblio data, and items data if $embeditems is set to true.
>+
>+=cut
>+
>+sub GetMarcBiblioHistory {
>+    my $biblionumber = shift;
>+    my $embeditems   = shift || 0;
>+    if (not defined $biblionumber) {
>+        warn "GetMarcBiblio called with undefined biblionumber";
>+        return;
>+    }
>+    my $dbh = C4::Context->dbh;
>+    my $sth = $dbh->prepare("SELECT marcxml,history FROM biblioitems WHERE biblionumber=? ");
>+    $sth->execute($biblionumber);
>+    my $row = $sth->fetchrow_hashref;
>+    my $marcxml = StripNonXmlChars( $row->{'marcxml'} );
>+    MARC::File::XML->default_record_format( C4::Context->preference('marcflavour') );
>+    my $record = MARC::Record->new();
>+    my ($history, $borrowers) = HistoryParse( $row->{'history'} );
>+    $borrowers = HistoryBorrowerDetails( $borrowers );
>+    $history = HistoryMapUsers( $history, $borrowers );
>+    if ($marcxml) {
>+        $record = eval { MARC::Record::new_from_xml( $marcxml, "utf8", C4::Context->preference('marcflavour') ) };
>+        if ($@) { warn " problem with :$biblionumber : $@ \n$marcxml"; }
>+            return unless $record;
>+            C4::Biblio::_koha_marc_update_bib_ids($record, '', $biblionumber, $biblionumber);
>+            C4::Biblio::EmbedItemsInMarcBiblio($record, $biblionumber) if ($embeditems);
>+            return ($record, $history);
>+    } else {
>+        return;
>+    }
>+}
>+
>+=head2 HistoryParse
>+
>+Input format (JSON): [ { datetime, borrowernumber, marcxml } ]
>+Output format: { tag } { subfield } [ { borrowernumber, data, datetime } ]
>+
>+=cut
>+
>+sub HistoryParse {
>+    my $json = shift;
>+    if (!defined $json) {
>+        warn "HistoryParse: Missing JSON parameter";
>+        return undef;
>+    }
>+    my $input = decode_json($json);
>+    # Reindex the dataset; order all values by datetime under each tag name.
>+    my $borrowers = { };
>+    my $output = { };
>+    for my $item (@{$input}) {
>+        my $datetime = $item->{datetime};
>+        $borrowers->{ $item->{borrowernumber }} = undef;
>+        my $record = MARC::File::XML::decode($item->{marcxml});
>+        for my $field ($record->fields()) {
>+            my $tag = $field->tag();
>+            $output->{$tag} = {} unless defined $output->{$tag};
>+            for my $subfield ($field->subfields()) {
>+                # Build hash ref structure.
>+                my $subfield_tag = @{$subfield}[0];
>+                $output->{$tag}->{$subfield_tag} = []
>+                unless defined $output->{$tag}->{$subfield_tag};
>+                push( @{ $output->{$tag}->{$subfield_tag} }, HistoryParsedRecordNew(
>+                    @{ $subfield }[1], $item->{borrowernumber}, $datetime));
>+                }
>+            }
>+        }
>+    return ( $output, $borrowers );
>+}
>+
>+=head2 HistoryBorrowerDetails
>+
>+Take a hashref with borrowernumbers as keys, returns new hash with the
>+borrowernumber as key to hash containing firstname, surname and title.
>+
>+=cut
>+
>+sub HistoryBorrowerDetails {
>+    my $borrowernumbers = shift;
>+    my $dbh = C4::Context->dbh;
>+    my $output = { };
>+    my @numbers = keys %{$borrowernumbers};
>+    if (@numbers > 0) {
>+        # We now need to look up the numeric user id's and map to name and title strings.
>+        my $sth = $dbh->prepare(qq{SELECT
>+            borrowernumber, title, firstname, surname
>+            FROM borrowers
>+            WHERE borrowernumber IN}
>+            . '(' . join(',', ('?') x @numbers) . ')');
>+        $sth->execute(@numbers)
>+            or die ('BorrowerDetails: Unable to execute SELECT statement');
>+        while (my $row = $sth->fetchrow_hashref()) {
>+            $output->{ $row->{borrowernumber} } = { };
>+            $output->{ $row->{borrowernumber} }->{title}     = $row->{title};
>+            $output->{ $row->{borrowernumber} }->{firstname} = $row->{firstname};
>+            $output->{ $row->{borrowernumber} }->{surname}   = $row->{surname};
>+        }
>+        $sth->finish();
>+    }
>+    return $output;
>+}
>+
>+=head2 HistoryMapUsers
>+
>+=cut
>+
>+sub HistoryMapUsers {
>+    my ($history, $borrowers) = @_;
>+    my $mappedhistory = { };
>+    for my $tag (keys %{ $history }) {
>+        $mappedhistory->{ $tag } = { };
>+        for my $field (keys %{ $history->{ $tag } }) {
>+            $mappedhistory->{ $tag }->{ $field } = [ ];
>+            for my $entry (@{ $history->{ $tag }->{ $field } }) {
>+                # C4::Dates does not allow me to print time, only dates.
>+                #my $newtime = C4::Dates->new( $history->{ $tag }->{ $field }->{ $time }->{timestamp}, 'iso' )->output('syspref');
>+                my $newitem = { };
>+                my $user = $borrowers->{ $entry->{borrowernumber} };
>+
>+                # Maybe change this to work on original instead of copy?
>+                $newitem->{title}          = $user->{title};
>+                $newitem->{firstname}      = $user->{firstname};
>+                $newitem->{surname}        = $user->{surname};
>+                $newitem->{borrowernumber} = $entry->{borrowernumber};
>+                $newitem->{data}           = $entry->{data};
>+                $newitem->{datetime}       = $entry->{datetime};
>+
>+                push ( @{ $mappedhistory->{ $tag }->{ $field } } , $newitem);
>+            }
>+        }
>+    }
>+    return $mappedhistory;
>+}
>+
>+=head2 HistoryParsedRecordNew
>+
>+=cut
>+
>+sub HistoryParsedRecordNew {
>+    my ($value, $borrowernumber, $datetime) = @_;
>+    my $newrecord = {
>+        data           => $value,
>+        borrowernumber => $borrowernumber,
>+        datetime       => $datetime,
>+    };
>+    return $newrecord;
>+}
>+
>+=head2 HistoryRecordNew
>+
>+=cut
>+
>+sub HistoryRecordNew {
>+    my ( $record ) = @_;
>+    my $encoding = C4::Context->preference("marcflavour");
>+    my $hash_record = {
>+        borrowernumber => C4::Context->userenv()->{number},
>+        datetime       => POSIX::strftime('%Y-%m-%d %T', localtime(time)),
>+        marcxml        => $record->as_xml_record($encoding),
>+    };
>+    return $hash_record;
>+}
>+
>+=head2 HistoryUpdate
>+
>+=cut
>+
>+sub HistoryUpdate {
>+    my ( $record, $limit, $collection ) = @_;
>+    if ( !$record ) {
>+        warn "HistoryUpdate: Called with undefined record";
>+        return;
>+    }
>+    if ( ref($record) ne 'HASH' ) {
>+        warn "HistoryUpdate: Record parameter is not a hashref";
>+        return;
>+    }
>+    $limit = 10 unless defined $limit;
>+    # We need something to append history to, even if there is none.
>+    $collection = [] unless defined $collection;
>+    # Truncate history to $histsize - 1, need space for new entry.
>+    splice(@{$collection}, $limit - 1);
>+    push(@{$collection}, $record);
>+    # Sort the resulting array, newest entries first.
>+    # Removed; used to be integer timestamps. Is now MySQL date strings.
>+    #@{$collection} = sort { $a->{timestamp} < $b->{timestamp} } @{$collection};
>+    @{$collection} = sort { $a->{datetime} lt $b->{datetime} } @{$collection};
>+    return $collection;
>+}
>+
>+1;
>+
>+__END__
>+
>+=head1 AUTHOR
>+
>+Koha Development Team <http://koha-community.org/>
>+
>+Paul POULAIN paul.poulain@free.fr
>+
>+Joshua Ferraro jmf@liblime.com
>+
>+=cut
>+
>diff --git a/cataloguing/addbiblio.pl b/cataloguing/addbiblio.pl
>index 7c8b66f..7f8e6d5 100755
>--- a/cataloguing/addbiblio.pl
>+++ b/cataloguing/addbiblio.pl
>@@ -36,7 +36,7 @@ use C4::ImportBatch;
> use C4::Charset;
> use Koha::BiblioFrameworks;
> use Koha::DateUtils;
>-
>+use Koha::MetadataRecord::History;
> use Koha::Libraries;
> 
> use Date::Calc qw(Today);
>@@ -279,10 +279,13 @@ sub GetMandatoryFieldZ3950 {
> =cut
> 
> sub create_input {
>-    my ( $tag, $subfield, $value, $index_tag, $tabloop, $rec, $authorised_values_sth,$cgi ) = @_;
>-    
>+
>+    my ( $tag, $subfield, $value, $index_tag, $tabloop, $rec, $authorised_values_sth,$cgi, $history ) = @_;
>+
>     my $index_subfield = CreateKey(); # create a specifique key for each subfield
> 
>+    my $taghistory = $history->{$tag}->{$subfield};
>+
>     $value =~ s/"/&quot;/g;
> 
>     # if there is no value provided but a default value in parameters, get it
>@@ -300,7 +303,7 @@ sub create_input {
>         # And <<USER>> with surname (?)
>         my $username=(C4::Context->userenv?C4::Context->userenv->{'surname'}:"superlibrarian");
>         $value=~s/<<USER>>/$username/g;
>-    
>+
>     }
>     my $dbh = C4::Context->dbh;
> 
>@@ -322,6 +325,7 @@ sub create_input {
>         value          => $value,
>         maxlength      => $tagslib->{$tag}->{$subfield}->{maxlength},
>         random         => CreateKey(),
>+        history        => $taghistory,
>     );
> 
>     if(exists $mandatory_z3950->{$tag.$subfield}){
>@@ -377,6 +381,7 @@ sub create_input {
>             maxlength => $subfield_data{maxlength},
>             readonly  => ($is_readonly) ? 1 : 0,
>             authtype  => $tagslib->{$tag}->{$subfield}->{authtypecode},
>+            history   => $taghistory,
>         };
> 
>     # it's a plugin field
>@@ -398,6 +403,7 @@ sub create_input {
>                 maxlength      => $subfield_data{maxlength},
>                 javascript     => $plugin->javascript,
>                 noclick        => $plugin->noclick,
>+                history        => $taghistory,
>             };
>         } else {
>             warn $plugin->errstr;
>@@ -410,6 +416,7 @@ sub create_input {
>                 size      => 67,
>                 maxlength => $subfield_data{maxlength},
>                 readonly  => 0,
>+                history   => $taghistory,
>             };
>         }
> 
>@@ -422,6 +429,7 @@ sub create_input {
>             value     => $value,
>             size      => 67,
>             maxlength => $subfield_data{maxlength},
>+            history   => $taghistory,
>         };
> 
>     }
>@@ -442,6 +450,7 @@ sub create_input {
>                 id        => $subfield_data{id},
>                 name      => $subfield_data{id},
>                 value     => $value,
>+                history   => $taghistory,
>             };
> 
>         }
>@@ -454,6 +463,7 @@ sub create_input {
>                 size      => 67,
>                 maxlength => $subfield_data{maxlength},
>                 readonly  => 0,
>+                history   => $taghistory,
>             };
> 
>         }
>@@ -479,11 +489,11 @@ sub format_indicator {
> }
> 
> sub build_tabs {
>-    my ( $template, $record, $dbh, $encoding,$input ) = @_;
>+    my ( $template, $record, $dbh, $encoding, $input, $history ) = @_;
> 
>     # fill arrays
>     my @loop_data = ();
>-    my $tag;
>+    #my $tag;
> 
>     my $branch_limit = C4::Context->userenv ? C4::Context->userenv->{"branch"} : "";
>     my $query = "SELECT authorised_value, lib
>@@ -499,7 +509,6 @@ sub build_tabs {
>     my @BIG_LOOP;
>     my %seen;
>     my @tab_data; # all tags to display
>-    
>     foreach my $used ( @$usedTagsLib ){
>         push @tab_data,$used->{tagfield} if not $seen{$used->{tagfield}};
>         $seen{$used->{tagfield}}++;
>@@ -536,7 +545,6 @@ sub build_tabs {
> 		}
> 		# loop through each field
>                 foreach my $field (@fields) {
>-                    
>                     my @subfields_data;
>                     if ( $tag < 10 ) {
>                         my ( $value, $subfield );
>@@ -556,7 +564,7 @@ sub build_tabs {
>                             @subfields_data,
>                             &create_input(
>                                 $tag, $subfield, $value, $index_tag, $tabloop, $record,
>-                                $authorised_values_sth,$input
>+                                $authorised_values_sth,$input, $history
>                             )
>                         );
>                     }
>@@ -571,7 +579,7 @@ sub build_tabs {
>                                 @subfields_data,
>                                 &create_input(
>                                     $tag, $subfield, $value, $index_tag, $tabloop,
>-                                    $record, $authorised_values_sth,$input
>+                                    $record, $authorised_values_sth,$input, $history
>                                 )
>                             );
>                         }
>@@ -599,7 +607,7 @@ sub build_tabs {
>                             @subfields_data,
>                             &create_input(
>                                 $tag, $subfield, '', $index_tag, $tabloop, $record,
>-                                $authorised_values_sth,$input
>+                                $authorised_values_sth,$input, $history
>                             )
>                         );
>                     }
>@@ -645,11 +653,12 @@ sub build_tabs {
>                            # always include in the form regardless of the hidden setting - bug 2206
>                     next
>                       if ( $tagslib->{$tag}->{$subfield}->{tab} ne $tabloop );
>+
> 			push(
>                         @subfields_data,
>                         &create_input(
>                             $tag, $subfield, '', $index_tag, $tabloop, $record,
>-                            $authorised_values_sth,$input
>+                            $authorised_values_sth,$input, $history
>                         )
>                     );
>                 }
>@@ -666,7 +675,7 @@ sub build_tabs {
>                         tagfirstsubfield => $subfields_data[0],
>                         fixedfield       => $tag < 10?1:0,
>                     );
>-                    
>+
>                     push @loop_data, \%tag_data ;
>                 }
>             }
>@@ -755,6 +764,7 @@ $mandatory_z3950 = GetMandatoryFieldZ3950($frameworkcode);
> # -- Global
> 
> my $record   = -1;
>+my $history  = undef;
> my $encoding = "";
> my (
> 	$biblionumbertagfield,
>@@ -765,7 +775,7 @@ my (
> );
> 
> if (($biblionumber) && !($breedingid)){
>-	$record = GetMarcBiblio($biblionumber);
>+    ($record, $history) = GetMarcBiblioHistory($biblionumber);
> }
> if ($breedingid) {
>     ( $record, $encoding ) = MARCfindbreeding( $breedingid ) ;
>@@ -898,7 +908,7 @@ if ( $op eq "addbiblio" ) {
>         }
>     } else {
>     # it may be a duplicate, warn the user and do nothing
>-        build_tabs ($template, $record, $dbh,$encoding,$input);
>+        build_tabs ($template, $record, $dbh,$encoding, $input, $history);
>         $template->param(
>             biblionumber             => $biblionumber,
>             biblioitemnumber         => $biblioitemnumber,
>@@ -945,7 +955,8 @@ elsif ( $op eq "delete" ) {
>             $record = $urecord;
>         };
>     }
>-    build_tabs( $template, $record, $dbh, $encoding,$input );
>+
>+    build_tabs( $template, $record, $dbh, $encoding, $input, $history );
>     $template->param(
>         biblionumber             => $biblionumber,
>         biblionumbertagfield        => $biblionumbertagfield,
>diff --git a/installer/data/mysql/atomicupdate/add_history.sql b/installer/data/mysql/atomicupdate/add_history.sql
>new file mode 100644
>index 0000000..4c5c424
>--- /dev/null
>+++ b/installer/data/mysql/atomicupdate/add_history.sql
>@@ -0,0 +1 @@
>+ALTER TABLE biblioitems ADD COLUMN `history` LONGTEXT DEFAULT NULL AFTER `marcxml`;
>diff --git a/koha-tmpl/intranet-tmpl/prog/css/addbiblio.css b/koha-tmpl/intranet-tmpl/prog/css/addbiblio.css
>index b8c6da3..3bb2745 100644
>--- a/koha-tmpl/intranet-tmpl/prog/css/addbiblio.css
>+++ b/koha-tmpl/intranet-tmpl/prog/css/addbiblio.css
>@@ -16,6 +16,11 @@
> 	text-decoration : none;
> }
> 
>+.buttonHistory {
>+	font-weight : bold;
>+	text-decoration : none;
>+}
>+
> a.expandfield {
> 	text-decoration : none;
> }
>@@ -128,6 +133,7 @@ a.tagnum {
> .linktools a:hover { background-color: #FFC; }
> .subfield_controls { margin : 0 .5em; }
> .readonly { border-width : 1px; border-style: inset; padding-left : 15px; background: #EEE url(../img/locked.png) center left no-repeat; width:29em; }
>+.subfield_history { margin : 0 .5em; float: right; }
> 
> #cataloguing_additem_itemlist {
> 	margin-bottom : 1em;
>@@ -167,4 +173,18 @@ tbody tr.active td {
>     position: absolute;
>     top: 50%;
>     width: 15em;
>-}
>\ No newline at end of file
>+}
>+
>+.history_container {
>+	display: none;
>+}
>+
>+.history_table {
>+	width: 100%;
>+	display: none;
>+	float: right;
>+	clear: both;
>+	font-size: 75%;
>+	width: 75%;
>+	margin: 0.3em;
>+}
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt
>index c66d24a..97091a9 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt
>@@ -659,10 +659,13 @@ function Changefwk() {
>                     [% IF ( mv.readonly == 1 ) %]
>                     <input type="text" id="[%- mv.id -%]" name="[%- mv.name -%]" value="[%- mv.value -%]" class="input_marceditor readonly" tabindex="1" size="[%- mv.size -%]" maxlength="[%- mv.maxlength -%]" readonly="readonly" />
>                     [% ELSE %]
>-                    <input type="text" id="[%- mv.id -%]" name="[%- mv.name -%]" value="[%- mv.value -%]" class="input_marceditor" tabindex="1" size="[%- mv.size -%]" maxlength="[%- mv.maxlength -%]" />
>+                    <input type="text" id="[%- mv.id -%]" name="[%- mv.name -%]" value="[%- mv.value -%]" class="input_marceditor" tabindex="1" size="[%- mv.size -%]" maxlength="[%- mv.maxlength -%]" onclick="historyToggle('[%- mv.id -%]_history'); return false;"/>
>                     [% END %]
>+
>                     [% IF ( mv.authtype ) %]
>-                    <span class="subfield_controls"><a href="#" class="buttonDot tag_editor" onclick="openAuth(this.parentNode.parentNode.getElementsByTagName('input')[1].id,'[%- mv.authtype -%]','biblio'); return false;" tabindex="1" title="Tag editor">Tag editor</a></span>
>+
>+					<a href="#" class="buttonDot tag_editor" onclick="openAuth(this.parentNode.parentNode.getElementsByTagName('input')[1].id,'[%- mv.authtype -%]','biblio'); return false;" tabindex="1" title="Tag editor">Tag editor</a>
>+                    </span>
>                     [% END %]
>                 [% ELSIF ( mv.type == 'text_complex' ) %]
>                     <input type="text" id="[%- mv.id -%]" name="[%- mv.name -%]" value="[%- mv.value -%]" class="input_marceditor framework_plugin" tabindex="1" size="[%- mv.size -%]" maxlength="[%- mv.maxlength -%]" />
>@@ -670,7 +673,8 @@ function Changefwk() {
>                         [% IF mv.noclick %]
>                             <a href="#" class="buttonDot tag_editor disabled" tabindex="-1" title="No popup"></a>
>                         [% ELSE %]
>-                            <a href="#" id="buttonDot_[% mv.id %]" class="buttonDot tag_editor framework_plugin" tabindex="1" title="Tag editor">Tag editor</a>
>+
>+                            <a href="#" id="buttonDot_[% mv.id %]" class="buttonDot tag_editor framework_plugin" tabindex="2" title="Tag editor">Tag editor</a>
>                         [% END %]
>                     </span>
>                     [% mv.javascript %]
>@@ -700,8 +704,25 @@ function Changefwk() {
>                     </a>
>                 [% END %]
>                 </span>
>-                
>+
>+                [% IF ( subfield_loo.history ) %]
>+                <span class="subfield_history">
>+                    <a href="#" class="buttonHistory" onclick="HistoryToggle('[% mv.id %]_history'); return false;">
>+                        <img src="[% interface %]/[% theme %]/img/icon-history.png" alt="History" title="Display history for this field" />
>+                    </a>
>+                </span>
>+
>+                <table class="history_table" id="[%- mv.id -%]_history" name="[%- mv.name -%]_history">
>+                    <tr><th>Value</th><th>Date</th><th>User</th></tr>
>+                    [% FOREACH hist IN subfield_loo.history %]
>+						<tr><td><a href="#" onclick="HistoryRollback('[% mv.id %]', '[% hist.data %]'); return false;">[% hist.data %]</a></td>
>+                        <td>[% hist.datetime %]</td>
>+                        <td>[% hist.title %] [% hist.firstname %] [% hist.surname %]</td></tr>
>+                    [% END %]
>+                </table>
>+                [% END %]
>             </div>
>+
>             
>         [% END %]
> 
>diff --git a/koha-tmpl/intranet-tmpl/prog/img/icon-history.png b/koha-tmpl/intranet-tmpl/prog/img/icon-history.png
>new file mode 100644
>index 0000000000000000000000000000000000000000..c65d0c6750c915ec1fc7c47eae52a329d0358404
>GIT binary patch
>literal 513
>zcmV+c0{;DpP)<h;3K|Lk000e1NJLTq000mG000mO1^@s6AM^iV00006VoOIv0A>JY
>z0A>feN^JlD010qNS#tmYF*N`HF*N}<USmxF000McNliru-v=8CG7*r|`GEic0gg#T
>zK~y-)jndDHj!_f`@XzylBMYlBLzI+B24(SBSlHN88kuDEHjC3(veCp6$-*CS7TC@t
>zi&=?SP|A-tDaB?Mq)hD8&}78szIDAN@2T5;o_oLF?>XOdPos*6keP$;J=l&he8UQs
>zGxKMSYlg`Yu^&s=fz6fv8opvSGanA{%@DbV+ZAaApGy_Hs`G1^c_||Ps^ECZ++Y>Q
>z@CGwjz&6~+fkL^FnHM{i>a9Y4fw9bdiA}hJEt&Z#GY=LqxDXM$J0<%Z#>+*o_#MO#
>z{I0B?!3T74wp&2w%Kj`f-}y(BKEXYl$4n<8#>;cTqlnm8aem=_T~T?ySLJuAgA<r2
>z2bZz80)NLT+#FaKjt%{O|8?88hcJbcnfbH^v~9bNt8LqE#gn#eVHQX5wo@`q<7j3s
>z)mci@R&f|}m@F2i3uK`g9^wB*L@eSo)-l<PKxoQ*?8F1?D_7SubG|b|(q0+oj}`e!
>zX3j^1?g#=CWv$^0dYSonV0#pSyLeuzSkBD#x@Ug?PJNe_qfZo{00000NkvXXu0mjf
>DfPUe<
>
>literal 0
>HcmV?d00001
>
>diff --git a/koha-tmpl/intranet-tmpl/prog/js/cataloging.js b/koha-tmpl/intranet-tmpl/prog/js/cataloging.js
>index 6f55930..7c0591d 100644
>--- a/koha-tmpl/intranet-tmpl/prog/js/cataloging.js
>+++ b/koha-tmpl/intranet-tmpl/prog/js/cataloging.js
>@@ -55,6 +55,16 @@ function openAuth(tagsubfieldid,authtype,source) {
>     newin=window.open("../authorities/auth_finder.pl?source="+source+"&authtypecode="+authtype+"&index="+tagsubfieldid+"&value_mainstr="+encodeURI(mainmainstring)+"&value_main="+encodeURI(mainstring), "_blank",'width=700,height=550,toolbar=false,scrollbars=yes');
> }
> 
>+function HistoryToggle (tagid)
>+{
>+	$('#' + tagid).toggle();
>+}
>+
>+function HistoryRollback (tagid, value)
>+{
>+	$('#' + tagid).val(value);
>+}
>+
> function ExpandField(index) {
>     var original = document.getElementById(index); //original <div>
>     var divs = original.getElementsByTagName('div');
>diff --git a/t/db_dependent/BiblioHistory.t b/t/db_dependent/BiblioHistory.t
>new file mode 100644
>index 0000000..c1a2c48
>--- /dev/null
>+++ b/t/db_dependent/BiblioHistory.t
>@@ -0,0 +1,256 @@
>+#!/usr/bin/perl
>+
>+use Modern::Perl;
>+use t::lib::TestBuilder;
>+use t::lib::Mocks;
>+use Test::More tests => 6;
>+use POSIX qw(strftime);
>+
>+use MARC::Record;
>+use MARC::Field;
>+use JSON qw(encode_json);
>+
>+use Koha::MetadataRecord::History;
>+
>+BEGIN {
>+    use_ok('C4::Biblio');
>+}
>+
>+our $dbh = C4::Context->dbh;
>+$dbh->{AutoCommit} = 0;
>+$dbh->{RaiseError} = 0;
>+
>+$dbh->do(q|DELETE FROM issues|);
>+$dbh->do(q|DELETE FROM items|);
>+$dbh->do(q|DELETE FROM borrowers|);
>+$dbh->do(q|DELETE FROM branches|);
>+$dbh->do(q|DELETE FROM categories|);
>+$dbh->do(q|DELETE FROM biblioitems|);
>+
>+my $builder = t::lib::TestBuilder->new();
>+
>+my $branch = $builder->build(
>+    {
>+        source => 'Branch',
>+    }
>+);
>+
>+my $category = $builder->build(
>+    {
>+        source => 'Category',
>+    }
>+);
>+
>+my $patron1 = $builder->build(
>+    {
>+        source => 'Borrower',
>+        value  => {
>+            title        => 'Mr/Mrs',
>+            firstname    => 'First name',
>+            surname      => 'Surname',
>+            categorycode => $category->{categorycode},
>+            branchcode   => $branch->{branchcode},
>+        },
>+    }
>+);
>+
>+my $patron2 = $builder->build(
>+    {
>+        source => 'Borrower',
>+        value  => {
>+            title        => 'Mr/Mrs 2',
>+            firstname    => 'First name 2',
>+            surname      => 'Surname 2',
>+            categorycode => $category->{categorycode},
>+            branchcode   => $branch->{branchcode},
>+        },
>+    }
>+);
>+
>+my $biblio = $builder->build(
>+    {
>+        source => 'Biblio',
>+        value  => {
>+            branchcode => $branch->{branchcode},
>+        },
>+    }
>+);
>+
>+C4::Context->_new_userenv('DUMMY_SESSION_ID');
>+C4::Context->set_userenv( $patron1->{borrowernumber},
>+    $patron1->{userid}, 'usercnum', $patron1->{firstname}, $patron1->{surname},
>+    $branch->{branchcode}, 'My library', 0 );
>+
>+# Item with no pre-existing history backlog.
>+my $record1 = MARC::Record->new();
>+my $field1  = MARC::Field->new(
>+    245, '1', '0',
>+    'a' => 'First Title',
>+    'c' => 'Some dude'
>+);
>+$record1->append_fields($field1);
>+
>+my $record2 = MARC::Record->new();
>+my $field2  = MARC::Field->new(
>+    245, '1', '0',
>+    'a' => 'Second Title',
>+    'c' => 'Former dude'
>+);
>+$record2->append_fields($field2);
>+
>+my $biblioitem1 = $builder->build(
>+    {
>+        source => 'Biblioitem',
>+        value  => {
>+            marcxml => $record1->as_xml(),
>+            history => undef,
>+        }
>+    }
>+);
>+
>+# items{tagname}{subfield}{timestamp}{borrowernumber|timestamp|marcxml}
>+my $items = [
>+    {
>+        borrowernumber => $patron1->{borrowernumber},
>+        datetime       => '2012-08-12 12:00:10',
>+        marcxml        => $record1->as_xml(),
>+    },
>+    {
>+        borrowernumber => $patron2->{borrowernumber},
>+        datetime       => '2012-08-12 12:00:10',
>+        marcxml        => $record2->as_xml(),
>+    }
>+];
>+
>+# Item with previous history entries.
>+my $biblioitem2 = $builder->build(
>+    {
>+        source => 'Biblioitem',
>+        value  => {
>+            marcxml => $record2->as_xml(),
>+            history => encode_json($items),
>+        }
>+    }
>+);
>+
>+my $expected_history = {
>+    245 => {
>+        a => [
>+            {
>+                borrowernumber => $patron1->{borrowernumber},
>+                data           => 'First Title',
>+                datetime       => '2012-08-12 12:00:10'
>+            },
>+            {
>+                borrowernumber => $patron2->{borrowernumber},
>+                data           => 'Second Title',
>+                datetime       => '2012-08-12 12:00:10'
>+            },
>+        ],
>+        c => [
>+            {
>+                borrowernumber => $patron1->{borrowernumber},
>+                data           => 'Some dude',
>+                datetime       => '2012-08-12 12:00:10'
>+            },
>+            {
>+                borrowernumber => $patron2->{borrowernumber},
>+                data           => 'Former dude',
>+                datetime       => '2012-08-12 12:00:10'
>+            },
>+        ],
>+    },
>+};
>+
>+my $expected_users = {
>+    $patron1->{borrowernumber} => undef,
>+    $patron2->{borrowernumber} => undef,
>+};
>+
>+# Tests the decoding and reindexing of the data, without any borrower
>+# information implanted into the history itself.
>+
>+my ( $history, $users ) = C4::Biblio::HistoryParse( encode_json($items) );
>+
>+is_deeply( $history, $expected_history, 'HistoryParse: Output' );
>+is_deeply( $users,   $expected_users,   'HistoryParse: Users' );
>+
>+# Tests the extraction of full borrower details based on the borrowernumber.
>+my $expected_details = {
>+    $patron1->{borrowernumber} => {
>+        firstname => $patron1->{firstname},
>+        surname   => $patron1->{surname},
>+        title     => $patron1->{title},
>+    },
>+    $patron2->{borrowernumber} => {
>+        firstname => $patron2->{firstname},
>+        surname   => $patron2->{surname},
>+        title     => $patron2->{title},
>+    }
>+};
>+
>+my $details = HistoryBorrowerDetails($expected_users);
>+is_deeply( $details, $expected_details, 'HistoryBorrowerDetails: Output' );
>+
>+{
>+    my $expected_mapped_history = {
>+        245 => {
>+            a => [
>+                {
>+                    borrowernumber => $patron1->{borrowernumber},
>+                    data           => 'First Title',
>+                    firstname      => $patron1->{firstname},
>+                    surname        => $patron1->{surname},
>+                    title          => $patron1->{title},
>+                    datetime       => '2012-08-12 12:00:10'
>+                },
>+                {
>+                    borrowernumber => $patron2->{borrowernumber},
>+                    data           => 'Second Title',
>+                    firstname      => $patron2->{firstname},
>+                    surname        => $patron2->{surname},
>+                    title          => $patron2->{title},
>+                    datetime       => '2012-08-12 12:00:10'
>+                },
>+            ],
>+            c => [
>+                {
>+                    borrowernumber => $patron1->{borrowernumber},
>+                    data           => 'Some dude',
>+                    firstname      => $patron1->{firstname},
>+                    surname        => $patron1->{surname},
>+                    title          => $patron1->{title},
>+                    datetime       => '2012-08-12 12:00:10'
>+                },
>+                {
>+                    borrowernumber => $patron2->{borrowernumber},
>+                    data           => 'Former dude',
>+                    firstname      => $patron2->{firstname},
>+                    surname        => $patron2->{surname},
>+                    title          => $patron2->{title},
>+                    datetime       => '2012-08-12 12:00:10'
>+                },
>+            ]
>+        }
>+    };
>+
>+    $history = C4::Biblio::HistoryMapUsers( $expected_history, $details );
>+
>+    is_deeply( $history, $expected_mapped_history, 'HistoryMapUsers: Output' );
>+
>+  #my $record;
>+  #( $record, $history ) = GetMarcBiblioHistory( $biblioitem2->{biblionumber} );
>+
>+    #is_deeply( $record, $record2 );
>+    #is_deeply( $history, $expected_mapped_history );
>+}
>+
>+# The C4::Context will provide the expected user borrowernumber.
>+my $record = HistoryRecordNew($record1);
>+
>+my $expected_record = {
>+    borrowernumber => C4::Context->userenv()->{number},
>+    marcxml        => $record1->as_xml_record(),
>+    datetime       => POSIX::strftime( '%Y-%m-%d %T', localtime ),
>+};
>+is_deeply( $record, $expected_record );
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 14367: 41642 | 56613 | 64243 | 84841 | 84842 | 84859 | 88827 | 88828 | 88829 | 102065 | 102066 | 102067 | 102173 | 102174 | 102175 | 110138 | 110139 | 110140 | 110225 | 110226 | 110227 | 110442 | 110443 | 110444 | 110489 | 110490 | 110491 | 116621 | 120480 | 121025 | 121076 | 126413 | 126414 | 131214 | 131215 | 145655 | 145656 | 146997 | 146998