Bug 5755

Summary: Control facets truncation on OPAC result page
Product: Koha Reporter: Frédéric Demians <f.demians>
Component: SearchingAssignee: Frédéric Demians <f.demians>
Status: CLOSED FIXED QA Contact: Bugs List <koha-bugs>
Severity: enhancement    
Priority: PATCH-Sent (DO NOT USE) CC: chris, fridolin.somers, julian.maurice, katrin.fischer, nengard
Version: 3.4   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Proposed patch
Version with Frydolin syspref name
New version rebased on HEAD

Description Frédéric Demians 2011-02-14 08:23:25 UTC
On OPAC result page facets are truncated to 20 characters. On some OPAC
layout, it's not enough. A new syspref FacetOpacMaxLength defines the
length to cut facets if necessary.
Comment 1 Frédéric Demians 2011-02-14 08:30:31 UTC
Created attachment 3143 [details] [review]
Proposed patch
Comment 2 Fridolin Somers 2011-02-15 11:24:32 UTC
(In reply to comment #1)
> Created attachment 3143 [details] [review]
> Proposed patch

I think "FacetOpacMaxLength" is too ambiguous.
It may signify the number of biblio used in the computation of facets.

Also, Search.pm is for both OPAC and Intranet. So both will be impacted by the system pref, no?

I suggest "FacetLabelMaxLength".

Regards,
Comment 3 Frédéric Demians 2011-02-15 16:32:01 UTC
> I think "FacetOpacMaxLength" is too ambiguous.  It may signify the
> number of biblio used in the computation of facets.

An English speaking person may have to say his word on that. 

> Also, Search.pm is for both OPAC and Intranet. So both will be
> impacted by the system pref, no?

Yes, you get a point.

> I suggest "FacetLabelMaxLength".

Or FacetTruncationLength?

Thanks.
Comment 4 Fridolin Somers 2011-02-16 07:49:32 UTC
(In reply to comment #3)
> 
> Or FacetTruncationLength?
> 
Ok, but with "label" : FacetLabelTruncationLength

Regards,
Comment 5 Frédéric Demians 2011-02-16 15:58:11 UTC
Created attachment 3161 [details] [review]
Version with Frydolin syspref name
Comment 6 Frédéric Demians 2011-02-16 15:58:41 UTC
> Version with Frydolin syspref name

Fridolyn...
Comment 8 Julian Maurice 2011-03-11 10:43:47 UTC
Patch does not apply. I think it comes from
'\ No newline at end of file'
lines.
These lines are present in both proposed patches.
Comment 9 Frédéric Demians 2011-03-11 12:06:02 UTC
(In reply to comment #8)
> Patch does not apply. I think it comes from
> '\ No newline at end of file'
> lines.
> These lines are present in both proposed patches.

Thanks Julian for trying. This is due to a merge conflict. This patch affects syspref definition files which have changed since the patch submission. I will try to find time to rebase it to master and resubmit.
Comment 10 Frédéric Demians 2011-03-11 12:17:15 UTC
Created attachment 3283 [details] [review]
New version rebased on HEAD
Comment 11 Julian Maurice 2011-03-11 12:55:48 UTC
Tested, it works.
Comment 12 Chris Cormack 2011-03-16 23:25:25 UTC
Are we sure we dont want to add an updatedatabase for this?

That means no one who ever upgrades can get this new feature. Only people who newly install can ... 

Please submit a follow up patch doing the updatedatabase to insert the syspref, defaulting it to 20, so that people upgrading can now (or in the future) turn this on.
Comment 13 Frédéric Demians 2011-03-17 06:12:07 UTC
> Are we sure we dont want to add an updatedatabase for this?  That
> means no one who ever upgrades can get this new feature. Only people
> who newly install can ... 

It isn't actually correct. People will just have to go in Administration
> System preferences > Searching tab > Result Display section. Their,
they will find FacetLabelTruncationLength syspref since the patch add it
to search.tmpl.
Comment 14 Katrin Fischer 2011-03-17 06:49:58 UTC
I am not sure this will not add confusion. If I understand this correctly there will be no entry in the sys pref table until the sys pref is changed? 
Perhaps we should not do this different from the other sys prefs without a good reason.
Comment 15 Frédéric Demians 2011-03-17 09:11:42 UTC
> I am not sure this will not add confusion. If I understand this
> correctly there will be no entry in the sys pref table until the sys
> pref is changed? 

You understand correctly!

Chris, please, just confirm you want an DB update for this syspref. I
will send it. Thanks.
Comment 16 Chris Cormack 2011-03-18 01:41:06 UTC
Yes please, lets do it the same as the others a least until 3.4 is released :)
Comment 17 Nicole C. Engard 2011-03-19 15:38:17 UTC
Signed off on the two patches.
Comment 18 Chris Cormack 2011-03-22 08:16:19 UTC
Pushed please test