Skip to content

Commit

Permalink
pkp#10506 Fix calls to removed user group collector
Browse files Browse the repository at this point in the history
  • Loading branch information
Vitaliy-1 committed Nov 29, 2024
1 parent e6186ed commit d0a4189
Show file tree
Hide file tree
Showing 13 changed files with 69 additions and 151 deletions.
5 changes: 2 additions & 3 deletions api/v1/submissions/PKPSubmissionController.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@
use PKP\submissionFile\SubmissionFile;
use PKP\userGroup\UserGroup;


class PKPSubmissionController extends PKPBaseController
{
use AnonymizeData;
Expand Down Expand Up @@ -630,7 +629,7 @@ public function add(Request $illuminateRequest): JsonResponse
Repo::userGroup()->assignUserToGroup(
$user->getId(),
$submitAsUserGroup->id
);
);
}
}

Expand Down Expand Up @@ -845,7 +844,7 @@ public function submit(Request $illuminateRequest): JsonResponse
$contextId = $context->getId();
$userGroups = UserGroup::withContextIds($contextId)->cursor();



/** @var GenreDAO $genreDao */
$genreDao = DAORegistry::getDAO('GenreDAO');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
use PKP\submission\reviewRound\ReviewRoundDAO;
use PKP\submission\SubmissionCommentDAO;
use PKP\user\User;
use PKP\userGroup\UserGroup;
use Symfony\Component\Mailer\Exception\TransportException;

