-
Notifications
You must be signed in to change notification settings - Fork 0
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
[Dataset] [Search] Quick search on click territory tag #118
Conversation
93cc90e
to
7a2e4ac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @AlitaBernachot, your solution looks and works well ! Just left some minor comments on suggestions or things you may have forgotten.
I think you also need to run npx nx format
and/or configure your IDE to format on save, which should eventually make the CI pass.
apps/datahub/src/app/dataset/dataset-information/dataset-information.component.ts
Outdated
Show resolved
Hide resolved
apps/datahub/src/app/dataset/dataset-information/dataset-information.component.ts
Show resolved
Hide resolved
7a2e4ac
to
c7a7148
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Thanks for the adaptions @AlitaBernachot !
(req) => { | ||
if ( | ||
req.body.query?.ids?.values.includes( | ||
'775b7660-b03b-443a-9817-be82ecd0ef07' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for my understanding, why is this condition actually needed, as the command is currently only called for this dataset?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops I forggot to make it generic, I updated to:
if (req.body.query?.ids?.values.includes(id))
The condition is needed because there is no other way to identify the request dedicated to the dataset info since /geonetwork/srv/api/search/records/_search?bucket=bucket
is also used to populate filters, queryscore, etc...
if there was a route like /geonetwork/srv/api/search/records/dataset/775b7660-b03b-443a-9817-be82ecd0ef07
we wouldnt need to analyse the request payload, a simple intercept cy.intercept('POST',
/geonetwork/srv/api/search/records/dataset/775b7660-b03b-443a-9817-be82ecd0ef07
) would have been sufficient.
17ced4f
to
7bac8f7
Compare
console.log('query', query) | ||
console.log('keyword.key', keyword.key) | ||
console.log('keyword.key', keyword) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remember to get rid of console.log before merging ;)
This PR enables the click on "Territoire" tag, redirecting the user to the search page with checked filter on selected tag.
To enable the Territoire filter, update line in
default.toml
: