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.

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

pkp/pkp-lib#i9963 API Endpoint to download review files #10112

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Vitaliy-1
Copy link
Collaborator

No description provided.

@taslangraham
Copy link
Contributor

taslangraham commented Aug 2, 2024

The implemented endpoint seems to support the use-case described in #9964. Assigned files can be retrieved by sending the appropriate fileStage(4) in the fileStages param.

// Get review files reviewer has access to
$files = collect();
if (in_array(SubmissionFile::SUBMISSION_FILE_REVIEW_FILE, $fileStages)) {
$files = $files->merge(
Repo::submissionFile()->getCollector()
->filterByReviewIds([$reviewAssignment->getId()])
->filterByFileStages([SubmissionFile::SUBMISSION_FILE_REVIEW_FILE, SubmissionFile::SUBMISSION_FILE_INTERNAL_REVIEW_FILE])
->getMany()
);
}

@Vitaliy-1 @ewhanson @jardakotesovec

public function effect()
{
$user = $this->request->getUser();
if (!$user instanceof User) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This make sense in theory but doesn't allow for admin access (site admin, journal manager, etc.) or other elevated roles like editors. As a site admin, I would expect all routes to be accessible via the site admin API key. I saw you said you based this on the ReviewAssignmentAccessPolicy. I think it makes sense to keep this as part of the ReviewAssignmentFileAccessPolicy as-is but to conditionally apply the policy check like ReviewAssignmentAccessPolicy is here:

//
// Reviewer role
//
if (isset($roleAssignments[Role::ROLE_ID_REVIEWER])) {
// 1) Reviewers can access whitelisted operations ...
$reviewerSubmissionAccessPolicy = new PolicySet(PolicySet::COMBINING_DENY_OVERRIDES);
$reviewerSubmissionAccessPolicy->addPolicy(new RoleBasedHandlerOperationPolicy($request, Role::ROLE_ID_REVIEWER, $roleAssignments[Role::ROLE_ID_REVIEWER]));
// 2) ... but only if they have been assigned to the submission as reviewers.
$reviewerSubmissionAccessPolicy->addPolicy(new ReviewAssignmentAccessPolicy($request, $permitDeclined));
$submissionAccessPolicy->addPolicy($reviewerSubmissionAccessPolicy);
}

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.

3 participants