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

Openaire suggestions (publication claim) #1638

Merged
merged 47 commits into from
Feb 15, 2024

Conversation

LucaGiamminonni
Copy link
Contributor

@LucaGiamminonni LucaGiamminonni commented May 4, 2022

References

Unique code in this PR can be found via this diff:
4Science/dspace-angular@CST-5337...4Science:dspace-angular:CST-5249_suggestion

The suggestion feature requires the researcher profile feature, so it is required to:

  • uncomment the dspace.object.owner section in the authority.cfg
  • uncomment the property researcher-profile.entity-type = Person in the researcher-profile.cfg

The features included in this PR are the result of the OpenAIRE Call Innovation funded project "Enrich local data via the OpenAIRE Graph” awarded by 4Science (https://www.openaire.eu/open-call-winner-phase-1-4science). It provides a closer integration between DSpace and two OpenAIRE services, the Notification Broker and the OpenAIRE REST API.
Detailed documentation about the aims of the project, the implementation and the configuration options is available at https://4science.github.io/oaire-eld/#/publication-claim
This PR regarding only the publication claim.

How to Test

First, you must enable the following Researcher Profile settings by default:

researcher-profile.entity-type = Person
# Set this to a Collection that accepts "Person" entities. This example is the "People" Collection from the entities demo data.
researcher-profile.collection.uuid = 9398affe-a977-4992-9a1d-6f00908a259f
# Optional, but useful to make the Researcher Profile publicly available automatically.
researcher-profile.set-new-profile-visible = true
# Optional, but useful in testing. Lets you delete & recreate the Profile easily.
researcher-profile.hard-delete.enabled = true

Then, do the following:

  1. Create a normal EPerson account with the same name as an author findable at https://explore.openaire.eu/ (e.g. "Tim Donohue")
  2. Login as that EPerson -> Profile -> Researcher Profile -> Click "Create New". This creates your Researcher Profile page.
  3. Click View for that newly created Person entity. Copy the UUID of that Person entity (from your browser URL).
  4. Logout & Login as an Admin user
  5. Go to "Processes" in Admin Menu
  6. Create a new Process, choosing "import-oaire-suggestions"
  7. Select the Parameter "--single-researcher" and add the UUID of the Person entity created in steps 2-3 above.
  8. Run that Process.
  9. Now go to the Notifications menu & select "Publication Claim". Some suggestions should appear from https://explore.openaire.eu/ based on the name in your Researcher Profile.

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.

@LucaGiamminonni LucaGiamminonni changed the title Cst 5249 suggestion Openaire suggestions (publication claim) May 4, 2022
@lgtm-com
Copy link

lgtm-com bot commented May 4, 2022

This pull request introduces 27 alerts when merging a798ded into 37ebe25 - view on LGTM.com

new alerts:

  • 27 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Jul 7, 2022

This pull request introduces 16 alerts when merging de6b533 into fae355a - view on LGTM.com

new alerts:

  • 16 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Jul 8, 2022

This pull request introduces 16 alerts when merging d8e96d8 into fae355a - view on LGTM.com

new alerts:

  • 16 for Unused variable, import, function or class

@tdonohue tdonohue self-requested a review July 14, 2022 14:41
Copy link

github-actions bot commented Feb 7, 2024

Hi @LucaGiamminonni,
Conflicts have been detected against the base branch.
Please resolve these conflicts as soon as you can. Thanks!

@tdonohue tdonohue self-requested a review February 8, 2024 15:39
@FrancescoMolinaro
Copy link
Contributor

