Skip to content

Commit

Permalink
#9914 changes after code review
Browse files Browse the repository at this point in the history
  • Loading branch information
@bozana
bozana committed Jun 6, 2024
1 parent 208ca40 commit fd6b973
Show file tree
Hide file tree
Showing 7 changed files with 247 additions and 46 deletions.
29 changes: 28 additions & 1 deletion classes/user/Collector.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class Collector implements CollectorInterface
public DAO $dao;

public string $orderBy = self::ORDERBY_ID;
public array $orderByUserGroupIds = [];
public string $orderDirection = 'ASC';
public ?array $orderLocales = null;
public ?array $userGroupIds = null;
Expand Down Expand Up @@ -206,7 +207,6 @@ public function filterByWorkflowStageIds(?array $workflowStageIds): self
return $this;
}


/**
* Limit results to users with user groups in these context IDs
*/
Expand Down Expand Up @@ -380,6 +380,17 @@ public function orderBy(string $sorter, string $direction = self::ORDER_DIR_DESC
return $this;
}

/**
* Order the results additionally by user group ID
*
* @param array $userGroupIds The IDs in the order the user query result should be ordered by
*/
public function orderByUserGroupIds(array $userGroupIds): self
{
$this->orderByUserGroupIds = $userGroupIds;
return $this;
}

/**
* Limit the number of objects retrieved
*/
Expand Down Expand Up @@ -732,6 +743,22 @@ protected function buildSearchFilter(Builder $query): self
*/
protected function buildOrderBy(Builder $query): self
{
if (!empty($this->orderByUserGroupIds)) {
$query->addSelect('uugob.user_group_id')
->leftJoin('user_user_groups AS uugob', 'uugob.user_id', '=', 'u.user_id');
switch (DB::getDriverName()) {
case 'mysql':
$userGroupOrderBy = implode(', ', $this->orderByUserGroupIds);
$query->orderByRaw("FIELD(uugob.user_group_id, {$userGroupOrderBy}) ASC");
break;
case 'pgsql':
$userGroupOrderBy = array_map(fn ($item) => 'uugob.user_group_id=' . $item, $this->orderByUserGroupIds);
$userGroupOrderBy = implode(', ', $userGroupOrderBy);
$query->orderByRaw("({$userGroupOrderBy}) ASC");
break;
}
}

$orderByFields = [self::ORDERBY_ID => 'u.user_id'];
if ($orderByField = $orderByFields[$this->orderBy] ?? null) {
$query->orderBy($orderByField, $this->orderDirection);
Expand Down
3 changes: 2 additions & 1 deletion classes/user/Repository.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,8 @@ public function mergeUsers(int $oldUserId, int $newUserId)
$userGroups = Repo::userGroup()->userUserGroups($oldUserId);
foreach ($userGroups as $userGroup) {
if (!Repo::userGroup()->userInGroup($newUserId, $userGroup->getId())) {
Repo::userGroup()->assignUserToGroup($newUserId, $userGroup->getId());
$mastheadStatus = Repo::userGroup()->getUserUserGroupMastheadStatus($oldUserId, $userGroup->getId());
Repo::userGroup()->assignUserToGroup($newUserId, $userGroup->getId(), null, null, $mastheadStatus);
}
}

Expand Down
112 changes: 102 additions & 10 deletions classes/userGroup/Repository.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
use APP\core\Request;
use APP\core\Services;
use APP\facades\Repo;
use Carbon\Carbon;
use Illuminate\Support\Collection;
use Illuminate\Support\LazyCollection;
use PKP\core\Core;
use PKP\db\DAORegistry;
use PKP\pages\about\AboutContextHandler;
use PKP\plugins\Hook;
use PKP\security\Role;
use PKP\services\PKPSchemaService;
Expand Down Expand Up @@ -147,6 +149,11 @@ public function add(UserGroup $userGroup): int

Hook::call('UserGroup::add', [$userGroup]);

// Clear editorial masthead cache if the new role should be added to the masthead
// Because it is a new role, no need to clear editorial history chache
if ($userGroup->getMasthead()) {
AboutContextHandler::forgetEditorialMastheadCache($userGroup->getContextId());
}
return $userGroup->getId();
}

Expand All @@ -158,6 +165,12 @@ public function edit(UserGroup $userGroup, array $params)

$this->dao->update($newUserGroup);

// Clear editorial masthead and history cache if the role is on the masthead
if ($userGroup->getMasthead()) {
AboutContextHandler::forgetEditorialMastheadCache($userGroup->getContextId());
AboutContextHandler::forgetEditorialHistoryCache($userGroup->getContextId());
}

Repo::userGroup()->get($newUserGroup->getId());
}

Expand Down Expand Up @@ -241,6 +254,23 @@ public function userUserGroups(int $userId, ?int $contextId = null, ?UserUserGro
return $collector->getMany();
}

/**
* Return all context IDs for masthead user groups the given user is or was assigned to
*
* @return Collection of context IDs
*/
public function getUserUserGroupsContextIds(int $userId): Collection
{
return Repo::userGroup()
->getCollector()
->filterByUserIds([$userId])
->filterByUserUserGroupStatus(UserUserGroupStatus::STATUS_ALL)
->filterByMasthead(true)
->getQueryBuilder()
->pluck('context_id')
->unique();
}

