DSpace / RestContract

REST Contract for DSpace 7-8
https://wiki.lyrasis.org/display/DSDOC8x/
37 stars 48 forks source link

[CST-5347] Researcher profile rest contract #180

Closed LucaGiamminonni closed 2 years ago

LucaGiamminonni commented 2 years ago

These changes concern the introduction of a rest repository for researcher profiles. It is part of the migration issue of ORCID from cris (https://github.com/DSpace/DSpace/issues/8157).

The researcher profile, modeled as an item of a specific type (such as Person), is associated with a specific ePerson. An ePerson can only have one profile.

tdonohue commented 2 years ago

@LucaGiamminonni : If possible, could you write up a high level summary of how a Profile is planned to be implemented? I tried to review this PR today, but I came away with a number of high level questions:

  1. How is a Profile different from a Person Entity (e.g. https://demo7.dspace.org/entities/person/b1b2c768-bda1-448a-a073-fc541e8b24d9)? Currently, we already have the basics of a Researcher Profile represented in the Person Entity. It's unclear to me how this new endpoint is related to the existing Person Entity, and why we cannot use Person Entities to represent a user's Profile.
  2. How would this Profile be represented in the database layer or Java API layer? Is the plan for it to be a separate table, or is it just a different way of representing the relationship between an EPerson object and a Person Entity?
  3. I'd also prefer not to put this under /api/cris/, as I think that'd imply it's a separate object type specific to CRIS systems. It likely should be alongside the existing EPerson or Item endpoints. We might consider whether this would be more appropriate under /api/eperson/profiles or similar...but I might be able to provide a better suggestion once I understand the design.

Overall, I'm not against the general idea of having a Researcher Profile. But, I need to better understand the proposed high-level design here before I can accurately review this REST Contract. Currently, I see a large amount of overlap with Person Entities in this REST Contract -- and I'm wanting to better understand how these two are related.

LucaGiamminonni commented 2 years ago

Hi @tdonohue,

  1. in cris the researcher profile is nothing more than a Person item (or an item with the configured type) which is linked to an eperson through authority with the cris.owner metadata. This specific endpoint allows you to do specific operations related to the researcher profile without using the more generic ItemRestRepository. The profile id coincides with the eperson owner's id (the search is done using the cris.owner metadata).
  2. the profile model does not require additional tables, it is modeled with an item anyway.
  3. yes I agree that we can change the path with /api/eperson/profiles or something similar
LucaGiamminonni commented 2 years ago

Hi @tdonohue, thanks for your feedbacks. I have updated the contract, let me know if it is ok for you now. Thanks again.

LucaGiamminonni commented 2 years ago

I changed the contract for:

abollini commented 2 years ago

Documentation about the ORCID functionalities is available in the DSpace-CRIS technical documentation pp 76-89 of pdf https://github.com/4Science/DSpace/releases/download/dspace-cris-2022.01.01/dspace-cris-2022.01.01.pdf it can moved to the dspace wiki once the contribution is finalized. It is almost accurate also for the porting with the exception of the features related to nested metadata that cannot be ported to DSpace as this concept is not yet supported in DSpace

tdonohue commented 2 years ago

@LucaGiamminonni : Could you add the one minor change to this REST Contract that was suggested above? https://github.com/DSpace/RestContract/pull/180#discussion_r875966142

This is ready to merge except for that minor change

LucaGiamminonni commented 2 years ago

Hi @tdonohue, I made the minor change.

tdonohue commented 2 years ago

Thanks @LucaGiamminonni ! This now looks good. I gave approval previously & with this change, @benbosman 's final feedback above has been addressed.

Merging this immediately. If we find other small changes are needed they can be done in a separate PR.