Attachment 168823 Details for Bug 29560 – Bug 29560: (follow-up) Split prepare and store logic

[patch] Bug 29560: (follow-up) Split prepare and store logic

Bug-29560-follow-up-Split-prepare-and-store-logic.patch (text/plain), 12.57 KB, created by Martin Renvoize (ashimema) on 2024-07-11 13:34:57 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2024-07-11 13:34:57 UTC

Size: 12.57 KB

patch

obsolete

>From 678c3d6a82e9d61e3781f7b3abeb41e66ea4cf4a Mon Sep 17 00:00:00 2001
>From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>Date: Wed, 10 Jul 2024 12:22:06 +0100
>Subject: [PATCH] Bug 29560: (follow-up) Split prepare and store logic
>
>This allows for future removal of C4::Biblio::prepare_marc_host by
>splitting the logic for generating the host field and storing the link
>in the child record.
>---
> Koha/Biblio.pm        | 174 +++++++++++++++++++++++++-----------------
> Koha/REST/V1/Items.pm |  32 ++++----
> 2 files changed, 119 insertions(+), 87 deletions(-)
>
>diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm
>index 614406d2041..f41398aa5cb 100644
>--- a/Koha/Biblio.pm
>+++ b/Koha/Biblio.pm
>@@ -1764,163 +1764,201 @@ sub get_marc_hostinfo_only {
>     return $hostinfo;
> }
> 
>-=head3 link_marc_host
>+=head3 generate_marc_host_field
> 
>-=cut
>+  my $link_field = $biblio->generate_marc_host_field;
>+  $child->link_marc_host( $link_field );
> 
>-sub link_marc_host {
>-    my ( $self, $params ) = @_;
>+This method generates a MARC link field from the biblio that can be added to child
>+records to link them to the host record.
> 
>-    my $host = Koha::Biblios->find( $params->{biblionumber} );
>-    return unless $host;
>+NOTE: This replicates and partially enhances C4::Biblio::prepare_marc_host(). We should merge
>+functionality from C4::Biblio::PrepareMarcHost() too and then replace all calls to those methods
>+with this one and remove those alternatives from the codebase.
>+
>+=cut
>+
>+sub generate_marc_host_field {
>+    my ($self) = @_;
> 
>     my $marcflavour = C4::Context->preference('marcflavour');
>-    my $marc_host = $host->metadata->record;
>+    my $marc_host   = $self->metadata->record;
>     my %sfd;
>-    my $field;
>     my $host_field;
>+    my $link_field;
> 
>     if ( $marcflavour eq 'MARC21' ) {
>+
>         # Author
>-        if ( $field =
>-            $marc_host->field('100') || $marc_host->field('110') || $marc_host->field('111') )
>-        {
>-            my $s = $field->as_string('ab');
>+        if ( $host_field = $marc_host->field('100') || $marc_host->field('110') || $marc_host->field('111') ) {
>+            my $s = $host_field->as_string('ab');
>             if ($s) {
>                 $sfd{a} = $s;
>             }
>         }
>-        # Title
>-        if ( $field = $marc_host->field('245') ) {
>-            my $s = $field->as_string('ab');
>+
>+        # Edition
>+        if ( $host_field = $marc_host->field('250') ) {
>+            my $s = $host_field->as_string('ab');
>             if ($s) {
>-                $sfd{t} = $s;
>+                $sfd{b} = $s;
>             }
>         }
>-        # Uniform title
>-        if ( $field = $marc_host->field('240') ) {
>-            my $s = $field->as_string('a');
>+
>+        # Publication
>+        if ( $host_field = $marc_host->field('260') ) {
>+            my $s = $host_field->as_string('abc');
>             if ($s) {
>-                $sfd{s} = $s;
>+                $sfd{d} = $s;
>             }
>         }
>-        # Publication
>-        if ( $field = $marc_host->field('260') ) {
>-            my $s = $field->as_string('abc');
>+
>+        # Uniform title
>+        if ( $host_field = $marc_host->field('240') ) {
>+            my $s = $host_field->as_string('a');
>             if ($s) {
>-                $sfd{d} = $s;
>+                $sfd{s} = $s;
>             }
>         }
>-        # Edition
>-        if ( $field = $marc_host->field('250') ) {
>-            my $s = $field->as_string('ab');
>+
>+        # Title
>+        if ( $host_field = $marc_host->field('245') ) {
>+            my $s = $host_field->as_string('ab');
>             if ($s) {
>-                $sfd{b} = $s;
>+                $sfd{t} = $s;
>             }
>         }
>+
>         # ISSN
>-        if ( $field = $marc_host->field('022') ) {
>-            my $s = $field->as_string('a');
>+        if ( $host_field = $marc_host->field('022') ) {
>+            my $s = $host_field->as_string('a');
>             if ($s) {
>                 $sfd{x} = $s;
>             }
>         }
>+
>         # ISBN
>-        if ( $field = $marc_host->field('020') ) {
>-            my $s = $field->as_string('a');
>+        if ( $host_field = $marc_host->field('020') ) {
>+            my $s = $host_field->as_string('a');
>             if ($s) {
>                 $sfd{z} = $s;
>             }
>         }
>         if ( C4::Context->preference('UseControlNumber') ) {
>+
>             # Control number
>-            if ( $field = $marc_host->field('001') ) {
>-                $sfd{w} = $field->data(),;
>+            if ( $host_field = $marc_host->field('001') ) {
>+                $sfd{w} = $host_field->data(),;
>             }
>+
>             # Control number identifier
>-            if ( $field = $marc_host->field('003') ) {
>-                $sfd{w} = '('.$field->data().')'.$sfd{w};
>+            if ( $host_field = $marc_host->field('003') ) {
>+                $sfd{w} = '(' . $host_field->data() . ')' . $sfd{w};
>             }
>         }
>-        $host_field = MARC::Field->new( 773, '0', ' ', %sfd );
>-    }
>-    elsif ( $marcflavour eq 'UNIMARC' ) {
>+        $link_field = MARC::Field->new( 773, '0', ' ', %sfd );
>+    } elsif ( $marcflavour eq 'UNIMARC' ) {
> 
>         #author
>-        if ( $field =
>-            $marc_host->field('700') || $marc_host->field('710') || $marc_host->field('720') )
>-        {
>-            my $s = $field->as_string('ab');
>+        if ( $host_field = $marc_host->field('700') || $marc_host->field('710') || $marc_host->field('720') ) {
>+            my $s = $host_field->as_string('ab');
>             if ($s) {
>                 $sfd{a} = $s;
>             }
>         }
> 
>-        #title
>-        if ( $field = $marc_host->field('200') ) {
>-            my $s = $field->as_string('a');
>-            if ($s) {
>-                $sfd{t} = $s;
>-            }
>-        }
>-
>         #place of publicaton
>-        if ( $field = $marc_host->field('210') ) {
>-            my $s = $field->as_string('a');
>+        if ( $host_field = $marc_host->field('210') ) {
>+            my $s = $host_field->as_string('a');
>             if ($s) {
>                 $sfd{c} = $s;
>             }
>         }
> 
>         #date of publication
>-        if ( $field = $marc_host->field('210') ) {
>-            my $s = $field->as_string('d');
>+        if ( $host_field = $marc_host->field('210') ) {
>+            my $s = $host_field->as_string('d');
>             if ($s) {
>                 $sfd{d} = $s;
>             }
>         }
> 
>         #edition statement
>-        if ( $field = $marc_host->field('205') ) {
>-            my $s = $field->as_string();
>+        if ( $host_field = $marc_host->field('205') ) {
>+            my $s = $host_field->as_string();
>             if ($s) {
>                 $sfd{e} = $s;
>             }
>         }
> 
>+        #title
>+        if ( $host_field = $marc_host->field('200') ) {
>+            my $s = $host_field->as_string('a');
>+            if ($s) {
>+                $sfd{t} = $s;
>+            }
>+        }
>+
>         #URL
>-        if ( $field = $marc_host->field('856') ) {
>-            my $s = $field->as_string('u');
>+        if ( $host_field = $marc_host->field('856') ) {
>+            my $s = $host_field->as_string('u');
>             if ($s) {
>                 $sfd{u} = $s;
>             }
>         }
> 
>         #ISSN
>-        if ( $field = $marc_host->field('011') ) {
>-            my $s = $field->as_string('a');
>+        if ( $host_field = $marc_host->field('011') ) {
>+            my $s = $host_field->as_string('a');
>             if ($s) {
>                 $sfd{x} = $s;
>             }
>         }
> 
>         #ISBN
>-        if ( $field = $marc_host->field('010') ) {
>-            my $s = $field->as_string('a');
>+        if ( $host_field = $marc_host->field('010') ) {
>+            my $s = $host_field->as_string('a');
>             if ($s) {
>                 $sfd{y} = $s;
>             }
>         }
>-        if ( $field = $marc_host->field('001') ) {
>-            $sfd{0} = $field->data(),;
>+        if ( $host_field = $marc_host->field('001') ) {
>+            $sfd{0} = $host_field->data(),;
>         }
>-        $host_field = MARC::Field->new( 461, '0', ' ', %sfd );
>+        $link_field = MARC::Field->new( 461, '0', ' ', %sfd );
>+    }
>+
>+    return $link_field;
>+}
>+
>+=head3 link_marc_host
>+
>+  $biblio->link_marc_host({ field => $link_field});
>+  $biblio->link_marc_host({ host => $biblio });
>+  $biblio->link_marc_host({ host => $biblionumber });
>+
>+Links a child marc record to the parent. Expects either a pre-formed link field as generated by
>+$parent->get_link_field, the biblio object or biblionumber of the host to link to.
>+
>+=cut
>+
>+sub link_marc_host {
>+    my ( $self, $params ) = @_;
>+
>+    my $host_link_field;
>+    if ( $params->{field} ) {
>+        $host_link_field = $params->{field};
>+    } elsif ( ref( $params->{host} ) eq 'Koha::Biblio' ) {
>+        $host_link_field = $params->{host}->generate_marc_host_field;
>+    } else {
>+        my $host = Koha::Biblios->find( $params->{host} );
>+        $host_link_field = $host->generate_marc_host_field;
>     }
> 
>     my $marc_record = $self->metadata->record;
>-    $marc_record->append_fields($host_field);
>+    $marc_record->append_fields($host_link_field);
> 
>-    C4::Biblio::ModBiblioMarc($marc_record, $self->biblionumber);
>+    C4::Biblio::ModBiblioMarc( $marc_record, $self->biblionumber );
>     return $self;
> }
> 
>diff --git a/Koha/REST/V1/Items.pm b/Koha/REST/V1/Items.pm
>index 2612cf326ee..cc1a8fef634 100644
>--- a/Koha/REST/V1/Items.pm
>+++ b/Koha/REST/V1/Items.pm
>@@ -292,6 +292,7 @@ sub bundled_items {
>     };
> }
> 
>+
> =head3 add_to_bundle
> 
> Controller function that handles adding items to this bundle
>@@ -302,7 +303,7 @@ sub add_to_bundle {
>     my $c = shift->openapi->valid_input or return;
> 
>     my $item_id = $c->param('item_id');
>-    my $item = Koha::Items->find( $item_id );
>+    my $item    = Koha::Items->find($item_id);
> 
>     return $c->render_resource_not_found("Item")
>         unless $item;
>@@ -316,24 +317,22 @@ sub add_to_bundle {
>     return $c->render_resource_not_found("Bundle item")
>         unless $bundle_item;
> 
>-    my $add_link = $c->validation->param('body')->{'marc_link'} // 0;
>+    my $add_link = $body->{'marc_link'} // 0;
>     return try {
>         my $options = {
>             force_checkin => $body->{force_checkin},
>-            ignore_holds => $body->{ignore_holds},
>+            ignore_holds  => $body->{ignore_holds},
>         };
> 
>-        my $link = $item->add_to_bundle($bundle_item, $options);
>+        my $link = $item->add_to_bundle( $bundle_item, $options );
>         if ($add_link) {
>-            $bundle_item->biblio->link_marc_host(
>-                { biblionumber => $item->biblio->biblionumber } );
>+            $bundle_item->biblio->link_marc_host( { host => $item->biblio } );
>         }
>         return $c->render(
>             status  => 201,
>-            openapi => $bundle_item
>+            openapi => $bundle_item->to_api
>         );
>-    }
>-    catch {
>+    } catch {
>         if ( ref($_) eq 'Koha::Exceptions::Object::DuplicateID' ) {
>             return $c->render(
>                 status  => 409,
>@@ -343,8 +342,7 @@ sub add_to_bundle {
>                     key        => $_->duplicate_id
>                 }
>             );
>-        }
>-        elsif ( ref($_) eq 'Koha::Exceptions::Item::Bundle::BundleIsCheckedOut' ) {
>+        } elsif ( ref($_) eq 'Koha::Exceptions::Item::Bundle::BundleIsCheckedOut' ) {
>             return $c->render(
>                 status  => 409,
>                 openapi => {
>@@ -360,8 +358,7 @@ sub add_to_bundle {
>                     error_code => 'checked_out'
>                 }
>             );
>-        }
>-        elsif ( ref($_) eq 'Koha::Exceptions::Checkin::FailedCheckin' ) {
>+        } elsif ( ref($_) eq 'Koha::Exceptions::Checkin::FailedCheckin' ) {
>             return $c->render(
>                 status  => 409,
>                 openapi => {
>@@ -369,8 +366,7 @@ sub add_to_bundle {
>                     error_code => 'failed_checkin'
>                 }
>             );
>-        }
>-        elsif ( ref($_) eq 'Koha::Exceptions::Item::Bundle::ItemHasHolds' ) {
>+        } elsif ( ref($_) eq 'Koha::Exceptions::Item::Bundle::ItemHasHolds' ) {
>             return $c->render(
>                 status  => 409,
>                 openapi => {
>@@ -378,8 +374,7 @@ sub add_to_bundle {
>                     error_code => 'reserved'
>                 }
>             );
>-        }
>-        elsif ( ref($_) eq 'Koha::Exceptions::Item::Bundle::IsBundle' ) {
>+        } elsif ( ref($_) eq 'Koha::Exceptions::Item::Bundle::IsBundle' ) {
>             return $c->render(
>                 status  => 400,
>                 openapi => {
>@@ -387,8 +382,7 @@ sub add_to_bundle {
>                     error_code => 'failed_nesting'
>                 }
>             );
>-        }
>-        else {
>+        } else {
>             $c->unhandled_exception($_);
>         }
>     };
>-- 
>2.45.2

Actions: View | Diff | Splinter Review

Attachments on bug 29560: 135090 | 135091 | 137509 | 137510 | 156299 | 156300 | 159081 | 159082 | 168720 | 168721 | 168722 | 168723 | 168816 | 168817 | 168818 | 168819 | 168820 | 168821 | 168822 | 168823 | 168824 | 168825 | 168885 | 168886 | 168887 | 168888 | 168889 | 168968