[Koha-bugs] [Bug 20447] Add support for MARC holdings records

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447

--- Comment #126 from Josef Moravec <josef.moravec at gmail.com> ---
Comment on attachment 87346
  --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87346
Bug 20447: MARC Holdings support

Review of attachment 87346:
 --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=20447&attachment=87346)
-----------------------------------------------------------------

::: C4/Biblio.pm
@@ +2828,5 @@
>  
>      $itemnumbers = [] unless defined $itemnumbers;
>  
> +    if ( !$skip_holdings && C4::Context->preference('SummaryHoldings') && !@$itemnumbers ) {
> +        require Koha::Holdings;

You don't need this require

::: C4/Items.pm
@@ +1690,5 @@
>              AND biblionumber = ?
>      |, undef, $tobiblioitem, $tobiblio, $itemnumber, $frombiblio );
>      if ($return == 1) {
> +        # Check if the moved item is attached to a holdings record
> +        my $item = my $item = Koha::Items->find($itemnumber);

double "my $item"

::: C4/Search.pm
@@ +2252,5 @@
>          }
>  
> +        # Fetch summary holdings
> +        if (C4::Context->preference('SummaryHoldings')) {
> +            $summary_holdings = Koha::Holdings->search({ biblionumber => $oldbiblio->{biblionumber}, deleted_on => undef })->unblessed;

why do you unbless here?

::: Koha/Holding.pm
@@ +53,5 @@
> +    my ($self) = @_;
> +
> +    $self->{_metadata} ||= Koha::Holdings::Metadatas->find({ holding_id => $self->id });
> +
> +    return $self->{_metadata};

Should be changed, according to bug 22407, see
https://wiki.koha-community.org/wiki/Coding_Guidelines#PERL15:_Object-oriented_code_and_the_Koha::_namespace

You should use something like 

my $metadata = $self->_result->metadata;
return Koha::Holdings::Metadata->_new_from_dbic($metadata);

@@ +111,5 @@
> +    my ($self) = @_;
> +
> +    $self->{_items} ||= Koha::Items->search( { holding_id => $self->holding_id() } );
> +
> +    return wantarray ? $self->{_items}->as_list : $self->{_items};

PERL15 rule applies here too

::: Koha/Template/Plugin/Holdings.pm
@@ +71,5 @@
> +            my $sth = C4::Context->dbh->prepare($query);
> +            $sth->execute($holding->{'holdingbranch'});
> +            my $b = $sth->fetchrow_hashref();
> +            push @parts, $b->{'branchname'} if $b;
> +            $sth->finish();

Why do you use sql instead of relation method  like $holding->library ?

::: catalogue/detail.pl
@@ +194,5 @@
>  
> +# Summary holdings
> +my $summary_holdings;
> +if (C4::Context->preference('SummaryHoldings')) {
> +    $summary_holdings = Koha::Holdings->search({ biblionumber => $biblionumber, deleted_on => undef })->unblessed;

Why do you unbless?

::: t/db_dependent/Koha/Holding.t
@@ +22,5 @@
> +use t::lib::TestBuilder;
> +
> +use C4::Biblio;
> +
> +use Koha::BiblioFramework;

Singular class is imported through plural class, it is not needed to be used
explicitaly.

::: t/db_dependent/Koha/Holdings.t
@@ +22,5 @@
> +use t::lib::TestBuilder;
> +
> +use C4::Biblio;
> +
> +use Koha::BiblioFramework;

Not needed to use singular class

-- 
You are receiving this mail because:
You are watching all bug changes.