Orcid queue

[LucaGiamminonni]

References

Includes #1618
Requires backend PR: DSpace/DSpace#8266

This PR add the orcid queue to force synchronization with ORCID in the orcid page.

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

• My PR passes TSLint validation using yarn run lint
• My PR doesn't introduce circular dependencies
• My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
• My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
• If my PR includes new, third-party dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.

[lgtm-com]

This pull request introduces 6 alerts when merging 95b704f into 7278b15 - view on LGTM.com

new alerts:

• 6 for Unused variable, import, function or class

[lgtm-com]

This pull request introduces 13 alerts when merging 49e2a5b into 37ebe25 - view on LGTM.com

new alerts:

• 13 for Unused variable, import, function or class

[lgtm-com]

This pull request introduces 15 alerts when merging 983db9c into dd6ff7f - view on LGTM.com

new alerts:

• 15 for Unused variable, import, function or class

[lgtm-com]

This pull request introduces 4 alerts when merging 529384a into 8591727 - view on LGTM.com

new alerts:

• 4 for Unused variable, import, function or class

[lgtm-com]

This pull request introduces 4 alerts when merging e04404b into 41bdd62 - view on LGTM.com

new alerts:

• 4 for Unused variable, import, function or class

[lgtm-com]

This pull request introduces 4 alerts when merging 9850174 into 50f9688 - view on LGTM.com

new alerts:

• 4 for Unused variable, import, function or class

[artlowel]

You can find a compare view showing only the changes in this PR here

[artlowel]

Thanks @LucaGiamminonni!

Syncing Publications

If add relationships to publications to my Person entity, they show up in the queue section on the orcid settings page, however I can't seem to send those to orcid afterwards, either manual or using the script.

In manual mode I get a 422 on the POST to /orcidhistories with the message: Errors occurs during ORCID object validation. Error codes: type.required. The script output also contains that same message for every publication.

Unlinking Publications

If I click the red unlink button next to a publication in the queue it is removed from that list. However, perhaps it would be a better idea to leave it there, simply with that button highlighted? Because if you accidentally click that button there doesn't seem to be a way to undo it, the publication is gone forever.

Syncing funding data

The info added in #1618 to the "funding preferences" box says that it will send related Project entities, but the Projects linked to my Person entity don't seem to show up in the queue. I also tried adding some related OrgUnits, but those don't show up either.

Syncing profile data

Could you provide some more information on how the "Profile Preferences" sync works?
Which exact metadata fields (or relationships) are meant by "Biographical data"? By default it doesn't seem (the person metadata schema)[https://demo7.dspace.org/admin/registries/metadata/person] has a field for a biography. There are many person.identifier fields which of those would be synced, and which wouldn't?

None of these "Profile Preferences" properties seem to show up in the queue section, so how would the sync work? Only with the backend script? or is it possible to sync these manually as well?

[artlowel]

The purpose of the @dataService annotation is to indicate to the linkservice that this service has findById and findAllById methods for objects of given type.

After #742 is fixed this will no longer be possible but for now we need enforce this behavior manually.

I'd add those find methods to it and rename the service to OrcidHistoryDataService. The alternative is to leave out the @dataservice annotation, but that would mean we don't have a dataservice for these types of objects, so if it's needed later (e.g. to resolve links on other objects) it would have to be created then.

[artlowel]

Same comment. Please add findById and findAllById methods for objects with type ORCID_QUEUE and rename the service to OrcidQueueDataService, or remove the annotation

[atarix83]

@artlowel @tdonohue

we have updated both this and rest PR in order to refresh the queue when the preferences are changed. This means that also publication added when the option is disable should appear in the queue once the synchronization option is enabled.

Here a couple of videos on how it should work

Giuseppe.Digilio.mp4

Giuseppe.Digilio.1.mp4

[tdonohue]

@LucaGiamminonni and @atarix83 : I've gotten further in testing, and I'm now able to get the Queue to populate & I can get basic things synced to ORCID. So, the basics work.

However, I find the usability of this feature to be very bad. I'm having a great amount of difficulty figuring out how to resolve validation errors -- the only way to figure out a solution is to dig into the code (which is not something a normal user can do).

