Bug 7031 - More options for AdvancedSearchTypes
Summary: More options for AdvancedSearchTypes
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Ian Walls
QA Contact: Marcel de Rooy
URL:
Keywords:
: 4329 4481 5538 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-10-14 16:46 UTC by Ian Walls
Modified: 2019-02-15 01:09 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:


Attachments
Screenshot of proposed change: OPAC advanced search (44.05 KB, image/png)
2011-10-15 17:00 UTC, Ian Walls
Details
Screenshot of proposed changes: staff search (39.57 KB, image/png)
2011-10-15 17:01 UTC, Ian Walls
Details
Screenshot of proposed changes: system preference change (15.09 KB, image/png)
2011-10-15 17:01 UTC, Ian Walls
Details
Proposed Patch (19.59 KB, patch)
2011-10-15 17:04 UTC, Ian Walls
Details | Diff | Splinter Review
Updated patch (19.89 KB, patch)
2011-12-21 20:28 UTC, Ian Walls
Details | Diff | Splinter Review
Follow-up patch with markup corrections (4.72 KB, patch)
2011-12-27 16:30 UTC, Owen Leonard
Details | Diff | Splinter Review
screenshot of advanced search (36.87 KB, image/png)
2011-12-27 22:20 UTC, Katrin Fischer
Details
Bug 7031 Follow-up: Staff search displays OPAC description instead of the normal description for the authorized values (977 bytes, patch)
2012-02-24 14:19 UTC, Kyle M Hall
Details | Diff | Splinter Review
Enh 7031: More options for Advanced Search (20.37 KB, patch)
2012-02-24 15:03 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Enh 7031: More options for Advanced Search (20.41 KB, patch)
2012-02-24 16:10 UTC, Owen Leonard
Details | Diff | Splinter Review
Follow-up patch with markup corrections (4.72 KB, patch)
2012-02-24 16:10 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Follow-up: Staff search displays OPAC description instead of the normal description for the authorized values (1.06 KB, patch)
2012-02-24 16:11 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7031 Follow-up, More options for AdvancedSearchTypes (4.75 KB, patch)
2012-02-24 19:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7031: Followup for typo in catalogue search (1.01 KB, patch)
2012-03-09 17:57 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Ian Walls 2011-10-14 16:46:24 UTC
This development aims to enhance the AdvancedSearchTypes to support more than a single type at a time, so libraries can search collection codes AND itemtypes AND shelving locations AND other items fields in the future.

By replacing the choice of AdvancedSearchTypes with free text, we can allow the library to specify multiple fields, and in their preferred order.  Each field can then be put into it's own tab in the search interface, allowing for a more dense storage of values while not sacrificing functionality.

This enhancement obsoletes bug 4329, as it covers more than just Shelving Locations, and also in the staff client (and is syspref controlled).
Comment 1 Ian Walls 2011-10-14 16:47:22 UTC
*** Bug 4329 has been marked as a duplicate of this bug. ***
Comment 2 Ian Walls 2011-10-15 17:00:36 UTC
Created attachment 5896 [details]
Screenshot of proposed change: OPAC advanced search

Shows tabs for different search limit fields
Comment 3 Ian Walls 2011-10-15 17:01:09 UTC
Created attachment 5897 [details]
Screenshot of proposed changes:  staff search

Shows tabs in staff search
Comment 4 Ian Walls 2011-10-15 17:01:42 UTC
Created attachment 5898 [details]
Screenshot of proposed changes:  system preference change

Shows change in system preference display
Comment 5 Ian Walls 2011-10-15 17:04:27 UTC Comment hidden (obsolete)
Comment 6 Paul Poulain 2011-10-24 11:38:14 UTC
Updating Version : This ENH will be for Koha 3.8
Comment 7 Paul Poulain 2011-10-25 15:05:49 UTC
Bug versionned for master. entries will be made against rel_3_8 once the patch has been applied (see thread about that on koha-devel yesterday)
Comment 8 Owen Leonard 2011-11-10 21:09:14 UTC
I'm really glad to see this in the works. Some superficial comments:

- The new <script> block in opac-advsearch.tt should be inside <head>
- The markup in catalogue/advsearch.tt has a missing </div> somewhere which 
  breaks the display of the language-chooser
- The description of the system preference needs to be much clearer, including
  saying what kind of input is allowed. It says:

Show checkboxes for the [    ] item fields on the OPAC and staff advanced search pages, in the order listed. 'itemtypes', 'ccode' and 'loc' supported at this time.

...but it doesn't say what those terms mean or that they must be pipe-delimited.

Maybe beyond the scope of this bug, maybe not: If we're working on this feature why not take the time now to split it into staff-specific and OPAC-specific preferences? For a library like mine which uses itemtype as a circ-policy code it doesn't make much sense to expose it to the patrons, but it does make sense for the staff.
Comment 9 Ian Walls 2011-12-21 20:26:40 UTC
Owen,


<script> blocked moved up a line, inside <head>.

I cannot find a <div> breaking my language display... Firebug isn't picking it up.  Can you give me a more pointy pointer?

Description in system preferences updated to be more explicit and clear.


Splitting the system preference into two would be very easy to do, but it's outside the scope of this project, and would introduce a database update where one is not strictly necessary.
Comment 10 Ian Walls 2011-12-21 20:28:05 UTC Comment hidden (obsolete)
Comment 11 Owen Leonard 2011-12-27 16:30:27 UTC Comment hidden (obsolete)
Comment 12 Katrin Fischer 2011-12-27 22:20:25 UTC
Created attachment 6938 [details]
screenshot of advanced search

I tested this and have some questions/notes:

1) I was searching for a special combination of itemtype A, location B, and collection code C. The search turned up a record where no item had this combination. There was one item with my itemtype A, while the other item had itemtype D, location B, and collection code C. 
Is it possible to make the search more specific here?

2) Why use phrase for itype, but not for ccode or loc?
No results match your search  with limit(s): 'mc-itype,phr:BK or mc-itype,phr:CF or mc-itype,phr:CR mc-ccode:"NFIC" mc-loc:"CART" or mc-loc:"CHILD"'.  

3) I wonder if it's a bit confusing to show the heading as a link when only one limit is in use.

4) The "box" is a bit bigger than the other search options on the advanced search page. Perhaps a minor CSS glitch?

5) I agree with owen that this should be slit into separate preferences for staff and OPAC.

6) Staff search displays OPAC description instead of the normal description for the authorized values (loc and ccode).

Failing QA for 6). 1) should be documented or improved if possible.
Comment 13 Jared Camins-Esakov 2011-12-28 16:52:26 UTC
Katrin,

(In reply to comment #12)
> Created attachment 6938 [details]
> screenshot of advanced search
> 
> I tested this and have some questions/notes:
> 
> 1) I was searching for a special combination of itemtype A, location B, and
> collection code C. The search turned up a record where no item had this
> combination. There was one item with my itemtype A, while the other item had
> itemtype D, location B, and collection code C. 
> Is it possible to make the search more specific here?

Not with GRS-1 indexing. DOM indexing would make it possible.

> 2) Why use phrase for itype, but not for ccode or loc?
> No results match your search  with limit(s): 'mc-itype,phr:BK or
> mc-itype,phr:CF or mc-itype,phr:CR mc-ccode:"NFIC" mc-loc:"CART" or
> mc-loc:"CHILD"'.  

I'm not sure, but I would think that adding a phrase index and using that would be preferable here (however, I wouldn't hold up the QA process for it if I were the one signing off). Of course, while we're using GRS-1 indexing, it probably doesn't really matter all that much anyway.
Comment 14 Kyle M Hall 2012-02-24 14:19:55 UTC Comment hidden (obsolete)
Comment 15 Owen Leonard 2012-02-24 14:54:48 UTC
(In reply to comment #10)
> Created attachment 6910 [details] [review]
> Updated patch

This patch no longer applies.
Comment 16 Kyle M Hall 2012-02-24 15:03:52 UTC Comment hidden (obsolete)
Comment 17 Owen Leonard 2012-02-24 16:10:19 UTC
Created attachment 7859 [details] [review]
[SIGNED-OFF] Enh 7031: More options for Advanced Search
Comment 18 Owen Leonard 2012-02-24 16:10:50 UTC Comment hidden (obsolete)
Comment 19 Owen Leonard 2012-02-24 16:11:44 UTC
Created attachment 7861 [details] [review]
[SIGNED-OFF] Follow-up: Staff search displays OPAC description instead of the normal description for the authorized values
Comment 20 Owen Leonard 2012-02-24 16:12:45 UTC
I have signed off on the patches not written by me and re-attached everything in the order they need to be applied.
Comment 21 Kyle M Hall 2012-02-24 19:31:32 UTC
Created attachment 7864 [details] [review]
[SIGNED-OFF] Bug 7031 Follow-up, More options for AdvancedSearchTypes

This follow-up patch corrects some markup errors and removes some
markup which I think is superfluous:

- Using template FILTER command to remove invalid punctuation from
  ID attributes (to quiet the HTML validator)
- Removing <fieldset> since options are bordered by the tab box now
- Correcting the conditional which controls the table markup
  to prevent extra/missing </tr>
- Adding missing ID attribute in the OPAC template

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 22 Marcel de Rooy 2012-03-09 17:57:48 UTC
Created attachment 8127 [details] [review]
Bug 7031: Followup for typo in catalogue search

Trivial patch
Comment 23 Marcel de Rooy 2012-03-09 18:15:25 UTC
QA Comment:
Nice feature, introduced quite economically. Code looks good (especially noting the buildQuery environment and the search scripts;). Clear changes.
Remember a trivial merge conflict in css file while applying.
Added a followup for a simple typo in catalogue/search.pl (making no real difference but for the record)
Tested it "diagonally".
Marking as Passed QA.
Comment 24 Paul Poulain 2012-03-12 17:12:11 UTC
QA comment:

After applying the 4 patches, I get:
"my" variable $total masks earlier declaration in same scope at catalogue/search.pl line 706.

that was here before the patch
(perl -wc catalogue/search.pl)

so it's OK

staff-global.css says:
+.adsearch {

isn't there a typo here ? shouldn't it be advsearch  (add a v), as it's everywhere else ? I've grepped "adsearch", and it appears nowhere except in staff-global.css, so I think it's a typo.

If you confirm it is, just switch back to "passed QA", i'll do the fix & push this (great) feature, as everything seems to work well.

Note : I tested with my UNIMARC setup, and everything worked out of the box, well done !
Comment 25 Paul Poulain 2012-03-14 13:46:17 UTC
Patch pushed, with a follow-up:
* replaced adsearch by advsearch in css (typo fix)
* replaced some tabs in .css by 4 spaces
Comment 26 Owen Leonard 2013-01-22 19:20:11 UTC
*** Bug 5538 has been marked as a duplicate of this bug. ***
Comment 27 Chris Cormack 2013-10-05 01:28:32 UTC
*** Bug 4481 has been marked as a duplicate of this bug. ***