/**
* return whether a user is in a user group
*/
Expand All @@ -256,18 +286,40 @@ public function userInGroup(int $userId, int $userGroupId): bool
/**
* return whether an active user in a user group should be displayed on the masthead
*/
public function userOnMasthead(int $userId, int $userGroupId): bool
public function userOnMasthead(int $userId, ?int $userGroupId): bool
{
$userGroup = Repo::userGroup()->get($userGroupId);
if (!$userGroup->getMasthead()) {
return false;
if ($userGroupId) {
$userGroup = Repo::userGroup()->get($userGroupId);
if (!$userGroup->getMasthead()) {
return false;
}
}
return UserUserGroup::withUserId($userId)
$query = UserUserGroup::withUserId($userId)
->withActive()
->withMasthead();
if ($userGroupId) {
$query->withUserGroupId($userGroupId);
}
return $query->get()->isNotEmpty();
}

/**
* Get user masthead status for a user group the user is currently active in
*/
public function getUserUserGroupMastheadStatus(int $userId, int $userGroupId): UserUserGroupMastheadStatus
{
$masthead = UserUserGroup::withUserId($userId)
->withUserGroupId($userGroupId)
->withActive()
->withMasthead()
->get()
->isNotEmpty();
->pluck('masthead');
switch ($masthead[0]) {
case 1:
return UserUserGroupMastheadStatus::STATUS_ON;
case 0:
return UserUserGroupMastheadStatus::STATUS_OFF;
case null:
return UserUserGroupMastheadStatus::STATUS_NULL;
}
}

/**
Expand All @@ -282,9 +334,20 @@ public function contextHasGroup(int $contextId, int $userGroupId): bool
->getCount() > 0;
}

public function assignUserToGroup(int $userId, int $userGroupId, ?string $startDate = null, ?string $endDate = null, ?UserUserGroupMastheadStatus $mastheadStatus = null): UserUserGroup
/**
* Assign a user to a role
*
* @param string|null $startDate The date in ISO (YYYY-MM-DD HH:MM:SS) format
* @param string|null $endDate The date in ISO (YYYY-MM-DD HH:MM:SS) format
*/
public function assignUserToGroup(int $userId, int $userGroupId, ?string $startDate = null, ?string $endDate = null, ?UserUserGroupMastheadStatus $mastheadStatus = null): ?UserUserGroup
{
if ($endDate && !Carbon::parse($endDate)->isFuture()) {
return null;
}

$dateStart = $startDate ?? Core::getCurrentDate();
$userGroup = Repo::userGroup()->get($userGroupId);
// user_user_group's masthead does not inherit from the user_group's masthead,
// it needs to be specified (when accepting an invitations).
switch ($mastheadStatus) {
Expand All @@ -295,7 +358,11 @@ public function assignUserToGroup(int $userId, int $userGroupId, ?string $startD
$masthead = 0;
break;
default:
$masthead = null;
$masthead = $userGroup->getMasthead() ? 1 : null;
}
// Clear editorial masthead cache if a new user is assigned to a masthead role
if ($userGroup->getMasthead()) {
AboutContextHandler::forgetEditorialMastheadCache($userGroup->getContextId());
}
return UserUserGroup::create([
'userId' => $userId,
Expand All @@ -306,19 +373,41 @@ public function assignUserToGroup(int $userId, int $userGroupId, ?string $startD
]);
}

/**
* Remove all user role assignments. This should be used only when merging i.e. fully deleting an user.
*/
public function deleteAssignmentsByUserId(int $userId, ?int $userGroupId = null): bool
{
if (!$userGroupId) {
$contextIds = $this->getUserUserGroupsContextIds($userId);
foreach ($contextIds as $contextId) {
AboutContextHandler::forgetEditorialMastheadCache($contextId);
AboutContextHandler::forgetEditorialHistoryCache($contextId);
}
}

$query = UserUserGroup::withUserId($userId);

if ($userGroupId) {
$query->withUserGroupId($userGroupId);
$userGroup = $this->get($userGroupId);
if ($userGroup->getMasthead()) {
AboutContextHandler::forgetEditorialMastheadCache($contextId);
AboutContextHandler::forgetEditorialHistoryCache($contextId);
}
}

return $query->delete();
}

public function endAssignments(int $contextId, int $userId, ?int $userGroupId = null): void
{
// Clear editorial masthead and history cache if the user was displayed on the masthead for the given role
if ($this->userOnMasthead($userId, $userGroupId)) {
AboutContextHandler::forgetEditorialMastheadCache($contextId);
AboutContextHandler::forgetEditorialHistoryCache($contextId);
}

$dateEnd = Core::getCurrentDate();
$query = UserUserGroup::withContextId($contextId)
->withUserId($userId)
Expand Down Expand Up @@ -481,6 +570,9 @@ public function installSettings($contextId, $filename)
}
}

AboutContextHandler::forgetEditorialMastheadCache($contextId);
AboutContextHandler::forgetEditorialHistoryCache($contextId);

return true;
}

Expand Down
2 changes: 1 addition & 1 deletion locale/en/manager.po
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1860,7 +1860,7 @@ msgstr "Yes, require submitting authors to upload one or more of these files."
msgid "manager.setup.genres.submitRequired.no"
msgstr "No, allow new submissions without these files."

msgid "manager.setup.appereance.editorialMasthead.description"
msgid "manager.setup.appearance.editorialMasthead.description"
msgstr "Define the order of editorial masthead roles for public display."

msgid "manager.settings.wizard"
Expand Down
Loading

0 comments on commit fd6b973

Please sign in to comment.