Here's a few examples:

1. Initially, I created a Profile and added the person.country = "United States". I saw the change appear in my queue, so I tried syncing it to ORCID. But, I got a validation error saying Invalid ISO 3611 country. There's no such thing as an ISO 3611 country code ...but I know you mean ISO 3166 (This is a bug in the error message)
   • So, I looked up my ISO 3166 country code. I changed "United States" to "USA". But, I got the same error.
   • So, then I realized there's both a 2-digit code and a 3-digit code. The two digit code is "US", so I tried that and it finally worked.
   • POSSIBLE FIX: If this is always a 2-digit code, then we should update the error to say Invalid ISO 3166 2-digit country code (or similar)
2. Then I created a new relationship of an existing Publication to my Profile. I saw that change also appear in my queue. I tried syncing it to ORCID, but I got a validation error saying The type is required.
   • This error message is not useful at all. I couldn't figure out what "type" it was talking about as my Entity already has a type which is "Publication". So I had to go looking in the code. It appears that a Publication cannot be synced without a dc.type metadata field (and if you have any non-standard value there then you'll need to update the mapping in mapConverter-dspace-to-orcide-publication-type.properties or it will still fail).
   • I went back and edited the Publication to add a dc.type = Book and tried to sync it again.
   • This time, I got a different error saying "The submission to ORCID failed because the resource sent to ORCID registry is not valid."
   • However, no information was provided as to what was wrong. I see nothing in the dspace.log, and the item still appears in the Queue.
   • If I look in the database, I see that the orcid_history table has the failure details. It shows this detailed exception:

     400 Bad Request: There is an issue with your data or the API endpoint. 
     405 Method Not Allowed: endpoint and method mismatch. 
     415 Unsupported Media Type: data must be in XML or JSON format. 
     Full validation error: java.xml.bind.UnmarshalException
     [org.xml.sax.SAXParseException; lineNumber: 8; columnNumber: 40; cvc-minInclusive-valid: 
     Value '1838' is not facet-valid with respect to minInclusive '1900' for type 'year'.] 
     (cvc-minInclusive-valid: Value '1838' is not facet-valid with respect to minInclusive '1900' for type 'year'.)
     
     ORCID could not process the data, because they were invalid
     

   • This information appears nowhere in the UI or in the dspace.log, so it's difficult to locate. (This appears to be a bug, because the error is impossible to debug without database level access.)
   • In this case though the error is correct, this Publication had a date of 1838, which appears to be invalid with ORCID. So, I went back and edited the date to be 1938.
   • After changing the invalid date, I was able to successfully sync this Publication to my ORCID profile.

These two examples tell me that the code overall works, but it's very difficult to diagnose synchronization failures. We need to find a way to:

1. Provide more useful error messages to let users know how to resolve the issue. So, for example, the error "The type is required" is not useful at all. It should say something like "The required 'dc.type' field is either missing or invalid."
2. Provide some way to share the ORCID error information that ends up in orcid_history database table. That message is the only way I was able to figure out the second error. This error should probably appear in dspace.log (I couldn't find it there) at a minimum. Ideally, if there's some way to share it in the UI that'd also be nice. (Currently, though, it appears the information in orcid_history is not visible in the UI at all)

[atarix83]

@tdonohue

We agree that the problems reported could be confusing from the user prospective and that the usability need to be improved.

• regarding the problem with the country code and dc.type we have changed the messages as you suggested
• regarding the problem with the date, we're aware that the UX is not ideal due to those validation errors. We tried to handle the most common ones, but we missed this one. We're going to manage this case with a specific error code. Unfortunately, the message coming from ORCID is very technical, cannot be translated and most of time not comprehensible to end users. We'll add to the message to the dspace log file other than in the DB in order to make it more accessible.

Generally speaking validation problems are also due to the fact the the user not always is guided in entering metadata like in the edit item page where no checks are done. For these reasons we have an extra feature in DSpace-CRIS that allow editing of existing item in a submission like fashion, it was also proposed to be ported to DSpace DSpace/DSpace#2876 it could be a better solution on the long run

[artlowel]

I tested this again and I had similar issues getting the sync to work to the ones @tdonohue describes, so I gotta say I agree with him: it works in theory, but you're going to need a lot of trial and error to get it to work in practice.

I'm also not sure a submission-like admin item edit page will suffice. One of the problems both @tdonohue and I had was that dc.type wasn't a value recognized by ORCID. But the types I used were simply options from our common_types valuepairs used in the default submission form. So perhaps it's not a bad idea to include a mapping of dspace's types to ORCID's types by default

[tdonohue]

@artlowel : Just a quick note. There is an existing mapping of DSpace "dc.type" values to ORCID types in the new dspace/config/crosswalks/orcid/mapConverter-dspace-to-orcid-publication-type.properties file in DSpace/DSpace#8266 I just double checked and it looks like that mapping file now includes all the default common_types (I think it might have been missing a few the last time I tested).

In any case, I'll test this again today and see if it's working better based on the recent backend updates.

UPDATE: After testing again, I see the dc.type usability issue is "resolved". If the dc.type is missing or not in the mapping, then it is now sent to ORCID as "Other" (since mapping cannot be determined) instead of throwing an odd error to the user.

[atarix83]

@artlowel

as said by @tdonohue orcid type mapping has been updated with the types used in dspace, so probably you're runninig over an old version of the REST PR please could you try again with latest code?

[tdonohue]

👍 Thanks @LucaGiamminonni and @atarix83 . I tested this again today and was able to successfully get a Publication , Person information and a Project to all sync to the ORCID Sandbox.

I also found that there were some minor usability enhancements since I last tested (on Friday). So, for example, if the dc.type field is missing in a Publication or not in the mapping, then ORCID is sent a type of "Other" (instead of throwing a validation error back to the user). I also found that when a validation error does occur, the details now also appear in the backend's logs (dspace.log) which is an improvement.

These improvements are great, but I still feel that we could do better to improve the behavior whenever you hit a validation error. That said, I think we can workaround that with some detailed documentation & improve it in 7.4

Here's a few examples that I stumbled upon in my most recent testing:

• To Sync a DSpace Project (to a Funding in ORCID)
  • Project MUST be linked to an OrgUnit in DSpace
  • Linked OrgUnit MUST have an organization.address.addressCountry which corresponds to a 2-digit country code (e.g. US, BA, etc)
  • Linked OrgUnit MUST have a valid orgunit identifier. The easiest way to "fake" one is to add organization.identifier.rin = 12345.

There are likely additional requirements that I haven't stumbled on yet, but this is just to point out that while syncing works for both Publications and Projects there are some unwritten requirements for each to work. We MUST make sure to add those requirements to documentation until we are able to provide better usability to users.

Overall though, because this works (when requirements are met), I feel we should move this forward for 7.3. Thanks again @LucaGiamminonni and @atarix83 for your hard work on this feature.

[tdonohue]

@artlowel : When you have a chance tomorrow, I'd like you to verify that your feedback has been addressed. Please consider writing up any usability suggestions you have as comments here & we can make sure they are created as follow-up tickets and/or documentation.

[artlowel]

as said by @tdonohue orcid type mapping has been updated with the types used in dspace, so probably you're runninig over an old version of the DSpace/DSpace#8266 please could you try again with latest code?

You're right, that's no longer an issue with the latest version of the backend code.

My main bits of feedback that haven't been addressed so far are:

Unlinking Publications

If I click the red unlink button next to a publication in the queue it is removed from that list. However, perhaps it would be a better idea to leave it there, simply with that button highlighted? Because if you accidentally click that button there doesn't seem to be a way to undo it, the publication is gone forever.

As well as the feedback I had on the original PR that it would be good to have a way to reach the settings page from somewhere other than that button on the Person entity page. E.g. a button in the "Researcher Profile" section of the Profile page

But I'm ok with addressing those in a later PR

[tdonohue]

Merging as this and the frontend PR are at +2. Future updates (especially to usability) will be created as follow-up tickets to work on for 7.4. (I'll create a few tickets based on the usability feedback in this PR)