-
Notifications
You must be signed in to change notification settings - Fork 492
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 : Adding hidden metadata fields (Ontoportal integration) #10503
CVOC : Adding hidden metadata fields (Ontoportal integration) #10503
Conversation
|
||
#### Hidden HTML Fields | ||
|
||
To enlarge javascript possibilities of [:CVocConf](https://guides.dataverse.org/en/6.3/installation/config.html#cvocconf) configured service, the child fields of the configured field had been added to the dataset metadata view. |
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.
Suggested change: External Controlled Vocabulary scripts, configured via :CVocConf, can now access the values of managed fields as well as the term-uri-field for use in constructing the metadata view for a 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.
Suggested change accepted and commit
<f:passThroughAttribute name="hidden" value="hidden" /> | ||
<f:passThroughAttribute name="data-cvoc-metadata-name" | ||
value="#{cvPart.key.datasetFieldType.name}" /> | ||
</h:outputText> |
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 think this works. A possible alternate design would be to use a data-cvoc-managed-fields element in the term uri span that has a json object with managed field names: values. That might be more SPA friendly. Not sure how how much effort it might take on the Java side to construct the value or whether changing is worth the effort at this point. @luddaniel - any thoughts?
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.
In order to avoid introducing any blocking changes, we have opted to add hidden metadata. So existing Javascript files like skosmos.js or ror.js don't have to be modified to keep working.
I imagine that for SPA, metadata will be retrieved via API, probably using an endpoint providing the JSON representation of a datasetversion (based on its ID ?). But maybe I'm wrong ?
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 don't think there's any backward compatibility issue in either design - the old scripts are either ignoring extra hidden DOM elements or extra attributes in the DOM element they normally look at.
My point about SPA-friendly may be incorrect - I was assuming, like I think you are, that the SPA would be getting the list of fields/values via some API, as JSON, and then guessing that dumping the JSON for the necessary fields into an attribute would be less work than generating more DOM elements. That may or may not be the case. I wouldn't be surprised if the SPA needs breaking changes in the cvoc mechanism anyway, so this could be something we discuss when the SPA cvoc design starts.
Overall, I'm raising this for two reasons - one is just to write down the possible alternative somewhere, and two is to see if you thought this was worth making a change now. If not (doesn't sound like it) I don't think it's a big enough issue to hold up the PR/ go discuss in a larger group, etc., so I can go ahead and approve the PR as is.
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.
Hello @qqmyers , we discussed with @jeromeroucou to understand the depth of your point, here are our views :
- There is no simple path to anticipate the idea of preparing key/values fetching of cvoc metadata (most of it is done in JSF, cvoc is a static setting, fields are dynamics and can be repeatables, Get JSON Representation of a Dataset requires ATM API Token so it cannot be use by javascript for now...)
- We agree to treat this out of this PR
- We agree with you that we must anticipate SPA portability (we discused about it in this meeting https://groups.google.com/g/dataverse-community/c/4eTd3kfJeBU/m/fPQix_bUAAAJ) and I have mentionned it here #9276#issuecomment-1830172630 Import subject : what will happen of CVOC feature under SPA Dataverse architecture ? but no one seems to have created the related issue in https://github.com/IQSS/dataverse-frontend
- First idea we have got was to add in Get JSON Representation of a Dataset endpoint the id of the related cvoc conf to the field when applicable then offer an new endpoint to fetch cvoc details of the field... We could take time to specify this before the SPA version is released.
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.
FWIW: My initial thought would just be to have a get DatasetPage.getCvocManagedFields(DatasetField dsf) that would get called on line 160 and generate the serialized json for the data-cvoc-managedfields attribute. There shouldn't be any problem navigating from the dsf to the required values for the managed child fields in Java - I agree it would be hard to generate it in the JSF. How the SPA would assemble the info is tbd, but it seems like a cleaner solution to avoid extra hidden fields that have to be generated and that the Ontoportal JS has to go find.
In any case, not too hard to add to the attribute at some point and deprecate the hidden fields if they are used in many scripts.
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.
Following agreements on next steps of #10145
What this PR does / why we need it:
Dataset metadata view of a CVOC field is by default only
term-uri-field
value of theCVocConf:
Historically, this field is an idenfier (ORCID ID or skomos permalink), allowing to reconstruct data depending on user language (ex. skosmos.js#L31) while loading the related javascript file (
js-url
) ofCVocConf:
.Ontoportal uses 2 fields to be able to reconstruct the data. In order to minimize code modifications, we decided to add missing child fields as hidden
<span>
.HTML Before PR :
HTML After PR :
Allowing to do this in Javascript :
Full javascript : ontoportal.js (POC)
Suggestions on how to test this:
Check html fields on Dataset metadata view using
CVocConf:
that usesmanaged-fields
.Does this PR introduce a user interface change?
No