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

Re-add "blind copy reviewers on editor decision" feature #4834

Closed
asmecher opened this issue Jun 19, 2019 · 14 comments · Fixed by #6728
Closed

Re-add "blind copy reviewers on editor decision" feature #4834

asmecher opened this issue Jun 19, 2019 · 14 comments · Fixed by #6728
Assignees
Labels
Community:2:Priority Any issue that has been identified through research or feedback as a major community priority. Hosting Bug reports and feature requests from Publishing Services's hosted clients.

Comments

@asmecher
Copy link
Member

Describe the problem you would like to solve
Editors would like a way to copy reviewers on the editor decision email sent to the author including the review details.

Describe the solution you'd like
OJS 2.x used to have a checkbox for this purpose (and still has some related locale keys). Reconstitute this feature the way it used to work in OJS 2.x.

Who is asking for this feature?
See e.g.: #743 (comment)

@asmecher asmecher added Community:2:Priority Any issue that has been identified through research or feedback as a major community priority. Hosting Bug reports and feature requests from Publishing Services's hosted clients. labels Jun 19, 2019
@dgardnerHK
Copy link

From an editor. We've just moved up to OJS v3.1 and I was so surprised to find that copying reviewers into the editorial decision is no longer a simple button click. I have had so much positive feedback over the years from reviewers about being able to see the editorial decision and the other reviewer's feedback that I must keep this going but it is going to be extra work and might get forgotten sometimes. I also worry that if I don't do it reviewers might begin to wonder whether their efforts are worthwhile and getting sufficient reviewers is my biggest headache.
Hope you can bring it back soon.

@NateWr
Copy link
Contributor

NateWr commented Oct 12, 2020

The following mockup suggests a a simple checkbox to be added to the editor decisions modal. When implementing this checkbox, the following things should be checked:

  • Only reviewers with completed reviews should be listed here. It is common for there to be many reviewers assigned but only a few (sometimes less than half) actually completing a review.
  • The option to BCC reviewers should be hidden when the option "do not send email notification" is selected.

bcc-reviewers

@ajnyga
Copy link
Collaborator

ajnyga commented Nov 18, 2020

+1 for this, some journals have asked for this. Apparently it is a common practice in some journals that the reviewers get to see the reports from other reviewers.

@alexxxmendonca
Copy link
Contributor

+1

and maybe have a non-blind copy as well, thinking about open review.

@jonasraoni jonasraoni self-assigned this Feb 6, 2021
jonasraoni added a commit to jonasraoni/pkp-lib that referenced this issue Feb 7, 2021
jonasraoni added a commit to jonasraoni/pkp-lib that referenced this issue Feb 7, 2021
jonasraoni added a commit to jonasraoni/pkp-lib that referenced this issue Feb 7, 2021
@asmecher
Copy link
Member Author

asmecher commented Feb 7, 2021

(Comments here: #6728 (comment))

jonasraoni added a commit to jonasraoni/pkp-lib that referenced this issue Feb 8, 2021
jonasraoni added a commit to jonasraoni/pkp-lib that referenced this issue Mar 11, 2021
jonasraoni added a commit to jonasraoni/pkp-lib that referenced this issue Mar 11, 2021
jonasraoni added a commit to jonasraoni/pkp-lib that referenced this issue Mar 11, 2021
@jonasraoni
Copy link
Contributor

@asmecher I've created a PR for stable-3_3_0. If everything is ok, I can update the one for /main...

PKP-LIB

But the PR to run the tests had a strange result :)

@asmecher
Copy link
Member Author

The code looks good, @jonasraoni! Please go ahead and forward-port to main.

@asmecher asmecher added this to the OJS/OMP/OPS 3.3.0-4 milestone Mar 11, 2021
@asmecher
Copy link
Member Author

But the PR to run the tests had a strange result :)

I thought this was just a race condition -- in fact restarting the tests did fix one of the broken PHP7.x tests -- but the 8.x tests appear to be failing on https://github.com/pkp/pkp-lib/pull/6855/files#diff-292418b042047b0b51e9084eee6931fbab4e76fb45f171010fdc8ce309d594fdR12:

