Merge pull request #11314 from Vitaliy-1/stable-3_5_0-i11252-fix

#11252 Fix review assignment collector

Author: Vitaliy-1

api/v1/_submissions/PKPBackendSubmissionsController.php

@@ -391,9 +391,6 @@ public function getReviewAssignments(Request $illuminateRequest): JsonResponse
                 case 'published':
                     $collector->filterByPublished(true);
                     break;
-                case 'allReviewRounds':
-                    $collector->filterByReviewerIds([$currentUser->getId()]);
-                    break;
             }
         }
 

classes/security/authorization/internal/ReviewAssignmentAccessPolicy.php

@@ -69,8 +69,7 @@ public function effect(): int
 
         $reviewAssignment = Repo::reviewAssignment()->getCollector()
             ->filterBySubmissionIds([$submission->getId()])
-            ->filterByReviewerIds([$user->getId()])
-            ->filterByLastReviewRound(true)
+            ->filterByReviewerIds([$user->getId()], true)
             ->getMany()
             ->first();
 

classes/submission/reviewAssignment/Collector.php

@@ -282,17 +282,24 @@ public function orderBySubmissionId(string $direction = self::ORDER_DIR_ASC): st
     public function getQueryBuilder(): Builder
     {
         $q = DB::table($this->dao->table . ' as ra')
-            // Aggregate the latest review round number for the given assignment
+            // Aggregating data regarding latest review round and stage. For OMP the latest round isn't equal to the round with the highest number per submission
             ->leftJoinSub(
-                DB::table('review_rounds', 'rr')
-                    ->select('rr.submission_id')
-                    ->selectRaw('MAX(rr.round) as current_round')
-                    ->selectRaw('MAX(rr.stage_id) as current_stage')
-                    ->groupBy('rr.submission_id'),
-                'agrr',
-                fn (JoinClause $join) => $join->on('ra.submission_id', '=', 'agrr.submission_id')
-            )
-            ->select(['ra.*']);
+                DB::table('review_assignments as rr')
+                    ->select(['rr.submission_id', 'rr.stage_id'])
+                    ->selectRaw('MAX(rr.round) as latest_round')
+                    ->groupBy('rr.submission_id', 'rr.stage_id')
+                    ->leftJoinSub(
+                        DB::table('review_assignments as rs')
+                            ->select('rs.submission_id')
+                            ->selectRaw('MAX(rs.stage_id) as latest_stage')
+                            ->groupBy('rs.submission_id'),
+                        'rsmax',
+                        fn (JoinClause $join) => $join->on('rr.submission_id', '=', 'rsmax.submission_id')
+                    )
+                    ->whereColumn('rr.stage_id', 'rsmax.latest_stage'), // Take the highest round only from the latest stage
+                'rrmax',
+                fn (JoinClause $join) => $join->on('ra.submission_id', '=', 'rrmax.submission_id')
+            );
 
         $q->when(
             $this->contextIds !== null,
@@ -314,8 +321,8 @@ public function getQueryBuilder(): Builder
 
         $q->when($this->isLastReviewRound || $this->isIncomplete, function (Builder $q) {
             $q
-                ->whereColumn('ra.round', '=', 'agrr.current_round') // assignments from the last review round only
-                ->whereColumn('ra.stage_id', '=', 'agrr.current_stage') // assignments for the current review stage only (for OMP)
+                ->whereColumn('ra.round', '=', 'rrmax.latest_round') // assignments from the last review round only
+                ->whereColumn('ra.stage_id', '=', 'rrmax.stage_id') // assignments for the current review stage only (for OMP)
                 ->when(
                     $this->isIncomplete,
                     fn (Builder $q) => $q
@@ -450,19 +457,33 @@ public function getQueryBuilder(): Builder
                 ->when(
                     $this->isLastReviewRoundByReviewer,
                     fn (Builder $q) => $q
-                        // Aggregate the latest review round number for the given assignment and reviewer
                         ->leftJoinSub(
+                            // Determine the last review round the reviewer has assignments in
                             DB::table('review_assignments as ramax')
-                                ->select(['ramax.submission_id', 'ramax.reviewer_id'])
+                                ->select(['ramax.submission_id', 'ramax.reviewer_id', 'ramax.stage_id'])
                                 ->selectRaw('MAX(ramax.round) as latest_round')
-                                ->selectRaw('MAX(ramax.stage_id) as latest_stage')
-                                ->whereIn('ramax.reviewer_id', $this->reviewerIds)
-                                ->groupBy(['ramax.submission_id', 'ramax.reviewer_id']),
-                            'agrmax',
-                            fn (JoinClause $join) => $join->on('ra.submission_id', '=', 'agrmax.submission_id')
+                                ->groupBy(['ramax.submission_id', 'ramax.reviewer_id', 'ramax.stage_id'])
+                                /*
+                                 * Reviewers might have assignments in the Internal but not External review stage.
+                                 * Must aggregate data regarding the last review stage the reviewer has assignments in
+                                 */
+                                ->leftJoinSub(
+                                    DB::table('review_assignments as rsmax')
+                                        ->select(['rsmax.submission_id', 'rsmax.reviewer_id'])
+                                        ->selectRaw('MAX(rsmax.stage_id) as latest_stage')
+                                        ->groupBy(['rsmax.submission_id', 'rsmax.reviewer_id'])
+                                        ->where('rsmax.reviewer_id', $this->reviewerIds),
+                                    'rssmax',
+                                    fn (JoinClause $join) => $join->on('ramax.submission_id', '=', 'rssmax.submission_id')
+                                )
+                                ->whereColumn('ramax.reviewer_id', 'rssmax.reviewer_id') // Take only selected reviewers
+                                ->whereColumn('ramax.stage_id', 'rssmax.latest_stage'),  // Take only the current stage
+                        'raamax',
+                            fn (JoinClause $join) => $join->on('ra.submission_id', '=', 'raamax.submission_id')
                         )
-                        ->whereColumn('ra.round', 'agrmax.latest_round') // assignments from the last review round per reviewer only
-                        ->whereColumn('ra.stage_id', 'agrmax.latest_stage') // assignments for the current review stage only (for OMP)
+                        ->whereColumn('ra.reviewer_id', 'raamax.reviewer_id') // Finally fitler by reviewers
+                        ->whereColumn('ra.stage_id', 'raamax.stage_id') // Finally filter by the latest review stage
+                        ->whereColumn('ra.round', 'raamax.latest_round') // Finally filter by the latest review round
                 )
         );