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

Topic search refactor #603

Merged
merged 38 commits into from
Jan 4, 2021
Merged

Conversation

amanji
Copy link
Contributor

@amanji amanji commented Dec 18, 2020

This PR introduces major updates to search (v4).

Topic searches in previous versions were based on Credential attributes that were linked back to their owner Topics. This resulted in duplicate Topic results when query terms match multiple Credentials. v4 introduces Topic search endpoints that are based on Topic attributes. Topic attributes are an aggregation of their owned Credential attributes. This change will require rebuilding SOLR indexes.

Changelog:

Server

  • Introduces new search API endpoints for Topic (/v4/search/topic) and Credential (/v4/search/credential). Search API endpoints for Credential are derived from v3
  • New index fields introduced for Topic that allow searching on Name, Address and Source ID
  • Fixes issue with filtering Topic/Credential by category facets, where result set was not narrowed down by selection
  • General code cleanup

Client

  • Upgrades basic and advanced search to v4 from v3
  • Adds new facets to advanced search for Credential types
  • Upgrades autocomplete to v3 from v2, which expands autocomplete beyond Names to include Address and Source ID
  • Improves advanced search handling of URL query parameters to allow for bookmarking of queries. URL query parameters are also carried over from basic search to advanced search
  • Aligns historical facet filtering for basic and advanced search
  • General code cleanup

@amanji amanji requested review from ianco and esune December 18, 2020 18:15
@amanji amanji linked an issue Dec 21, 2020 that may be closed by this pull request
server/vcr-server/api/v3/serializers/search.py Outdated Show resolved Hide resolved
server/vcr-server/api/v3/views/rest.py Outdated Show resolved Hide resolved
@ianco
Copy link
Contributor

ianco commented Dec 24, 2020

Everything looking good so far - tested with the demo issuer/controller and bc registries issuer.

Only issue is that search by BC business number (e.g. BC1234567) will show duplicates in the search results.

@amanji
Copy link
Contributor Author

amanji commented Dec 24, 2020

Everything looking good so far - tested with the demo issuer/controller and bc registries issuer.

Only issue is that search by BC business number (e.g. BC1234567) will show duplicates in the search results.

Adding updates to client-side code in this PR

esune
esune previously approved these changes Jan 4, 2021
Copy link
Member

@esune esune left a comment

Choose a reason for hiding this comment

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

Changes look good. I have however not tried running them locally, let me know if you need me to do so to confirm.

@amanji
Copy link
Contributor Author

amanji commented Jan 4, 2021

Changes look good. I have however not tried running them locally, let me know if you need me to do so to confirm.

Might be worthwhile, although I know @ianco was running some local tests as well.

@ianco
Copy link
Contributor

ianco commented Jan 4, 2021

Changes look good. I have however not tried running them locally, let me know if you need me to do so to confirm.

Might be worthwhile, although I know @ianco was running some local tests as well.

I'm testing now ...

@ianco
Copy link
Contributor

ianco commented Jan 4, 2021

Everything looking good so far - tested with the demo issuer/controller and bc registries issuer.
Only issue is that search by BC business number (e.g. BC1234567) will show duplicates in the search results.

Adding updates to client-side code in this PR

I'm still seeing the same behaviour - duplicated results when searching on BC Business Number (e.g. BC1234567)

@ianco ianco merged commit f8fec1c into bcgov:master Jan 4, 2021
@amanji amanji deleted the topic-search-refactor branch January 5, 2021 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants