-
Notifications
You must be signed in to change notification settings - Fork 490
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
CVOC : add HTTP request headers (Ontoportal integration) #10331
CVOC : add HTTP request headers (Ontoportal integration) #10331
Conversation
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.
Looks good. Note I asked for a combined release note in #10404 about the overall changes to the CVOC syntax/mechanism. This PR is matched by a doc/schema update over in gdcc/dataverse-external-vocab-support#19.
This pull request is in need of documentation. It probably shouldn't have been approved. But I'll add what I can @jeromeroucou while I'm testing it. I'll also merge from develop to get the latest code. |
@jeromeroucou I don't have permission to push to your branch:
Can you please...
|
FWIW: There is documentation in https://github.com/gdcc/dataverse-external-vocab-support/pull/19/files |
FWIW: There is a combined release note in #10404. There is no documentation of the contents of the Cvoc conf setting in the guides as it references the repository with all of the scripts and the schema etc. |
@jeromeroucou no pressure, but there's a box you can check to allow edits: |
Hi @pdurbin I merge develop branch and add a minimal release note snippet. But I doesn't have the checkbox "Allow edits by maintainers". Is it related to this discussion ? If needed, I'll verify to allow you to push on |
A combined release note yes, but nothing at the future version of https://guides.dataverse.org/en/6.2/installation/config.html#cvocconf I'm not very happy with this situation that there is basically no documentation in the guides proper. Pull requests like this one are changing the behavior of the main app. I'm not picky about where in the guides the documentation goes. Would you prefer it in https://guides.dataverse.org/en/6.2/admin/metadatacustomization.html#using-external-vocabulary-services than https://guides.dataverse.org/en/6.2/installation/config.html#cvocconf ? @jeromeroucou thanks for merging the latest from the develop and adding a release not snippet. I think the lack of docs is the last thing (though I haven't tested anything). As for giving me push access to your entire fork, I didn't realize that's what I was asking for. I always assumed checking that box (which you don't have!) allowed me to push to that specific branch, not the entire repo. |
I have two suggested changes and I also can't see how to commit to or create a PR against your branch. f4da07d puts in a sleep to address a failing test and fa4a03c notes that adding headers is now allowed in the docs (also notes we have an ROR example script along with SkosMos and ORCID these days. |
@qqmyers I would suggest making a PR into this PR, like I did: |
rewrite :CVocConf docs, explain where to find readme.md
For some reason, Recherche-Data-Gouv isn't showing as a possible target when I try to make a PR: https://github.com/IQSS/dataverse/compare/develop...GlobalDataverseCommunityConsortium:dataverse:10316_cvoc_http_headers_QA_changes?expand=1 . I'm not sure why. |
@qqmyers I had to hack on the URL like this: Recherche-Data-Gouv/dataverse@10316_cvoc_http_headers...IQSS:dataverse:10316-docs |
OK - that worked - I was able to make PR #5. Thanks! |
…voc_http_headers_QA_changes 10316 cvoc http headers qa changes
I configured the following:
I used https://github.com/gdcc/dataverse-external-vocab-support/blob/02928cb3148143506e41c7904cdf4f934a595bad/scripts/ror.js as a starting point but added the "headers" object from this PR and put the js in the logos directory because it's served up. Here's my Docker container:
Lookup in ROR for author affiliation works fine and the headers seem to be present in the three places I was asked to check: Merging. |
What this PR does / why we need it:
Add HTTP headers customization for :CVocConf settings. This is needed for Ontoportal integration as a external vocabulary services, which required API KEY provided by HTTP request headers.
Which issue(s) this PR closes:
Closes #10316
Suggestions on how to test this:
Add
:CVocConf
to a external vocabulary service, withheader
customization :Replace
XXX
below by the external vocabulary service protocol configured.Verify if
span[data-cvoc-protocol="XXX"][data-cvoc-headers]
in advanced search page and dataset page (with metadata tab open) have the configuration previously configured. Verify ifinput[data-cvoc-protocol="XXX"][data-cvoc-headers]
in edition dataset page have also the configuration.Does this PR introduce a user interface change? If mockups are available, please link/include them here:
No
Docs: