Bugzilla – Attachment 79578 Details for
Bug 20797
If an Ill request has an associated bib record, the detail view should contain a link to the record
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20996: Remove prefix use of borrower category
Bug-20996-Remove-prefix-use-of-borrower-category.patch (text/plain), 13.21 KB, created by
Magnus Enger
on 2018-09-28 13:17:12 UTC
(
hide
)
Description:
Bug 20996: Remove prefix use of borrower category
Filename:
MIME Type:
Creator:
Magnus Enger
Created:
2018-09-28 13:17:12 UTC
Size:
13.21 KB
patch
obsolete
>From 960d7e5c7b3b15646b8552181172b2287e643551 Mon Sep 17 00:00:00 2001 >From: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> >Date: Tue, 3 Jul 2018 11:36:58 +0100 >Subject: [PATCH] Bug 20996: Remove prefix use of borrower category > >This patch removes the potential use of borrower category as a ILL >request ID prefix. It makes no sense. We provide the ability for a site >to define a request prefix based on branch, there is no use case for >using the borrower category. Add to this that the borrower for every >request was being retrieved in order to get the category, it's a huge >performance hit also. > >We also now require the <branch> block in the <interlibrary_loans> block >and complain if it's not present. The request prefix should be defined >in this block. > >This patch also improves the performance of the API request that returns all >requests, optionally including additional data. > >It also deprecates the overloaded TO_JSON method and moves the request >augmentation code into the API route's controller. It may be that we >want to shift it out of there at some point, but it is fine where it is >for now. > >Signed-off-by: Magnus Enger <magnus@enger.priv.no> > >https://bugs.koha-community.org/show_bug.cgi?id=20797 >--- > Koha/Illrequest.pm | 47 ++-------- > Koha/Illrequest/Config.pm | 22 +++-- > Koha/REST/V1/Illrequests.pm | 103 ++++++++++++++++++--- > about.pl | 7 ++ > ill/ill-requests.pl | 9 +- > koha-tmpl/intranet-tmpl/prog/en/modules/about.tt | 5 + > .../prog/en/modules/ill/ill-requests.tt | 2 +- > 7 files changed, 130 insertions(+), 65 deletions(-) > >diff --git a/Koha/Illrequest.pm b/Koha/Illrequest.pm >index 99b97bdbe4..1f4501c135 100644 >--- a/Koha/Illrequest.pm >+++ b/Koha/Illrequest.pm >@@ -704,8 +704,7 @@ sub getLimits { > =head3 getPrefix > > my $prefix = $abstract->getPrefix( { >- brw_cat => $brw_cat, >- branch => $branch_code, >+ branch => $branch_code > } ); > > Return the ILL prefix as defined by our $params: either per borrower category, >@@ -715,13 +714,8 @@ per branch or the default. > > sub getPrefix { > my ( $self, $params ) = @_; >- my $brn_prefixes = $self->_config->getPrefixes('branch'); >- my $brw_prefixes = $self->_config->getPrefixes('brw_cat'); >- >- return $brw_prefixes->{$params->{brw_cat}} >- || $brn_prefixes->{$params->{branch}} >- || $brw_prefixes->{default} >- || ""; # "the empty prefix" >+ my $brn_prefixes = $self->_config->getPrefixes(); >+ return $brn_prefixes->{$params->{branch}} || ""; # "the empty prefix" > } > > #### Illrequests Imports >@@ -954,12 +948,7 @@ file. > > sub id_prefix { > my ( $self ) = @_; >- my $brw = $self->patron; >- my $brw_cat = "dummy"; >- $brw_cat = $brw->categorycode >- unless ( 'HASH' eq ref($brw) && $brw->{deleted} ); > my $prefix = $self->getPrefix( { >- brw_cat => $brw_cat, > branch => $self->branchcode, > } ); > $prefix .= "-" if ( $prefix ); >@@ -991,6 +980,10 @@ sub _censor { > Overloaded I<TO_JSON> method that takes care of inserting calculated values > into the unblessed representation of the object. > >+TODO: This method does nothing and is not called anywhere. However, bug 74325 >+touches it, so keeping this for now until both this and bug 74325 are merged, >+at which point we can sort it out and remove it completely >+ > =cut > > sub TO_JSON { >@@ -999,32 +992,6 @@ sub TO_JSON { > my $object = $self->SUPER::TO_JSON(); > $object->{id_prefix} = $self->id_prefix; > >- if ( scalar (keys %$embed) ) { >- # Augment the request response with patron details if appropriate >- if ( $embed->{patron} ) { >- my $patron = $self->patron; >- $object->{patron} = { >- firstname => $patron->firstname, >- surname => $patron->surname, >- cardnumber => $patron->cardnumber >- }; >- } >- # Augment the request response with metadata details if appropriate >- if ( $embed->{metadata} ) { >- $object->{metadata} = $self->metadata; >- } >- # Augment the request response with status details if appropriate >- if ( $embed->{capabilities} ) { >- $object->{capabilities} = $self->capabilities; >- } >- # Augment the request response with library details if appropriate >- if ( $embed->{library} ) { >- $object->{library} = Koha::Libraries->find( >- $self->branchcode >- )->TO_JSON; >- } >- } >- > return $object; > } > >diff --git a/Koha/Illrequest/Config.pm b/Koha/Illrequest/Config.pm >index 28e80ce0f6..617de7df0f 100644 >--- a/Koha/Illrequest/Config.pm >+++ b/Koha/Illrequest/Config.pm >@@ -117,6 +117,17 @@ sub available_backends { > return \@backends; > } > >+=head has_branch >+ >+Return whether a 'branch' block is defined >+ >+=cut >+ >+sub has_branch { >+ my ( $self ) = @_; >+ return $self->{configuration}->{raw_config}->{branch}; >+} >+ > =head3 partner_code > > $partner_code = $config->partner_code($new_code); >@@ -150,18 +161,15 @@ sub limits { > > =head3 getPrefixes > >- my $prefixes = $config->getPrefixes('brw_cat' | 'branch'); >+ my $prefixes = $config->getPrefixes(); > >-Return the prefix for ILLs defined by our config. >+Return the branch prefix for ILLs defined by our config. > > =cut > > sub getPrefixes { >- my ( $self, $type ) = @_; >- die "Unexpected type." unless ( $type eq 'brw_cat' || $type eq 'branch' ); >- my $values = $self->{configuration}->{prefixes}->{$type}; >- $values->{default} = $self->{configuration}->{prefixes}->{default}; >- return $values; >+ my ( $self ) = @_; >+ return $self->{configuration}->{prefixes}->{branch}; > } > > =head3 getLimitRules >diff --git a/Koha/REST/V1/Illrequests.pm b/Koha/REST/V1/Illrequests.pm >index 6e3eace01d..59e3f82a44 100644 >--- a/Koha/REST/V1/Illrequests.pm >+++ b/Koha/REST/V1/Illrequests.pm >@@ -20,6 +20,9 @@ use Modern::Perl; > use Mojo::Base 'Mojolicious::Controller'; > > use Koha::Illrequests; >+use Koha::Illrequestattributes; >+use Koha::Libraries; >+use Koha::Patrons; > use Koha::Libraries; > > =head1 NAME >@@ -38,34 +41,104 @@ sub list { > my $c = shift->openapi->valid_input or return; > > my $args = $c->req->params->to_hash // {}; >- my $filter; >- my $output = []; > > # Create a hash where all keys are embedded values > # Enables easy checking > my %embed; >+ my $args_arr = (ref $args->{embed} eq 'ARRAY') ? $args->{embed} : [ $args->{embed} ]; > if (defined $args->{embed}) { >- %embed = map { $_ => 1 } @{$args->{embed}}; >+ %embed = map { $_ => 1 } @{$args_arr}; > delete $args->{embed}; > } > >- for my $filter_param ( keys %$args ) { >- my @values = split(/,/, $args->{$filter_param}); >- $filter->{$filter_param} = \@values; >+ my $requests = Koha::Illrequests->unblessed; >+ >+ # Identify patrons & branches that >+ # we're going to need and get them >+ my $to_fetch = {}; >+ $to_fetch->{patrons} = {} if $embed{patron}; >+ $to_fetch->{branches} = {} if $embed{library}; >+ $to_fetch->{capabilities} = {} if $embed{capabilities}; >+ foreach my $req(@{$requests}) { >+ $to_fetch->{patrons}->{$req->{borrowernumber}} = 1 if $embed{patron}; >+ $to_fetch->{branches}->{$req->{branchcode}} = 1 if $embed{library}; >+ $to_fetch->{capabilities}->{$req->{backend}} = 1 if $embed{capabilities}; > } > >- my $requests = Koha::Illrequests->search($filter); >+ # Fetch the patrons we need >+ my $patron_arr = []; >+ if ($embed{patron}) { >+ my @patron_ids = keys %{$to_fetch->{patrons}}; >+ if (scalar @patron_ids > 0) { >+ my $where = { >+ borrowernumber => { -in => \@patron_ids } >+ }; >+ $patron_arr = Koha::Patrons->search($where)->unblessed; >+ } >+ } > >- if ( scalar (keys %embed) ) >- { >- # Need to embed stuff >- my @results = map { $_->TO_JSON(\%embed) } $requests->as_list; >- return $c->render( status => 200, openapi => \@results ); >+ # Fetch the branches we need >+ my $branch_arr = []; >+ if ($embed{library}) { >+ my @branchcodes = keys %{$to_fetch->{branches}}; >+ if (scalar @branchcodes > 0) { >+ my $where = { >+ branchcode => { -in => \@branchcodes } >+ }; >+ $branch_arr = Koha::Libraries->search($where)->unblessed; >+ } > } >- else >- { >- return $c->render( status => 200, openapi => $requests ); >+ >+ # Fetch the capabilities we need >+ if ($embed{capabilities}) { >+ my @backends = keys %{$to_fetch->{capabilities}}; >+ if (scalar @backends > 0) { >+ foreach my $bc(@backends) { >+ my $backend = Koha::Illrequest->new->load_backend($bc); >+ $to_fetch->{$bc} = $backend->capabilities; >+ } >+ } > } >+ >+ >+ # Now we've got all associated users and branches, >+ # we can augment the request objects >+ foreach my $req(@{$requests}) { >+ my $r = Koha::Illrequests->new->find($req->{illrequest_id}); >+ $req->{id_prefix} = $r->id_prefix; >+ foreach my $p(@{$patron_arr}) { >+ if ($p->{borrowernumber} == $req->{borrowernumber}) { >+ $req->{patron} = { >+ firstname => $p->{firstname}, >+ surname => $p->{surname}, >+ cardnumber => $p->{cardnumber} >+ }; >+ last; >+ } >+ } >+ foreach my $b(@{$branch_arr}) { >+ if ($b->{branchcode} eq $req->{branchcode}) { >+ $req->{library} = $b; >+ last; >+ } >+ } >+ if ($embed{metadata}) { >+ my $metadata = Koha::Illrequestattributes->search( >+ { illrequest_id => $req->{illrequest_id} }, >+ { columns => [qw/type value/] } >+ )->unblessed; >+ my $meta_hash = {}; >+ foreach my $meta(@{$metadata}) { >+ $meta_hash->{$meta->{type}} = $meta->{value}; >+ } >+ $req->{metadata} = $meta_hash; >+ } >+ if ($embed{capabilities}) { >+ $req->{capabilities} = $to_fetch->{$req->{backend}}; >+ } >+ } >+ >+ return $c->render( status => 200, openapi => $requests ); > } > > 1; >diff --git a/about.pl b/about.pl >index 0e88d8a0d0..349dfc9668 100755 >--- a/about.pl >+++ b/about.pl >@@ -282,6 +282,13 @@ if ( C4::Context->preference('ILLModule') ) { > $warnILLConfiguration = 1; > } > >+ >+ if ( !$ill_config_from_file->{branch} ) { >+ # branch not defined >+ $template->param( ill_branch_not_defined => 1 ); >+ $warnILLConfiguration = 1; >+ } >+ > $template->param( warnILLConfiguration => $warnILLConfiguration ); > } > >diff --git a/ill/ill-requests.pl b/ill/ill-requests.pl >index 47cfc97050..6b7b76ace5 100755 >--- a/ill/ill-requests.pl >+++ b/ill/ill-requests.pl >@@ -53,9 +53,14 @@ my ( $template, $patronnumber, $cookie ) = get_template_and_user( { > } ); > > # Are we able to actually work? >-my $backends = Koha::Illrequest::Config->new->available_backends; >+my $cfg = Koha::Illrequest::Config->new; >+my $backends = $cfg->available_backends; >+my $has_branch = $cfg->has_branch; > my $backends_available = ( scalar @{$backends} > 0 ); >-$template->param( backends_available => $backends_available ); >+$template->param( >+ backends_available => $backends_available, >+ has_branch => $has_branch >+); > > if ( $backends_available ) { > if ( $op eq 'illview' ) { >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/about.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/about.tt >index 49d521043f..8353e549f4 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/about.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/about.tt >@@ -289,6 +289,11 @@ > The ILL module is enabled, but no 'partner_code' defined in koha-conf.xml. Falling back to the hardcoded 'ILLLIBS'. > </td></tr> > [%END %] >+ [% IF ill_branch_not_defined %] >+ <tr><th scope="row"><b>Warning</b> </th><td> >+ The ILL module is enabled, but no 'branch' block is defined in koha-conf.xml. You must define this block before use. >+ </td></tr> >+ [%END %] > [% IF ill_partner_code_doesnt_exist %] > <tr><th scope="row"><b>Warning</b> </th><td> > The ILL module is enabled, but the configured 'partner_code' ([% ill_partner_code_doesnt_exist | html %]) is not defined on the system. >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt >index 8bc9268364..19f4289a95 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt >@@ -301,7 +301,7 @@ > <div id="bd"> > <div id="yui-main"> > <div id="interlibraryloans" class="yui-b"> >- [% IF !backends_available %] >+ [% IF !backends_available || !has_branch %] > <div class="dialog message">ILL module configuration problem. Take a look at the <a href="/cgi-bin/koha/about.pl#sysinfo">about page</a></div> > [% ELSE %] > [% INCLUDE 'ill-toolbar.inc' %] >-- >2.11.0
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=79578">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 20797
:
75454
|
75485
|
75486
|
75656
|
75870
|
79495
|
79496
|
79497
|
79578
|
79579
|
79580
|
79581
|
79582
|
79946
|
79947
|
80018
|
80019
|
80021
|
80022
|
80038
|
80040
|
80047
|
80048
|
80049
|
80055
|
80071
|
80072
|
80073
|
80120
|
80121
|
80122