Bugzilla – Attachment 51329 Details for
Bug 13074
C4::Items::_build_default_values_for_mod_marc should use Koha::Cache
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 13074: Use Koha::Cache to cache the defaults values of a MARC record
Bug-13074-Use-KohaCache-to-cache-the-defaults-valu.patch (text/plain), 10.71 KB, created by
Tomás Cohen Arazi (tcohen)
on 2016-05-09 14:14:48 UTC
(
hide
)
Description:
Bug 13074: Use Koha::Cache to cache the defaults values of a MARC record
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2016-05-09 14:14:48 UTC
Size:
10.71 KB
patch
obsolete
>From bb65b79d26be1960e2a7916d1dee72a375f3ff0e Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Tue, 3 May 2016 11:08:47 +0100 >Subject: [PATCH] Bug 13074: Use Koha::Cache to cache the defaults values of a > MARC record > >With the global %default_values_for_mod_from_marc variable, the changes >made to the marc_subfield_structure table and especially the links >between MARC and DB fields are not safe and might be outdated (if a >field is linked/unlinked) > >Test plan: >Under Plack: >- Link the barcode field, edit a record and set a barcode. >- Remove the mapping for the barcode field and then update again the > barcode of the record. >The items.barcode DB field must not have been updated. > >Without this patch, the field should have been updated. > >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >--- > C4/Items.pm | 17 ++++-- > admin/biblio_framework.pl | 2 + > admin/marc_subfields_structure.pl | 2 + > admin/marctagstructure.pl | 3 + > t/db_dependent/Items.t | 116 +++++++++++++++++++++++++++++++++++++- > 5 files changed, 132 insertions(+), 8 deletions(-) > >diff --git a/C4/Items.pm b/C4/Items.pm >index 8ee4169..a46c60c 100644 >--- a/C4/Items.pm >+++ b/C4/Items.pm >@@ -449,12 +449,14 @@ Returns item record > > =cut > >-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 $cache = Koha::Cache->get_instance(); >+ my $cache_key = "default_value_for_mod_marc-$frameworkcode"; >+ my $cached = $cache->get_from_cache($cache_key); >+ return $cached if $cached; >+ > my $marc_structure = C4::Biblio::GetMarcStructure( 1, $frameworkcode ); > my $default_values = { > barcode => undef, >@@ -487,15 +489,18 @@ sub _build_default_values_for_mod_marc { > uri => undef, > withdrawn => 0, > }; >+ my %default_values_for_mod_from_marc; > while ( my ( $field, $default_value ) = each %$default_values ) { > my $kohafield = $field; > $kohafield =~ s|^([^\.]+)$|items.$1|; >- $default_values_for_mod_from_marc{$frameworkcode}{$field} = >+ $default_values_for_mod_from_marc{$field} = > $default_value > if C4::Koha::IsKohaFieldLinked( > { kohafield => $kohafield, frameworkcode => $frameworkcode } ); > } >- return $default_values_for_mod_from_marc{$frameworkcode}; >+ >+ $cache->set_in_cache($cache_key, \%default_values_for_mod_from_marc); >+ return \%default_values_for_mod_from_marc; > } > > sub ModItemFromMarc { >diff --git a/admin/biblio_framework.pl b/admin/biblio_framework.pl >index 79a0db1..6f8fb6c 100755 >--- a/admin/biblio_framework.pl >+++ b/admin/biblio_framework.pl >@@ -80,6 +80,7 @@ if ( $op eq 'add_form' ) { > } > $cache->clear_from_cache("MarcStructure-0-$frameworkcode"); > $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); >+ $cache->clear_from_cache("default_value_for_mod_marc-$frameworkcode"); > $op = 'list'; > } elsif ( $op eq 'delete_confirm' ) { > my $framework = Koha::BiblioFrameworks->find($frameworkcode); >@@ -109,6 +110,7 @@ if ( $op eq 'add_form' ) { > } > $cache->clear_from_cache("MarcStructure-0-$frameworkcode"); > $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); >+ $cache->clear_from_cache("default_value_for_mod_marc-$frameworkcode"); > $op = 'list'; > } > >diff --git a/admin/marc_subfields_structure.pl b/admin/marc_subfields_structure.pl >index 22d4398..bfaa3dc 100755 >--- a/admin/marc_subfields_structure.pl >+++ b/admin/marc_subfields_structure.pl >@@ -341,6 +341,7 @@ elsif ( $op eq 'add_validate' ) { > $sth_update->finish; > $cache->clear_from_cache("MarcStructure-0-$frameworkcode"); > $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); >+ $cache->clear_from_cache("default_value_for_mod_marc-$frameworkcode"); > > print $input->redirect("/cgi-bin/koha/admin/marc_subfields_structure.pl?tagfield=$tagfield&frameworkcode=$frameworkcode"); > exit; >@@ -384,6 +385,7 @@ elsif ( $op eq 'delete_confirmed' ) { > } > $cache->clear_from_cache("MarcStructure-0-$frameworkcode"); > $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); >+ $cache->clear_from_cache("default_value_for_mod_marc-$frameworkcode"); > print $input->redirect("/cgi-bin/koha/admin/marc_subfields_structure.pl?tagfield=$tagfield&frameworkcode=$frameworkcode"); > exit; > >diff --git a/admin/marctagstructure.pl b/admin/marctagstructure.pl >index 7728687..7298d6a 100755 >--- a/admin/marctagstructure.pl >+++ b/admin/marctagstructure.pl >@@ -164,6 +164,7 @@ if ($op eq 'add_form') { > } > $cache->clear_from_cache("MarcStructure-0-$frameworkcode"); > $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); >+ $cache->clear_from_cache("default_value_for_mod_marc-$frameworkcode"); > } > print $input->redirect("/cgi-bin/koha/admin/marctagstructure.pl?searchfield=$tagfield&frameworkcode=$frameworkcode"); > exit; >@@ -190,6 +191,7 @@ if ($op eq 'add_form') { > $sth2->execute($searchfield, $frameworkcode); > $cache->clear_from_cache("MarcStructure-0-$frameworkcode"); > $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); >+ $cache->clear_from_cache("default_value_for_mod_marc-$frameworkcode"); > } > $template->param( > searchfield => $searchfield, >@@ -355,5 +357,6 @@ sub duplicate_framework { > } > $cache->clear_from_cache("MarcStructure-0-$newframeworkcode"); > $cache->clear_from_cache("MarcStructure-1-$newframeworkcode"); >+ $cache->clear_from_cache("default_value_for_mod_marc-$frameworkcode"); > } > >diff --git a/t/db_dependent/Items.t b/t/db_dependent/Items.t >index 2b990c1..eec8fa3 100755 >--- a/t/db_dependent/Items.t >+++ b/t/db_dependent/Items.t >@@ -26,7 +26,7 @@ use Koha::Library; > use t::lib::Mocks; > use t::lib::TestBuilder; > >-use Test::More tests => 9; >+use Test::More tests => 10; > > use Test::Warn; > >@@ -553,13 +553,125 @@ subtest 'C4::Biblio::EmbedItemsInMarcBiblio' => sub { > $schema->storage->txn_rollback; > }; > >+ >+subtest 'C4::Items::_build_default_values_for_mod_marc' => sub { >+ plan tests => 4; >+ >+ $schema->storage->txn_begin(); >+ >+ # Clear cache >+ $C4::Context::context->{marcfromkohafield} = undef; >+ $C4::Biblio::inverted_field_map = undef; >+ >+ my $builder = t::lib::TestBuilder->new; >+ my $framework = $builder->build({ >+ source => 'BiblioFramework', >+ }); >+ # Link biblio.biblionumber and biblioitems.biblioitemnumber to avoid _koha_marc_update_bib_ids to fail with 'no biblio[item]number tag for framework" >+ $builder->build({ >+ source => 'MarcSubfieldStructure', >+ value => { >+ frameworkcode => $framework->{frameworkcode}, >+ kohafield => 'biblio.biblionumber', >+ tagfield => '999', >+ tagsubfield => 'c', >+ } >+ }); >+ $builder->build({ >+ source => 'MarcSubfieldStructure', >+ value => { >+ frameworkcode => $framework->{frameworkcode}, >+ kohafield => 'biblioitems.biblioitemnumber', >+ tagfield => '999', >+ tagsubfield => 'd', >+ } >+ }); >+ my $mss_itemnumber = $builder->build({ >+ source => 'MarcSubfieldStructure', >+ value => { >+ frameworkcode => $framework->{frameworkcode}, >+ kohafield => 'items.itemnumber', >+ tagfield => '952', >+ tagsubfield => '9', >+ } >+ }); >+ >+ my $mss_barcode = $builder->build({ >+ source => 'MarcSubfieldStructure', >+ value => { >+ frameworkcode => $framework->{frameworkcode}, >+ kohafield => 'items.barcode', >+ tagfield => '952', >+ tagsubfield => 'p', >+ } >+ }); >+ >+ # Create a record with a barcode >+ my ($biblionumber) = get_biblio( $framework->{frameworkcode} ); >+ my $item_record = new MARC::Record; >+ my $a_barcode = 'a barcode'; >+ my $barcode_field = MARC::Field->new( >+ '952', ' ', ' ', >+ p => $a_barcode, >+ ); >+ $item_record->append_fields( $barcode_field ); >+ my (undef, undef, $item_itemnumber) = AddItemFromMarc($item_record, $biblionumber); >+ >+ # Make sure everything has been set up >+ my $item = GetItem($item_itemnumber); >+ is( $item->{barcode}, $a_barcode, 'Everything has been set up correctly, the barcode is defined as expected' ); >+ >+ # Delete the barcode field and save the record >+ $item_record->delete_fields( $barcode_field ); >+ ModItemFromMarc($item_record, $biblionumber, $item_itemnumber); >+ $item = GetItem($item_itemnumber); >+ is( $item->{barcode}, undef, 'The default value should have been set to the barcode, the field is mapped to a kohafield' ); >+ >+ # Re-add the barcode field and save the record >+ $item_record->append_fields( $barcode_field ); >+ ModItemFromMarc($item_record, $biblionumber, $item_itemnumber); >+ $item = GetItem($item_itemnumber); >+ is( $item->{barcode}, $a_barcode, 'Everything has been set up correctly, the barcode is defined as expected' ); >+ >+ # Remove the mapping >+ my $dbh = C4::Context->dbh; >+ $dbh->do(q| >+ DELETE FROM marc_subfield_structure >+ WHERE kohafield = 'items.barcode' >+ AND frameworkcode = ? >+ |, undef, $framework->{frameworkcode} ); >+ >+ # And make sure the caches are cleared >+ $C4::Context::context->{marcfromkohafield} = undef; >+ $C4::Biblio::inverted_field_map = undef; >+ my $cache = Koha::Cache->get_instance(); >+ $cache->clear_from_cache("default_value_for_mod_marc-" . $framework->{frameworkcode} ); >+ >+ # Update the MARC field with another value >+ $item_record->delete_fields( $barcode_field ); >+ my $another_barcode = 'another_barcode'; >+ my $another_barcode_field = MARC::Field->new( >+ '952', ' ', ' ', >+ p => $another_barcode, >+ ); >+ $item_record->append_fields( $another_barcode_field ); >+ # The DB value should not have been updated >+ ModItemFromMarc($item_record, $biblionumber, $item_itemnumber); >+ $item = GetItem($item_itemnumber); >+ is ( $item->{barcode}, $a_barcode, 'items.barcode is not mapped anymore, so the DB column has not been updated' ); >+ >+ $schema->storage->txn_rollback; >+}; >+ > # Helper method to set up a Biblio. > sub get_biblio { >+ my ( $frameworkcode ) = @_; >+ $frameworkcode //= ''; > my $bib = MARC::Record->new(); > $bib->append_fields( > MARC::Field->new('100', ' ', ' ', a => 'Moffat, Steven'), > MARC::Field->new('245', ' ', ' ', a => 'Silence in the library'), > ); >- my ($bibnum, $bibitemnum) = AddBiblio($bib, ''); >+ my ($bibnum, $bibitemnum) = AddBiblio($bib, $frameworkcode); > return ($bibnum, $bibitemnum); > } >-- >2.7.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 13074
:
51126
|
51138
|
51329
|
51330
|
52249
|
52250
|
52254
|
52255