orcidProfile plugin option for email is unimplemented

[ctgraham]

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

orcidProfile/templates/settingsForm.tpl

Lines 48 to 50 in 65b4c3b

	{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.

orcidProfile/OrcidProfilePlugin.inc.php

Lines 705 to 723 in 65b4c3b

	/**
	* 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);
	}
	}
	}

[ctgraham]

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]

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

[ohilbig01]

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

[ctgraham]

New PR filed. Thanks, @ohilbig01 !

[asmecher]

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

[nils-stefan-weiher]

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

[ajnyga]

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]

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

[ajnyga]

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

[ajnyga]

@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]

Sounds like a plan to me. Looks good here.

[asmecher]

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