Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

search: add simple params for the backend calls #265

Merged
merged 1 commit into from
May 15, 2020
Merged

search: add simple params for the backend calls #265

merged 1 commit into from
May 15, 2020

Conversation

jma
Copy link
Contributor

@jma jma commented May 4, 2020 •
edited
Loading

For search boxes, the AND operator should be applied by default and it
should be robust to the query syntax errors.

Co-Authored-by: Johnny Mariéthoz Johnny.Mariethoz@rero.ch

Why are you opening this PR?

Dependencies

How to test?

  • Enter several queries to the public view search box. Check that the AND operator is used by default. Test several queries with parameters that causes query syntax errors.

Note: see (https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-simple-query-string-query.html#simple-query-string-syntax) for the supported syntax documentation.

Code review check list

  • Commit message template compliance.
  • Commit message without typos.
  • File names.
  • Functions names.
  • Functions docstrings.
  • Unnecessary commited files?
  • Extracted translations?

@jma jma mentioned this pull request May 4, 2020
Merged
7 tasks
@jma jma marked this pull request as ready for review May 13, 2020 05:08
@jma jma requested review from zannkukai and sebdeleze May 13, 2020 05:08
sebdeleze
sebdeleze approved these changes May 13, 2020
View reviewed changes
Copy link

@sebdeleze sebdeleze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK for me, but just a suggestion: As simple query seems to be the default behavior, we could put this params by default in ng-core search. In this way, you don't have to add a specific preFilters configuration in rero-ils-ui.

@jma jma force-pushed the maj-simple-query branch 2 times, most recently from f8b9f9c to 65623cd Compare May 13, 2020 09:53
@jma
Copy link
Contributor Author

jma commented May 13, 2020

OK for me, but just a suggestion: As simple query seems to be the default behavior, we could put this params by default in ng-core search. In this way, you don't have to add a specific preFilters configuration in rero-ils-ui.

yes you're right, but probably one day we will have and advanced search box...

@jma
search: use simple query for the search views
3854332 
* Uses AND operator by default.
* Makes the search engine more robust to the query syntax errors.
* Uses more simple syntax for boolean operator AND: +,OR: |, NOT: -.
* Fixes suggestion highlights by escaping regex special characters.

Co-Authored-by: Johnny Mariéthoz <Johnny.Mariethoz@rero.ch>
iGormilhit
iGormilhit approved these changes May 15, 2020
View reviewed changes
Copy link
Contributor

@iGormilhit iGormilhit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit message approved.

@jma jma merged commit 8bad931 into rero:dev May 15, 2020
@jma jma deleted the maj-simple-query branch July 14, 2020 04:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zannkukai zannkukai zannkukai approved these changes

@sebdeleze sebdeleze sebdeleze approved these changes

@iGormilhit iGormilhit iGormilhit approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jma @iGormilhit @sebdeleze @zannkukai