[11-Mar-2021 18:42:49 UTC] PHP Fatal error:  Uncaught TypeError: count(): Argument #1 ($var) must be of type Countable|array, null given in /home/travis/build/pkp/ojs/cache/t_compile/690e812ff452b7e03c7305ace893860ee4c93d06^32ff1066ce03ba06db492913fd0e6f222fcf1f6d_0.app.controllersmodalseditorDe.php:24
Stack trace:
#0 /home/travis/build/pkp/ojs/lib/pkp/lib/vendor/smarty/smarty/libs/sysplugins/smarty_template_resource_base.php(123): content_604a649c4c4ff4_63093318(Object(Smarty_Internal_Template))
#1 /home/travis/build/pkp/ojs/lib/pkp/lib/vendor/smarty/smarty/libs/sysplugins/smarty_template_compiled.php(114): Smarty_Template_Resource_Base->getRenderedTemplateCode(Object(Smarty_Internal_Template))
#2 /home/travis/build/pkp/ojs/lib/pkp/lib/vendor/smarty/smarty/libs/sysplugins/smarty_internal_template.php(216): Smarty_Template_Compiled->render(Object(Smarty_Internal_Template))
#3 /home/travis/build/pkp/ojs/lib/pkp/lib/vendor/smarty/smarty/libs/sysplugins/smarty_internal_template.php(385): Smarty_Internal_Template->render()
#4 /home/travis/build/pkp/ojs/cache/t_compile/690e812ff452b7e03c7305ace893860ee4c93d06^f98a67abb74c9aa755e1eb9577e5d7f9f966bd33_0.app.controllersmodalseditorDe.php(125): Smarty_Internal_Template->_subTemplateRender('app:controllers...', NULL, '690e812ff452b7e...', 0, 3600, Array, 0, false)
#5 /home/travis/build/pkp/ojs/lib/pkp/lib/vendor/smarty/smarty/libs/sysplugins/smarty_template_resource_base.php(123): content_604a649c494609_64128568(Object(Smarty_Internal_Template))
#6 /home/travis/build/pkp/ojs/lib/pkp/lib/vendor/smarty/smarty/libs/sysplugins/smarty_template_compiled.php(114): Smarty_Template_Resource_Base->getRenderedTemplateCode(Object(Smarty_Internal_Template))
#7 /home/travis/build/pkp/ojs/lib/pkp/lib/vendor/smarty/smarty/libs/sysplugins/smarty_internal_template.php(216): Smarty_Template_Compiled->render(Object(Smarty_Internal_Template))
#8 /home/travis/build/pkp/ojs/lib/pkp/lib/vendor/smarty/smarty/libs/sysplugins/smarty_internal_templatebase.php(232): Smarty_Internal_Template->render(false, 0)
#9 /home/travis/build/pkp/ojs/lib/pkp/lib/vendor/smarty/smarty/libs/sysplugins/smarty_internal_templatebase.php(116): Smarty_Internal_TemplateBase->_execute(Object(Smarty_Internal_Template), NULL, '690e812ff452b7e...', NULL, 0)
#10 /home/travis/build/pkp/ojs/lib/pkp/classes/template/PKPTemplateManager.inc.php(1072): Smarty_Internal_TemplateBase->fetch('controllers/mod...', NULL, '690e812ff452b7e...', NULL)
#11 /home/travis/build/pkp/ojs/lib/pkp/classes/form/Form.inc.php(197): PKPTemplateManager->fetch('controllers/mod...')
#12 /home/travis/build/pkp/ojs/lib/pkp/classes/controllers/modals/editorDecision/form/EditorDecisionForm.inc.php(122): Form->fetch(Object(Request), NULL, false)
#13 /home/travis/build/pkp/ojs/lib/pkp/controllers/modals/editorDecision/form/EditorDecisionWithEmailForm.inc.php(169): EditorDecisionForm->fetch(Object(Request), NULL, false)
#14 /home/travis/build/pkp/ojs/lib/pkp/classes/controllers/modals/editorDecision/PKPEditorDecisionHandler.inc.php(444): EditorDecisionWithEmailForm->fetch(Object(Request))
#15 /home/travis/build/pkp/ojs/lib/pkp/classes/controllers/modals/editorDecision/PKPEditorDecisionHandler.inc.php(154): PKPEditorDecisionHandler->_initiateEditorDecision(Array, Object(Request), 'PromoteForm')
#16 /home/travis/build/pkp/ojs/lib/pkp/classes/core/PKPRouter.inc.php(395): PKPEditorDecisionHandler->promote(Array, Object(Request))
#17 /home/travis/build/pkp/ojs/lib/pkp/classes/core/PKPComponentRouter.inc.php(257): PKPRouter->_authorizeInitializeAndCallRequest(Array, Object(Request), Array)
#18 /home/travis/build/pkp/ojs/lib/pkp/classes/core/Dispatcher.inc.php(144): PKPComponentRouter->route(Object(Request))
#19 /home/travis/build/pkp/ojs/lib/pkp/classes/core/PKPApplication.inc.php(364): Dispatcher->dispatch(Object(Request))
#20 /home/travis/build/pkp/ojs/index.php(68): PKPApplication->execute()
#21 {main}
  thrown in /home/travis/build/pkp/ojs/cache/t_compile/690e812ff452b7e03c7305ace893860ee4c93d06^32ff1066ce03ba06db492913fd0e6f222fcf1f6d_0.app.controllersmodalseditorDe.php on line 24

