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

pkp/pkp-lib#10744 Increase opportunity for ORCID work/review deposits #10756

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

taslangraham
Copy link
Contributor

For #10744

@taslangraham taslangraham marked this pull request as ready for review January 3, 2025 15:05
@@ -777,6 +778,9 @@ public function reviewRead($args, $request)
->withType(Notification::NOTIFICATION_TYPE_REVIEW_ASSIGNMENT)
->delete();

// Deposit review to ORCID
(new SendReviewToOrcid($reviewAssignment->getId()))->execute();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is great, but it got me thinking: do you think we should still send it again directly after this when the reviewer is thanked as well? It seems redundant to do it twice in quick succession, but curious what you think. If we do want to remove, it, it will need to be removed here:

(new SendReviewToOrcid($reviewAssignment->getId()))->execute();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now that you've mentioned it, I do agree that it is a bit redundant and there isn't anything gained from doing it when the reviewer is thanked if it was already done when the review was confirmed. I think it's best to have it at just the confirmation step.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed the call from thank reviewer flow

@taslangraham taslangraham force-pushed the i10744-main branch 2 times, most recently from 34a863e to 94423fa Compare January 10, 2025 23:47
@taslangraham taslangraham merged commit ce214a4 into pkp:main Jan 13, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants