31550 – ILS-DI: Add OPAC description for some item fields

Bug 31550 - ILS-DI: Add OPAC description for some item fields

Summary: ILS-DI: Add OPAC description for some item fields

Status:	Failed QA

Alias:	None

Product:	Koha
Classification:	Unclassified
Component:	Web services (show other bugs)
Version:	Main
Hardware:	All All

Importance:	P5 - low enhancement
Assignee:	Thibaud Guillot (thibaud_g)
QA Contact:	Testopia

URL:
Keywords:

Depends on:
Blocks:

Reported:	2022-09-12 08:45 UTC by Thibaud Guillot (thibaud_g)
Modified:	2023-10-12 16:39 UTC (History)
CC List:	4 users (show)

See Also:	25650
Change sponsored?:	---
Patch complexity:	---
Documentation contact:
Documentation submission:
Text to go in the release notes:	This enhancement adds the OPAC descriptions for some item fields to the XML returned from ILS-DI requests. This includes these fields: location, ccode, permanent_location, notforloan, itemlost, withdrawn, damaged, and restricted. The OPAC descriptions have _description added to the item field name, for example: <ccode_description>Reference (OPAC description)</ccode_description>.
Version(s) released in:
Circulation function:

Attachments
Bug 31552: Add fields to get opac description on ILSDI web service (2.17 KB, patch) 2022-09-12 09:28 UTC, Thibaud Guillot (thibaud_g)	Details \| Diff \| Splinter Review
Bug 31550: Add fields to get opac description on ILSDI web service (2.17 KB, patch) 2022-09-12 09:30 UTC, Thibaud Guillot (thibaud_g)	Details \| Diff \| Splinter Review
Bug 31550: (follow-up) fix bareword issue (988 bytes, patch) 2023-01-04 10:32 UTC, Thibaud Guillot (thibaud_g)	Details \| Diff \| Splinter Review
Bug 31550: Add fields to get opac description on ILSDI web service (2.16 KB, patch) 2023-01-04 18:57 UTC, David Nind	Details \| Diff \| Splinter Review
Bug 31550: (follow-up) fix bareword issue (1.01 KB, patch) 2023-01-04 18:57 UTC, David Nind	Details \| Diff \| Splinter Review
Bug 31550: (follow-up) fix bareword issue (1.01 KB, patch) 2023-08-11 14:24 UTC, Thibaud Guillot (thibaud_g)	Details \| Diff \| Splinter Review
Bug 31550: Add unit tests (1.88 KB, patch) 2023-08-11 14:25 UTC, Thibaud Guillot (thibaud_g)	Details \| Diff \| Splinter Review
Bug 31550: Add fields to get opac description on ILSDI web service (2.16 KB, patch) 2023-08-11 14:26 UTC, Thibaud Guillot (thibaud_g)	Details \| Diff \| Splinter Review
Bug 31550: (follow-up) fix bareword issue (1.01 KB, patch) 2023-08-11 14:26 UTC, Thibaud Guillot (thibaud_g)	Details \| Diff \| Splinter Review
Bug 31550: Add unit tests (1.88 KB, patch) 2023-08-11 14:27 UTC, Thibaud Guillot (thibaud_g)	Details \| Diff \| Splinter Review
Bug 31550: Add fields to get opac description on ILSDI web service (2.16 KB, patch) 2023-08-26 22:11 UTC, David Nind	Details \| Diff \| Splinter Review
Bug 31550: (follow-up) fix bareword issue (1.01 KB, patch) 2023-08-26 22:11 UTC, David Nind	Details \| Diff \| Splinter Review
Bug 31550: Add unit tests (1.93 KB, patch) 2023-08-26 22:11 UTC, David Nind	Details \| Diff \| Splinter Review
Show Obsolete (10) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Thibaud Guillot (thibaud_g) 2022-09-12 08:45:30 UTC

It may be useful to get the opac description on the ILSDI web service with a specific suffix for some item fields. In relation to bug 25650, this is to add 'ccode', 'permanent_location', 'notforloan', 'itemlost', 'withdrawn', 'damaged' and 'restricted'.

Comment 1 Thibaud Guillot (thibaud_g) 2022-09-12 09:28:39 UTC Comment hidden (obsolete)

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 2 Thibaud Guillot (thibaud_g) 2022-09-12 09:29:31 UTC Comment hidden (obsolete)

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

Comment 3 Thibaud Guillot (thibaud_g) 2022-09-12 09:30:43 UTC

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

Comment 4 David Nind 2022-09-13 19:12:29 UTC

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

Comment 5 Thibaud Guillot (thibaud_g) 2022-11-03 08:32:25 UTC

(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)

Comment 6 Thibaud Guillot (thibaud_g) 2023-01-02 09:48:11 UTC

Anyone get the same error than David ? So I tested right now and it's working for me

Comment 7 David Nind 2023-01-04 00:56:31 UTC

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

Comment 8 Thibaud Guillot (thibaud_g) 2023-01-04 10:32:47 UTC

Created attachment 144979 [details] [review]
Bug 31550: (follow-up) fix bareword issue

Comment 9 Thibaud Guillot (thibaud_g) 2023-01-04 10:33:22 UTC

(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

Comment 10 David Nind 2023-01-04 18:57:48 UTC

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>

Comment 11 David Nind 2023-01-04 18:57:53 UTC

Created attachment 145042 [details] [review]
Bug 31550: (follow-up) fix bareword issue

Signed-off-by: David Nind <david@davidnind.com>

Comment 12 David Nind 2023-01-04 18:58:57 UTC

(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.

Comment 13 Marcel de Rooy 2023-02-24 10:10:41 UTC

Please add unit tests.

Comment 14 Thibaud Guillot (thibaud_g) 2023-08-11 14:24:57 UTC

Created attachment 154370 [details] [review]
Bug 31550: (follow-up) fix bareword issue

Signed-off-by: David Nind <david@davidnind.com>

Comment 15 Thibaud Guillot (thibaud_g) 2023-08-11 14:25:04 UTC

Created attachment 154371 [details] [review]
Bug 31550: Add unit tests

Comment 16 Thibaud Guillot (thibaud_g) 2023-08-11 14:26:46 UTC

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>

Comment 17 Thibaud Guillot (thibaud_g) 2023-08-11 14:26:52 UTC

Created attachment 154373 [details] [review]
Bug 31550: (follow-up) fix bareword issue

Signed-off-by: David Nind <david@davidnind.com>

Comment 18 Thibaud Guillot (thibaud_g) 2023-08-11 14:27:03 UTC

Created attachment 154374 [details] [review]
Bug 31550: Add unit tests

Comment 19 David Nind 2023-08-26 22:11:16 UTC

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>

Comment 20 David Nind 2023-08-26 22:11:18 UTC

Created attachment 154859 [details] [review]
Bug 31550: (follow-up) fix bareword issue

Signed-off-by: David Nind <david@davidnind.com>

Comment 21 David Nind 2023-08-26 22:11:21 UTC

Created attachment 154860 [details] [review]
Bug 31550: Add unit tests

Signed-off-by: David Nind <david@davidnind.com>

Comment 22 David Nind 2023-08-26 22:22:26 UTC

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

Comment 23 Nick Clemens (kidclamp) 2023-10-12 16:07:40 UTC

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

Comment 24 Tomás Cohen Arazi (tcohen) 2023-10-12 16:39:03 UTC

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.