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

Add Web of Science Reviewer Recognition Service plugin for OJS 3.4 #301

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Alex-nz
Copy link

@Alex-nz Alex-nz commented Aug 7, 2024

Add Web of Science Reviewer Recognition Service plugin for OJS 3.4 into the plugin gallery. This plugin is the heavily rebranded old Publons plugin, hence I put these together in the file.

@Alex-nz
Copy link
Author

Alex-nz commented Aug 8, 2024

Hi PKP team, can we please get this PR reviewed?

@@ -9768,6 +9768,41 @@
<description>Minor improvements for Portuguese translations.</description>
</release>
</plugin>
<plugin category="generic" product="webOfScience">
Copy link

Choose a reason for hiding this comment

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

Hi @Alex-nz, thanks a lot for the contribution!
I went through the plugin code and would have a few comments/questions:

  • The handler https://github.com/clarivate/wos_reviewer_recognition_plugin_ojs_3/blob/main/WOSHandler.php is missing the authorization function. This way everybody could call the function exportReview. I believe that addRoleAssigment should be used, so that only reviewers (an user with a reviewer role) can call the function, and it should also be checked if the request context corresponds to the submission context (having the submission from the reviewAssignment) -- so that a reviewer from one journal cannot call that function for a review in another journal (that maybe has no plugin enabled) -- e.g. if the parameter reviewId, that the function expects, would be manipulated. I believe the WOS service would surely not work correctly for example in case the user is not a reviewer any more, or for a review in another journal, but the safety should be ensured by coding it.
  • When are these lines the case: https://github.com/clarivate/wos_reviewer_recognition_plugin_ojs_3/blob/main/WOSHandler.php#L191 and https://github.com/clarivate/wos_reviewer_recognition_plugin_ojs_3/blob/main/WOSHandler.php#L214 -- when and how would $_SERVER["HTTP_WOS_URL"] be set?
  • Instead of using php curl you should rather use Guzzle -- you can get the HTTP client from the Application class. This client is properly configured for proxies if the server is behind one.
  • I think <certification type="reviewed"/> should be used below.
    Please let me know if you would have any questions and what do you think about the suggestions...
    Thanks a lot!

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the review @bozana. I would have to defer answering your comments to the developer who worked on this plugin. @v0k1c when you are back from vacation, can you please look into this?

@Alex-nz
Copy link
Author

Alex-nz commented Sep 9, 2024

Hi @bozana , we've made the requested updates and updated the PR, can you please re-review?

I think should be used below.

^ I've used the same certification type that we were given when published the plugin previously, under Publons name. I have updated it to "reviewed" as you suggested.

@bozana
Copy link

bozana commented Sep 10, 2024

Hi @Alex-nz,
I would have two more comments:

  • I think in the function exportReview it should be also checked that the $request->getContext()->getId() is the same as the submission->getData('contextId')
  • Why are you using https://fonts.googleapis.com/css?family=Roboto? -- We have chosen the fonts under consideration to support wide range of languages etc. They are downloaded and provided with OJS. So plugins would preferably use them (you would not need to do anything for that).
    This way, using 'https://fonts.googleapis.com/css?family=Roboto' would provide user information (IP) address to google (which would be good to inform/ask the user about). Thus, if you would really need that CSS, maybe to download it and provide it within the plugin, but I believe there should be no need to use anything extra.

And finally, let me double check regarding the certification level with the team...

Thanks a lot!

@bozana
Copy link

bozana commented Sep 10, 2024

I have forgotten to include @v0k1c into my comment above :-( Sorry! But now... :-)

@Alex-nz
Copy link
Author

Alex-nz commented Sep 10, 2024

thank you @bozana , @v0k1c and I are looking into your comments.

@Alex-nz
Copy link
Author

Alex-nz commented Sep 17, 2024

@bozana, @v0k1c has made the changes as requested and I've updated this PR to reflect the new release. Can you please re-review?

@v0k1c
Copy link

v0k1c commented Sep 18, 2024

Hi @bozana! I've just double checked, and Open Sans (although available within the theme plugin?) is not loaded by default? All I'm seeing that is loaded is Noto Sans and Fontawesome. Would it be safe to import/enable it through theme plugin, or what is the preferred way to use additional fonts that are available there? Asking because I can't find anything related to this within the available documentation.

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