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

Port to OMP #22

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Port to OMP #22

wants to merge 1 commit into from

Conversation

defstat
Copy link

@defstat defstat commented Aug 24, 2017

No description provided.

* @return $string
*/
function addCheckOrcidButton($output, &$templateMgr) {
$sessionManager = SessionManager::getManager();
Copy link
Member

Choose a reason for hiding this comment

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

Space-based indents.

* @param $templateMgr TemplateManager
* @return $string
*/
function addCheckOrcidButton($output, &$templateMgr) {
Copy link
Member

Choose a reason for hiding this comment

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

Generally speaking, references on objects aren't needed any more; I'd suggest removing throughout unless they're needed.

}
return false;
}

/**
* Output filter adds ORCiD interaction to OJS login form.
Copy link
Member

Choose a reason for hiding this comment

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

Should this refer to OJS specifically?

if (preg_match('/<form.*id="editAuthor"[^>]+>/', $output, $matches, PREG_OFFSET_CAPTURE)) {
$match = $matches[0][0];
$offset = $matches[0][1];
$context = Request::getContext();
Copy link
Member

Choose a reason for hiding this comment

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

If it's possible, I'd suggest exposing the $request object to this function rather than calling Request::(whatever) statically. Static calls to request functions are deprecated.

// if (is_array($params['path'])) {
// $params['path'] = array_merge($path, $params['path']);
// } elseif (!empty($params['path'])) {
// $params['path'] = array_merge($path, array($params['path']));
Copy link
Member

Choose a reason for hiding this comment

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

Dead code left behind?

<?php

/**
* @file classes/user/UserSettingsDAO.inc.php
Copy link
Member

Choose a reason for hiding this comment

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

Class name/path incorrect

/**
* Constructor
*/
function __construct() {
Copy link
Member

Choose a reason for hiding this comment

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

Wherever possible, remove unneeded (empty) constructors.

@@ -33,4 +32,8 @@
<message key="plugins.generic.orcidProfile.author.submission.success">Your submission has successfully been associated with your ORCID iD.</message>
<message key="plugins.generic.orcidProfile.author.submission.failure">Your submission could not be successfully associated with your ORCID iD. Please contact the journal manager with your name, ORCID, and details of your submission.</message>
<message key="plugins.generic.orcidProfile.authFailure">OJS was not able to communicate with the ORCID service. Please contact the journal manager with your name, ORCID iD, and details of your submission.</message>
<message key="plugins.generic.orcidProfile.linkMessage">Connect with ORCID</message>
<message key="plugins.generic.orcidProfile.oauthLoginError">OJS was not able to communicate with the ORCID service. Please contact the journal manager with your name, ORCID, and details of your submission.</message>
Copy link
Member

Choose a reason for hiding this comment

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

Refers to OJS

$givenName = $profile['given-names']['value'];
$familyName = $profile['family-name']['value'];

$response = "The ORCID " . $orcid . " correspond to a registered researcher - Name: " . $givenName . " - Family Name: " . $familyName;
Copy link
Member

Choose a reason for hiding this comment

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

English-language text should be localized.

</style>
{/literal}
<div id='orcidLoginLink'>
<a href="{$orcidProfileOauthPath|escape}authorize?client_id={$orcidClientId|urlencode}&response_type=code&scope=/authenticate&redirect_uri={url|urlencode page="orcidapi" op="orcidAuthorize" targetOp=$targetOp params=$params escape=false}">
Copy link
Member

Choose a reason for hiding this comment

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

I think all the &s need to be escaped as &amp;.

{**
* plugins/generic/orcidProfile/orcidProfile.tpl
*
* Copyright (c) 2015-2016 University of Pittsburgh
Copy link
Member

Choose a reason for hiding this comment

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

Dates for new files should all be -2017

* plugins/generic/orcidProfile/orcidProfile.tpl
*
* Copyright (c) 2015-2016 University of Pittsburgh
* Copyright (c) 2014-2016 Simon Fraser University Library
Copy link
Member

Choose a reason for hiding this comment

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

All "Simon Fraser University Library" copyrights should be changed to "Simon Fraser University"

@asmecher
Copy link
Member

Thanks, @defstat -- let me know when you've had a chance to look through my comments (and perform a rebase -- currently this won't merge) and I'll take a second look.

@nils-stefan-weiher
Copy link

@asmecher @defstat Isn't the plugin already working with OMP 3.1.1?
Can this be closed?

@asmecher
Copy link
Member

asmecher commented Jul 4, 2019

@defstat, I don't believe it's released for OMP yet; is there a compatible version that you're aware of?

@defstat
Copy link
Author

defstat commented Jul 8, 2019

@asmecher during Hirmeos project there where changes on the orcidProfile plugin regarding porting in OMP (for OMP 1.2 back then). EKT's OMP platform (http://ebooks.epublishing.ekt.gr) had incorporated the changes. That plugin (https://github.com/defstat/orcidProfileLink) had also been developed in order for the orcid of the monographs author to be displayed on the monographs landing page.
I am not sure about the current stage of all of these and whether they can still be used on current version of OMP (I am pretty sure that they need some work to be able to work on the current release of OMP)

@asmecher
Copy link
Member

asmecher commented Jul 8, 2019

@defstat, hmm, it probably needs some review and testing. The orcidProfile plugin has never been included in the OMP codebase or flagged in the Plugin Gallery for compatibility with OMP, and those are the two places I'd expect to see it if it were released for OMP.

Base automatically changed from master to main February 18, 2021 01:35
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.

3 participants