@jonasraoni
Copy link
Contributor

jonasraoni commented Mar 12, 2021

@asmecher I even didn't watch this build's output, because the other PRs I submitted had the same race condition issue... Thanks for noticing, and also for talking about "restarting the tests", now I found the small restart link =]

count(null)
PHP < 7.2 = Great
PHP 7.2 = Warning...
PHP 8 = Die!

There's nothing in the documentation about the fatal error yet, but better to search across the code base to find more cases before they pop.

Also, did someone investigate these race conditions?

asmecher added a commit that referenced this issue Mar 12, 2021
…iewers

Closes #4834 Added "blind copy reviewers on editor decision".
asmecher added a commit that referenced this issue Mar 12, 2021
…cc-reviewers

Closes #4834 Added "blind copy reviewers on editor decision".
@asmecher
Copy link
Member Author

Merged PRs for both main and stable-3_3_0, thanks!

Also, did someone investigate these race conditions?

Not recently -- we have some outstanding planning and refactoring to do on the future of our test system (i.e. potentially migrating from Travis to Github Actions) and that'll need to happen before we dig into debugging the quirks.

@jonasraoni
Copy link
Contributor

@asmecher what about the fatal error in PHP 8? Are you going to proceed with a global check? Just asking because I didn't commit the fix.

About the Github Actions, I checked fast and it looks promising, looks like it supports Docker too, so we could have some slim pre-built images with all the tooling installed to speedup the process.

@asmecher
Copy link
Member Author

@jonasraoni, sorry, I had assumed there was a fix in place now. Can you look into a specific fix for this? (i.e. making sure that $reviewers is set in all cases where the new template is called?) There is some discussion of a global fix here, but it's too invasive for stable-3_3_0: #6783

@asmecher asmecher reopened this Mar 12, 2021
asmecher added a commit that referenced this issue Mar 12, 2021
@asmecher
Copy link
Member Author

Never mind, I got it: 295fbe3

asmecher added a commit that referenced this issue Mar 13, 2021
asmecher added a commit that referenced this issue Mar 13, 2021
asmecher added a commit that referenced this issue Mar 13, 2021
@jonasraoni
Copy link
Contributor

I've skimmed the code and for me it should be working, so I was going to investigate it better...
Anyway thanks, I'm a bit disabled after falling over my hand 😂

leonardorodriguesds pushed a commit to leonardorodriguesds/pkp-lib that referenced this issue Apr 5, 2021
Edit UserDAO to insert state and titration addicional fields

Add additional fields in UserDetailsForm

Add additional fields texts en_US

Add additional fields texts en_ES

Add additional fields texts pt_BR

Add additional fields to ContactForm and RegistrationForm

Add additional fields State and Titration to User profile
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community:2:Priority Any issue that has been identified through research or feedback as a major community priority. Hosting Bug reports and feature requests from Publishing Services's hosted clients.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants