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.

Sign up for GitHub

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

Jump to bottom

pkp/pkp-lib#10280 Additional data for submissions/{ID}/participants API #10383

Merged
merged 1 commit into from
Sep 19, 2024
+98 −7
Merged

pkp/pkp-lib#10280 Additional data for submissions/{ID}/participants API #10383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions api/v1/submissions/PKPSubmissionController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -902,7 +902,7 @@ public function getParticipants(Request $illuminateRequest): JsonResponse
$context = $request->getContext();
$submission = $this->getAuthorizedContextObject(Application::ASSOC_TYPE_SUBMISSION);
$args = $illuminateRequest->input();
$stageId = $args['stageId'] ?? null;
$stageId = $args['stageId'] ?? $illuminateRequest->route('stageId') !== null ? (int) $illuminateRequest->route('stageId') : null;

if (!$submission || $submission->getData('contextId') !== $context->getId()) {
return response()->json([
Expand All @@ -919,7 +919,7 @@ public function getParticipants(Request $illuminateRequest): JsonResponse

$map = Repo::user()->getSchemaMap();
foreach ($usersIterator as $user) {
$data[] = $map->summarizeReviewer($user);
$data[] = $map->summarizeReviewer($user, ['submission' => $submission, 'stageId' => $stageId]);
}

return response()->json($data, Response::HTTP_OK);
Expand Down
4 changes: 2 additions & 2 deletions classes/stageAssignment/StageAssignment.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public function scopeWithStageIds(Builder $query, ?array $stageIds): Builder
public function scopeWithSubmissionIds(Builder $query, ?array $submissionIds): Builder
{
return $query->when($submissionIds !== null, function ($query) use ($submissionIds) {
return $query->whereIn('submission_id', $submissionIds);
return $query->whereIn('stage_assignments.submission_id', $submissionIds);
Copy link
Collaborator

Choose a reason for hiding this comment

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

The additional stage_assignments table name shouldn't be necessary here. Were you running into any issues with it?

Copy link
Contributor Author

@taslangraham taslangraham Sep 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

When using the scopeWithSubmissionIds filter in combination with another filter, such as scopeWithContextId, which performs joins with tables containing a column named submission_id, you'll encounter an ambiguous column error because the column name submission_id exists in multiple tables in the query.

So for example the following query would throw the error below

$stageAssignments = StageAssignment::withSubmissionIds([$submission->getId()])
     ->withStageIds($stageId ? [$stageId] : [])
     ->withUserId($user->getId())
     ->withContextId($this->context->getId()) // joins submissions table which also has a submission_id column
     ->get();

Error

SQLSTATE[23000]: Integrity constraint violation: 1052 Column 'submission_id' in where clause is ambiguous (Connection: mysql, SQL: select * from `stage_assignments` inner join `submissions` on `stage_assignments`.`submission_id` = `submissions`.`submission_id` where `submission_id` in (9) and exists (select * from `user_group_stage` where `stage_assignments`.`user_group_id` = `user_group_stage`.`user_group_id` and `stage_id` in (5)) and `user_id` = 3 and `submissions`.`context_id` = 1)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good to know!

});
}

Expand Down Expand Up @@ -142,7 +142,7 @@ public function scopeWithContextId(Builder $query, ?int $contextId): Builder
{
return $query->when($contextId !== null, function ($query) use ($contextId) {
return $query->join('submissions', 'stage_assignments.submission_id', '=', 'submissions.submission_id')
->where('submissions.context_id', $contextId);
->where('submissions.context_id', $contextId);
});
}
}
60 changes: 57 additions & 3 deletions classes/user/maps/Schema.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,12 @@
use Illuminate\Support\Enumerable;
use PKP\db\DAORegistry;
use PKP\plugins\Hook;
use PKP\security\Role;
use PKP\services\PKPSchemaService;
use PKP\stageAssignment\StageAssignment;
use PKP\user\User;
use PKP\workflow\WorkflowStageDAO;
use Submission;

class Schema extends \PKP\core\maps\Schema
{
Expand Down Expand Up @@ -51,9 +55,9 @@ public function summarize(User $item): array
*
* Includes properties with the apiSummary flag in the user schema.
*/
public function summarizeReviewer(User $item): array
public function summarizeReviewer(User $item, array $auxiliaryData = []): array
{
return $this->mapByProperties(array_merge($this->getSummaryProps(), ['reviewsActive', 'reviewsCompleted', 'reviewsDeclined', 'reviewsCancelled', 'averageReviewCompletionDays', 'dateLastReviewAssignment', 'reviewerRating']), $item);
return $this->mapByProperties(array_merge($this->getSummaryProps(), ['reviewsActive', 'reviewsCompleted', 'reviewsDeclined', 'reviewsCancelled', 'averageReviewCompletionDays', 'dateLastReviewAssignment', 'reviewerRating']), $item, $auxiliaryData);
}

/**
Expand Down Expand Up @@ -98,9 +102,11 @@ public function summarizeManyReviewers(Enumerable $collection): Enumerable
/**
* Map schema properties of a user to an assoc array
*
* @param array $auxiliaryData - Associative array used to provide supplementary data needed to populate properties on the response.
*
* @hook UserSchema::getProperties::values [[$this, &$output, $user, $props]]
*/
protected function mapByProperties(array $props, User $user): array
protected function mapByProperties(array $props, User $user, array $auxiliaryData = []): array
{
$output = [];

Expand Down Expand Up @@ -180,6 +186,54 @@ protected function mapByProperties(array $props, User $user): array
}
}
}
break;
case 'stageAssignments':
$submission = $auxiliaryData['submission'];
$stageId = $auxiliaryData['stageId'];

if((!isset($submission) || !isset($stageId)) || (!($submission instanceof Submission) || !is_numeric($auxiliaryData['stageId']))) {
$output['stageAssignments'] = [];
break;
}

// Get User's stage assignments for submission.
// Note:
// - A User can potentially have multiple assignments for a submission.
// - A User can potentially have multiple assignments for a stage of a submission.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@asmecher, I realized I've been a bit confused about this so I wanted to confirm before this is merged: could you confirm these two assumptions noted? I thought these were the case but I'm less sure after our recent discussion.

Copy link
Member

Choose a reason for hiding this comment

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

Let's say (using our default user groups) a user is assigned to a submission as both Section Editor and Layout Editor. Then they'll have a row in stage_assignments for each assignment. By default both the Section Editor and Layout Editor user groups are both active in the Production stage (per user_group_stage). So with the stage filter, this code should get the stage assignments for both Section Editor and Layout Editor. Does that clarify?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@asmecher, yes, thanks!

$stageAssignments = StageAssignment::withSubmissionIds([$submission->getId()])
->withStageIds($stageId ? [$stageId] : [])
->withUserId($user->getId())
->withContextId($this->context->getId())
->get();

$results = [];

foreach ($stageAssignments as $stageAssignment /**@var StageAssignment $stageAssignment*/) {
// Get related user group info for stage assignment
$userGroup = Repo::userGroup()->get($stageAssignment->userGroupId);

// Only prepare data for non-reviewer participants
if ($userGroup->getRoleId() !== Role::ROLE_ID_REVIEWER) {
$entry = [
'stageAssignmentId' => $stageAssignment->id,
'stageAssignmentUserGroup' => $userGroup->getAllData(),
'stageAssignmentStageId' => $stageId,
'recommendOnly' => (bool)$stageAssignment->recommendOnly,
'canChangeMetadata' => (bool)$stageAssignment->canChangeMetadata
];

$workflowStageDao = DAORegistry::getDAO('WorkflowStageDAO'); /** @var WorkflowStageDAO $workflowStageDao */
$entry['stageAssignmentStage'] = [
'id' => $stageId,
'label' => __($workflowStageDao->getTranslationKeyFromId($stageId)),
];

$results[] = $entry;
}

$output['stageAssignments'] = $results;
}

break;
default:
$output[$prop] = $user->getData($prop);
Expand Down
37 changes: 37 additions & 0 deletions schemas/user.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,43 @@
"validation": [
"nullable"
]
},
"stageAssignments": {
"apiSummary": true,
"readOnly": true,
"type": "array",
"items": {
"type": "object",
"properties": {
"stageAssignmentId": {
"type": "integer"
},
"stageAssignmentUserGroup": {
"type": "object",
"$ref": "#/definitions/UserGroup"
},
"stageAssignmentStageId": {
"type": "integer"
},
"stageAssignmentStage": {
"type": "object",
"properties": {
"id": {
"type": "string"
},
"label": {
"type": "string"
}
}
},
"canChangeMetadata": {
"type": "boolean"
},
"recommendOnly": {
"type": "boolean"
}
}
}
}
}
}