Bug 17624 - Add Staff Collection status for items in C4::XSLT
Summary: Add Staff Collection status for items in C4::XSLT
Status: CLOSED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-14 14:15 UTC by Marcel de Rooy
Modified: 2017-12-04 10:32 UTC (History)
3 users (show)

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


Attachments
Bug 17624: Hide not-for-loan items on search results' availability (9.03 KB, patch)
2016-11-16 12:46 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17624: [Follow-up] Consistency between Availability and Location (4.18 KB, patch)
2016-11-16 12:46 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17624: Separate XSLT item status for Staff Collection (3.98 KB, patch)
2016-11-30 14:42 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17624: [Follow-up] Consistency between Availability and Location (4.56 KB, patch)
2016-11-30 14:42 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2016-11-14 14:15:07 UTC

    
Comment 1 Marcel de Rooy 2016-11-16 12:46:29 UTC
Created attachment 57540 [details] [review]
Bug 17624: Hide not-for-loan items on search results' availability

Currently, not for loan items are listed as copies for reference on the
OPAC XSLT search results.
This reports add a preference to hide or show these not-for-loan items on
the OPAC XSLT results view. It is: OPACXSLTResultsHideNFL.
By default, it will still show them. So, no behavior change here.

Note: This patch also removed a double line in the pref file that resulted in
a small empty table row at the end of the Appearance section for OPAC
preferences.

Test plan:
[1] Run the dbrev via the web installer. Verify that the pref is still off.
    This means that not-for-loans should be listed on the Availability
    line as "Copies available for reference".
[2] Set the pref to Hide.
[3] Verify that the not-for-loans are not shown in the search results.
    Note that you still see them in the details view (no problem: they
    are listed just as what they are: not for loan).
[4] Set OpacItemLocation to Location. Look at the search results again.
Comment 2 Marcel de Rooy 2016-11-16 12:46:32 UTC
Created attachment 57541 [details] [review]
Bug 17624: [Follow-up] Consistency between Availability and Location

If OpacItemLocation is set to location, the opac results xslt shows
something like:
    Availability: Copies available for loan: Library(1).
                  Copies available for reference: Library(1).
    Location(s):  Location_1 callnumber_1

Note that the reference item is not listed under Location, since the logic
for that section is different. Simply stated: Available is created by two
separate if statements, but Location is one if-else statement.

This patch changes the Location logic to two if statements. In this
process some single choose/when's are replaced by simple if's. Actually,
the change is quite minimal.

Test plan:
[1] Set OPACXSLTResultsHideNFL to Show.
    Set OpacItemLocation to Location.
[2] Use a biblio with two items. Put data in callnumber and location.
    Mark one item as not for loan.
[3] Verify that you now see both items in the Location line of the
    search results.
Comment 3 Marcel de Rooy 2016-11-16 15:29:26 UTC
Hmm Still looking little bit at another approach
Comment 4 Marcel de Rooy 2016-11-25 12:46:12 UTC
What about hiding Staff collection only?
Comment 5 Josef Moravec 2016-11-25 14:18:33 UTC
(In reply to Marcel de Rooy from comment #4)
> What about hiding Staff collection only?

+1 

It does make more sense to me
Comment 6 Katrin Fischer 2016-11-26 13:59:49 UTC
I think we should not start hardcode special behaviour to configurable sample values. A lot of people use them differently or change them, which is something we never have discouraged. It would be safer to make this a configuration option, as we have in other cases.
Comment 7 Marcel de Rooy 2016-11-28 09:39:40 UTC
(In reply to Katrin Fischer from comment #6)
> I think we should not start hardcode special behaviour to configurable
> sample values. A lot of people use them differently or change them, which is
> something we never have discouraged. It would be safer to make this a
> configuration option, as we have in other cases.

I agree.
As you know, notforloan<0, notforloan==0 and notforloan>0 are tested already. It seems that we do not test ==1 though.
Comment 8 Katrin Fischer 2016-11-28 10:42:12 UTC
Hm, but all positive values show a specific behaviour already - you can't put a hold on them and you can't check them out without override. (notforloan>0)
Comment 9 Marcel de Rooy 2016-11-30 14:42:12 UTC
Created attachment 57833 [details] [review]
Bug 17624: Separate XSLT item status for Staff Collection

In order to differentiate a regular not for loan with a not-for-loan that
has the status "Staff Collection", this patch slightly adjusts the status
in C4::XSLT::buildKohaItemsNamespace. This status is used in the OPAC
results XSLT.

If you do not have items with notforloan>=2, there will be no change in
behavior.

If you use notforloan=2 and did not change its authorised value description,
a count for Staff Collection will be added in the Availability line. The
items with notforloan==2 will no longer be viewed as "for reference".

Higher values for notforloan will no longer be listed too as "for
reference". (Same for a modified notforloan==2.)

Note: I also considered using a plugin here or a specific preference. This
solution is by far the simplest. Currently, we are presenting the item
status in various places on various ways. A better solution would depend
on consolidating the item status code in Koha::Item.

Test plan:
[1] Pick a biblio with three items: one should be for loan, set the other
    two to Not for loan and Staff Collection.
[2] Look for this biblio on OPAC search results. Check Availability.
Comment 10 Marcel de Rooy 2016-11-30 14:42:16 UTC
Created attachment 57834 [details] [review]
Bug 17624: [Follow-up] Consistency between Availability and Location

If OpacItemLocation is set to location, the opac results xslt shows
something like:
    Availability: Copies available for loan: Library(1).
                  Copies available for reference: Library(1).
    Location(s):  Location_1 callnumber_1

Note that the reference item is not listed under Location, since the logic
for that section is different. Simply stated: Available is created by two
separate if statements, but Location is one if-else statement.

This patch changes the Location logic to two if statements. In this
process some single choose/when's are replaced by simple if's. Actually,
the change is quite minimal.

NOTE: A conditional <br> tag is removed from the XSLT between the reference
section and the status counts.

Test plan:
[1] Set OpacItemLocation to Location.
[2] Use a biblio with two items. Put data in callnumber and location.
    Mark one item as not for loan.
[3] Verify that you now see both items in the Location line of the
    search results.
Comment 11 Marcel de Rooy 2016-11-30 14:56:34 UTC
Apart from consolidating item status code, we also should take into account the relation between Not-for-loan and Use-restrictions on the item record when resolving this more globally.
Comment 12 Katrin Fischer 2016-11-30 15:11:31 UTC
I am no fan of this approach. I think hardcoding behaviour for numeric values will break behaviour for libraries. I am still in favor of this being a configuration option.
Comment 13 Marcel de Rooy 2016-11-30 15:17:47 UTC
(In reply to Katrin Fischer from comment #12)
> I am no fan of this approach. I think hardcoding behaviour for numeric
> values will break behaviour for libraries. I am still in favor of this being
> a configuration option.

I have spent hours on other options. I will leave it at this point for now.
Comment 14 Marcel de Rooy 2017-03-20 08:53:14 UTC
Blocking this patch until a marvel(l)ous idea comes up..