Bug 8209

Summary: "Did you mean?" suggestions based on authorities
Product: Koha Reporter: Jared Camins-Esakov <jcamins>
Component: SearchingAssignee: Jared Camins-Esakov <jcamins>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: eb, fcapovilla, jonathan.druart, katrin.fischer, m.de.rooy, paul.poulain
Version: 3.10   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 8212, 8726    
Attachments: Bug 8209: "Did you mean?" from authorities
Bug 8209: "Did you mean?" from authorities
Bug 8209: "Did you mean?" from authorities
Bug 8209: "Did you mean?" from authorities
[SIGNED-OFF] Bug 8209: "Did you mean?" from authorities
Bug 8209: "Did you mean?" from authorities
SIGNED-OFF-Bug-8209-Did-you-mean-from-authorities.
Bug 8209 follow-up: fix failing test in AuthorityFile suggestion plugin
Bug 8209 follow-up: another correction for BuildSummary API
Bug 8209 follow-up: another correction for BuildSummary API

Description Jared Camins-Esakov 2012-06-07 14:31:07 UTC
One of the ideas behind authority records is that users who search for one term should have related terms (according to the authority file) suggested to them. At the moment, Koha doesn't do that. Adding an authority searching step to regular searches and displaying any suggestions in a "Did you mean" bar at the top of the results would be very useful.
Comment 1 Jared Camins-Esakov 2012-07-26 21:44:46 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2012-08-02 05:11:55 UTC
Sorry, Jared:
Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y
Applying: Bug 8209: "Did you mean?" from authorities
/home/katrin/kohaclone/.git/rebase-apply/patch:834: trailing whitespace.
 
fatal: sha1 information is lacking or useless (C4/AuthoritiesMarc.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 8209: "Did you mean?" from authorities
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
Comment 3 Jared Camins-Esakov 2012-08-02 12:14:32 UTC Comment hidden (obsolete)
Comment 4 Katrin Fischer 2012-08-02 15:12:37 UTC
Hi Jared, 
the new patch applies, a few first findings:

1) Tiny: +our $NAME    = 'EmbedItems';  looks suspicious to me.

2) Tiny: The database update message has a tiny typo:
Upgrade to 3.09.00.XXX done (Add system preference DidYouMeanFromAuthorities))

3) Blocker: The suggestions don't show up correctly after activating the feature in system preferences:

Did you mean: HASH(0x42a70e0) HASH(0x42a6ed0) HASH(0x429fbd0) HASH(0x429f630)

4) Might be data: I don't get results for the an=[0-9]* searches. I am not sure if this is my database or a general problem, but will wait with testing until 3) is fixed.

5) Suggestion: Use links instead of buttons for display of the suggestions.

6) Tiny: If I have no results for my search and no suggestions are generated, the "Sorry, no suggestions." section covers my "Powered by Koha".

The variant for deactivated Javascript works very nicely, except for 3).

Sorry to fail QA :(

Because Paul pushed so many authority related featured today: I rebased the patch to current master shortly before I started testing.
commit 9226ae46419a5c54447c7d6e27b6163d2c6fd95a
Comment 5 Jared Camins-Esakov 2012-08-02 16:12:18 UTC Comment hidden (obsolete)
Comment 6 Jared Camins-Esakov 2012-08-02 16:48:42 UTC Comment hidden (obsolete)
Comment 7 Katrin Fischer 2012-08-03 06:46:55 UTC Comment hidden (obsolete)
Comment 8 Marcel de Rooy 2012-08-30 08:25:34 UTC
QA Comment:
Code looks good to me. Tested only a bit, refer to signoff.

One question: A template block pertaining to koha_spsuggest is removed. This appears dead code to me. But it is in several other templates too. If we remove it here, could we remove it elsewhere too? Perhaps in a followup or under a separate report?

Passed QA
Comment 9 Jared Camins-Esakov 2012-08-30 10:53:43 UTC
(In reply to comment #8)
> One question: A template block pertaining to koha_spsuggest is removed. This
> appears dead code to me. But it is in several other templates too. If we
> remove it here, could we remove it elsewhere too? Perhaps in a followup or
> under a separate report?

Yes, the spsuggest code is dead. I'll open a new report and submit a patch for it.
Comment 10 Jared Camins-Esakov 2012-09-10 07:53:27 UTC Comment hidden (obsolete)
Comment 11 wajasu 2012-09-10 17:48:08 UTC Comment hidden (obsolete)
Comment 12 Tomás Cohen Arazi 2012-09-10 19:18:41 UTC
*** Bug 5705 has been marked as a duplicate of this bug. ***
Comment 13 Paul Poulain 2012-09-13 09:48:20 UTC
Patch pushed to master.

1- I love this feature (and it worked out of the box with UNIMARC)
2- I love the way it's coded, with svc,... ++ for adding webservices
3- will require some work to make it work with Solr... (I feel Koha::SuggestionEngine will have to be moved to Koha::SearchEngine::Zebra::Suggestions or something like that, but that will be for later)
4- you continue introducing OOP with Class::Accessor. I'm a little bit uncomfortable having different OOP methods in Koha. We should debate of this later I think (that will be with you as RM, wont start this discussion now ;-) )
Comment 14 Jared Camins-Esakov 2012-09-13 11:40:13 UTC Comment hidden (obsolete)
Comment 15 Paul Poulain 2012-09-13 12:15:33 UTC
follow-up fixes the test errors
Comment 16 Jared Camins-Esakov 2012-09-13 15:36:31 UTC Comment hidden (obsolete)
Comment 17 Paul Poulain 2012-09-13 15:48:27 UTC
(In reply to comment #16)
> Created attachment 12194 [details] [review]
> Bug 8209 follow-up: another correction for BuildSummary API

Status set to "in discussion" ? shouldn't it be "need signoff" ?
Comment 18 Jared Camins-Esakov 2012-09-13 15:50:39 UTC
Yes, sorry. I got only halfway through the status changing dance.
Comment 19 Jonathan Druart 2012-09-14 12:08:30 UTC
Created attachment 12217 [details] [review]
Bug 8209 follow-up: another correction for BuildSummary API

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 20 Jared Camins-Esakov 2012-09-27 12:29:30 UTC
The follow-up is a bugfix, not an enhancement.
Comment 21 Marcel de Rooy 2012-09-27 12:44:07 UTC
(In reply to comment #20)
> The follow-up is a bugfix, not an enhancement.

Latest followup fixes a display problem. Label was a hash ref instead of a name.

Passed QA
Comment 22 Paul Poulain 2012-10-01 14:41:03 UTC
Follow-up pushed to master
Comment 23 Jared Camins-Esakov 2012-12-03 13:18:27 UTC
This feature is available on Master.
Comment 24 Katrin Fischer 2018-02-12 21:25:51 UTC
*** Bug 8571 has been marked as a duplicate of this bug. ***