-
Notifications
You must be signed in to change notification settings - Fork 32
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
[DH] Metadata-Quality widget update #741
Conversation
Affected libs:
|
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!! I think the customization options for the quality will be brought back later on but in a more flexible way. For now this should be more than enough.
Do you think it's possible to add an end-to-end test for the quality indicator? There's already a mechanism in the datahub e2e tests to mock a configuration, it could be worthwile to mock a config with the md quality enabled and check whether the widgets show up and the sorting work. This might necessitate running the pipelines on the support services docker compo as well.
This can be done in another PR though.
fix: CI fix: CI fix: CI add pipeline fix: CI add pipeline
1130470
to
0192cd9
Compare
Thanks for your review @jahow. I've added a test but not documentation by now. Tests are now covering approximatively :
|
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.
Added a comment on the docker composition, thanks for adding an e2e test!!
support-services/docker-compose.yml
Outdated
ES_HOST: http://elasticsearch:9200 | ||
RECORDS_INDEX: gn-records | ||
depends_on: | ||
init: |
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.
Do you think you could invert the dependency, and make init
depend on this? init
will do a reindexation of the catalog so that would mean the md quality widget would show up right away without needing a manual indexation
|
||
describe('metadata quality widget enabled', () => { | ||
beforeEach(() => { | ||
// this will enable spatial filtering |
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.
// this will enable spatial filtering | |
// this will enable metadata quality |
it('should reindex records', () => { | ||
cy.login('admin', 'admin', false) | ||
cy.visit( | ||
`http://localhost:8080/geonetwork/srv/eng/admin.console#/tools` | ||
) | ||
cy.intercept({ | ||
method: 'PUT', | ||
url: 'http://localhost:8080/geonetwork/srv/api/site/index?reset=false&asynchronous=true', | ||
}).as('indexingRecordsXHR') | ||
cy.get('[class=panel-body]').find('button').first().click() | ||
cy.wait('@indexingRecordsXHR') | ||
}) |
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.
See my comment on the docker composition; also this check here is unfortunately not enough to guarantee that all records are indeed indexed correctly, which is why I added a more robust logic in the init container. See:
# finally check that the index has records in it | |
recordsCount=0 | |
while [ "$recordsCount" = '0' ]; | |
do | |
response=$( | |
curl -s "http://$host/geonetwork/srv/api/search/records/_search" \ | |
-H 'Accept: application/json, text/plain, */*' \ | |
-H 'Content-Type: application/json;charset=UTF-8' \ | |
-H "Cookie: JSESSIONID=$jsessionid; XSRF-TOKEN=$xsrf_token" \ | |
-H "X-XSRF-TOKEN: $xsrf_token" \ | |
--data-raw '{"size":0}' | |
) | |
recordsCount=$(echo $response | sed 's/.*"hits":{"total":{"value":\([0-9]\+\).*/\1/g') | |
echo "Records found: $recordsCount" | |
sleep 2 | |
done |
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.
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.
If I set a
reset=false
query param, it doesn't work (400 on datahub). Any idea ?
Maybe that's because the index wasn't created yet? I'm not sure. You can try using reset=false&asynchronous=true
since is what you did in the e2e test.
edit: just tried and indeed, indexing with reset=false
doesn't work because then all search requests in the datahub will fail, probably because the index is misconfigured.
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.
I pushed a commit trying to address that, let's see if the CI succeeds
9d92082
to
f18229c
Compare
…cords Also adapts datasets e2e to run without indexing again
f18229c
to
3888a57
Compare
Metadata Quality widget simplifier
Removes config parameters for Metadata Quality widget.
Changes
SORTABLE
,DISPLAY_WIDGET_IN_DETAIL
,DISPLAY_WIDGET_IN_SEARCH
,DISPLAY_TITLE
,DISPLAY_DESCRIPTION
,DISPLAY_TOPIC
,DISPLAY_KEYWORDS
,DISPLAY_LEGAL_CONSTRAINTS
,DISPLAY_CONTACT
,DISPLAY_UPDATE_FREQUENCY
,DISPLAY_ORGANISATION
from Toml configuration fileTODO