@frabacche and @abollini: I also tested this again from the frontend & re-reviewed the code. Overall, it is looking better, but I'm still finding a few bugs/issues.

  1. I'd recommend calling this page "Publication Claim" (singular) as that's more similar to "Quality Assurance" (the other page under "notifications"). Also, the breadcrumbs on the "Publication Claim" page should likely say "Publication Claim" instead of "Suggestions" Having two names for this page is confusing:
    breadcrumbs-should-match-header

  2. When logged in as an EPerson with suggestions in "Publication Claim", the breadcrumbs disappear if you click on the link to look at your suggestions. The breadcrumbs here should likely be "Home > Publication Claim > Tim Donohue". Additionally, the title of this page says "OpenAIRE" twice. We likely could simplify it to just Suggestions for [name] from OpenAIRE Graph
    suggestions-when-logged-in-as-submitter

  3. I'm not sure if this is a bug, but the "DateScorer" is always returning 0 and I'm not sure how to "set a min/max date in the profile"? What does that mean? It also doesn't seem to be possible to add "education achievements" in DSpace yet. Maybe we need to fix this text?
    DateScorer-always-zero

  4. If you are logged in as an EPerson with Suggestions, you cannot get back to the suggestions page if you initially visit the page & then leave. Here's what I see:

    • Login as an EPerson who has a Profile with Suggestions
    • After login, go to your Profile (or MyDSpace). You will see the notice that suggestions exist. Click "Review the suggestions"
    • That brings you to the suggestions page. Now, go immediately back to your Profile or MyDSpace (without performing any actions on the suggestions page)
    • The Notice on those pages is now gone. There is no way to get back to your suggestions.
    • If you logout & login again, the notification will appear again. This appears to be the only way to get back to the suggestions page.
    • I'd recommend that either the Notification always exist on your Profile / MyDSpace until you take an action on all suggestions. Or, maybe there needs to be a "Publication Claim" link that appears in the User dropdown menu when you have suggestions?

Beyond those bugs, I've rereviewed the code. Most of my feedback has been addressed (thanks!). However, there are still some areas that were overlooked which I've highlighted again below. Overall, this is looking better!

Hi @tdonohue , points 1,2 and 4 have been addressed, point 3 is a backend specification and @frabacche will get there from the backend request.
I hope this works for you.

@frabacche
Copy link
Contributor

frabacche commented Feb 9, 2024

@frabacche and @abollini: I also tested this again from the frontend & re-reviewed the code. Overall, it is looking better, but I'm still finding a few bugs/issues.

  1. I'd recommend calling this page "Publication Claim" (singular) as that's more similar to "Quality Assurance" (the other page under "notifications"). Also, the breadcrumbs on the "Publication Claim" page should likely say "Publication Claim" instead of "Suggestions" Having two names for this page is confusing:
    breadcrumbs-should-match-header

  2. When logged in as an EPerson with suggestions in "Publication Claim", the breadcrumbs disappear if you click on the link to look at your suggestions. The breadcrumbs here should likely be "Home > Publication Claim > Tim Donohue". Additionally, the title of this page says "OpenAIRE" twice. We likely could simplify it to just Suggestions for [name] from OpenAIRE Graph
    suggestions-when-logged-in-as-submitter

  3. I'm not sure if this is a bug, but the "DateScorer" is always returning 0 and I'm not sure how to "set a min/max date in the profile"? What does that mean? It also doesn't seem to be possible to add "education achievements" in DSpace yet. Maybe we need to fix this text?
    DateScorer-always-zero

  4. If you are logged in as an EPerson with Suggestions, you cannot get back to the suggestions page if you initially visit the page & then leave. Here's what I see:

    • Login as an EPerson who has a Profile with Suggestions
    • After login, go to your Profile (or MyDSpace). You will see the notice that suggestions exist. Click "Review the suggestions"
    • That brings you to the suggestions page. Now, go immediately back to your Profile or MyDSpace (without performing any actions on the suggestions page)
    • The Notice on those pages is now gone. There is no way to get back to your suggestions.
    • If you logout & login again, the notification will appear again. This appears to be the only way to get back to the suggestions page.
    • I'd recommend that either the Notification always exist on your Profile / MyDSpace until you take an action on all suggestions. Or, maybe there needs to be a "Publication Claim" link that appears in the User dropdown menu when you have suggestions?

Beyond those bugs, I've rereviewed the code. Most of my feedback has been addressed (thanks!). However, there are still some areas that were overlooked which I've highlighted again below. Overall, this is looking better!

