Reviewers' identities visible to other reviewers in discussion

[amandastevens]

Issue 2976 introduced a discussion grid for reviewers and the ability to create discussions between editors, reviewers, and authors. Participants' identities are hidden automatically from other participants, depending on the review type. The issue says is it's supposed to work as follows:
• In open reviews: all users see each other, including the reviewers, they see all other open reviewers (what do you think?)
• in blind reviews: authors only see the editors; reviewers see the editors; editors see everybody
• in mixed reviews (both blind and open): authors only see editors and open reviewers: blind reviewers only see editors; open reviewers see authors and editors but not the blind reviewers

I tested this on two different OJS 3.1.2 installs and found that reviewers' identities were exposed to other reviewers for both blind and double blind submissions. As an editor I created a discussion topic in the review stage and added the three reviewers to the discussion. The reviewers had all been requested to do a blind review. Then I logged in as one of the reviewers and I could see the names of the other two reviewers in the discussion. I tried this again with a double blind review and the same thing occurred.

Here is a screenshot of the discussion topic when I'm logged in as a reviewer.

[ajnyga]

Just to make sure. This happens if an editors starts a discussion and adds all the reviewers as participants?

The discussion system currently works so that all people added to a discussion can see each other. What blind reviewers can not do is to start that kind of discussions themselves. They can not see each other as suggested participants.

But we already discussed with Alec that some sort of anonymization could be introduced in case an editor starts a discussion like that. I will come up with a pr soon.

[ajnyga]

So looked into this and it is complicated.

Everything works ok as long as the editor does not start a discussion with several blind editors.

If such discussion is created, then it is hard to keep that anonymized. I can easily hide the names from the participants list, but if one of the reviewers answers the message, her name will show up in the "Last reply" and "From" fields. Also I think that it is a bad decision to hide the names because all participants will see each other's answers. So a reviewer might just see the editors name, but the answer ends up to other reviewers as well.

So probably we need a combination of warnings (for the editor when she selects several blind reviewers to a discussion and for the reviewers if they receive a discussion with several blind reviewers and/or authors) and anonymized user names. So instead Firstname Lastname you see something like Reviewer #1234.

I have been talking about this with Alec in Slack, so let's see first what he thinks.

[asmecher]

@amandastevens, would you be OK with a warning before a user is allowed to create a new discussion is created in a way that breaks anonymity? I think a full solution to this will only be possible once we overhaul the email communication system (e.g. permitting OJS to receive email replies from users). That would allow us to provide email-based communication between users that shouldn't know each others' identities, which we can't really offer otherwise.

[NateWr]

I've bumped this up to Critical and assigned it to the 3.1.2-2 milestone. During the sprint in Pittsburgh we saw a case where Blind reviewers were exposed to the Author before any discussions were created. This seems to undermine anonymous review.

This may have been because the Author had other editorial roles in the system, which would mean that this issue doesn't effect very many people. But regardless the create discussions form should show/hide info based on their assigned role in the submission, like other parts of the workflow.

[ajnyga]

Most likely it is a situation where the author is also an editor in the same context.

If current user is editor, all reviewers are considered: https://github.com/pkp/pkp-lib/blob/master/controllers/grid/queries/form/QueryForm.inc.php#L234

If current user is author, only open reviewers are considered: https://github.com/pkp/pkp-lib/blob/master/controllers/grid/queries/form/QueryForm.inc.php#L253

I think the code was written back when we always allowed editors access to anything. So it uses checks like $user->hasRole(array(ROLE_ID_MANAGER), $context->getId()) instead of array_intersect(array(ROLE_ID_MANAGER), $userRoles).

So without testing, I would say that an easy fix would be to add if ($user->hasRole(array(ROLE_ID_SITE_ADMIN), CONTEXT_SITE) || array_intersect(array(ROLE_ID_MANAGER), $userRoles) || array_intersect(array(ROLE_ID_SUB_EDITOR), $userRoles) ) { to https://github.com/pkp/pkp-lib/blob/master/controllers/grid/queries/form/QueryForm.inc.php#L234

[ajnyga]

But, I think that the depending on the outcome of the discussion above, the whole way of creating the list of participants should be considered. There are a lot of different cases that are hard to think of and while the reviewers do not have an actual stage assigment and there are different kinds of reviewers, things get complicated quickly.

[NateWr]

Yes, I think your suggestion above for the array_intersect will resolve the issue.

Can we rename $userRoles to $userRoleAssignments? Just because we use $userRoles elsewhere in the system to identify roles-in-context but here we want roles-in-submission. 👍

[asmecher]

@NateWr, do you mind if I assign you to this one in order to get your proposed fix merged for 3.1.2-2? If you want to address the critical issue and file the rest separately, that's OK with me.

[NateWr]

Thanks for doing the hard work on this one @ajnyga. I'll backport to stable once the test has passed.

PR:
#5119

Tests only:
pkp/ojs#2487

[ajnyga]

"hard work"

[NateWr]

Also, I can confirm that the original issue (reviewers seeing other reviewers) only occurs when an editor adds them all to the same discussion. This is less critical, but I'd suggest one of the following:

• Block discussions that involve more than one reviewer. Return an error that says you can't send a discussion to more than one reviewer.
• Create a separate discussion for each assigned reviewer. When more than one reviewer is selected, a note should appear in the form to let them know this will happen.

I'm kind of in favor of the first one.

[ajnyga]

The bad thing with option 1 is that it will also block you from creating a new review that involves several open review requests. But I guess that can be taken into account.

[NateWr]

Merged and cherry-picked to stable. I've taken the Critical Issue label off and assigned this to the 3.3 (unconfirmed) milestone.

[dihagan]

I would prefer a warning on this issue, for cases when there is a group of reviewers (say an advisory committee) who need to come to a consensus.

[BagiraHun]

There is a bug in https://github.com/pkp/pkp-lib/blob/master/controllers/grid/queries/form/QueryForm.inc.php#L243

Due type mismatch of the two value, you have to cast them to same integer type (int) or you have to use == operator.

My solution is: https://github.com/BagiraHun/pkp-lib/blob/blind_reviewer_author_patch/controllers/grid/queries/form/QueryForm.inc.php#L243

[bozana]

PRs:
pkp-lib: #6629
ojs: pkp/ojs#3000 (only submodule update)
omp: pkp/omp#920 (only submodule update)
ops: pkp/ops#113 (only submodule update)

[bozana]

@NateWr, could you please take a look at the PR above?
When all well I would provide the OMP and OPS PRs for submodule update...

[bozana]

In the review stage also the guest editors, translators and funding coordinators can be assigned. In such a case it is then possible to select them in the review discussion participants list. Should they be considered and how -- are they allowed to see the participants names (e.g. like editors/section editors are)?

[ajnyga]

Guest editor is the same as section editor, right?

I think that if a role like translator has access to the review stage, then they should be able to see the names. Journals can control this by selecting the stages different roles can access?