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: implement advanced search #569

Merged
merged 1 commit into from
May 17, 2021
Merged

search: implement advanced search #569

merged 1 commit into from
May 17, 2021

Conversation

Garfield-fr
Copy link
Contributor

@Garfield-fr Garfield-fr commented Apr 21, 2021 •
edited
Loading

Co-Authored-by: Bertrand Zuchuat [email protected]

Why are you opening this PR?

Dependencies

My PR depends on PR(s):

How to test?

  • Test advanced and simple search on some views

Examples

  • documents: identifiedBy.value:9782412052716
  • documents: identifiedBy.type:bf\:Isbn AND identifiedBy.value:9782412052716
  • patron: city:Martigny
  • vendors: name:fnac
  • organisation: gnd.preferred_name:Deputazione

Code review check list

  • Commit message template compliance.
  • Commit message without typos.
  • File names.
  • Functions names.
  • Functions docstrings.
  • Unnecessary commited files?
  • Cypress tests successful?

@iGormilhit iGormilhit changed the title search filter: implement advanced search on brief view search: implement advanced search on brief view Apr 27, 2021
@iGormilhit iGormilhit added f: professional ui Professional interface f: search labels Apr 27, 2021
@iGormilhit iGormilhit added this to the v1.3.0 milestone Apr 27, 2021
@Garfield-fr Garfield-fr requested a review from zannkukai April 27, 2021 11:47
iGormilhit
iGormilhit approved these changes Apr 28, 2021
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 proposition:

search: implement advanced search

* Updates routes to add an advanced search filter.
* Fixes the number of parameters for the component that implements
  RecordSearchPage.
* Closes rero/rero-ils#1824.

@iGormilhit iGormilhit changed the title search: implement advanced search on brief view search: implement advanced search Apr 28, 2021
@Garfield-fr Garfield-fr force-pushed the zub-search-filter branch 3 times, most recently from 49efb31 to e5ba32f Compare April 29, 2021 09:27
@pronguen pronguen self-requested a review April 30, 2021 11:59
pronguen
pronguen reviewed Apr 30, 2021
View reviewed changes
Copy link
Contributor

@pronguen pronguen left a comment

Choose a reason for hiding this comment

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

I didn't tested it in detail but it worked well 👍

Only one comment about wording

  • "Advanced Search" should be renamed in "Expert search" (because in the future we will have a real advanced search as in RERO Explore, so we need two distinct names)
  • Don't put a capital letter to search: "Expert search"

benerken
benerken reviewed Apr 30, 2021
View reviewed changes
Copy link
Contributor

@benerken benerken left a comment •
edited by lauren-d
Loading

Choose a reason for hiding this comment

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

  • (Not blocking) The "expert search" is usable if you know the keys for your search.
    The librarian needs an easy and quick way to access the list of keys.

  • (Not blocking) increase the size of the search box when the librarian is in the documents.

@Garfield-fr Garfield-fr force-pushed the zub-search-filter branch from e5ba32f to 08bce0a Compare May 4, 2021 06:21
@Garfield-fr
Copy link
Contributor Author

  • "Advanced Search" should be renamed in "Expert search" (because in the future we will have a real advanced search as in RERO Explore, so we need two distinct names)
  • Don't put a capital letter to search: "Expert search"

Fixed

@jma jma force-pushed the zub-search-filter branch 4 times, most recently from 994830b to 2a23197 Compare May 17, 2021 10:04
@Garfield-fr @jma
search filter: implement advanced search on brief view
3a4666f 
* Updates routes to add advanced search filter.
* Fixes number of parameters for component implement RecordSearchPage.
* Closes rero/rero-ils#1824.
* Uses ng-core version 1.8.x.
* Fixes tests.
* Adds a new karma package to display the test name in the console:
  `karma-spec-reporter`.

Co-Authored-by: Bertrand Zuchuat <[email protected]>
@jma jma force-pushed the zub-search-filter branch from 2a23197 to 3a4666f Compare May 17, 2021 10:27
@iGormilhit iGormilhit merged commit 086c897 into rero:dev May 17, 2021
@Garfield-fr Garfield-fr deleted the zub-search-filter branch June 15, 2021 04:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pronguen pronguen pronguen left review comments

@benerken benerken benerken approved these changes

@iGormilhit iGormilhit iGormilhit approved these changes

@zannkukai zannkukai zannkukai approved these changes

@lauren-d lauren-d Awaiting requested review from lauren-d

Assignees

@Garfield-fr Garfield-fr

Labels
f: professional ui Professional interface f: search
Projects
None yet
Milestone
v1.3.0
Development

Successfully merging this pull request may close these issues.

Toggle in each brief view to enable the standard query instead of the simple query
5 participants
@Garfield-fr @iGormilhit @pronguen @zannkukai @benerken