class PKPReviewerGridHandler extends GridHandler
Expand Down Expand Up @@ -472,10 +473,11 @@ public function getUsersNotAssignedAsReviewers($args, $request)
{
$context = $request->getContext();
$term = $request->getUserVar('term');
$reviewerUserGroupIds = Repo::userGroup()->getCollector()
->filterByContextIds([$context->getId()])
->filterByRoleIds([Role::ROLE_ID_REVIEWER])

$reviewerUserGroupIds = UserGroup::withContextIds([$context->getId()])
->withRoleIds([Role::ROLE_ID_REVIEWER])
->getIds();

$users = Repo::user()->getCollector()
->filterExcludeUserGroupIds(iterator_to_array($reviewerUserGroupIds))
->searchPhrase($term)
Expand Down Expand Up @@ -563,7 +565,7 @@ public function updateReinstateReviewer($args, $request)
$template = Repo::emailTemplate()->getByKey($context->getId(), ReviewerReinstate::getEmailTemplateKey());
$mailable = new ReviewerReinstate($context, $submission, $reviewAssignment);

if($this->createMail($mailable, $request->getUserVar('personalMessage'), $template, $user, $reviewer)) {
if ($this->createMail($mailable, $request->getUserVar('personalMessage'), $template, $user, $reviewer)) {
Repo::emailLogEntry()->logMailable(SubmissionEmailLogEventType::REVIEW_REINSTATED, $mailable, $submission, $user);
}
}
Expand Down Expand Up @@ -622,7 +624,7 @@ public function updateResendRequestReviewer($args, $request)
$template = Repo::emailTemplate()->getByKey($context->getId(), ReviewerResendRequest::getEmailTemplateKey());
$mailable = new ReviewerResendRequest($context, $submission, $reviewAssignment);

if($this->createMail($mailable, $request->getUserVar('personalMessage'), $template, $user, $reviewer)) {
if ($this->createMail($mailable, $request->getUserVar('personalMessage'), $template, $user, $reviewer)) {
Repo::emailLogEntry()->logMailable(SubmissionEmailLogEventType::REVIEW_RESEND, $mailable, $submission, $user);
}
}
Expand Down Expand Up @@ -661,7 +663,7 @@ public function updateUnassignReviewer($args, $request)
$template = Repo::emailTemplate()->getByKey($context->getId(), ReviewerUnassign::getEmailTemplateKey());
$mailable = new ReviewerUnassign($context, $submission, $reviewAssignment);

if($this->createMail($mailable, $request->getUserVar('personalMessage'), $template, $user, $reviewer)) {
if ($this->createMail($mailable, $request->getUserVar('personalMessage'), $template, $user, $reviewer)) {
Repo::emailLogEntry()->logMailable(SubmissionEmailLogEventType::REVIEW_CANCEL, $mailable, $submission, $user);
}
}
Expand Down
3 changes: 1 addition & 2 deletions classes/install/PKPInstall.php
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,7 @@ public function createData()
$adminUserGroup->save();

// Assign the user to the admin user group
$repository = Repo::userGroup();
$repository->assignUserToGroup($user->getId(), $adminUserGroup->id);
Repo::userGroup()->assignUserToGroup($user->getId(), $adminUserGroup->id);

// Add initial site data
/** @var SiteDAO */
Expand Down
17 changes: 7 additions & 10 deletions classes/invitation/stepTypes/SendInvitationStep.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,21 @@
*
* @brief create accept invitation steps.
*/

namespace PKP\invitation\stepTypes;

use APP\core\Application;
use Exception;
use PKP\components\forms\invitation\UserDetailsForm;
use PKP\context\Context;
use PKP\facades\Repo;
use PKP\invitation\core\Invitation;
use PKP\invitation\sections\Email;
use PKP\invitation\sections\Form;
use PKP\invitation\sections\Sections;
use PKP\invitation\steps\Step;
use PKP\mail\mailables\UserRoleAssignmentInvitationNotify;
use PKP\user\User;
use PKP\userGroup\UserGroup;
use stdClass;

class SendInvitationStep extends InvitationStepTypes
Expand All @@ -36,7 +37,7 @@ class SendInvitationStep extends InvitationStepTypes
public function getSteps(?Invitation $invitation, Context $context, ?User $user): array
{
$steps = [];
if(!$invitation && !$user) {
if (!$invitation && !$user) {
$steps[] = $this->invitationSearchUser();
}
$steps[] = $this->invitationDetailsForm($context);
Expand All @@ -54,7 +55,7 @@ private function invitationSearchUser(): stdClass
__('userInvitation.searchUser.stepName'),
'form',
'UserInvitationSearchFormStep',
__('userInvitation.searchUser.stepDescription'),
__('userInvitation.searchUser.stepDescription'),
);
$sections->addSection(
null,
Expand Down Expand Up @@ -167,19 +168,15 @@ private function invitationInvitedEmail(Context $context): stdClass

/**
* Get all user groups
* @param Context $context
* @return array
*/
private function getAllUserGroups(Context $context): array
{
$allUserGroups = [];
$userGroups = Repo::userGroup()->getCollector()
->filterByContextIds([$context->getId()])
->getMany();
$userGroups = UserGroup::withContextIds([$context->getId()])->get();
foreach ($userGroups as $userGroup) {
$allUserGroups[] = [
'value' => (int) $userGroup->getId(),
'label' => $userGroup->getLocalizedName(),
'value' => (int) $userGroup->id,
'label' => $userGroup->getLocalizedData('name'),
'disabled' => false
];
}
Expand Down
4 changes: 2 additions & 2 deletions classes/security/authorization/CanAccessSettingsPolicy.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ public function effect()
// At least one user group must be an admin, or a manager with setup access.
$userGroups = $this->getAuthorizedContextObject(Application::ASSOC_TYPE_USER_GROUP);
foreach ($userGroups as $userGroup) {
if ($userGroup->getRoleId() == ROLE_ID_SITE_ADMIN) {
if ($userGroup->roleId == Role::ROLE_ID_SITE_ADMIN) {
return AuthorizationPolicy::AUTHORIZATION_PERMIT;
}
if ($userGroup->getRoleId() == Role::ROLE_ID_MANAGER && $userGroup->getPermitSettings()) {
if ($userGroup->roleId == Role::ROLE_ID_MANAGER && $userGroup->permitSettings) {
return AuthorizationPolicy::AUTHORIZATION_PERMIT;
}
}
Expand Down
26 changes: 13 additions & 13 deletions classes/submission/maps/Schema.php
Original file line number Diff line number Diff line change
Expand Up @@ -541,12 +541,12 @@ function (Role $role) {
->withStageIds([$stageId])
->get();

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

// Replaces StageAssignmentDAO::getBySubmissionAndUserIdAndStageId
$stageAssignments = StageAssignment::withSubmissionIds([$submission->getId()])
Expand All @@ -557,18 +557,18 @@ function (Role $role) {
foreach ($stageAssignments as $stageAssignment) {
$userGroup = UserGroup::find($stageAssignment->userGroupId);
$stageAssignmentsOverview[] = [
"roleId" => $userGroup?->roleId ?? null,
"recommendOnly" => $stageAssignment->recommendOnly,
"canChangeMetadata" => $stageAssignment->canChangeMetadata,
"userId" => $stageAssignment->userId
'roleId' => $userGroup?->roleId ?? null,
'recommendOnly' => $stageAssignment->recommendOnly,
'canChangeMetadata' => $stageAssignment->canChangeMetadata,
'userId' => $stageAssignment->userId
];
}
}
$stage['currentUserAssignedRoles'] = array_values(array_unique($currentUserAssignedRoles));
// FIXME - $stageAssignments are just temporarly added until https://github.com/pkp/pkp-lib/issues/10480 is ready
$stage['stageAssignments'] = $stageAssignmentsOverview;
if(!$stage['currentUserAssignedRoles']) {
if(in_array(Role::ROLE_ID_MANAGER, $currentRoles)) {
if (!$stage['currentUserAssignedRoles']) {
if (in_array(Role::ROLE_ID_MANAGER, $currentRoles)) {
$stage['currentUserAssignedRoles'][] = Role::ROLE_ID_MANAGER;
}
}
Expand Down Expand Up @@ -764,8 +764,8 @@ protected function checkDecisionPermissions(int $stageId, Submission $submission
if (!$makeRecommendation && !$makeDecision) {
$userGroups = Repo::userGroup()->userUserGroups($user->getId(), $contextId);
foreach ($userGroups as $userGroup) {
if (in_array($userGroup->getRoleId(), [Role::ROLE_ID_MANAGER, Role::ROLE_ID_SITE_ADMIN])) {
if (!$userGroup->getRecommendOnly()) {
if (in_array($userGroup->roleId, [Role::ROLE_ID_MANAGER, Role::ROLE_ID_SITE_ADMIN])) {
if (!$userGroup->recommendOnly) {
$makeDecision = true;
} else {
$makeRecommendation = true;
Expand Down
16 changes: 8 additions & 8 deletions classes/template/PKPTemplateManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@
use PKP\submission\GenreDAO;
use PKP\submission\PKPSubmission;
use PKP\submissionFile\SubmissionFile;
use PKP\userGroup\UserGroup;
use Smarty;
use Smarty_Internal_Template;
use PKP\userGroup\UserGroup;

require_once('./lib/pkp/lib/vendor/smarty/smarty/libs/plugins/modifier.escape.php'); // Seems to be needed?
Expand Down Expand Up @@ -988,7 +988,7 @@ public function setupBackendPage()

if ($request->getContext()) {
if (count(array_intersect([Role::ROLE_ID_MANAGER, Role::ROLE_ID_SITE_ADMIN, Role::ROLE_ID_SUB_EDITOR, Role::ROLE_ID_ASSISTANT, Role::ROLE_ID_REVIEWER, Role::ROLE_ID_AUTHOR], $userRoles))) {
if(Config::getVar('features', 'enable_new_submission_listing')) {
if (Config::getVar('features', 'enable_new_submission_listing')) {
if (count(array_intersect([Role::ROLE_ID_MANAGER, Role::ROLE_ID_SITE_ADMIN, Role::ROLE_ID_SUB_EDITOR, Role::ROLE_ID_ASSISTANT], $userRoles))) {
$dashboardViews = Repo::submission()->getDashboardViews($request->getContext(), $request->getUser(), [Role::ROLE_ID_MANAGER, Role::ROLE_ID_SITE_ADMIN, Role::ROLE_ID_SUB_EDITOR, Role::ROLE_ID_ASSISTANT]);
$requestedPage = $router->getRequestedPage($request);
Expand Down Expand Up @@ -1017,7 +1017,7 @@ public function setupBackendPage()
'submenu' => $viewsData
];
}
if(count(array_intersect([ Role::ROLE_ID_REVIEWER], $userRoles))) {
if (count(array_intersect([ Role::ROLE_ID_REVIEWER], $userRoles))) {
$dashboardViews = Repo::submission()->getDashboardViews($request->getContext(), $request->getUser(), [Role::ROLE_ID_REVIEWER]);
$requestedPage = $router->getRequestedPage($request);
$requestedOp = $router->getRequestedOp($request);
Expand All @@ -1038,7 +1038,7 @@ public function setupBackendPage()
'icon' => 'ReviewAssignments',
];
}
if(count(array_intersect([ Role::ROLE_ID_AUTHOR], $userRoles))) {
if (count(array_intersect([ Role::ROLE_ID_AUTHOR], $userRoles))) {
$dashboardViews = Repo::submission()->getDashboardViews($request->getContext(), $request->getUser(), [Role::ROLE_ID_AUTHOR]);
$requestedPage = $router->getRequestedPage($request);
$requestedOp = $router->getRequestedOp($request);
Expand Down Expand Up @@ -1113,7 +1113,7 @@ public function setupBackendPage()
}

$userGroups = (array) $router->getHandler()->getAuthorizedContextObject(Application::ASSOC_TYPE_USER_GROUP);
$hasSettingsAccess = array_reduce($userGroups, fn ($carry, $userGroup) => $carry || $userGroup->getPermitSettings(), false);
$hasSettingsAccess = array_reduce($userGroups, fn ($carry, $userGroup) => $carry || $userGroup->permitSettings, false);
if ($hasSettingsAccess) {
$menu['settings'] = [
'name' => __('navigation.settings'),
Expand Down Expand Up @@ -1344,15 +1344,15 @@ public function display($template = null, $cache_id = null, $compile_id = null,
$query->where('user_id', $user->getId())
->where(function ($q) {
$q->whereNull('date_end')
->orWhere('date_end', '>', now());
->orWhere('date_end', '>', now());
})
->where(function ($q) {
$q->whereNull('date_start')
->orWhere('date_start', '<=', now());
->orWhere('date_start', '<=', now());
});
})
->get();

$userRoles = [];
foreach ($userGroups as $userGroup) {
$userRoles[] = (int) $userGroup->role_id;
Expand Down
10 changes: 5 additions & 5 deletions classes/userGroup/Repository.php
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ public function installSettings(?int $contextId, string $filename): bool
// If has manager role then permitMetadataEdit can't be overridden
if (in_array($roleId, self::NOT_CHANGE_METADATA_EDIT_PERMISSION_ROLES)) {
$permitMetadataEdit = $setting->getAttribute('permitMetadataEdit') === 'true';
$permitSettings = $setting->getAttribute('permitSettings');
$permitSettings = $setting->getAttribute('permitSettings') === 'true';
}

$defaultStages = explode(',', (string) $setting->getAttribute('stages'));
Expand All @@ -494,7 +494,7 @@ public function installSettings(?int $contextId, string $filename): bool
'isDefault' => true,
'showTitle' => true,
'masthead' => $masthead,
'permit_settings' => $permitSettings,
'permitSettings' => $permitSettings,
]);

// Save the UserGroup instance to the database
Expand All @@ -507,9 +507,9 @@ public function installSettings(?int $contextId, string $filename): bool
$stageId = (int) trim($stageId);
if ($stageId >= WORKFLOW_STAGE_ID_SUBMISSION && $stageId <= WORKFLOW_STAGE_ID_PRODUCTION) {
UserGroupStage::create([
'context_id' => $contextId,
'user_group_id' => $userGroupId,
'stage_id' => $stageId,
'contextId' => $contextId,
'userGroupId' => $userGroupId,
'stageId' => $stageId,
]);
}
}
Expand Down
56 changes: 1 addition & 55 deletions classes/userGroup/UserGroup.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
use Illuminate\Database\Eloquent\Relations\HasMany;
use PKP\core\PKPApplication;
use PKP\core\traits\ModelWithSettings;
use PKP\facades\Locale;
use PKP\facades\Repo;
use PKP\plugins\Hook;
use PKP\services\PKPSchemaService;
Expand Down Expand Up @@ -65,30 +64,10 @@ class UserGroup extends Model
'showTitle',
'permitSelfRegistration',
'permitMetadataEdit',
'permitSettings' => 'boolean',
'permitSettings',
'masthead',
];

/**
* The attributes that should be cast.
*
* @return array<string, string>
*/
protected function casts(): array
{
return [
'contextId' => 'integer',
'roleId' => 'integer',
'isDefault' => 'boolean',
'showTitle' => 'boolean',
'permitSelfRegistration' => 'boolean',
'permitMetadataEdit' => 'boolean',
'masthead' => 'boolean',
'permitSettings' => 'boolean',
// multilingual attributes will be handled through accessors and mutators
];
}

/**
* Get the settings table name
*/
Expand Down Expand Up @@ -383,39 +362,6 @@ protected function scopeWithPublicationIds(EloquentBuilder $builder, array $publ
});
}

/**
* Set the name attribute.
*
*/
public function setNameAttribute($value): void
{
if (is_string($value)) {
$value = $this->localizeNonLocalizedData($value);
}
$this->setData('name', $value);
}

/**
* Set the abbrev attribute.
*
*/
public function setAbbrevAttribute($value): void
{
if (is_string($value)) {
$value = $this->localizeNonLocalizedData($value);
}
$this->setData('abbrev', $value);
}

/**
* Localize non-localized data.
*
*/
protected function localizeNonLocalizedData(string $value): array
{
return [Locale::getLocale() => $value];
}

/**
* Find a UserGroup by ID and optional context ID.
*
Expand Down
Loading

0 comments on commit d0a4189

Please sign in to comment.