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.

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

search: add nested aggregations support #157

Merged
merged 1 commit into from
Apr 8, 2020

Conversation

sebdeleze
Copy link

@sebdeleze sebdeleze commented Mar 31, 2020

  • Adds nested aggregations support.
  • Refactors RecordSearchComponent.
  • Unsubscribes from observables when components are destroyed.
  • Creates a RecordSearchService for handling aggregations filters.
  • Adapts menu component in order to include query parameters.
  • Sets aggregations buckets size to 8 in tester application.
  • Fixes "combineLatest" RxJS function deprecation warnings.
  • Exports RecordSearchService in public-api to use it outside ng-core.
  • Initializes filters to launch first search on homepage.

Co-Authored-by: Alicia Zangger alicia.zangger@rero.ch
Co-Authored-by: Johnny Mariéthoz Johnny.Mariethoz@rero.ch
Co-Authored-by: Sébastien Délèze sebastien.deleze@rero.ch

@sebdeleze sebdeleze added the wip label Mar 31, 2020
@sebdeleze sebdeleze force-pushed the sed-search-aggregation-filters branch 2 times, most recently from 35f5012 to 9b036e3 Compare March 31, 2020 19:11
@sebdeleze sebdeleze removed the wip label Mar 31, 2020
@sebdeleze
Copy link
Author

sebdeleze commented Mar 31, 2020

@AoNoOokami I didn't have a real example of hierarchical facets. Could you test this PR on your side?

@sebdeleze sebdeleze requested review from jma and AoNoOokami March 31, 2020 19:18
@sebdeleze sebdeleze added the wip label Apr 1, 2020
@sebdeleze sebdeleze force-pushed the sed-search-aggregation-filters branch from ca5b48c to 051cc2d Compare April 1, 2020 06:11
Copy link
Contributor

@jma jma left a comment

Choose a reason for hiding this comment

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

please check spelling in you commit message: at lease improvments. In any case I would rename this PR as search: add nested aggregations support

@sebdeleze sebdeleze force-pushed the sed-search-aggregation-filters branch from 051cc2d to 089c133 Compare April 1, 2020 14:56
@sebdeleze sebdeleze changed the title search: aggregations improvments search: add nested aggregations support Apr 1, 2020
@sebdeleze sebdeleze force-pushed the sed-search-aggregation-filters branch from 089c133 to 0962562 Compare April 1, 2020 18:58
@sebdeleze sebdeleze removed the request for review from AoNoOokami April 1, 2020 19:00
@sebdeleze sebdeleze removed the wip label Apr 1, 2020
@sebdeleze sebdeleze requested a review from jma April 1, 2020 19:01
@sebdeleze sebdeleze force-pushed the sed-search-aggregation-filters branch 2 times, most recently from 78181dd to e70e94d Compare April 6, 2020 12:13
@sebdeleze
Copy link
Author

@jma Everything should be resolved, there are just a few points left for which I have left comments.

@sebdeleze sebdeleze force-pushed the sed-search-aggregation-filters branch 2 times, most recently from db1571a to 71c5d18 Compare April 6, 2020 18:40
@sebdeleze sebdeleze requested a review from jma April 6, 2020 19:08
@sebdeleze sebdeleze force-pushed the sed-search-aggregation-filters branch from 71c5d18 to cdf8279 Compare April 7, 2020 07:26
@sebdeleze sebdeleze requested review from AoNoOokami and jma and removed request for jma April 7, 2020 07:26
* Adds nested aggregations support.
* Refactors RecordSearchComponent.
* Unsubscribes from observables when components are destroyed.
* Creates a RecordSearchService for handling aggregations filters.
* Adapts menu component in order to include query parameters.
* Sets aggregations buckets size to 8 in tester application.
* Fixes "combineLatest" RxJS function deprecation warnings.
* Exports RecordSearchService in public-api to use it outside ng-core.
* Initializes filters to launch first search on homepage.

Co-Authored-by: Alicia Zangger alicia.zangger@rero.ch
Co-Authored-by: Johnny Mariéthoz Johnny.Mariethoz@rero.ch
Co-Authored-by: Sébastien Délèze sebastien.deleze@rero.ch
@sebdeleze sebdeleze force-pushed the sed-search-aggregation-filters branch from cdf8279 to fd0fd70 Compare April 7, 2020 08:52
Copy link

@AoNoOokami AoNoOokami left a comment

Choose a reason for hiding this comment

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

Tested with rero/rero-ils#852 and rero/rero-ils-ui#208, on both public and admin views.

@sebdeleze sebdeleze merged commit 0d2d688 into dev Apr 8, 2020
@sebdeleze sebdeleze deleted the sed-search-aggregation-filters branch April 8, 2020 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants