Bugzilla – Attachment 123758 Details for
Bug 27526
Remove Mod/AddItemFromMarc from additem.pl
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 27526: Remove AddItemFromMarc from additem
Bug-27526-Remove-AddItemFromMarc-from-additem.patch (text/plain), 15.94 KB, created by
Martin Renvoize (ashimema)
on 2021-08-11 07:55:22 UTC
(
hide
)
Description:
Bug 27526: Remove AddItemFromMarc from additem
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2021-08-11 07:55:22 UTC
Size:
15.94 KB
patch
obsolete
>From 71ea1b91b3008c78cd8eff3a6ac912a482073801 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Fri, 22 Jan 2021 11:46:38 +0100 >Subject: [PATCH] Bug 27526: Remove AddItemFromMarc from additem > >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > cataloguing/additem.pl | 248 ++++++++++-------- > .../prog/en/modules/cataloguing/additem.tt | 8 +- > 2 files changed, 144 insertions(+), 112 deletions(-) > >diff --git a/cataloguing/additem.pl b/cataloguing/additem.pl >index 3c588d0655..567f29adc6 100755 >--- a/cataloguing/additem.pl >+++ b/cataloguing/additem.pl >@@ -40,6 +40,8 @@ use C4::Context; > use C4::Circulation qw( LostItem ); > use C4::Koha qw( GetAuthorisedValues ); > use C4::ClassSource qw( GetClassSources GetClassSource ); >+use C4::Barcodes; >+use C4::Barcodes::ValueBuilder; > use Koha::DateUtils qw( dt_from_string ); > use Koha::Items; > use Koha::ItemTypes; >@@ -486,20 +488,9 @@ if ($prefillitem) { > } > > #------------------------------------------------------------------------------- >+my $current_item; > if ($op eq "additem") { > >- #------------------------------------------------------------------------------- >- # rebuild >- my @tags = $input->multi_param('tag'); >- my @subfields = $input->multi_param('subfield'); >- my @values = $input->multi_param('field_value'); >- # build indicator hash. >- my @ind_tag = $input->multi_param('ind_tag'); >- my @indicator = $input->multi_param('indicator'); >- my $xml = TransformHtmlToXml(\@tags,\@subfields,\@values,\@indicator,\@ind_tag, 'ITEM'); >- my $record = MARC::Record::new_from_xml($xml, 'UTF-8'); >- >- # type of add > my $add_submit = $input->param('add_submit'); > my $add_duplicate_submit = $input->param('add_duplicate_submit'); > my $add_multiple_copies_submit = $input->param('add_multiple_copies_submit'); >@@ -513,141 +504,178 @@ if ($op eq "additem") { > $add_duplicate_submit = 1 if ($prefillitem); > $justaddeditem = 1; > >- # if autoBarcode is set to 'incremental', calculate barcode... >- if ( C4::Context->preference('autoBarcode') eq 'incremental' ) { >- $record = _increment_barcode($record, $frameworkcode); >+ my @columns = Koha::Items->columns; >+ my $biblio = Koha::Biblios->find($biblionumber); >+ my $item = Koha::Item->new; >+ $item->biblionumber($biblio->biblionumber); >+ for my $c ( @columns ) { >+ if ( $c eq 'more_subfields_xml' ) { >+ my @more_subfields_xml = $input->multi_param("items.more_subfields_xml"); >+ my @unlinked_item_subfields; >+ for my $subfield ( @more_subfields_xml ) { >+ my $v = $input->param('items.more_subfields_xml_' . $subfield); >+ push @unlinked_item_subfields, $subfield, $v; >+ } >+ if ( @unlinked_item_subfields ) { >+ my $marc = MARC::Record->new(); >+ # use of tag 999 is arbitrary, and doesn't need to match the item tag >+ # used in the framework >+ $marc->append_fields(MARC::Field->new('999', ' ', ' ', @unlinked_item_subfields)); >+ $marc->encoding("UTF-8"); >+ $item->more_subfields_xml($marc->as_xml("USMARC")); >+ next; >+ } >+ $item->more_subfields_xml(undef); >+ } else { >+ my $v = $input->param("items.".$c); >+ next unless defined $v; >+ $item->$c($v); >+ } > } > >- my $addedolditem = TransformMarcToKoha( $record ); >+ # if autoBarcode is set to 'incremental', calculate barcode... >+ if ( not $item->barcode && C4::Context->preference('autoBarcode') eq 'incremental' ) { >+ my ( $barcode ) = C4::Barcodes::ValueBuilder::incremental::get_barcode; >+ $item->barcode($barcode); >+ } > > # If we have to add or add & duplicate, we add the item > if ( $add_submit || $add_duplicate_submit ) { > > # check for item barcode # being unique >- my $exist_itemnumber = get_item_from_barcode( $addedolditem->{'barcode'} ); >- push @errors, "barcode_not_unique" if ($exist_itemnumber); >- >- # if barcode exists, don't create, but report The problem. >- unless ($exist_itemnumber) { >- my ( $oldbiblionumber, $oldbibnum, $oldbibitemnum ) = AddItemFromMarc( $record, $biblionumber ); >- >- # Pushing the last created item cookie back >- if ($prefillitem && defined $record) { >- my $itemcookie = $input->cookie( >- -name => 'LastCreatedItem', >- # We encode_base64url the whole freezed structure so we're sure we won't have any encoding problems >- -value => encode_base64url( freeze( $record ) ), >- -HttpOnly => 1, >- -expires => '' >- ); >- >- $cookie = [ $cookie, $itemcookie ]; >- } >+ if ( Koha::Items->search({ barcode => $item->barcode })->count ) { >+ # if barcode exists, don't create, but report The problem. >+ push @errors, "barcode_not_unique"; >+ } >+ else { >+ $item->store->discard_changes; >+ >+ # FIXME This need to be rewritten, we must store $item->unblessed instead >+ ## Pushing the last created item cookie back >+ #if ($prefillitem && defined $record) { >+ # my $itemcookie = $input->cookie( >+ # -name => 'LastCreatedItem', >+ # # We encode_base64url the whole freezed structure so we're sure we won't have any encoding problems >+ # -value => encode_base64url( freeze( $record ) ), >+ # -HttpOnly => 1, >+ # -expires => '' >+ # ); >+ >+ # $cookie = [ $cookie, $itemcookie ]; >+ #} > > } > $nextop = "additem"; >- if ($exist_itemnumber) { >- $itemrecord = $record; >- } >+ >+ >+ # FIXME reset item to the item we were editing >+ #if ($exist_itemnumber) { >+ >+ # $itemrecord = $record; >+ #} >+ $current_item = $item->unblessed; > } > > # If we have to add & duplicate > if ($add_duplicate_submit) { >- $itemrecord = $record; > if (C4::Context->preference('autoBarcode') eq 'incremental') { >- $itemrecord = _increment_barcode($itemrecord, $frameworkcode); >+ my ( $barcode ) = C4::Barcodes::ValueBuilder::incremental::get_barcode; >+ $current_item->{barcode} = $barcode; > } > else { > # we have to clear the barcode field in the duplicate item record to make way for the new one generated by the javascript plugin >- my ($tagfield,$tagsubfield) = &GetMarcFromKohaField( "items.barcode" ); >- my $fieldItem = $itemrecord->field($tagfield); >- $itemrecord->delete_field($fieldItem); >- $fieldItem->delete_subfields($tagsubfield); >- $itemrecord->insert_fields_ordered($fieldItem); >+ $current_item->{barcode} = undef; # FIXME or delete? > } >- $itemrecord = removeFieldsForPrefill($itemrecord) if ($prefillitem); >+ # FIXME This subroutine needs to be adjusted >+ # We want to pass $item >+ # $itemrecord = removeFieldsForPrefill($itemrecord) if ($prefillitem); > } > > # If we have to add multiple copies > if ($add_multiple_copies_submit) { > >- use C4::Barcodes; >- my $barcodeobj = C4::Barcodes->new; >- my $copynumber = $addedolditem->{'copynumber'}; >- my $oldbarcode = $addedolditem->{'barcode'}; >- my ($tagfield,$tagsubfield) = &GetMarcFromKohaField( "items.barcode" ); >- my ($copytagfield,$copytagsubfield) = &GetMarcFromKohaField( "items.copynumber" ); >+ my $copynumber = $current_item->{copynumber}; >+ my $oldbarcode = $current_item->{barcode}; > >- # If there is a barcode and we can't find their new values, we can't add multiple copies >- my $testbarcode; >+ # If there is a barcode and we can't find their new values, we can't add multiple copies >+ my $testbarcode; >+ my $barcodeobj = C4::Barcodes->new; > $testbarcode = $barcodeobj->next_value($oldbarcode) if $barcodeobj; >- if ($oldbarcode && !$testbarcode) { >+ if ( $oldbarcode && !$testbarcode ) { > >- push @errors, "no_next_barcode"; >- $itemrecord = $record; >- >- } else { >- # We add each item >+ push @errors, "no_next_barcode"; >+ $itemrecord = $record; > >- # For the first iteration >- my $barcodevalue = $oldbarcode; >- my $exist_itemnumber; >+ } >+ else { >+ # We add each item >+ >+ # For the first iteration >+ my $barcodevalue = $oldbarcode; >+ my $exist_itemnumber; >+ >+ for ( my $i = 0 ; $i < $number_of_copies ; ) { >+ >+ # If there is a barcode >+ if ($barcodevalue) { >+ >+# Getting a new barcode (if it is not the first iteration or the barcode we tried already exists) >+ $barcodevalue = $barcodeobj->next_value($oldbarcode) >+ if ( $i > 0 || $exist_itemnumber ); >+ >+ # Putting it into the record >+ if ($barcodevalue) { >+ if ( C4::Context->preference("autoBarcode") eq >+ 'hbyymmincr' && $i > 0 ) >+ { # The first copy already contains the homebranch prefix >+ # This is terribly hacky but the easiest way to fix the way hbyymmincr is working >+ # Contrary to what one might think, the barcode plugin does not prefix the returned string with the homebranch >+ # For a single item, it is handled with some JS code (see cataloguing/value_builder/barcode.pl) >+ # But when adding multiple copies we need to prefix it here, >+ # so we retrieve the homebranch from the item and prefix the barcode with it. >+ my $homebranch = $current_item->{homebranch}; >+ $barcodevalue = $homebranch . $barcodevalue; >+ } >+ $current_item->{barcode} = $barcodevalue; >+ } > >+ # Checking if the barcode already exists >+ $exist_itemnumber = Koha::Items->search({ barcode => $barcodevalue })->count; >+ } > >- for (my $i = 0; $i < $number_of_copies;) { >+ # Updating record with the new copynumber >+ if ($copynumber) { >+ $current_item->{copynumber} = $copynumber; >+ } > >- # If there is a barcode >- if ($barcodevalue) { >+ # Adding the item >+ if ( !$exist_itemnumber ) { >+ delete $current_item->{itemnumber}; >+ $current_item = Koha::Item->new($current_item)->store( >+ { skip_record_index => 1 } )->discard_changes->unblessed; >+ set_item_default_location($current_item->{itemnumber}); > >- # Getting a new barcode (if it is not the first iteration or the barcode we tried already exists) >- $barcodevalue = $barcodeobj->next_value($oldbarcode) if ($i > 0 || $exist_itemnumber); >+# We count the item only if it was really added >+# That way, all items are added, even if there was some already existing barcodes >+# FIXME : Please note that there is a risk of infinite loop here if we never find a suitable barcode >+ $i++; > >- # Putting it into the record >- if ($barcodevalue) { >- if ( C4::Context->preference("autoBarcode") eq 'hbyymmincr' && $i > 0 ) { # The first copy already contains the homebranch prefix >- # This is terribly hacky but the easiest way to fix the way hbyymmincr is working >- # Contrary to what one might think, the barcode plugin does not prefix the returned string with the homebranch >- # For a single item, it is handled with some JS code (see cataloguing/value_builder/barcode.pl) >- # But when adding multiple copies we need to prefix it here, >- # so we retrieve the homebranch from the item and prefix the barcode with it. >- my ($hb_field, $hb_subfield) = GetMarcFromKohaField( "items.homebranch" ); >- my $homebranch = $record->subfield($hb_field, $hb_subfield); >- $barcodevalue = $homebranch . $barcodevalue; >+ # Only increment copynumber if item was really added >+ $copynumber++ if ( $copynumber && $copynumber =~ m/^\d+$/ ); > } >- $record->field($tagfield)->update($tagsubfield => $barcodevalue); >- } >- >- # Checking if the barcode already exists >- $exist_itemnumber = get_item_from_barcode($barcodevalue); >- } >- # Updating record with the new copynumber >- if ( $copynumber ){ >- $record->field($copytagfield)->update($copytagsubfield => $copynumber); >- } >- >- # Adding the item >- if (!$exist_itemnumber) { >- my ( $oldbiblionumber, $oldbibnum, $oldbibitemnum ) = >- AddItemFromMarc( $record, $biblionumber, { skip_record_index => 1 } ); > >- # We count the item only if it was really added >- # That way, all items are added, even if there was some already existing barcodes >- # FIXME : Please note that there is a risk of infinite loop here if we never find a suitable barcode >- $i++; >- # Only increment copynumber if item was really added >- $copynumber++ if ( $copynumber && $copynumber =~ m/^\d+$/ ); >- } >- >- # Preparing the next iteration >- $oldbarcode = $barcodevalue; >- } >+ # Preparing the next iteration >+ $oldbarcode = $barcodevalue; >+ } > >- my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::BIBLIOS_INDEX }); >- $indexer->index_records( $biblionumber, "specialUpdate", "biblioserver" ); >+ my $indexer = Koha::SearchEngine::Indexer->new( >+ { index => $Koha::SearchEngine::BIBLIOS_INDEX } ); >+ $indexer->index_records( $biblionumber, "specialUpdate", >+ "biblioserver" ); > >- undef($itemrecord); >- } >- } >+ undef($current_item); >+ } >+ } > if ($frameworkcode eq 'FA' && $fa_circborrowernumber){ > print $input->redirect( > '/cgi-bin/koha/circ/circulation.pl?' >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt >index 666c38d275..a360a70d1e 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt >@@ -246,7 +246,9 @@ > </fieldset> > <input type="hidden" name="indicator" value=" " /> > <input type="hidden" name="indicator" value=" " /> >- <input type="hidden" name="itemnumber" value="[% itemnumber | html %]" /> >+ [% IF op != 'add_item' %] >+ <input type="hidden" name="itemnumber" value="[% itemnumber | html %]" /> >+ [% END %] > > <fieldset class="action"> [% IF op != 'saveitem' %] > <input type="submit" name="phony_submit" value="phony_submit" id="phony_submit" style="display:none;" onclick="return false;" /> >@@ -271,7 +273,9 @@ > [% ELSE %] > <input type="hidden" name="tag" value="[% itemtagfield | html %]" /> > <input type="hidden" name="subfield" value="[% itemtagsubfield | html %]" /> >- <input type="hidden" name="itemnumber" value="[% itemnumber | html %]" /> >+ [% IF op != 'add_item' %] >+ <input type="hidden" name="itemnumber" value="[% itemnumber | html %]" /> >+ [% END %] > <input type="submit" value="Save changes" onclick="return Check(this.form)"> > <input type="button" id="addnewitem" value="Add a new item"> > <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% biblionumber | uri %]">Cancel</a> >-- >2.20.1
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=123758">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 27526
:
115679
|
115680
|
116006
|
117806
|
117807
|
117808
|
121589
|
121590
|
121591
|
122288
|
122318
|
122655
|
122656
|
122657
|
122658
|
122676
|
122677
|
122681
|
122682
|
122694
|
122695
|
122696
|
122946
|
122947
|
122948
|
122949
|
122950
|
122951
|
122952
|
122953
|
122954
|
122957
|
122958
|
122959
|
122974
|
123006
|
123007
|
123110
|
123329
|
123372
|
123415
|
123757
|
123758
|
123759
|
123760
|
123761
|
123762
|
123763
|
123764
|
123765
|
123766
|
123767
|
123768
|
123769
|
123770
|
123771
|
123772
|
125069
|
125605
|
125606
|
125607
|
125608
|
125609
|
125610
|
125611
|
125612
|
125613
|
125614
|
125615
|
125616
|
125617
|
125618
|
125619
|
125620
|
125818
|
125839
|
125863
|
126034
|
126051
|
126143
|
126144
|
126393
|
127025
|
127032