Description
Thibaud Guillot (thibaud_g)
2022-09-12 08:45:30 UTC
Created attachment 140431 [details] [review] Bug 31552: Add fields to get opac description on ILSDI web service Test plan 1) Active your ILS-DI syspref 2) Have some authorised values linked to one or more concern fields 3) Choose an item and go to 'cgi-bin/koha/opac/ilsdi.pl?service=GetRecords&id=<itemId>' 4) The XML file contains item fields with some of them the code associated to the authorized values. 5) Apply this patch 6) Repeat step 3 and see the new fields with specific "_description" suffix added and contains opac description from authorized values Comment on attachment 140431 [details] [review] Bug 31552: Add fields to get opac description on ILSDI web service >From e618b742c79ba17b4418685c229e27dd1de244c0 Mon Sep 17 00:00:00 2001 >From: Thibaud Guillot <thibaud.guillot@biblibre.com> >Date: Mon, 12 Sep 2022 11:04:07 +0200 >Subject: [PATCH] Bug 31550: Add fields to get opac description on ILSDI web > service > >Test plan > >1) Active your ILS-DI syspref >2) Have some authorised values linked to one or more concern >fields >3) Choose an item and go to 'cgi-bin/koha/opac/ilsdi.pl?service=GetRecords&id=<itemId>' >4) The XML file contains item fields with some of them the code associated to the authorized values. >5) Apply this patch >6) Repeat step 3 and see the new fields with specific "_description" >suffix added and contains opac description from authorized values > >https://bugs.koha-community.org/show_bug.cgi?id=31550 >--- > C4/ILSDI/Services.pm | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > >diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm >index f6bf6bcc14..b8e23c4a5b 100644 >--- a/C4/ILSDI/Services.pm >+++ b/C4/ILSDI/Services.pm >@@ -253,10 +253,15 @@ sub GetRecords { > $item{'homebranchname'} = $home_library ? $home_library->branchname : ''; > $item{'holdingbranchname'} = $holding_library ? $holding_library->branchname : ''; > >- if ($item->location) { >- my $authorised_value = Koha::AuthorisedValues->find_by_koha_field({ kohafield => 'items.location', authorised_value => $item->location }); >- if ($authorised_value) { >- $item{location_description} = $authorised_value->opac_description; >+ my @item_fields_description = ( 'location', 'ccode', 'permanent_location', 'notforloan', 'itemlost', 'withdrawn', 'damaged', 'restricted' ); >+ >+ foreach (@item_fields_description) { >+ >+ if ( $item->$_ ) { >+ my $authorised_value = Koha::AuthorisedValues->find_by_koha_field( { kohafield => 'items.' . $_, authorised_value => $item->$_ } ); >+ if ($authorised_value) { >+ $item{ $_ . _description } = $authorised_value->opac_description; >+ } > } > } > >-- >2.25.1 Created attachment 140432 [details] [review] Bug 31550: Add fields to get opac description on ILSDI web service Test plan 1) Active your ILS-DI syspref 2) Have some authorised values linked to one or more concern fields 3) Choose an item and go to 'cgi-bin/koha/opac/ilsdi.pl?service=GetRecords&id=<itemId>' 4) The XML file contains item fields with some of them the code associated to the authorized values. 5) Apply this patch 6) Repeat step 3 and see the new fields with specific "_description" suffix added and contains opac description from authorized values I'm getting this error after applying the patch (using koha-testing-docker - flush_memcached, restart_all, and browser cached cleared after patch applied): Could not compile /kohadevbox/koha/opac/ilsdi.pl: Attempt to reload C4/ILSDI/Services.pm aborted. Compilation failed in require at /kohadevbox/koha/opac/ilsdi.pl line 22. BEGIN failed--compilation aborted at /kohadevbox/koha/opac/ilsdi.pl line 22. at /usr/share/perl5/CGI/Compile.pm line 144 in CGI::Compile::compile at /usr/share/perl5/CGI/Compile.pm line 144 141: my $code = $self->_eval($eval); 142: my $exception = $@; 143: 144: die "Could not compile $script: $exception" if $exception; 145: 146: subname "${package}::$subname", sub { 147: my @args = @_; Show function arguments .... Testing notes: 1. Added OPAC descriptions for CCODE authorised values. 2. Tested with sample data using: http://127.0.0.1:8081/cgi-bin/koha/opac/ilsdi.pl?service=GetRecords&id=262 (In reply to David Nind from comment #4) > I'm getting this error after applying the patch (using koha-testing-docker - > flush_memcached, restart_all, and browser cached cleared after patch > applied): > > Could not compile /kohadevbox/koha/opac/ilsdi.pl: Attempt to reload > C4/ILSDI/Services.pm aborted. > Compilation failed in require at /kohadevbox/koha/opac/ilsdi.pl line 22. > BEGIN failed--compilation aborted at /kohadevbox/koha/opac/ilsdi.pl line 22. > at /usr/share/perl5/CGI/Compile.pm line 144 > > in CGI::Compile::compile at /usr/share/perl5/CGI/Compile.pm line 144 > > 141: my $code = $self->_eval($eval); > 142: my $exception = $@; > 143: > 144: die "Could not compile $script: $exception" if $exception; > 145: > 146: subname "${package}::$subname", sub { > 147: my @args = @_; > > Show function arguments > .... > > Testing notes: > 1. Added OPAC descriptions for CCODE authorised values. > 2. Tested with sample data using: > http://127.0.0.1:8081/cgi-bin/koha/opac/ilsdi.pl?service=GetRecords&id=262 Thanks for your feeback, I restart from zero, apply this patch but I can't reproduce the bug.. this apparently comes from the ILS-DI service so I will try later like you in kohadevbox to see if the problem still be. (Thanks for you link but it concern your localhost - 127.0.0.1 so I can see your record) Anyone get the same error than David ? So I tested right now and it's working for me Hi Thibaud. Apologies for not retesting sooner! I'm getting this error trace message now after applying the patch: Could not compile /kohadevbox/koha/opac/ilsdi.pl: Bareword "_description" not allowed while "strict subs" in use at /kohadevbox/koha/C4/ILSDI/Services.pm line 262. Compilation failed in require at /kohadevbox/koha/opac/ilsdi.pl line 22. BEGIN failed--compilation aborted at /kohadevbox/koha/opac/ilsdi.pl line 22. at /usr/share/perl5/CGI/Compile.pm line 144 ... If I refresh the page again, I get this error trace message: Could not compile /kohadevbox/koha/opac/ilsdi.pl: Attempt to reload C4/ILSDI/Services.pm aborted. Compilation failed in require at /kohadevbox/koha/opac/ilsdi.pl line 22. BEGIN failed--compilation aborted at /kohadevbox/koha/opac/ilsdi.pl line 22. at /usr/share/perl5/CGI/Compile.pm line 144 ... Maybe this is because koha-testing-docker is using strict mode for the database? (I don't know enough about these things to offer an informed opinion....or how to disable struct mode). David Created attachment 144979 [details] [review] Bug 31550: (follow-up) fix bareword issue (In reply to David Nind from comment #7) > Hi Thibaud. > > Apologies for not retesting sooner! > > I'm getting this error trace message now after applying the patch: > > Could not compile /kohadevbox/koha/opac/ilsdi.pl: Bareword "_description" > not allowed while "strict subs" in use at > /kohadevbox/koha/C4/ILSDI/Services.pm line 262. > Compilation failed in require at /kohadevbox/koha/opac/ilsdi.pl line 22. > BEGIN failed--compilation aborted at /kohadevbox/koha/opac/ilsdi.pl line 22. > at /usr/share/perl5/CGI/Compile.pm line 144 > ... > > If I refresh the page again, I get this error trace message: > > Could not compile /kohadevbox/koha/opac/ilsdi.pl: Attempt to reload > C4/ILSDI/Services.pm aborted. > Compilation failed in require at /kohadevbox/koha/opac/ilsdi.pl line 22. > BEGIN failed--compilation aborted at /kohadevbox/koha/opac/ilsdi.pl line 22. > at /usr/share/perl5/CGI/Compile.pm line 144 > ... > > > Maybe this is because koha-testing-docker is using strict mode for the > database? (I don't know enough about these things to offer an informed > opinion....or how to disable struct mode). > > David Now it will be better I think ;) Thanks for your feedback again Created attachment 145041 [details] [review] Bug 31550: Add fields to get opac description on ILSDI web service Test plan 1) Active your ILS-DI syspref 2) Have some authorised values linked to one or more concern fields 3) Choose an item and go to 'cgi-bin/koha/opac/ilsdi.pl?service=GetRecords&id=<itemId>' 4) The XML file contains item fields with some of them the code associated to the authorized values. 5) Apply this patch 6) Repeat step 3 and see the new fields with specific "_description" suffix added and contains opac description from authorized values https://bugs.koha-community.org/show_bug.cgi?id=31550 Signed-off-by: David Nind <david@davidnind.com> Created attachment 145042 [details] [review] Bug 31550: (follow-up) fix bareword issue Signed-off-by: David Nind <david@davidnind.com> (In reply to Thibaud Guillot from comment #9) > Now it will be better I think ;) Thanks for your feedback again Thanks Thibaud! Now signed off. Please add unit tests. Created attachment 154370 [details] [review] Bug 31550: (follow-up) fix bareword issue Signed-off-by: David Nind <david@davidnind.com> Created attachment 154371 [details] [review] Bug 31550: Add unit tests Created attachment 154372 [details] [review] Bug 31550: Add fields to get opac description on ILSDI web service Test plan 1) Active your ILS-DI syspref 2) Have some authorised values linked to one or more concern fields 3) Choose an item and go to 'cgi-bin/koha/opac/ilsdi.pl?service=GetRecords&id=<itemId>' 4) The XML file contains item fields with some of them the code associated to the authorized values. 5) Apply this patch 6) Repeat step 3 and see the new fields with specific "_description" suffix added and contains opac description from authorized values https://bugs.koha-community.org/show_bug.cgi?id=31550 Signed-off-by: David Nind <david@davidnind.com> Created attachment 154373 [details] [review] Bug 31550: (follow-up) fix bareword issue Signed-off-by: David Nind <david@davidnind.com> Created attachment 154374 [details] [review] Bug 31550: Add unit tests Created attachment 154858 [details] [review] Bug 31550: Add fields to get opac description on ILSDI web service Test plan 1) Active your ILS-DI syspref 2) Have some authorised values linked to one or more concern fields 3) Choose an item and go to 'cgi-bin/koha/opac/ilsdi.pl?service=GetRecords&id=<itemId>' 4) The XML file contains item fields with some of them the code associated to the authorized values. 5) Apply this patch 6) Repeat step 3 and see the new fields with specific "_description" suffix added and contains opac description from authorized values https://bugs.koha-community.org/show_bug.cgi?id=31550 Signed-off-by: David Nind <david@davidnind.com> Created attachment 154859 [details] [review] Bug 31550: (follow-up) fix bareword issue Signed-off-by: David Nind <david@davidnind.com> Created attachment 154860 [details] [review] Bug 31550: Add unit tests Signed-off-by: David Nind <david@davidnind.com> Testing notes (using KTD): 1. Added OPAC descriptions for CCODE authorised values. 2. Tested with sample data using: http://127.0.0.1:8081/cgi-bin/koha/opac/ilsdi.pl?service=GetRecords&id=262 3. Before patch, there is only the CCODE in the returned XML, for example: <ccode>REF</ccode>. 4. After the patch, the CCODE OPAC description is also now included in the returned XML, for example: <ccode_description>Reference (OPAC description)</ccode_description> 5. Ran tests and these passed: prove t/db_dependent/AuthorisedValues.t I think at a minimum you want to use: get_description_by_koha_field that routine is cached so will avoid extra lookups. Possibly you could use 'strings_map' as the API does to expand the values - pinging RM fro opinion on that I'd use strings_map() as Nick suggests. It is supposed to return a stable data structure with enough information to know where things come from and how to handle it. Plus, it uses the methods Nick mentions are cached. |