Please consider the DateScorer surely can have a different score and is calculated on the backend (see corresponding backend pr at: DSpace/DSpace#8280) . At the moment the score can be 0 or 10. The Issue Date of the publication of suggestion is compared with a range. If the issue date is on the inside of the range datescorer returns 10; if it is on the outside of the range datascorer returns 0. The first range used is the one configured onto the researcher profile set with the following metadatas: person.range.maxdate, person.range.mindate. This configuration is not mandatory so if it does not exist - the educational date is considered - even if it is not configured on the spring context. Finally the birthdate is considered.

Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FrancescoMolinaro and @frabacche : This is almost ready to merge. However, I'm encountering two (new) bugs:

  1. When I click the "Ignore Suggestion" button, that particular suggestion remains on the page until I click reload in the browser. Here's what I'm doing:
    • Running the UI in production mode (yarn build:prod; yarn serve:ssr)
    • Login as an EPerson with a Researcher Profile that has suggestions
    • From one of the notifications, go to the list of suggestions
    • Click "Ignore Suggestion" button. Nothing appears to happen. (However, behind the scenes, I see the request did occur & succeed.)
    • Click Reload in the browser window. The suggestion now correctly disappears.
    • (This appears to be a caching or reloading issue, where the button doesn't cause the list of suggestions to refresh.)
  2. If I click "Approve & Import" on a suggestion, I can successfully create a Publication entity. But, that Publication is NOT linked to the Person entity that the suggestion was for. Here's what I'm doing:
    • Again, running the UI in production mode
    • Login as an EPerson that has suggestions for his/her Researcher Page (Person entity)
    • Once you see the notification, click on it to visit the suggestions
    • Click "Accept & Import" on one of them. Create a new Publication.
    • Upload a file, accept the license and Submit
    • The Publication is created. But, it is NOT linked to my Person entity.
    • Instead, I have to go to the Person entity, edit it & manually create a relationship to the new Publication, (Edit Person -> Relationships tab -> Add Publication). My assumption is this relationship should be created automatically because this Person is claiming this Publication as their own.

Beyond those two bugs, everything else appears to be working well.

Finally, I have one minor code suggestion below. Beyond that the code also looks good now.

src/assets/i18n/en.json5 Outdated Show resolved Hide resolved
@frabacche
Copy link
Contributor

@FrancescoMolinaro and @frabacche : This is almost ready to merge. However, I'm encountering two (new) bugs:

  1. When I click the "Ignore Suggestion" button, that particular suggestion remains on the page until I click reload in the browser. Here's what I'm doing:

    • Running the UI in production mode (yarn build:prod; yarn serve:ssr)
    • Login as an EPerson with a Researcher Profile that has suggestions
    • From one of the notifications, go to the list of suggestions
    • Click "Ignore Suggestion" button. Nothing appears to happen. (However, behind the scenes, I see the request did occur & succeed.)
    • Click Reload in the browser window. The suggestion now correctly disappears.
    • (This appears to be a caching or reloading issue, where the button doesn't cause the list of suggestions to refresh.)
  2. If I click "Approve & Import" on a suggestion, I can successfully create a Publication entity. But, that Publication is NOT linked to the Person entity that the suggestion was for. Here's what I'm doing:

    • Again, running the UI in production mode
    • Login as an EPerson that has suggestions for his/her Researcher Page (Person entity)
    • Once you see the notification, click on it to visit the suggestions
    • Click "Accept & Import" on one of them. Create a new Publication.
    • Upload a file, accept the license and Submit
    • The Publication is created. But, it is NOT linked to my Person entity.
    • Instead, I have to go to the Person entity, edit it & manually create a relationship to the new Publication, (Edit Person -> Relationships tab -> Add Publication). My assumption is this relationship should be created automatically because this Person is claiming this Publication as their own.

Beyond those two bugs, everything else appears to be working well.

Finally, I have one minor code suggestion below. Beyond that the code also looks good now.

  1. Ignore bug has been solved
  2. Suggestion import is a submission of a new item, so it's quite isolated. Author name is received and looked up on the workspace item: even though a single result is given - the link must be made by hand for safety reasons.

Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frabacche : Thanks for the quick fixes/response.

This is almost working. I'm basically a +1. But, I'm still finding that the "Ignore Suggestions" button only reloads sometimes.

Here's a video of the behavior I'm seeing.

  • I'm running the UI in production mode (yarn build:prod; yarn serve:ssr).
  • As you can see, if I click the "Ignore Suggestions" button once, it works. But, clicking it a second time on a different suggestion will NOT work (unless I click it twice)
    ignore-publication

This is the only remaining feedback that I have on this PR. If it's not easy to solve, we can log a bug ticket. But, ideally, it'd be nice to get this button working properly.

@FrancescoMolinaro
Copy link
Contributor

FrancescoMolinaro commented Feb 14, 2024

@frabacche : Thanks for the quick fixes/response.

This is almost working. I'm basically a +1. But, I'm still finding that the "Ignore Suggestions" button only reloads sometimes.

Here's a video of the behavior I'm seeing.

  • I'm running the UI in production mode (yarn build:prod; yarn serve:ssr).
  • As you can see, if I click the "Ignore Suggestions" button once, it works. But, clicking it a second time on a different suggestion will NOT work (unless I click it twice)

This is the only remaining feedback that I have on this PR. If it's not easy to solve, we can log a bug ticket. But, ideally, it'd be nice to get this button working properly.

Hi @tdonohue, I have made an improvement to the ignore suggestion button and it seems working in production mode (yarn build:prod; yarn serve:ssr).
Could you please give me confirmation once you have time to test it?
Many thanks in advance

@frabacche
Copy link
Contributor

Hi @tdonohue would you kindly restart the build process? We are afraid there's something randomly failing here. Thank you

Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FrancescoMolinaro and @frabacche : I've retested today and verified that the "Ignore Suggestion" button is now working properly.

However, I've noticed another odd issue in my testing. Whenever I click on the "review the suggestions" link in a notification, it reloads the entire application. In other words, it switches over to Server Side Rendering (SSR) instead of remaining on the client side (CSR).
review-suggestions-link

You can see this most easily by opening up your browser DevTools & clicking on that link. When you do so, you'll see on the "Network" tab that it waits on SSR to complete, and then reloads all the Javascript/CSS/logos for the application.

This seems like an unnecessary server-side request. Is there a way to fix this link to ensure it's remaining on the client side (like every other link in the application)?

I see no other issues at this time. So, I'm +1 this PR, but I think we need to fix the unnecessary SSR request.

@FrancescoMolinaro
Copy link
Contributor

@FrancescoMolinaro and @frabacche : I've retested today and verified that the "Ignore Suggestion" button is now working properly.

However, I've noticed another odd issue in my testing. Whenever I click on the "review the suggestions" link in a notification, it reloads the entire application. In other words, it switches over to Server Side Rendering (SSR) instead of remaining on the client side (CSR). review-suggestions-link

You can see this most easily by opening up your browser DevTools & clicking on that link. When you do so, you'll see on the "Network" tab that it waits on SSR to complete, and then reloads all the Javascript/CSS/logos for the application.

This seems like an unnecessary server-side request. Is there a way to fix this link to ensure it's remaining on the client side (like every other link in the application)?

I see no other issues at this time. So, I'm +1 this PR, but I think we need to fix the unnecessary SSR request.

Hello @tdonohue , I have fixed the unnecessary reload in SSR, the issue was due to the use of the normal href interpolated into the translation.
The normal HTML href on an achor tag seems to be always triggering a page reload.

May I kindly ask you to retest?

Thanks in advance

Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thank you @FrancescoMolinaro and @frabacche and @LucaGiamminonni (and whoever else has been involved with this PR). I've verified that the last fix works and all my feedback has now been addressed. Merging immediately.

@tdonohue tdonohue merged commit eb433c8 into DSpace:main Feb 15, 2024
13 checks passed
@tdonohue
Copy link
Member

NOTE Adding a needs documentation label to this PR until we have fully documented this in our pre-8.0 docs wiki space: https://wiki.lyrasis.org/display/DSDOC8x/

@tdonohue tdonohue added needs documentation PR is missing documentation. All new features and config changes require documentation. integration: QA Notifications Related to QA (Quality Assurance) notifications from external sources labels Feb 15, 2024
@tdonohue
Copy link
Member

Documentation for this feature is now at https://wiki.lyrasis.org/display/DSDOC8x/Publication+Claim

@tdonohue tdonohue removed the needs documentation PR is missing documentation. All new features and config changes require documentation. label Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high priority integration: OpenAIRE Related to integration with OpenAIRE integration: QA Notifications Related to QA (Quality Assurance) notifications from external sources new feature
Projects
No open projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants