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

CVOC : add HTTP request headers (Ontoportal integration) #10331

Merged
merged 8 commits into from
Apr 17, 2024

Conversation

jeromeroucou
Copy link
Contributor

@jeromeroucou jeromeroucou commented Feb 20, 2024

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, with header customization :

"headers": {
    "Accept": "application/xml",
    "Cache-Control": "no-cache"
}

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 if input[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:

@coveralls
Copy link

Coverage Status

coverage: 20.274% (-0.002%) from 20.276%
when pulling 61ecb32 on Recherche-Data-Gouv:10316_cvoc_http_headers
into 82585f9 on IQSS:develop.

@pdurbin pdurbin added the Size: 3 A percentage of a sprint. 2.1 hours. label Feb 28, 2024
@pdurbin pdurbin changed the title CVOC : add HTTP request headers CVOC : add HTTP request headers (Ontoportal integration) Feb 28, 2024
Copy link
Member

@qqmyers qqmyers left a 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.

@pdurbin pdurbin self-assigned this Apr 16, 2024
@pdurbin
Copy link
Member

pdurbin commented Apr 16, 2024

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.

@pdurbin
Copy link
Member

pdurbin commented Apr 16, 2024

@jeromeroucou I don't have permission to push to your branch:

$ git push Recherche-Data-Gouv 10316_cvoc_http_headers
ERROR: Permission to Recherche-Data-Gouv/dataverse.git denied to pdurbin.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

Can you please...

@pdurbin pdurbin assigned jeromeroucou and unassigned pdurbin Apr 16, 2024
@qqmyers
Copy link
Member

qqmyers commented Apr 16, 2024

@qqmyers
Copy link
Member

qqmyers commented Apr 16, 2024

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.

@pdurbin
Copy link
Member

pdurbin commented Apr 16, 2024

@jeromeroucou no pressure, but there's a box you can check to allow edits:

Screenshot 2024-04-16 at 5 06 31 PM

@jeromeroucou
Copy link
Contributor Author

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 dataverse project on Recherche-Data-Gouv organization.

@pdurbin
Copy link
Member

pdurbin commented Apr 17, 2024

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.

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.

@qqmyers
Copy link
Member

qqmyers commented Apr 17, 2024

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.

@pdurbin
Copy link
Member

pdurbin commented Apr 17, 2024

@qqmyers I would suggest making a PR into this PR, like I did:

@pdurbin pdurbin self-assigned this Apr 17, 2024
rewrite :CVocConf docs, explain where to find readme.md
@qqmyers
Copy link
Member

qqmyers commented Apr 17, 2024

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.

@pdurbin
Copy link
Member

pdurbin commented Apr 17, 2024

@qqmyers I had to hack on the URL like this:

Recherche-Data-Gouv/dataverse@10316_cvoc_http_headers...IQSS:dataverse:10316-docs

@qqmyers
Copy link
Member

qqmyers commented Apr 17, 2024

OK - that worked - I was able to make PR #5. Thanks!

…voc_http_headers_QA_changes

10316 cvoc http headers qa changes
@pdurbin
Copy link
Member

pdurbin commented Apr 17, 2024

I configured the following:

[
    {
        "headers": {
        "Accept": "application/json",
            "Authorization": "Basic YWxhZGRpbjpvcGVuc2VzYW1l"
        },
        "field-name": "authorAffiliation",
        "term-uri-field": "authorAffiliation",
        "js-url": ["/logos/cvoc/js/ror.js","/logos/cvoc/js/cvocutils.js"],
        "protocol": "ror",
        "retrieval-uri": "https://api.ror.org/organizations/{0}",
        "allow-free-text": true,
        "prefix": "https://ror.org/",
        "managed-fields": {},
        "languages":"",
        "vocabs": {
            "rors": {
                "uriSpace": "https://ror.org/"
            }
        },
        "retrieval-filtering": {
            "@context": {
                "termName": "https://schema.org/name",
                "scheme": "http://www.w3.org/2004/02/skos/core#inScheme",
                "lang": "@language",
                "content": "@value"
            },
            "scheme": {
                "pattern": "http://www.grid.ac/ontology/"
            },
            "termName": {
                "pattern": "{0}",
                "params": ["/name"]
            },
            "@type": {
                "pattern": "https://schema.org/Organization"
            }
        }
    }
]

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:

payara@dataverse:/dv/docroot$ find /dv/docroot/logos
/dv/docroot/logos
/dv/docroot/logos/temp
/dv/docroot/logos/cvoc
/dv/docroot/logos/cvoc/js
/dv/docroot/logos/cvoc/js/cvocutils.js
/dv/docroot/logos/cvoc/js/ror.js
/dv/docroot/logos/1
/dv/docroot/logos/1/9568-homer.png

Lookup in ROR for author affiliation works fine and the headers seem to be present in the three places I was asked to check:

Screenshot 2024-04-17 at 4 50 50 PM
Screenshot 2024-04-17 at 4 50 31 PM
Screenshot 2024-04-17 at 4 49 22 PM

Merging.

@pdurbin pdurbin merged commit e6b2661 into IQSS:develop Apr 17, 2024
12 checks passed
@pdurbin pdurbin added this to the 6.3 milestone Apr 17, 2024
@pdurbin pdurbin removed their assignment Apr 17, 2024
@jeromeroucou jeromeroucou deleted the 10316_cvoc_http_headers branch April 22, 2024 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Size: 3 A percentage of a sprint. 2.1 hours.
Projects
Status: 🚀 Done (Recherche Data Gouv)
Development

Successfully merging this pull request may close these issues.

Feature Request/Idea: Add new parameter "headers" into :CVocConf
4 participants