currentUserAssignedRoles not accurate in submission API

[jardakotesovec]

Describe the bug
In response from API .../api/v1/submissions/20,
there is field currentUserAssignedRoles, which is present in every stage (other fields deducted) as follows:

"stages": [
{
  "id": 1,
  "label": "Submission",
  "isActiveStage": true,
  "currentUserAssignedRoles": [
     4097
   ],
}
},
{
  "id": 3,
  "label": "Review",
  "isActiveStage": false,
  "currentUserAssignedRoles": [
    4097
  ],
},
{
  "id": 4,
  "label": "Copyediting",
  "isActiveStage": false,
  "currentUserAssignedRoles": [
    4097
  ],
},
{
  "id": 5,
  "label": "Production",
  "isActiveStage": false,
  "currentUserAssignedRoles": [
    4097
  ],
}

In this case it telling me that for this submission I do have ROLE_ID_ASSISTANT permission on every stage. But thats not accurate as in User&Roles -> Roles settings I selected only to be assigned to Submission&Copyediting stage.

The behaviour is same in 3.4, but I don't think its impactful - as 3.4 listing cares about assignment to any stage - https://github.com/pkp/ui-library/blob/main/src/components/ListPanel/submissions/SubmissionsListItem.vue#L574, not individual ones.

But for 3.5 I would like to use this information to decide which stages user has access to, similarly like we do in 3.4

What application are you using?
OJS, OMP or OPS version 3.4, 3.5

[jardakotesovec]

Reason is that in https://github.com/pkp/pkp-lib/blame/main/classes/submission/maps/Schema.php#L390
There is very similar code twice:

So first it finds stage assignments without stageId, and second time with stageId and its using both results. Therefore result of this is not stage specific, which I would suspect is by mistake, because currentUserAssignedRoles is created per stage.

I think just deleting the first part would solve this.

Additionally I would use it as excuse to sneak in the recommendOnly and canChangeMetadata booleans, since the stage assignments are already loaded there. Instead of just returning boolean.

            $currentUserAssignedRoles = [];
            if ($currentUser) {
                // Replaces StageAssignmentDAO::getBySubmissionAndUserIdAndStageId
                $stageAssignments = StageAssignment::withSubmissionIds([$submission->getId()])
                    ->withUserId($currentUser->getId() ?? 0)
                    ->get();

                foreach ($stageAssignments as $stageAssignment) {
                    $userGroup = $this->getUserGroup($stageAssignment->userGroupId);
                    if ($userGroup) {
                        $currentUserAssignedRoles[] = $userGroup->getRoleId();
                    }
                }

                // Replaces StageAssignmentDAO::getBySubmissionAndUserIdAndStageId
                $stageAssignments = StageAssignment::withSubmissionIds([$submission->getId()])
                    ->withUserId($currentUser->getId())
                    ->withStageIds([$stageId])
                    ->get();


                foreach ($stageAssignments as $stageAssignment) {
                    $userGroup = Repo::userGroup()->get($stageAssignment->userGroupId);
                    $currentUserAssignedRoles[] = (int) $userGroup->getRoleId();
                }
            }
            $stage['currentUserAssignedRoles'] = array_values(array_unique($currentUserAssignedRoles));

[Vitaliy-1]

@jardakotesovec, this sounds reasonable!

[jardakotesovec]

@Vitaliy-1 Before jumping on PR.

I think it make sense to solve this along with the #10360 , can you provide your thought on the suggestions there?

[jardakotesovec]

Closing this one in favour of #10480