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

orcidProfile plugin option for email is unimplemented #112

Open
ctgraham opened this issue Aug 14, 2019 · 12 comments · Fixed by #72, #73 or #74
Open

orcidProfile plugin option for email is unimplemented #112

ctgraham opened this issue Aug 14, 2019 · 12 comments · Fixed by #72, #73 or #74

Comments

@ctgraham
Copy link

Describe the bug
The orcidProfile plugin settings form offers an option "Send e-mail to request ORCID authorization from article authors on publication of a new issue", but this setting is not referenced anywhere in the code, so mail is always sent on publication.

Reported: https://forum.pkp.sfu.ca/t/ojs-3-1-2-1-orcid-collect-letters-cannot-be-disabled/55206

To Reproduce
Steps to reproduce the behavior:

  1. Go to orcidProfile plugin settings
  2. Check "Send e-mail to request ORCID authorization from article authors on publication of a new issue" and save settings
  3. Publish an issue with incomplete ORCID information for authors
  4. Mail will be sent regardless of the setting

What application are you using?
OJS or OMP version 3.1.2

Additional information

{fbvFormSection for="sendMailToAuthorOnPublication" title="plugins.generic.orcidProfile.manager.settings.mailSectionTitle" list="true"}
{fbvElement type="checkbox" name="sendMailToAuthorsOnPublication" label="plugins.generic.orcidProfile.manager.settings.sendMailToAuthorsOnPublication" id="sendMailToAuthorsOnPublication" checked=$sendMailToAuthorsOnPublication}
{/fbvFormSection}

vs.

/**
* handleEditorAction handles promoting a submission to production.
*
* @param $hookName string Name the hook was registered with
* @param $args array Hook arguments, &$submission, &$editorDecision, &$result, &$recommendation.
*
* @see EditorAction::recordDecision() The function calling the hook.
*/
public function handleEditorAction($hookName, $args) {
$submission =& $args[0];
$authorDao = DAORegistry::getDAO('AuthorDAO');
$authors = $authorDao->getBySubmissionId($submission->getId());
foreach ($authors as $author) {
$orcidAccessExpiresOn = Carbon\Carbon::parse($author->getData('orcidAccessExpiresOn'));
if ( $author->getData('orcidAccessToken') == null || $orcidAccessExpiresOn->isPast()) {
$this->sendAuthorMail($author, true);
}
}
}

asmecher referenced this issue Aug 14, 2019
pkp/pkp-lib#4998: Enable editorialAction hook only if email option is enabled.
@ctgraham
Copy link
Author

I haven't actually tested this patch, and I fear that it will not work as expected if register() is called without a context id.

We probably need something like:

		if ($contextId == null) {
			$contextId = $this->getCurrentContextId();
		}

@asmecher
Copy link
Member

Merged and cherry-picked the additional commit; thanks, @ctgraham!

@ohilbig01
Copy link

Hi @asmecher, I think there is an s missing in "sendMailToAuthorOnPublication"?
sendMailToAuthorsOnPublication worked for me.

@ctgraham ctgraham reopened this Aug 16, 2019
@ctgraham
Copy link
Author

New PR filed. Thanks, @ohilbig01 !

@asmecher
Copy link
Member

Thanks, @ohilbig01 and @ctgraham -- merged and cherry-picked to stable-3_1_2.

@nils-stefan-weiher
Copy link

Thanks @ohilbig01 and @ctgraham . That was a real big oversight from me.

@ajnyga
Copy link

ajnyga commented Dec 13, 2019

Hi,

Noticed a potential problem with this same hook.

The setting says "Send e-mail to request ORCID authorization from article authors on publication of a new issue"

But the setting enables this hook here: https://github.com/pkp/orcidProfile/blob/master/OrcidProfilePlugin.inc.php#L75 EditorAction::recordDecision
Which, to my knowledge, is not connected at all to publishing a new issue.

It is attached to this function https://github.com/pkp/orcidProfile/blob/master/OrcidProfilePlugin.inc.php#L716-L726 which does not check what kind of decision is done.

So the hook fires on every editorial decision I think. Meaning it could fire multiple times during the workflow. Explaining a recent forum post here: https://forum.pkp.sfu.ca/t/limit-orcid-plugin-notifications/57170

I already discussed this with @withanage and my suggestion is:

  1. Change the label for the setting to “Send e-mail to request ORCID authorization from article authors when the submission is moved to copy editing
  2. Edit the handleEditorAction function so that it will check the editor action and only fires when the action is "send to copyediting".

After that ORCIDs are collected and emails sent in three cases:

  1. A new user is registered
  2. Adding a new author to a submission (if the selection is checked in the form)
  3. Sending the submission to copy-editing (if the plugin setting is enabled)

@ctgraham
Copy link
Author

@ajnyga, do you want to write up a PR for this fix, or are you looking for someone else to pick it up?

@ctgraham ctgraham reopened this Dec 13, 2019
@ajnyga
Copy link

ajnyga commented Dec 17, 2019

Pull requests for
stable-3_1_2: #82
master: #83

@ajnyga
Copy link

ajnyga commented Jan 17, 2020

@ctgraham

Updated pull requests here:
stable-3_1_2: #82
master: #83

I am not changing the variable for now so that we can have this important fix implented asap. I will create a new issue about the variable name.

@ctgraham
Copy link
Author

Sounds like a plan to me. Looks good here.

@withanage withanage transferred this issue from pkp/pkp-lib Apr 7, 2020
@asmecher
Copy link
Member

@ajnyga, the PRs above were merged -- is this issue ready to be closed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants