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
: P5 - low enhancement (vote)
Assignee: Thibaud Guillot
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-09-12 08:45 UTC by Thibaud Guillot
Modified: 2023-10-12 16:39 UTC (History)
4 users (show)

See Also:
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:


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
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
Details | Diff | Splinter Review
Bug 31550: (follow-up) fix bareword issue (988 bytes, patch)
2023-01-04 10:32 UTC, Thibaud Guillot
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
Details | Diff | Splinter Review
Bug 31550: Add unit tests (1.88 KB, patch)
2023-08-11 14:25 UTC, Thibaud Guillot
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
Details | Diff | Splinter Review
Bug 31550: (follow-up) fix bareword issue (1.01 KB, patch)
2023-08-11 14:26 UTC, Thibaud Guillot
Details | Diff | Splinter Review
Bug 31550: Add unit tests (1.88 KB, patch)
2023-08-11 14:27 UTC, Thibaud Guillot
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

Note You need to log in before you can comment on or make changes to this bug.
Description Thibaud Guillot 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 2022-09-12 09:28:39 UTC Comment hidden (obsolete)
Comment 2 Thibaud Guillot 2022-09-12 09:29:31 UTC Comment hidden (obsolete)
Comment 3 Thibaud Guillot 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 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 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 2023-01-04 10:32:47 UTC
Created attachment 144979 [details] [review]
Bug 31550: (follow-up) fix bareword issue
Comment 9 Thibaud Guillot 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 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 2023-08-11 14:25:04 UTC
Created attachment 154371 [details] [review]
Bug 31550: Add unit tests
Comment 16 Thibaud Guillot 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 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 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 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.