Skip to content

Conversation

@Vitaliy-1
Copy link
Contributor

…rticipants

@Vitaliy-1 Vitaliy-1 force-pushed the i12009_task_participants branch from 80bea16 to fe9702f Compare November 11, 2025 09:54
@Vitaliy-1 Vitaliy-1 requested a review from ewhanson November 11, 2025 16:11
Copy link
Contributor

@ewhanson ewhanson left a comment

Choose a reason for hiding this comment

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

Thanks @Vitaliy-1. Just a few questions, mostly around how things are anonymized. Otherwise, looks good!


if ($currentUserReviewAssignments->isNotEmpty()) {
$currentUserHasDoubleBlindReview = (bool) $currentUserReviewAssignments->search(fn (ReviewAssignment $reviewAssignment) =>
$reviewAssignment->getReviewMethod() == ReviewAssignment::SUBMISSION_REVIEW_METHOD_DOUBLEANONYMOUS);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if it matters since they should both be ints but should this be checked with === instead?

foreach ($stageAssignments as $stageAssignment) {
if ($isReviewStage && $roleReviewer && $currentUserHasDoubleBlindReview) {
// Anonymize author if the current user is a reviewer with double-blind review
if ($stageAssignment->userGroup->roleId == Role::ROLE_ID_AUTHOR) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same with this ==.


foreach ($stageAssignments as $stageAssignment) {
if ($isReviewStage && $roleReviewer && $currentUserHasDoubleBlindReview) {
// Anonymize author if the current user is a reviewer with double-blind review
Copy link
Contributor

Choose a reason for hiding this comment

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

It may not matter, but it's not completely clear to me going through the code if the authors are being anonymized (i.e. they're included but with no identifiable information) or if they're rather completely excluded (i.e. not available as a participant at all). It seems like the code is excluding them outright, but wanted to double-check because the reference to anonymizing the author were a little confusing to follow.

$includedReviewAssignments = collect();
foreach ($filteredReviewAssignments as $reviewAssignment) { /** @var ReviewAssignment $reviewAssignment */

// Check whether participant should be anonymized
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment about "anonymized" as above.


return [
'id' => $this->id,
'id' => $this?->id,
Copy link
Contributor

Choose a reason for hiding this comment

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

In which cases would $this be potentially null?

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.

2 participants