|
Description
Katrin Fischer
2018-06-06 16:21:15 UTC
Created attachment 84692 [details] [review] Bug 20888: Allow use of boolean operator 'not' in item search Test plan: - Apply the patch - Check that you have a 'is not' option in the third box in item search - Check that you can exclude items from the search results with this 'is not' option - Check that you can still combine parameters with 'and' and 'or' - Check that the exclusion works for custom fields (see Administration > Item search fields) - Check that the exclusion works for custom fields linked to authorised values Please don't forget to add yourself as assignee :) Oops, sorry :) The patch does not pass the QA test : FAIL koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/itemsearch.tt FAIL filters missing_filter at line 237 ( var authorised_values = [% authorised_values_json %];) OK forbidden patterns OK git manipulation OK spelling OK tt_valid OK valid_template Created attachment 85281 [details] [review] Bug 20888: Allow use of boolean operator 'not' in item search Test plan: - Apply the patch - Check that you have a 'is not' option in the third box in item search - Check that you can exclude items from the search results with this 'is not' option - Check that you can still combine parameters with 'and' and 'or' - Check that the exclusion works for custom fields (see Administration > Item search fields) - Check that the exclusion works for custom fields linked to authorised values Fixed QA using $raw filter. I have created a dedicated bug for raw filter. Please rebase. Matthias, can you please rebase? :) Frido, I don't get the status of this BZ or your comment. The patch applies on master. What is to rebase exactly? Which bug are you referring to? What are you expecting me to do? (In reply to Matthias Meusburger from comment #9) > Frido, I don't get the status of this BZ or your comment. > > The patch applies on master. What is to rebase exactly? Which bug are you > referring to? What are you expecting me to do? Sorry its really not clear (even for me). I have created Bug 22596 for use of raw filter and added as dependency. Now I see the raw filter already there in your patch so forget what I said ;) Thanks Frido. We try to test today : this patch doesn't apply on a sandbox (Biblibre n°1) Created attachment 93347 [details] [review] Bug 20888: Allow use of boolean operator 'not' in item search Test plan: - Apply the patch - Check that you have a 'is not' option in the third box in item search - Check that you can exclude items from the search results with this 'is not' option - Check that you can still combine parameters with 'and' and 'or' - Check that the exclusion works for custom fields (see Administration > Item search fields) - Check that the exclusion works for custom fields linked to authorised values The patch has been rebased, it should apply correctly on the sandboxes now. Patch tested with a sandbox, by Amandine Zocca <azocca@ville-montauban.fr> Created attachment 93370 [details] [review] Bug 20888: Allow use of boolean operator 'not' in item search Test plan: - Apply the patch - Check that you have a 'is not' option in the third box in item search - Check that you can exclude items from the search results with this 'is not' option - Check that you can still combine parameters with 'and' and 'or' - Check that the exclusion works for custom fields (see Administration > Item search fields) - Check that the exclusion works for custom fields linked to authorised values Signed-off-by: Amandine Zocca <azocca@ville-montauban.fr> Wondering: If is translates to "like", should "not" not translate to "not like"? Currently it will be != which doesn't allow for truncation. Example would be a search for: Title does not contain a certain keyword. I think we should rename "is" to "is like" then. This way it will be consistent, with the following options: - is like - is not An "is" and/or "is not like" option may be added in the future, and it will still be consistent. What do you think? (In reply to Matthias Meusburger from comment #18) > I think we should rename "is" to "is like" then. This way it will be > consistent, with the following options: > > - is like > - is not > > An "is" and/or "is not like" option may be added in the future, and it will > still be consistent. > > What do you think? One problem we see with item search is that people expect it to work like the bibliographic search. For example, when searching for "Title is some words", they expect results to come up, but often forget to put the truncation. If the title actually is "these some words" the search will only find it if you use: Title is %some words". I think that's the reason, why we should not change the wording, we do use the SQL operator "like" and unless you use truncation, it works like an exact search. But it gives you the option to use the truncation. And I think we want the same behavior for adding the 'not' operator: If the libraries don't add truncation characters it will actually behave like !=, but they have the option to add them to make search a bit broader. Also: The hint below the search options will then be accurate, otherwise it would be misleading: You can use the following wildcard characters: % _ % matches any number of characters _ matches only a single character Ok, understood. I'll submit a new patch. Hi
- $('div.form-field-select-text select[name="f"]').change(function() {
+ $('div.form-field-select-text select.form-field-column').change(function() {
is this change necessary ?
New select has name "op".
Created attachment 97626 [details] [review] Bug 20888: Follow-up: 'is not' operator behaves as 'not like' Katrin, here's the modification you asked for: 'is not' now behaves like 'not like' (just like 'is' behaves like 'like') Created attachment 97627 [details] [review] Bug 20888: Follow-up: revert unecessary change on jquery selector Frido, you were right, this change was unnecessary and has been reverted. Hi Matts, have been testing: + <select name="op" class="form-field-not"> + <option value="like">is</option> + [% IF params.op == 'not like' %] + <option value="not like" selected="selected">is not</option> + [% ELSE %] + <option value="not like">is not</option> + [% END IF = not like... else = not like? I tested with a custom item search option for "Withdrawn" linked to the WITHDRAWN authorised value. If I pick "is not" Withdrawn I get the only withdrawn item. If I pick 'is' withdrawn... I get all the other items. It works ok on master for the 'is' case. IF = not like... else = not like? => Yes, this is made for "not like" to be selected if needed when going back to the search. The "is" option is just above. I tested with a custom item search option for "Withdrawn" linked to the WITHDRAWN authorised value. If I pick "is not" Withdrawn I get the only withdrawn item. If I pick 'is' withdrawn... I get all the other items. It works ok on master for the 'is' case. => I tested again, with the following configurations: Unimarc : custom withdrawn search field => OK Marc21 : custom withdrawn search field => OK Marc21 : default withdrawn search field => OK Could you possibly give it another go, Cait? Hi Matts, could you please rebase? I am stuck with a: error: could not build fake ancestor Created attachment 111265 [details] [review] Bug 20888: Allow use of boolean operator 'not' in item search Test plan: - Apply the patch - Check that you have a 'is not' option in the third box in item search - Check that you can exclude items from the search results with this 'is not' option - Check that you can still combine parameters with 'and' and 'or' - Check that the exclusion works for custom fields (see Administration > Item search fields) - Check that the exclusion works for custom fields linked to authorised values Signed-off-by: Amandine Zocca <azocca@ville-montauban.fr> Created attachment 111266 [details] [review] Bug 20888: Follow-up: 'is not' operator behaves as 'not like' Created attachment 111267 [details] [review] Bug 20888: Follow-up: revert unecessary change on jquery selector Created attachment 112453 [details] [review] Bug 20888: Allow use of boolean operator 'not' in item search Test plan: - Apply the patch - Check that you have a 'is not' option in the third box in item search - Check that you can exclude items from the search results with this 'is not' option - Check that you can still combine parameters with 'and' and 'or' - Check that the exclusion works for custom fields (see Administration > Item search fields) - Check that the exclusion works for custom fields linked to authorised values Signed-off-by: Amandine Zocca <azocca@ville-montauban.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 112454 [details] [review] Bug 20888: (follow-up) 'is not' operator behaves as 'not like' Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 112455 [details] [review] Bug 20888: (follow-up) revert unecessary change on jquery selector Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 112691 [details] [review] Bug 20888: Allow use of boolean operator 'not' in item search Test plan: - Apply the patch - Check that you have a 'is not' option in the third box in item search - Check that you can exclude items from the search results with this 'is not' option - Check that you can still combine parameters with 'and' and 'or' - Check that the exclusion works for custom fields (see Administration > Item search fields) - Check that the exclusion works for custom fields linked to authorised values Signed-off-by: Amandine Zocca <azocca@ville-montauban.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 112692 [details] [review] Bug 20888: (follow-up) 'is not' operator behaves as 'not like' Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 112693 [details] [review] Bug 20888: (follow-up) revert unecessary change on jquery selector Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Pushed to master for 20.11, thanks to everybody involved! |