Bug 30636 - ILS-DI shows incorrect availability when not for loan by item type
Summary: ILS-DI shows incorrect availability when not for loan by item type
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Web services (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: David Cook
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-04-28 01:42 UTC by David Cook
Modified: 2023-12-28 20:42 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes the ILS-DI web service so that requests where item types are not for loan are correctly returned, the same as for the OPAC. Currently, the item not for loan status is ignored and records are shown as available.
Version(s) released in:
22.11.00


Attachments
Bug 30636: Show notforloan availability correctly in ILS-DI (3.96 KB, patch)
2022-04-28 05:50 UTC, David Cook
Details | Diff | Splinter Review
Bug 30636: Show notforloan availability correctly in ILS-DI (3.97 KB, patch)
2022-04-28 05:51 UTC, David Cook
Details | Diff | Splinter Review
Bug 30636: Show notforloan availability correctly in ILS-DI (4.02 KB, patch)
2022-05-04 03:24 UTC, David Nind
Details | Diff | Splinter Review
Bug 30636: Show notforloan availability correctly in ILS-DI (4.05 KB, patch)
2022-06-06 13:52 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2022-04-28 01:42:47 UTC
If you've marked an item type as "Not for loan" in Koha, it'll show a "Status" of "Not for loan" in the catalogue, but the ILS-DI GetAvailability service will advertise it as still available.
Comment 1 David Cook 2022-04-28 04:40:16 UTC
Unfortunately, detail.tt uses item.notforloan_per_itemtype which is a column alias from a custom SQL query in C4/Items.pm. 

In theory, we could just add a Koha::Item->notforloan_per_itemtype method, but then it seems like Koha::Item->notforloan is often used as a boolean to determine if an item is not for loan... and sometimes Koha::Item->notforloan is used for its particular "Not for loan" description. 

It seems to me that we might be better off creating a new method like Koha::Item->is_not_for_loan() which returns a boolean response based on the "notforloan" value and whether it's "Not for loan" based on its item type.
Comment 2 David Cook 2022-04-28 05:50:35 UTC
Created attachment 134200 [details] [review]
Bug 30636: Show notforloan availability correctly in ILS-DI

This patch adds the Koha::Item->is_notforloan() method which checks
the item status of "notforloan" and it checks the effective item type
to see if it's marked as "notforloan". This change brings ILS-DI in line
with the catalogue which displays "Not for loan" for both those scenarios.

0. Don't apply patch yet
1. Enable ILS-DI system preference
http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=ILS-DI
2. Change Item types to "Not for Loan" for Books
http://localhost:8081/cgi-bin/koha/admin/itemtypes.pl?op=add_form&itemtype=BK
3. Note that book is listed as "Not for loan" in catalogue
http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=29&searchid=scs_1651115075364
4. Note that book is listed as "available" via ILS-DI
http://localhost:8080/cgi-bin/koha/ilsdi.pl?service=GetAvailability&id=29&id_type=bib

5. Apply patch and restart starman (ie koha-plack --restart kohadev)

6. Note that book is listed as "not available" and "Not for loan" via ILS-DI
http://localhost:8080/cgi-bin/koha/ilsdi.pl?service=GetAvailability&id=29&id_type=bib

--

7. prove t/db_dependent/Koha/Item.t
Comment 3 David Cook 2022-04-28 05:51:28 UTC
Created attachment 134201 [details] [review]
Bug 30636: Show notforloan availability correctly in ILS-DI

This patch adds the Koha::Item->is_notforloan() method which checks
the item status of "notforloan" and it checks the effective item type
to see if it's marked as "notforloan". This change brings ILS-DI in line
with the catalogue which displays "Not for loan" for both those scenarios.

0. Don't apply patch yet
1. Enable ILS-DI system preference
http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=ILS-DI
2. Change Item types to "Not for Loan" for Books
http://localhost:8081/cgi-bin/koha/admin/itemtypes.pl?op=add_form&itemtype=BK
3. Note that book is listed as "Not for loan" in catalogue
http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=29&searchid=scs_1651115075364
4. Note that book is listed as "available" via ILS-DI
http://localhost:8080/cgi-bin/koha/ilsdi.pl?service=GetAvailability&id=29&id_type=bib

5. Apply patch and restart starman (ie koha-plack --restart kohadev)

6. Note that book is listed as "not available" and "Not for loan" via ILS-DI
http://localhost:8080/cgi-bin/koha/ilsdi.pl?service=GetAvailability&id=29&id_type=bib

--

7. prove t/db_dependent/Koha/Item.t
Comment 4 David Nind 2022-05-04 03:24:07 UTC
Created attachment 134566 [details] [review]
Bug 30636: Show notforloan availability correctly in ILS-DI

This patch adds the Koha::Item->is_notforloan() method which checks
the item status of "notforloan" and it checks the effective item type
to see if it's marked as "notforloan". This change brings ILS-DI in line
with the catalogue which displays "Not for loan" for both those scenarios.

0. Don't apply patch yet
1. Enable ILS-DI system preference
http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=ILS-DI
2. Change Item types to "Not for Loan" for Books
http://localhost:8081/cgi-bin/koha/admin/itemtypes.pl?op=add_form&itemtype=BK
3. Note that book is listed as "Not for loan" in catalogue
http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=29&searchid=scs_1651115075364
4. Note that book is listed as "available" via ILS-DI
http://localhost:8080/cgi-bin/koha/ilsdi.pl?service=GetAvailability&id=29&id_type=bib

5. Apply patch and restart starman (ie koha-plack --restart kohadev)

6. Note that book is listed as "not available" and "Not for loan" via ILS-DI
http://localhost:8080/cgi-bin/koha/ilsdi.pl?service=GetAvailability&id=29&id_type=bib

--

7. prove t/db_dependent/Koha/Item.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 Katrin Fischer 2022-05-20 09:13:18 UTC
Looking here.
Comment 6 Katrin Fischer 2022-05-20 09:26:31 UTC
I believe the new is_notforloan is not complete as is:

* You check for the notforloan status in the item
* You check for the notforloan of the itemtype of the item

But: I think you need to use effective_itemtype here to catch itemtype on record level as well.
Comment 7 David Cook 2022-05-23 00:16:33 UTC
(In reply to Katrin Fischer from comment #6)
> I believe the new is_notforloan is not complete as is:
> 
> * You check for the notforloan status in the item
> * You check for the notforloan of the itemtype of the item
> 
> But: I think you need to use effective_itemtype here to catch itemtype on
> record level as well.

Nope. That's automagically handled by '$self->itemtype;'. See below:

=head3 itemtype

    my $itemtype = $item->itemtype;

    Returns Koha object for effective itemtype

=cut

sub itemtype {
    my ( $self ) = @_;
    return Koha::ItemTypes->find( $self->effective_itemtype );
}
Comment 8 Katrin Fischer 2022-06-06 13:46:18 UTC
(In reply to David Cook from comment #7)
> (In reply to Katrin Fischer from comment #6)
> > I believe the new is_notforloan is not complete as is:
> > 
> > * You check for the notforloan status in the item
> > * You check for the notforloan of the itemtype of the item
> > 
> > But: I think you need to use effective_itemtype here to catch itemtype on
> > record level as well.
> 
> Nope. That's automagically handled by '$self->itemtype;'. See below:
> 
> =head3 itemtype
> 
>     my $itemtype = $item->itemtype;
> 
>     Returns Koha object for effective itemtype
> 
> =cut
> 
> sub itemtype {
>     my ( $self ) = @_;
>     return Koha::ItemTypes->find( $self->effective_itemtype );
> }

Ah, I was looking for this, but still missed it. Thx for clarifying!
Comment 9 Katrin Fischer 2022-06-06 13:52:20 UTC
Created attachment 135729 [details] [review]
Bug 30636: Show notforloan availability correctly in ILS-DI

This patch adds the Koha::Item->is_notforloan() method which checks
the item status of "notforloan" and it checks the effective item type
to see if it's marked as "notforloan". This change brings ILS-DI in line
with the catalogue which displays "Not for loan" for both those scenarios.

0. Don't apply patch yet
1. Enable ILS-DI system preference
http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=ILS-DI
2. Change Item types to "Not for Loan" for Books
http://localhost:8081/cgi-bin/koha/admin/itemtypes.pl?op=add_form&itemtype=BK
3. Note that book is listed as "Not for loan" in catalogue
http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=29&searchid=scs_1651115075364
4. Note that book is listed as "available" via ILS-DI
http://localhost:8080/cgi-bin/koha/ilsdi.pl?service=GetAvailability&id=29&id_type=bib

5. Apply patch and restart starman (ie koha-plack --restart kohadev)

6. Note that book is listed as "not available" and "Not for loan" via ILS-DI
http://localhost:8080/cgi-bin/koha/ilsdi.pl?service=GetAvailability&id=29&id_type=bib

--

7. prove t/db_dependent/Koha/Item.t

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Tomás Cohen Arazi 2022-06-06 16:33:58 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!