It looks like this is already in Koha, but it checks the singleBranchMode, rather than counting the number of branches. So, if singleBranchMode is enabled, the branchcode limiter disappears, but a location limiter appears instead.

(In reply to comment #2)
It looks like I may be wrong. I'll do some more testing.

> It looks like this is already in Koha, but it checks the singleBranchMode,
> rather than counting the number of branches. So, if singleBranchMode is
> enabled, the branchcode limiter disappears, but a location limiter appears
> instead.

(In reply to comment #4)

I should also note that it didn't apply in its original state, but minor adjustments were all that were required to make it work.


> Created attachment 7700 [details] [review]
> Bug 7401 - Shelving Location facet instead of Branch facet when only 1 branch
> configured
> 
> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
> 
> Did some minor re-working so it uses the singleBranchMode
> system pref, instead of counting the number or branches.

Kyle,


SingleBranchMode is buggy, and should be phased out in favour of branch counting (in my opinion).  It seems unreasonable to me to have a system preference toggle when the number of configured branches would give you that information.  Turning on SingleBranchMode in a multi-branch context is just begging for problems.  Hence my usage of branch count instead of the system preference.

To the greater community:  opinions or thoughts on this?  I think we'd eliminate a fair number of problems by phasing SingleBranchMode out, but perhaps some of you know of a worthwhile use-case for keeping it (and keeping it in all it's currently used contexts).

Comment on attachment 7040 [details] [review]
Proposed Patch

Un-obsoleting patch due to debate between SingleBranchMode and counting @branchloop.

(In reply to comment #6)
Here are my thoughts: If you use branch counting instead of SingleBranchMode, then it limits the ability to use this. If a group if libraries use SingleBranchMode, why should they not be able to see the shelving facet? 

I understand the reasoning behind only using it for single libraries ( e.g. We don't want to show locations that don't exist in this library ). So, if SingleBranchMode is indeed to be deprecated, then you are right, and this should go back to branch counting.

As far as I can tell, SingleBranchMode is only used in two places, once in Search.pm and twice in Koha.pm, all instances involve facets.

> Kyle,
> 
> 
> SingleBranchMode is buggy, and should be phased out in favour of branch
> counting (in my opinion).  It seems unreasonable to me to have a system
> preference toggle when the number of configured branches would give you that
> information.  Turning on SingleBranchMode in a multi-branch context is just
> begging for problems.  Hence my usage of branch count instead of the system
> preference.
> 
> To the greater community:  opinions or thoughts on this?  I think we'd
> eliminate a fair number of problems by phasing SingleBranchMode out, but
> perhaps some of you know of a worthwhile use-case for keeping it (and keeping
> it in all it's currently used contexts).

Perhaps we should delete the SingleBranchMode system pref, and have C4::Context set it to enabled if the branch count is 1? That way we don't have to go counting the branches each time with GetBranchesLoop().

(In reply to comment #9)

I meant C4::Auth, not C4::Context.

> Perhaps we should delete the SingleBranchMode system pref, and have C4::Context
> set it to enabled if the branch count is 1? That way we don't have to go
> counting the branches each time with GetBranchesLoop().

QA comment:
* clean code
* I agree with the idea of removing SingleBranch mode, and the GetBranchCount is a 1st step in this direction
* a question though = why not having location facet even with multiple branches ?

marking passed QA, will push later, once i've the answer to question 3

(In reply to comment #14)

> * a question though = why not having location facet even with multiple
> branches ?

Reasoning is that a given location may not be used by all the libraries.

sorry, but this patch does not apply correctly anymore, because of bug 3216, applied 2 weeks ago.
The bug 3216 changes the content of the array, from:
my $library_facet = {
   idx   => 'branch',
   label => 'Libraries',
   tags  => [ qw/ 995b / ],
   expanded => '1',
};

to
$library_facet = {
 link_value  => 'branch',
 label_value => 'Libraries',
 tags        => [ '995', ],
 subfield    => 'b',
 expanded    => '1',
};


solving this conflict is not trivial, I won't do it myself, sorry for the annoyance to do it again

Comment on attachment 10154 [details] [review]
Bug 7401 - Shelving Location facet instead of Branch facet when only 1 branch configured

patch obsoleted, keeping the other signed-off one

QA comment: some concern already expressed and solved, passed QA

sorry, but does not apply once again. Please rebase and switch to passed QA again, and I'll take care of it promptly !

Epic fail :\
Global symbol "$itemtypes" requires explicit package name at /home/paul/koha.dev/koha-community/C4/Search.pm line 561.
Global symbol "$itemtypes" requires explicit package name at /home/paul/koha.dev/koha-community/C4/Search.pm line 561.
Global symbol "$itemtypes" requires explicit package name at /home/paul/koha.dev/koha-community/C4/Search.pm line 562.
Global symbol "$itemtypes" requires explicit package name at /home/paul/koha.dev/koha-community/C4/Search.pm line 563.
Global symbol "$itemtypes" requires explicit package name at /home/paul/koha.dev/koha-community/C4/Search.pm line 566.
BEGIN not safe after errors--compilation aborted at /home/paul/koha.dev/koha-community/C4/Search.pm line 1798.
Compilation failed in require at /home/paul/koha.dev/koha-community/opac/opac-search.pl line 47.
BEGIN failed--compilation aborted at /home/paul/koha.dev/koha-community/opac/opac-search.pl line 47.

My bad. I took a simple rebase for granted, and missed the fact that an additional variable and been added to GetRecords!

Testing this before pushing, I can't have it working:
 * setup on UNIMARC
 * I realized that there is a mistake for location. in Unimarc settings, the proper field is 995$e, not 995$c. I've changed Koha.pm, but it does not change anything
 * I've set SingleBranch=ON (I've 10+ branches on my setup)
 * when I do a search, I see no "Location" facet on OPAC or on Staff interface
 * in my database, there are locations :
portable lecannetdesmaures>SELECT location,count(*) FROM items GROUP BY location;
+--------------------------+----------+
| location                 | count(*) |
+--------------------------+----------+
| NULL                     |      289 |
|                          |       18 |
| Ados                     |     1063 |
| Album                    |      933 |
| BD adulte                |     1610 |
| BD enfant                |      265 |
| Bureau                   |       15 |
| Cédéroms adulte          |       95 |
| Cédéroms enfant          |        2 |
| Contes                   |      348 |
| Doc adultes              |        3 |
| Doc jeunesse             |        5 |
...

Should I do something else to see the facets ?

Here is an updated patch. This version is modified enough from the original that I think it should be signed-off on again.

Created attachment 12241 [details]
Shelving location facet showing "*" link

Is it to be expected that one of the facets is "*"--representing items with no shelving location I assume? Or is there a problem with my data? I don't think narrowing a search to items without shelving location is useful to patrons.

(In reply to comment #29)
> Created attachment 12241 [details]
> Shelving location facet showing "*" link
> 
> Is it to be expected that one of the facets is "*"--representing items with
> no shelving location I assume? Or is there a problem with my data? I don't
> think narrowing a search to items without shelving location is useful to
> patrons.

Your first assumption is correct. I don't know if we should alter it however, as that is how all the different facets work. If there are items with no value for the given facet, a "*" shows up to represent the 'no value' option.

> If there are items
> with no value for the given facet, a "*" shows up to represent the 'no
> value' option.

In what situation is this the case?

(In reply to comment #31)
> > If there are items
> > with no value for the given facet, a "*" shows up to represent the 'no
> > value' option.
> 
> In what situation is this the case?

It happens for branches, as well. But those are the only two though. If a branch has no branchname, it get's a "*" instead.

(In reply to comment #32)
> It happens for branches, as well. But those are the only two though. If a
> branch has no branchname, it get's a "*" instead.

By no branchname, do you mean the item has not homebranch or holdingbranch? I can't reproduce that output.

Even so, it seems like an awfully slim edge case. When would an item not have a branch name? Even in such a case, is it useful to the OPAC user to be able to refine their results to items which *don't* have a branch?

(In reply to comment #33)
> (In reply to comment #32)
> > It happens for branches, as well. But those are the only two though. If a
> > branch has no branchname, it get's a "*" instead.
> 
> By no branchname, do you mean the item has not homebranch or holdingbranch?
> I can't reproduce that output.
> 
> Even so, it seems like an awfully slim edge case. When would an item not
> have a branch name? Even in such a case, is it useful to the OPAC user to be
> able to refine their results to items which *don't* have a branch?

Actually, it appears to mean the branch exists, but doesn't have a name. I agree, it's not very useful. It's code Ian wrote, so I'm not sure if he had a particular reason for doing it that way. I'll post a new patch that should fix this issue.

Created attachment 12254 [details] [review]
[SIGNED-OFF] Bug 7401 - Shelving Location facet

This patch enables the shelving location facet as an
alternative to the branches fact in two situations:
A) SingleBranchMode is enabled
B) There is only one branch in the branches table

Test Plan:
1) Catalog multiple items with different shelving locations.
2) Test enable by enabling SingleBranchMode
3) Test enable by deleting all but one branch

Based on initial patch by Ian Walls.

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

Tested cases 2) and 3) successfully in OPAC and staff client

(In reply to comment #36)
> Created attachment 12254 [details] [review]
> [SIGNED-OFF] Bug 7401 - Shelving Location facet
> Signed-off-by: Owen Leonard <oleonard@myacpl.org>

patch looks good, passing QA…

$ koha-qa.pl  

testing 1 commit(s) (applied to commit 3f31617)
 * 663f5e5 Bug 7401 - Shelving Location facet
      C4/Branch.pm
      C4/Koha.pm
      C4/Search.pm
      koha-tmpl/intranet-tmpl/prog/en/includes/facets.inc
      koha-tmpl/opac-tmpl/prog/en/includes/opac-facets.inc
      opac/opac-search.pl

* C4/Branch.pm                                                             OK
* C4/Koha.pm                                                               OK
* C4/Search.pm                                                             OK
* opac/opac-search.pl                                                      OK
* koha-tmpl/intranet-tmpl/prog/en/includes/facets.inc                      OK
* koha-tmpl/opac-tmpl/prog/en/includes/opac-facets.inc                     OK

Patch pushed to master