I do not see that preference, can you please tell me where to find it? 
Thanks

I believe you are supposed to add one yourself in the "local use" tab, but I am not sure where this patch is headed with that approach. Marcel, can you please explain? There seems to be no other linked bugs to take this further.

(In reply to Ella from comment #2)
> I do not see that preference, can you please tell me where to find it? 
> Thanks

Sorry, it is a local preference. Note the Local tab in the Preferences section under Administration.

(In reply to Katrin Fischer from comment #3)
> I believe you are supposed to add one yourself in the "local use" tab, but I
> am not sure where this patch is headed with that approach. Marcel, can you
> please explain? There seems to be no other linked bugs to take this further.

Depending on further feedback and consensus, it will be rather easy to promote this to an official preference.

@Nick: Would this address your concerns about hiding call numbers for certain unavailable categories?

(In reply to Marcel de Rooy from comment #11)
> We start here with Checked out until On hold.
> Not sure if we should do exactly the same for the remaining
> not for loan categories.

Note that the last patch now fills that gap.

Testing notes (using koha-testing-docker):

Revised test plan:

1. Setup records and items to test:
   . Have records with multiple items and call numbers (one with 20 items and one with 50) that will appear in the OPAC results using the same keyword (use 'Add multiple copies of this item' option at the bottom of the add item form to bulk add multiple items).
   . Make sure (using batch item editing) that there are a mix of libraries, not for loan categories, call numbers, and damaged status'
   . As a bonus, add some additional not for loan categories (Administration > Basic parameters > Authorized values), such as 'Not for Loan' and 'Staff Collection'
   . Check out some items to patrons
2. Confirm the current behavour when searching in the OPAC (see the example at the end):
   . The first call number per branch is displayed under availablility in the results (for items available for loan and items available for reference)
   . Where there are multiple call numbers for an item, "..." is used to indicate more call numbers
   . For items not available, such as damaged: no call numbers are displayed.
3. Apply the patch
4. Update the database (updatedatabase when using koha-testing-docker)
5. Run flush_memcached and restart_all (when using koha-testing-docker)
6. Without changing the default values for the new system preferences, check that the current behavour still works (as per step 2). Note: the default setting is to display 2 call numbers for items available (only 1 was displayed before) and 1 call number for items not available (no call numbers were displayed before).
7. Experiment with different values for the new system preferences (for example 10 for resultsMaxItems and 0 for resultsMaxItemsUnavailable):
   . resultsMaxItems - Maximum number of available items displayed in search results
   . resultsMaxItemsUnavailable - Maximum number of unavailable items displayed in search results
8. Confirm that the correct number of call numbers are displayed.
9. Sign off!

Example
-------

Before patch applied:
~~~~~~~~~~~~~~~~~~~~~

Availability: Items available for loan: Centerville (16) Call number: PERL A1, .., Fairview (1) Call number: PERL A1, Franklin (7) Call number: PERL B2, .., Midway (9) Call number: 560.897 ZAK, .., Springfield (6) Call number: 965.009 PLO, ... Items available for reference: Fairview Not For Loan (1) Call number: PERL A1, Franklin Not For Loan (1) Call number: PERL B2, Midway Not For Loan (2) Call number: 560.897 ZAK, .., Centerville Not For Loan (1) Call number: PERL B2, Springfield Not For Loan (3) Call number: 965.009 PLO, ... Not available: Damaged (7). 

Patch applied:
~~~~~~~~~~~~~~

Availability: Items available for loan: Centerville (16) Call number: PERL A1, PERL A1, .., Fairview (1) Call number: PERL A1, PERL A1, Franklin (7) Call number: PERL A1, PERL A1, .., Midway (9) Call number: PERL A1, PERL A1, .., Springfield (6) Call number: PERL A1, PERL A1, ... Items available for reference: Fairview Not For Loan (1) Call number: PERL A1, PERL B2, Franklin Not For Loan (1) Call number: PERL A1, PERL B2, Midway Not For Loan (2) Call number: PERL A1, PERL B2, Centerville Not For Loan (1) Call number: PERL A1, PERL B2, Springfield Not For Loan (3) Call number: PERL A1, PERL B2, ... Not available: Damaged (7) [ PERL A1, .. ]. 

Note differences:
- Default number of call numbers peated for items available for loan (before patch was applied they didn't), 
- Not available items didn't originally dispaly Call numbers for Damaged

Different values
~~~~~~~~~~~~~~~~

Results with:
- resultsMaxItems: 10
- resultsMaxItemsUnavailable: 0

Availability: Items available for loan: Centerville (16) Call number: PERL A1, PERL A1, PERL B2, PERL B2, PERL B2, PERL B2, PERL B2, PERL B2, PERL B2, 560.897 ZAK, .., Fairview (1) Call number: PERL A1, PERL A1, PERL B2, PERL B2, PERL B2, PERL B2, PERL B2, PERL B2, PERL B2, 560.897 ZAK, Franklin (7) Call number: PERL A1, PERL A1, PERL B2, PERL B2, PERL B2, PERL B2, PERL B2, PERL B2, PERL B2, 560.897 ZAK, Midway (9) Call number: PERL A1, PERL A1, PERL B2, PERL B2, PERL B2, PERL B2, PERL B2, PERL B2, PERL B2, 560.897 ZAK, Springfield (6) Call number: PERL A1, PERL A1, PERL B2, PERL B2, PERL B2, PERL B2, PERL B2, PERL B2, PERL B2, 560.897 ZAK. Items available for reference: Fairview Not For Loan (1) Call number: PERL A1, PERL B2, 560.897 ZAK, 560.897 ZAK, PERL B2, 965.009 PLO, 965.009 PLO, 965.009 PLO, Franklin Not For Loan (1) Call number: PERL A1, PERL B2, 560.897 ZAK, 560.897 ZAK, PERL B2, 965.009 PLO, 965.009 PLO, 965.009 PLO, Midway Not For Loan (2) Call number: PERL A1, PERL B2, 560.897 ZAK, 560.897 ZAK, PERL B2, 965.009 PLO, 965.009 PLO, 965.009 PLO, Centerville Not For Loan (1) Call number: PERL A1, PERL B2, 560.897 ZAK, 560.897 ZAK, PERL B2, 965.009 PLO, 965.009 PLO, 965.009 PLO, Springfield Not For Loan (3) Call number: PERL A1, PERL B2, 560.897 ZAK, 560.897 ZAK, PERL B2, 965.009 PLO, 965.009 PLO, 965.009 PLO. Not available: Damaged (7). 

Results with:
- resultsMaxItems: 1
- resultsMaxItemsUnavailable: 0

Availability: Items available for loan: Centerville (16) Call number: PERL A1, .., Fairview (1) Call number: PERL A1, Franklin (7) Call number: PERL A1, .., Midway (9) Call number: PERL A1, .., Springfield (6) Call number: PERL A1, ... Items available for reference: Fairview Not For Loan (1) Call number: PERL A1, Franklin Not For Loan (1) Call number: PERL A1, Midway Not For Loan (2) Call number: PERL A1, .., Centerville Not For Loan (1) Call number: PERL A1, Springfield Not For Loan (3) Call number: PERL A1, ... Not available: Damaged (7). 

Results with:
- resultsMaxItems: 0
- resultsMaxItemsUnavailable: 0

Availability: Items available for loan: Centerville (16) Call number: PERL A1, .., Franklin (7) Call number: PERL A1, .., Midway (9) Call number: PERL A1, .., Springfield (6) Call number: PERL A1, ... Items available for reference: Fairview Not For Loan (1) Call number: PERL A1, Franklin Not For Loan (1) Call number: PERL A1, Midway Not For Loan (2) Call number: PERL A1, .., Centerville Not For Loan (1) Call number: PERL A1, Springfield Not For Loan (3) Call number: PERL A1, ... Not available: Checked out (1). Damaged (7). 

Note: no change from setting to 1 and 0

A couple of things I noted:

1. Should the call number be repeated if it is the same? That is, should only distinct call numbers be displayed for items available by library branch or not available categories?

2. Should using 0 for resultsMaxItems suppress the display of call numbers? That is the behavour when resultsMaxItemsUnavailable is 0.

(In reply to David Nind from comment #24)
> A couple of things I noted:
> 
> 1. Should the call number be repeated if it is the same? That is, should
> only distinct call numbers be displayed for items available by library
> branch or not available categories?
> 
> 2. Should using 0 for resultsMaxItems suppress the display of call numbers?
> That is the behavour when resultsMaxItemsUnavailable is 0.

Thx for testing, David! Great comments. Will be looking further shortly.

David,
This is the culprit:
$sp ||= 1 if $syspref eq 'resultsMaxItems'; # needs default

A zero becomes a 1 here. We should probably test for defined here with //.

(In reply to David Nind from comment #24)
> 1. Should the call number be repeated if it is the same? That is, should
> only distinct call numbers be displayed for items available by library
> branch or not available categories?

This is far from trivially adding a distinct keyword. We are relying here on the output of the xslt key function. Making this change would require a bit more than we are doing here now to control the number displayed.

> 2. Should using 0 for resultsMaxItems suppress the display of call numbers?
> That is the behavour when resultsMaxItemsUnavailable is 0.

The follow-up now resolves this.

I am seeing the same callnumbers for every branch, to recreate:
1 - sudo koha-mysql kohadev
2 - UPDATE items SET itemcallnumber=barcode;
3 - Add 5 items for a single branch (IPT) with IPT1-5 as barcodes and "IPT" as item callnumber
4 - Display in opac:
Availability: Items available for loan: Midway (1) Call number: 39999000013351, 39999000013375, 39999000013399, Centerville (3) Call number: 39999000013351, 39999000013375, 39999000013399, Fairview (1) Call number: 39999000013351, 39999000013375, 39999000013399, Institut Protestant de Théologie (5) Call number: 39999000013351, 39999000013375, 39999000013399, ...

(In reply to Nick Clemens from comment #29)
> I am seeing the same callnumbers for every branch, to recreate:
> 1 - sudo koha-mysql kohadev
> 2 - UPDATE items SET itemcallnumber=barcode;
> 3 - Add 5 items for a single branch (IPT) with IPT1-5 as barcodes and "IPT"
> as item callnumber
> 4 - Display in opac:
> Availability: Items available for loan: Midway (1) Call number:
> 39999000013351, 39999000013375, 39999000013399, Centerville (3) Call number:
> 39999000013351, 39999000013375, 39999000013399, Fairview (1) Call number:
> 39999000013351, 39999000013375, 39999000013399, Institut Protestant de
> Théologie (5) Call number: 39999000013351, 39999000013375, 39999000013399,
> ...

Thx for testing, Nick. Will have another look

 .../en/xslt/MARC21slim2OPACResults.xsl        | 383 +++++++-----------
 1 file changed, 144 insertions(+), 239 deletions(-)

David, would you mind to test this change again?
Hopefully, Nick could pick it up after that :)

(In reply to Marcel de Rooy from comment #28)
> (In reply to David Nind from comment #24)
> > 1. Should the call number be repeated if it is the same? That is, should
> > only distinct call numbers be displayed for items available by library
> > branch or not available categories?
> 
> This is far from trivially adding a distinct keyword. We are relying here on
> the output of the xslt key function. Making this change would require a bit
> more than we are doing here now to control the number displayed.

Looking back, I must have misinterpreted this comment. ;)

Created attachment 123526 [details] [review]
Bug 26302: Add dbrev for two new prefs

Adding two prefs:
    resultsMaxItems
    resultsMaxItemsUnavailable

Test plan:
Check Preferences/OPAC/Appearance.

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

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Created attachment 123527 [details] [review]
Bug 26302: Add resultbranch and other status in C4/XSLT

Groundwork for changes in the OPAC results xslt.

NOTE: Adds both new prefs too.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Created attachment 123528 [details] [review]
Bug 26302: Changes for substatus, resultbranch in XSLT.t

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Created attachment 123529 [details] [review]
Bug 26302: Refactor Availability block in OPAC results xslt

Decided finally to do a larger refactor here. Hopefully we improved
consistency and removed some redundancy. And the two new prefs provide
additional functionality.

Most significant changes:
[1] Clearer distinction of the Availability line in three parts:
    Available, Reference and Unavailable.
    For Unavailable we loop thru branches now too.
[2] Calling template to list item data (including location or ccode).
    Made the separate Location line obsolete.
[3] The tests around OPACItemLibrary are removed since we now look at
    resultbranch from XSLT.pm.
[4] Removed code replication for various 'other' statuses like Checked out.
[5] Obsoleted three xslt key indexes, singleBranchMode.
[6] Apply the two prefs to control number of listed items.

Test plan:
You may play with: OPACResultsLibrary (home/holding), OPACItemLocation
(library, callno, location, ccode), resultsMaxItems[Unavailable] (numbers),
Reference_NFL_Statuses (list of notforloan codes in reference part).

[1] Create a biblio with various items on a few branches. Fill call number,
    location and ccode too. Set home branch and holdingbranch differently.
[2] Toggle the preferences, and verify display within OAPC search results.

Example with prefs (home, callnumber, 2, 2):
Availability: Items available for loan: Centerville (2)Call number: perl A 4, PERL D 1. Items available for reference: Fairfield: Not For Loan (1)Call number: PERL A 5. Not available: Centerville: Checked out (1)Call number: PERL A 3. Centerville: Ordered (1)Call number: PERL B 1. Centerville: Staff Collection (2)Call number: PERL A 2, PERL E 1. Centerville: Withdrawn (1)Call number: PERL B 2. Fairfield: Withdrawn (1)Call number: PERL C 1.

Same data with prefs (holding, callnumber, 2, 2):
Availability: Items available for loan: Centerville (1)Call number: PERL D 1. Liberty (1)Call number: perl A 4. Items available for reference: Centerville: Not For Loan (1)Call number: PERL A 5. Not available: Centerville: Checked out (1)Call number: PERL A 3. Centerville: Staff Collection (1)Call number: PERL A 2. Centerville: Withdrawn (1)Call number: PERL B 2. Fairfield: Ordered (1)Call number: PERL B 1. Fairfield: Withdrawn (1)Call number: PERL C 1. Liberty: Staff Collection (1)Call number: PERL E 1.

Same data with prefs (holding, library, n/a, n/a):
Availability: Items available for loan: Centerville (1). Liberty (1). Items available for reference: Centerville: Not For Loan (1). Not available: Centerville: Checked out (1). Centerville: Staff Collection (1). Centerville: Withdrawn (1). Fairfield: Ordered (1). Fairfield: Withdrawn (1). Liberty: Staff Collection (1).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Created attachment 123531 [details] [review]
Bug 26302: Add dbrev for two new prefs

Adding two prefs:
    resultsMaxItems
    resultsMaxItemsUnavailable

Test plan:
Check Preferences/OPAC/Appearance.

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

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

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

Created attachment 123532 [details] [review]
Bug 26302: Add resultbranch and other status in C4/XSLT

Groundwork for changes in the OPAC results xslt.

NOTE: Adds both new prefs too.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

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

Created attachment 123533 [details] [review]
Bug 26302: Changes for substatus, resultbranch in XSLT.t

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

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

Created attachment 123534 [details] [review]
Bug 26302: Refactor Availability block in OPAC results xslt

Decided finally to do a larger refactor here. Hopefully we improved
consistency and removed some redundancy. And the two new prefs provide
additional functionality.

Most significant changes:
[1] Clearer distinction of the Availability line in three parts:
    Available, Reference and Unavailable.
    For Unavailable we loop thru branches now too.
[2] Calling template to list item data (including location or ccode).
    Made the separate Location line obsolete.
[3] The tests around OPACItemLibrary are removed since we now look at
    resultbranch from XSLT.pm.
[4] Removed code replication for various 'other' statuses like Checked out.
[5] Obsoleted three xslt key indexes, singleBranchMode.
[6] Apply the two prefs to control number of listed items.

Test plan:
You may play with: OPACResultsLibrary (home/holding), OPACItemLocation
(library, callno, location, ccode), resultsMaxItems[Unavailable] (numbers),
Reference_NFL_Statuses (list of notforloan codes in reference part).

[1] Create a biblio with various items on a few branches. Fill call number,
    location and ccode too. Set home branch and holdingbranch differently.
[2] Toggle the preferences, and verify display within OAPC search results.

Example with prefs (home, callnumber, 2, 2):
Availability: Items available for loan: Centerville (2)Call number: perl A 4, PERL D 1. Items available for reference: Fairfield: Not For Loan (1)Call number: PERL A 5. Not available: Centerville: Checked out (1)Call number: PERL A 3. Centerville: Ordered (1)Call number: PERL B 1. Centerville: Staff Collection (2)Call number: PERL A 2, PERL E 1. Centerville: Withdrawn (1)Call number: PERL B 2. Fairfield: Withdrawn (1)Call number: PERL C 1.

Same data with prefs (holding, callnumber, 2, 2):
Availability: Items available for loan: Centerville (1)Call number: PERL D 1. Liberty (1)Call number: perl A 4. Items available for reference: Centerville: Not For Loan (1)Call number: PERL A 5. Not available: Centerville: Checked out (1)Call number: PERL A 3. Centerville: Staff Collection (1)Call number: PERL A 2. Centerville: Withdrawn (1)Call number: PERL B 2. Fairfield: Ordered (1)Call number: PERL B 1. Fairfield: Withdrawn (1)Call number: PERL C 1. Liberty: Staff Collection (1)Call number: PERL E 1.

Same data with prefs (holding, library, n/a, n/a):
Availability: Items available for loan: Centerville (1). Liberty (1). Items available for reference: Centerville: Not For Loan (1). Not available: Centerville: Checked out (1). Centerville: Staff Collection (1). Centerville: Withdrawn (1). Fairfield: Ordered (1). Fairfield: Withdrawn (1). Liberty: Staff Collection (1).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

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

> Signed-off-by: Owen Leonard <oleonard@myacpl.org>
> 
> Signed-off-by: David Nind <david@davidnind.com>

Great. Thx Owen and David !

Overall this looks good, a few things:
1 - We should default the prefs to 1,0 to more closely mimic the behavior before
2 - Items with blank callnumbers now show: [Call number: ], before they only showed the count
3 - Not blocker: it would be nice to have a '-1' option for resultsMaxItemsUnavailable to preserve previous grouping - in large consortia I know they have not wanted to display individual NFL items

Before:
Availability: Items available for loan: Fairview (2) , Midway (1) , Union (1) Call number: J Qam. Items available for reference: Riverside Not For Loan (2) Call number: J Qam, .., Pleasant Valley Staff Collection (1) Call number: YA 615 Qam, Fairfield Not For Loan (1) Call number: FIC Qam. Not available: Ordered (3). Checked out (1). 

After:
Availability: Items available for loan: Fairview (2)Call number: , . Midway (1)Call number: . Union (1)Call number: J Qam. Items available for reference: Fairfield: Not For Loan (1)Call number: FIC Qam. Pleasant Valley: Staff Collection (1)Call number: YA 615 Qam. Riverside: Not For Loan (2)Call number: J Qam, YA 667 Qam. Not available: Fairview: Checked out (1)Call number: . Franklin: Ordered (1)Call number: YA Qam. Midway: Ordered (1)Call number: J 301 Qam. Springfield: Ordered (1)Call number: FIC 354 Qam.

Created attachment 123640 [details] [review]
Bug 26302: (QA follow-up) Reset preference values

Mimic current behavior.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Created attachment 123641 [details] [review]
Bug 26302: (QA follow-up) Prevent empty call numbers

This does not make the code easier, but here we go:

Suppose resultsMaxItems == 2.
If we would normally list two call numbers like:
    Liberty (2) [Call number: PERL F 1, PERL F 2].
If one call number would be empty, we now list:
    Liberty (2) [Call number: PERL F 1, ...].
And when both are empty, we only show a number:
    Liberty (2).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

(In reply to Nick Clemens from comment #47)

Thx

> 1 - We should default the prefs to 1,0 to more closely mimic the behavior
> before

Fixed in patch/follow-up 5

> 2 - Items with blank callnumbers now show: [Call number: ], before they only
> showed the count

Fixed in patch/follow-up 6

> 3 - Not blocker: it would be nice to have a '-1' option for
> resultsMaxItemsUnavailable to preserve previous grouping - in large
> consortia I know they have not wanted to display individual NFL items

Yeah can imagine. Willing to add it on a new report. Not here anymore.
Bug 28831

Created attachment 123643 [details] [review]
Bug 26302: (QA follow-up) Prevent empty call numbers

This does not make the code easier, but here we go:

Suppose resultsMaxItems == 2.
If we would normally list two call numbers like:
    Liberty (2) [Call number: PERL F 1, PERL F 2].
If one call number would be empty, we now list:
    Liberty (2) [Call number: PERL F 1, ...].
And when both are empty, we only show a number:
    Liberty (2).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Created attachment 123644 [details] [review]
Bug 26302: Add dbrev for two new prefs

Adding two prefs:
    resultsMaxItems
    resultsMaxItemsUnavailable

Test plan:
Check Preferences/OPAC/Appearance.

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

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Created attachment 123645 [details] [review]
Bug 26302: Add resultbranch and other status in C4/XSLT

Groundwork for changes in the OPAC results xslt.

NOTE: Adds both new prefs too.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Created attachment 123646 [details] [review]
Bug 26302: Changes for substatus, resultbranch in XSLT.t

Test plan:
Run t/db_dependent/XSLT.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Created attachment 123647 [details] [review]
Bug 26302: Refactor Availability block in OPAC results xslt

Decided finally to do a larger refactor here. Hopefully we improved
consistency and removed some redundancy. And the two new prefs provide
additional functionality.

Most significant changes:
[1] Clearer distinction of the Availability line in three parts:
    Available, Reference and Unavailable.
    For Unavailable we loop thru branches now too.
[2] Calling template to list item data (including location or ccode).
    Made the separate Location line obsolete.
[3] The tests around OPACItemLibrary are removed since we now look at
    resultbranch from XSLT.pm.
[4] Removed code replication for various 'other' statuses like Checked out.
[5] Obsoleted three xslt key indexes, singleBranchMode.
[6] Apply the two prefs to control number of listed items.

Test plan:
You may play with: OPACResultsLibrary (home/holding), OPACItemLocation
(library, callno, location, ccode), resultsMaxItems[Unavailable] (numbers),
Reference_NFL_Statuses (list of notforloan codes in reference part).

[1] Create a biblio with various items on a few branches. Fill call number,
    location and ccode too. Set home branch and holdingbranch differently.
[2] Toggle the preferences, and verify display within OAPC search results.

Example with prefs (home, callnumber, 2, 2):
Availability: Items available for loan: Centerville (2)Call number: perl A 4, PERL D 1. Items available for reference: Fairfield: Not For Loan (1)Call number: PERL A 5. Not available: Centerville: Checked out (1)Call number: PERL A 3. Centerville: Ordered (1)Call number: PERL B 1. Centerville: Staff Collection (2)Call number: PERL A 2, PERL E 1. Centerville: Withdrawn (1)Call number: PERL B 2. Fairfield: Withdrawn (1)Call number: PERL C 1.

Same data with prefs (holding, callnumber, 2, 2):
Availability: Items available for loan: Centerville (1)Call number: PERL D 1. Liberty (1)Call number: perl A 4. Items available for reference: Centerville: Not For Loan (1)Call number: PERL A 5. Not available: Centerville: Checked out (1)Call number: PERL A 3. Centerville: Staff Collection (1)Call number: PERL A 2. Centerville: Withdrawn (1)Call number: PERL B 2. Fairfield: Ordered (1)Call number: PERL B 1. Fairfield: Withdrawn (1)Call number: PERL C 1. Liberty: Staff Collection (1)Call number: PERL E 1.

Same data with prefs (holding, library, n/a, n/a):
Availability: Items available for loan: Centerville (1). Liberty (1). Items available for reference: Centerville: Not For Loan (1). Not available: Centerville: Checked out (1). Centerville: Staff Collection (1). Centerville: Withdrawn (1). Fairfield: Ordered (1). Fairfield: Withdrawn (1). Liberty: Staff Collection (1).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Created attachment 123648 [details] [review]
Bug 26302: (QA follow-up) Reset preference values

Mimic current behavior.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Created attachment 123649 [details] [review]
Bug 26302: (QA follow-up) Prevent empty call numbers

This does not make the code easier, but here we go:

Suppose resultsMaxItems == 2.
If we would normally list two call numbers like:
    Liberty (2) [Call number: PERL F 1, PERL F 2].
If one call number would be empty, we now list:
    Liberty (2) [Call number: PERL F 1, ...].
And when both are empty, we only show a number:
    Liberty (2).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Created attachment 124243 [details] [review]
Bug 26302: Make syspref's names start with uppercase

1. Do we really need status=>other to be passed?

2. Shouldn't the pref's names contain "OPAC"?

3. One test is failing

    #   Failed test 'All records have at least one item available'
    #   at t/db_dependent/Search.t line 500.
    #          got: 'false'
    #     expected: 'true'

(In reply to Jonathan Druart from comment #59)
> 3. One test is failing
> 
>     #   Failed test 'All records have at least one item available'
>     #   at t/db_dependent/Search.t line 500.
>     #          got: 'false'
>     #     expected: 'true'

I cannot replicate this after a reset_all..

(In reply to Jonathan Druart from comment #59)
> 1. Do we really need status=>other to be passed?

Yes, this is part of the change. An attempt to not pass it, may probably result in more obfuscated xslt code.

> 2. Shouldn't the pref's names contain "OPAC"?

Good point. They strictly relate to OPAC now. Will add a follow-up for it.

(In reply to Jonathan Druart from comment #60)
> (In reply to Jonathan Druart from comment #59)
> > 3. One test is failing
> > 
> >     #   Failed test 'All records have at least one item available'
> >     #   at t/db_dependent/Search.t line 500.
> >     #          got: 'false'
> >     #     expected: 'true'
> 
> I cannot replicate this after a reset_all..

Passes with me too.

Created attachment 124244 [details] [review]
Bug 26302: Make syspref's names start with uppercase

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Created attachment 124245 [details] [review]
Bug 26302: (QA follow-up) Prefix prefs with OPAC

git grep -l "ResultsMaxItems" | xargs sed -i -e "/ResultsMaxItems/ s/ResultsMaxItems/OPACResultsMaxItems/g"

And line:
koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slim2OPACResults.xsl:    <xsl:variable name="OPACResultsMaxItems" select="number(marc:sysprefs/marc:syspref[@name='resultsMaxItems']+0)"/>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

-    <xsl:variable name="resultsMaxItems" select="number(marc:sysprefs/marc:syspref[@name='resultsMaxItems']+0)"/>
-    <xsl:variable name="resultsMaxItemsUnavailable" select="number(marc:sysprefs/marc:syspref[@name='resultsMaxItemsUnavailable']+0)"/>
+    <xsl:variable name="ResultsMaxItems" select="number(marc:sysprefs/marc:syspref[@name='resultsMaxItems']+0)"/>
+    <xsl:variable name="ResultsMaxItemsUnavailable" select="number(marc:sysprefs/marc:syspref[@name='ResultsMaxItemsUnavailable']+0)"/>

Why did you miss the second occurrence on the first change and not on the second ?

(In reply to Marcel de Rooy from comment #65)
> -    <xsl:variable name="resultsMaxItems"
> select="number(marc:sysprefs/marc:syspref[@name='resultsMaxItems']+0)"/>
> -    <xsl:variable name="resultsMaxItemsUnavailable"
> select="number(marc:sysprefs/marc:
> syspref[@name='resultsMaxItemsUnavailable']+0)"/>
> +    <xsl:variable name="ResultsMaxItems"
> select="number(marc:sysprefs/marc:syspref[@name='resultsMaxItems']+0)"/>
> +    <xsl:variable name="ResultsMaxItemsUnavailable"
> select="number(marc:sysprefs/marc:
> syspref[@name='ResultsMaxItemsUnavailable']+0)"/>
> 
> Why did you miss the second occurrence on the first change and not on the
> second ?

Oops, forgot 'g' in my sed command.

Pushed to master for 21.11, thanks to everybody involved!

Buuuuuuuut, we also need this change in UNIMARC XSLT files right ?
And what about custom files ?

(In reply to Fridolin Somers from comment #68)
> Buuuuuuuut, we also need this change in UNIMARC XSLT files right ?
> And what about custom files ?

They have been following different paths for long time already.
Custom files obviously are not in scope here.