Bug 21833 - illrequests.medium is not displayed on ILL request detail page and ILL requests table
Summary: illrequests.medium is not displayed on ILL request detail page and ILL reques...
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Bugs List
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-11-14 13:24 UTC by Magnus Enger
Modified: 2023-06-09 09:38 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Display illrequests.medium as Request type (2.75 KB, patch)
2018-11-14 13:40 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 21833: Display illrequests.medium as Request type (2.78 KB, patch)
2018-11-21 18:30 UTC, Owen Leonard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Magnus Enger 2018-11-14 13:24:28 UTC
Request type is saved as illrequests.medium in the databse. This is displayed in two places: 

1. Detail ILL request view
/cgi-bin/koha/ill/ill-requests.pl?method=illview&illrequest_id=553

2. Edit request view
/cgi-bin/koha/ill/ill-requests.pl?method=edit_action&illrequest_id=553

But the value from illrequests.medium is never displayed, just N/A.
Comment 1 Magnus Enger 2018-11-14 13:40:12 UTC
Created attachment 82331 [details] [review]
Display illrequests.medium as Request type

To test:
- Make sure you have an ILL request with a value in illrequests.medium
- View the request in Detail view and Edit view, "N/A" is displayed for
  the Request type
- Apply the patch
- View the request in Detail and Edit view again, the value from
  illrequests.medium is now displayed as the Request type

This patch also brings the coding around illrequests.medium more in
line with the coding of similar attributes.
Comment 2 Owen Leonard 2018-11-21 18:30:20 UTC
Created attachment 82573 [details] [review]
Bug 21833: Display illrequests.medium as Request type

To test:
- Make sure you have an ILL request with a value in illrequests.medium
- View the request in Detail view and Edit view, "N/A" is displayed for
  the Request type
- Apply the patch
- View the request in Detail and Edit view again, the value from
  illrequests.medium is now displayed as the Request type

This patch also brings the coding around illrequests.medium more in
line with the coding of similar attributes.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 3 Marcel de Rooy 2018-11-23 07:13:07 UTC
Andrew,
Could you please confirm this (trivial) change?
Not sure if material type and medium are the same in ILL..

Marcel
Comment 4 Andrew Isherwood 2018-11-23 08:53:44 UTC
I believe this is a non-issue. Bug 20941 switched away from displaying medium in the UI since we seemingly don't use it any more, we store the material type as an illrequestattribute (the get_type function was introduced to retrieve the item's type), so I think any patches related to medium are no longer needed.

Backends will need to be adapted to display the "type" illrequestattribute. I mentioned it to Magnus as I believe he's the only other backend dev.
Comment 5 Magnus Enger 2019-05-16 13:19:27 UTC
True, the code now expects to find the value for "Request type" as a type=type in the illrequestattributes table. But the illrequests.medium field hs not been removed from the table structure. 

Personally I would expect to find the "Request type" in the illrequest-table, not as an attrribute. But that might be bias from the ILL systems I have been working with. 

Also, I think it would make sense to have both a "Request type" (Loan/Article copy) and a "Material type" (Book/DVD/microfiche).
Comment 6 Katrin Fischer 2023-01-13 10:33:08 UTC
I think there were changes after this was filed that changed things a bit:

Request type: This is now pulled from illrequestattributes.type for the big table and on the detail pages.

So what we need to do now is bring back the display of illrequests.medium, because it is no longer displayed at all.
Comment 7 Katrin Fischer 2023-01-13 10:33:32 UTC
Magnus, do you want this one to remain assigned to you?
Comment 8 Magnus Enger 2023-01-13 12:17:33 UTC
Unassigning.
Comment 9 Pedro Amorim 2023-06-06 09:22:43 UTC
Is removing the "medium" column an option? 
It's not being used anywhere as far as I can tell.
Comment 10 Katrin Fischer 2023-06-06 09:50:34 UTC
Older versions of backends might use it, it's hard to tell. We might need to have some kind of deprecation process to be on the save side.
Comment 11 Pedro Amorim 2023-06-06 11:17:42 UTC
I like the deprecation idea, I think having stuff in the database/code that is no longer used is bad for maintainability.
The core code does not check/query for illrequests.medium as far as my searching goes, so I think that really only leaves us with the possibility of backends using it.
Comment 12 Magnus Enger 2023-06-06 12:14:30 UTC
The Swedish "Libris backend" (https://github.com/Libriotech/koha-illbackend-libris) uses both illrequests.medium and illrequestattributes.type=type to store if a request is a loan or a copy. If the attribute is what is displayed I'm pretty sure we can do without illrequests.medium.
Comment 13 Katrin Fischer 2023-06-08 21:39:11 UTC
Our own backend has quite a bit of code using the medium, but it might date back to the type when it was still used. It would certainly need some work to clean-up. 

What could a deprecation process look like? How and where do we anounce it and how can we make it the most visible?
Comment 14 Pedro Amorim 2023-06-09 09:24:49 UTC
(In reply to Katrin Fischer from comment #13)
> Our own backend has quite a bit of code using the medium, but it might date
> back to the type when it was still used. It would certainly need some work
> to clean-up. 
> 
> What could a deprecation process look like? How and where do we anounce it
> and how can we make it the most visible?

Katrin, very fair points.
I think you're bringing up a bigger issue. Maybe we should consider a koha-ill mailing list, a koha-ill IRC chat, or something similar where these sort of topics could be discussed.
As it stands, there's no way of telling how many backends exist and what they're using.

Furthermore, we're also missing proper documentation on every backend capability that core checks for, and what their purpose is (e.g. "should_display_availability", "unmediated_ill", or "create_api" (this last one is only introduced in bug 30719). These are useful to know as a backend developer, because they effectively function as 'hooks' for core functionality that backends can make use of.

I believe we need a place where every ILL invested individual/organization can give their input on - and become aware of - updates and changes.
Comment 15 Katrin Fischer 2023-06-09 09:38:56 UTC
I like the "ILL invested individual/organization". :) I think a mailing list could be an option, but there is a certain danger of people missing to subscribe to it. Maybe we could keep it on the main lists, but have some clear tag or so? And add to a section in release notes?