Bug 10464 - Patron search on placing a hold doesn't use jqueryui.autocomplete
Summary: Patron search on placing a hold doesn't use jqueryui.autocomplete
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-06-13 17:00 UTC by Tomás Cohen Arazi
Modified: 2014-12-07 20:02 UTC (History)
4 users (show)

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


Attachments
Bug 10464 - Patron search on placing a hold should provide autocomplete (4.29 KB, patch)
2013-06-13 17:32 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 10464 - Patron search on placing a hold should provide autocomplete (4.39 KB, patch)
2013-06-13 18:27 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 10464 - Patron search on placing a hold should provide autocomplete (4.45 KB, patch)
2013-06-14 11:02 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 10464 - Patron search on placing a hold should provide autocomplete (4.48 KB, patch)
2013-06-16 15:20 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[PASSED QA] Bug 10464 - Patron search on placing a hold should provide autocomplete (4.62 KB, patch)
2013-06-16 15:34 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2013-06-13 17:00:10 UTC
We should provide the autocomplete feature present for the circulation search box if CircAutocompl set.
Comment 1 Tomás Cohen Arazi 2013-06-13 17:32:45 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi 2013-06-13 18:27:59 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2013-06-14 11:02:15 UTC Comment hidden (obsolete)
Comment 4 Katrin Fischer 2013-06-16 10:20:28 UTC
Hi Tomas,

I really like the idea :)

But I have found something that I'd like you to investigate: When sending the form without waiting for the autocomplete to take place (I used 's') you get to the suggestion list that Koha already did before your patch. This is good because I think it works without Javascript, so is a good fallback. But when I am on this screen, there is a Javascript error in my Firebug console:

$(...).autocomplete(...).data(...) is undefined
Line 335

Could you take a look? If it's not too much of a rewrite to fix, you can switch back to 'signed off'.
Comment 5 Tomás Cohen Arazi 2013-06-16 15:20:44 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2013-06-16 15:34:43 UTC
Created attachment 19042 [details] [review]
[PASSED QA] Bug 10464 - Patron search on placing a hold should provide autocomplete

We should provide the autocomplete feature, already present for the circulation search
box, if CircAutocompl is properly set; and leave as it currently works otherwise of
course.

This patch grabs the autocomplete handling javascript stuff from patron-search-box.inc
and puts an adjusted version on reserve/request.tt and also adjusts form and input
id's to match those configured in the autocomplete call. This changes where needed
due to name collisions with the patron-search-box.inc that is present everywhere.

To test:
- On a biblio record, go to the Holds tab, mine looks like
  /cgi-bin/koha/reserve/request.pl?biblionumber=2
- A search box is shown for patron searching.
- Search for a patron: it works as usual.
- Make sure CircAutocompl is 'Don\'t try'
- Apply the patch
- Click the Holds tab again, and do the same search: functionality should remain intact.
- Change CircAutocompl to 'Try'
- Click the Holds tab again
- Autocomplete should show you the possible options, and once you choose one result, it should work as usual.

Note: once you apply the patch the width of the search input field will be doubled. If you are oleonard and
prefer to stick with size=20, please amend the patch, I just prefered how it looked at size=40.

Regards
To+

Edit: Wrapped the JavaScript code with [% UNLESS ( borrowernumber || borrower_list ) %] to fix the behaviour
on the case of rendering the placing hold page instead.

Sponsored-by: Universidad Nacional de Córdoba
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script.
Adding a nice new autocomplete feature.
Comment 7 Galen Charlton 2013-06-17 01:37:23 UTC
Pushed to master.  Thanks, Tomás!

A refactoring to consider in the near future is tweaking the patron search auto-complete JavaScript so that it can be contained in an include template rather than duplicating it among the following templates:

koha-tmpl/intranet-tmpl/prog/en/includes/patron-search-box.inc
koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course.tt
p
Local template variables can be set via a TT include directive to set the form element IDs to apply the JavaScript to.
Comment 8 Tomás Cohen Arazi 2013-07-02 18:00:57 UTC
This patch has been pushed to 3.12.x, will be in 3.12.2.
Comment 9 Bernardo Gonzalez Kriegel 2013-07-13 18:03:09 UTC
Pushed to 3.10.x, will be in 3.10.8
Comment 10 Chris Hall 2013-07-18 09:02:53 UTC
Pushed to 3.8.x, will be in 3.8.15