Merge pull request #10754 from touhidurabir/i10751_main

#10751 EditorialMasthead page issue with eloquent based user group update
pkp · Jan 8, 2025 · 23b4712 · 23b4712
2 parents 4fd34f7 + 4aaf185
commit 23b4712
Show file tree

Hide file tree

Showing 13 changed files with 75 additions and 59 deletions.
diff --git a/classes/components/forms/context/PKPRestrictBulkEmailsForm.php b/classes/components/forms/context/PKPRestrictBulkEmailsForm.php
@@ -16,7 +16,7 @@
 namespace PKP\components\forms\context;
 
 use APP\core\Application;
-use Illuminate\Support\LazyCollection;
+use Illuminate\Support\Collection;
 use PKP\components\forms\FieldOptions;
 use PKP\components\forms\FormComponent;
 
@@ -31,7 +31,7 @@ class PKPRestrictBulkEmailsForm extends FormComponent
      *
      * @param string $action URL to submit the form to
      */
-    public function __construct($action, $context, LazyCollection $userGroups)
+    public function __construct($action, $context, Collection $userGroups)
     {
         $this->action = $action;
 

diff --git a/classes/invitation/invitations/userRoleAssignment/rules/AddUserGroupRule.php b/classes/invitation/invitations/userRoleAssignment/rules/AddUserGroupRule.php
@@ -32,7 +32,7 @@ public function passes($attribute, $value)
         // At this point, we know the user group exists; check if the user has it assigned
         if ($user = $this->invitation->getExistingUser()) {
             $userUserGroups = UserUserGroup::withUserId($user->getId())
-                ->withUserGroupId($value) // The $value is the userGroupId
+                ->withUserGroupIds([$value]) // The $value is the userGroupId
                 ->withActive()
                 ->get();
 

diff --git a/classes/mail/mailables/UserRoleAssignmentInvitationNotify.php b/classes/mail/mailables/UserRoleAssignmentInvitationNotify.php
@@ -188,7 +188,7 @@ public function setData(?string $locale = null): void
 
             foreach ($userGroups as $userGroup) {
                 $userUserGroups = UserUserGroup::withUserId($user->getId())
-                    ->withUserGroupId($userGroup->id)
+                    ->withUserGroupIds([$userGroup->id])
                     ->withActive()
                     ->get();
 

diff --git a/classes/services/PKPContextService.php b/classes/services/PKPContextService.php
@@ -562,7 +562,7 @@ public function add($context, $request)
 
         $assignmentExists = UserUserGroup::query()
             ->withUserId($currentUser->getId())
-            ->withUserGroupId($managerUserGroup->id)
+            ->withUserGroupIds([$managerUserGroup->id])
             ->exists();
 
         if (!$assignmentExists) {

diff --git a/classes/user/Repository.php b/classes/user/Repository.php
@@ -344,7 +344,7 @@ public function mergeUsers(int $oldUserId, int $newUserId)
         $submissionCommentDao = DAORegistry::getDAO('SubmissionCommentDAO'); /** @var SubmissionCommentDAO $submissionCommentDao */
         $submissionComments = $submissionCommentDao->getByUserId($oldUserId);
 
-        while ($submissionComment = $submissionComments->next()) {
+        while ($submissionComment = $submissionComments->next()) { /** @var \PKP\submission\SubmissionComment $submissionComment */
             $submissionComment->setAuthorId($newUserId);
             $submissionCommentDao->updateObject($submissionComment);
         }
@@ -369,7 +369,7 @@ public function mergeUsers(int $oldUserId, int $newUserId)
             // Check if the new user is already assigned to this user group
             $exists = UserUserGroup::query()
                 ->withUserId($newUserId)
-                ->withUserGroupId($userUserGroup->userGroupId)
+                ->withUserGroupIds([$userUserGroup->userGroupId])
                 ->exists();
 
             if (!$exists) {

diff --git a/classes/user/maps/Schema.php b/classes/user/maps/Schema.php
@@ -177,7 +177,7 @@ protected function mapByProperties(array $props, User $user, array $auxiliaryDat
                                 'recommendOnly' => (bool) $userGroup->recommendOnly,
                                 'dateStart' => UserUserGroup::withUserId($user->getId())
                                     ->withActive()
-                                    ->withUserGroupId($userGroup->id)
+                                    ->withUserGroupIds([$userGroup->id])
                                     ->pluck('date_start')->first()
                             ];
                         }

diff --git a/classes/userGroup/Repository.php b/classes/userGroup/Repository.php
@@ -13,23 +13,28 @@
 
 namespace PKP\userGroup;
 
-use Carbon\Carbon;
+use stdClass;
 use DateInterval;
-use Illuminate\Support\Collection;
-use Illuminate\Support\Facades\Cache;
-use Illuminate\Support\LazyCollection;
+use Carbon\Carbon;
 use PKP\core\Core;
-use PKP\db\DAORegistry;
+use APP\facades\Repo;
 use PKP\plugins\Hook;
+use PKP\site\SiteDAO;
 use PKP\security\Role;
+use PKP\db\DAORegistry;
+use PKP\facades\Locale;
+use APP\core\Application;
+use PKP\xml\PKPXMLParser;
+use Illuminate\Support\Collection;
 use PKP\services\PKPSchemaService;
-use PKP\site\SiteDAO;
-use PKP\userGroup\relationships\enums\UserUserGroupMastheadStatus;
-use PKP\userGroup\relationships\enums\UserUserGroupStatus;
-use PKP\userGroup\relationships\UserGroupStage;
-use PKP\userGroup\relationships\UserUserGroup;
 use PKP\validation\ValidatorFactory;
-use PKP\xml\PKPXMLParser;
+use Illuminate\Support\Facades\Cache;
+use Illuminate\Support\LazyCollection;
+use Illuminate\Database\Query\JoinClause;
+use PKP\userGroup\relationships\UserUserGroup;
+use PKP\userGroup\relationships\UserGroupStage;
+use PKP\userGroup\relationships\enums\UserUserGroupStatus;
+use PKP\userGroup\relationships\enums\UserUserGroupMastheadStatus;
 
 class Repository
 {
@@ -218,7 +223,7 @@ public function userInGroup(int $userId, int $userGroupId): bool
     {
         return UserUserGroup::query()
             ->withUserId($userId)
-            ->withUserGroupId($userGroupId)
+            ->withUserGroupIds([$userGroupId])
             ->withActive()
             ->exists();
     }
@@ -242,7 +247,7 @@ public function userOnMasthead(int $userId, ?int $userGroupId = null): bool
             ->withMasthead();
 
         if ($userGroupId) {
-            $query->withUserGroupId($userGroupId);
+            $query->withUserGroupIds([$userGroupId]);
         }
 
         return $query->exists();
@@ -256,7 +261,7 @@ public function getUserUserGroupMastheadStatus(int $userId, int $userGroupId): U
     {
         $masthead = UserUserGroup::query()
             ->withUserId($userId)
-            ->withUserGroupId($userGroupId)
+            ->withUserGroupIds([$userGroupId])
             ->withActive()
             ->pluck('masthead')
             ->first();
@@ -344,7 +349,7 @@ public function deleteAssignmentsByUserId(int $userId, ?int $userGroupId = null)
         $query = UserUserGroup::query()->withUserId($userId);
 
         if ($userGroupId) {
-            $query->withUserGroupId($userGroupId);
+            $query->withUserGroupIds([$userGroupId]);
             $userGroup = $this->get($userGroupId);
             if ($userGroup && $userGroup->masthead) {
                 self::forgetEditorialCache($userGroup->contextId);
@@ -374,7 +379,7 @@ public function endAssignments(int $contextId, int $userId, ?int $userGroupId =
             ->withActive();
 
         if ($userGroupId) {
-            $query->withUserGroupId($userGroupId);
+            $query->withUserGroupIds([$userGroupId]);
         }
 
         $query->update(['date_end' => $dateEnd]);
@@ -587,23 +592,32 @@ public function getMastheadUserIdsByRoleIds(array $mastheadRoles, int $contextId
             // Query that gets all users that are or were active in the given masthead roles
             // and that have accepted to be displayed on the masthead for the roles.
             // Sort the results by role ID and user family name.
-            $users = UserUserGroup::query()
-                ->withContextId($contextId)
-                ->withUserGroupIds($mastheadRoleIds)
-                ->withUserUserGroupStatus($userUserGroupStatus->value)
-                ->withUserUserGroupMastheadStatus(UserUserGroupMastheadStatus::STATUS_ON->value)
-                ->orderBy('user_groups.role_id', 'asc')
-                ->join('user_groups', 'user_user_groups.user_group_id', '=', 'user_groups.user_group_id')
-                ->join('users', 'user_user_groups.user_id', '=', 'users.user_id')
-                ->orderBy('users.family_name', 'asc')
-                ->get(['user_groups.user_group_id', 'users.user_id']);
-
-            // group unique user ids by UserGroup id
-            $userIdsByUserGroupId = $users->groupBy('user_group_id')->map(function ($group) {
-                return $group->pluck('user_id')->unique()->toArray();
-            })->toArray();
-
-            return $userIdsByUserGroupId;
+            $usersCollector = Repo::user()->getCollector();
+            $usersQuery = $usersCollector
+                ->filterByContextIds([$contextId])
+                ->filterByUserGroupIds($mastheadRoleIds)
+                ->filterByUserUserGroupStatus($userUserGroupStatus)
+                ->filterByUserUserGroupMastheadStatus(UserUserGroupMastheadStatus::STATUS_ON)
+                ->orderBy(
+                    $usersCollector::ORDERBY_FAMILYNAME,
+                    $usersCollector::ORDER_DIR_ASC, 
+                    [
+                        Locale::getLocale(),
+                        Application::get()->getRequest()->getSite()->getPrimaryLocale()
+                    ]
+                )
+                ->orderByUserGroupIds($mastheadRoleIds)
+                ->getQueryBuilder()
+                ->get();
+
+            // Get unique user IDs grouped by user group ID
+            $userIdsByUserGroupId = $usersQuery->mapToGroups(function (stdClass $item, int $key) {
+                return [$item->user_group_id => $item->user_id];
+            })->map(function ($item) {
+                return collect($item)->unique();
+            });
+
+            return $userIdsByUserGroupId->toArray();
         });
     }
 

diff --git a/classes/userGroup/relationships/UserUserGroup.php b/classes/userGroup/relationships/UserUserGroup.php
@@ -14,13 +14,13 @@
 
 namespace PKP\userGroup\relationships;
 
+use PKP\core\Core;
 use APP\facades\Repo;
+use PKP\userGroup\UserGroup;
+use Eloquence\Behaviours\HasCamelCasing;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Casts\Attribute;
 use Illuminate\Database\Eloquent\Relations\BelongsTo;
-use Eloquence\Behaviours\HasCamelCasing;
-use PKP\core\Core;
-use PKP\userGroup\UserGroup;
 
 class UserUserGroup extends \Illuminate\Database\Eloquent\Model
 {
@@ -52,9 +52,9 @@ public function scopeWithUserId(Builder $query, int $userId): Builder
         return $query->where('user_user_groups.user_id', $userId);
     }
 
-    public function scopeWithUserGroupId(Builder $query, int $userGroupId): Builder
+    public function scopeWithUserGroupIds(Builder $query, array $userGroupIds): Builder
     {
-        return $query->where('user_user_groups.user_group_id', $userGroupId);
+        return $query->whereIn('user_user_groups.user_group_id', $userGroupIds);
     }
 
     public function scopeWithActive(Builder $query): Builder

diff --git a/controllers/grid/settings/user/form/UserForm.php b/controllers/grid/settings/user/form/UserForm.php
@@ -143,7 +143,7 @@ public function saveUserGroupAssignments(Request $request): void
                     fn ($userGroupId) =>
                     UserUserGroup::query()
                         ->withUserId($this->userId)
-                        ->withUserGroupId($userGroupId)
+                        ->withUserGroupIds([$userGroupId])
                         ->withActive()
                         ->update(['date_end' => now()])
                 );

diff --git a/pages/about/AboutContextHandler.php b/pages/about/AboutContextHandler.php
@@ -21,6 +21,8 @@
 use APP\handler\Handler;
 use APP\template\TemplateManager;
 use DateTime;
+use Illuminate\Support\Collection;
+use PKP\context\Context;
 use PKP\facades\Locale;
 use PKP\orcid\OrcidManager;
 use PKP\plugins\Hook;
@@ -61,7 +63,7 @@ public function index($args, $request)
     }
 
 
-    private function getSortedMastheadUserGroups($context)
+    private function getSortedMastheadUserGroups(Context $context): Collection
     {
         $mastheadUserGroups = UserGroup::withContextIds([$context->getId()])
             ->masthead(true)
@@ -95,7 +97,7 @@ public function editorialMasthead($args, $request)
 
         // Get all user IDs grouped by user group ID for the masthead roles
         $allUsersIdsGroupedByUserGroupId = Repo::userGroup()->getMastheadUserIdsByRoleIds(
-            $mastheadRoles,
+            $mastheadRoles->all(),
             $context->getId()
         );
 
@@ -104,7 +106,7 @@ public function editorialMasthead($args, $request)
             foreach ($allUsersIdsGroupedByUserGroupId[$userGroupId] ?? [] as $userId) {
                 $user = Repo::user()->get($userId);
                 $userUserGroup = UserUserGroup::withUserId($user->getId())
-                    ->withUserGroupId($userGroupId)
+                    ->withUserGroupIds([$userGroupId])
                     ->withActive()
                     ->withMasthead()
                     ->first();
@@ -161,7 +163,7 @@ public function editorialHistory($args, $request)
 
         // get all user IDs grouped by user group ID for the masthead roles with ended status
         $allUsersIdsGroupedByUserGroupId = Repo::userGroup()->getMastheadUserIdsByRoleIds(
-            $mastheadRoles,
+            $mastheadRoles->all(),
             $context->getId(),
             UserUserGroupStatus::STATUS_ENDED
         );
@@ -171,7 +173,7 @@ public function editorialHistory($args, $request)
             foreach ($allUsersIdsGroupedByUserGroupId[$userGroupId] ?? [] as $userId) {
                 $user = Repo::user()->get($userId);
                 $userUserGroups = UserUserGroup::withUserId($user->getId())
-                    ->withUserGroupId($userGroupId)
+                    ->withUserGroupIds([$userGroupId])
                     ->withEnded()
                     ->withMasthead()
                     ->orderBy('date_start', 'desc')

diff --git a/pages/admin/AdminHandler.php b/pages/admin/AdminHandler.php
@@ -323,7 +323,7 @@ public function wizard($args, $request)
 
         $bulkEmailsEnabled = in_array($context->getId(), (array) $request->getSite()->getData('enableBulkEmails'));
         if ($bulkEmailsEnabled) {
-            $userGroups = UserGroup::withContextIds([$context->getId()])->get()->toArray();
+            $userGroups = UserGroup::withContextIds([$context->getId()])->get();
 
             $restrictBulkEmailsForm = new \PKP\components\forms\context\PKPRestrictBulkEmailsForm($apiUrl, $context, $userGroups);
             $components[$restrictBulkEmailsForm->id] = $restrictBulkEmailsForm->getConfig();

diff --git a/templates/frontend/pages/editorialHistory.tpl b/templates/frontend/pages/editorialHistory.tpl
@@ -16,10 +16,10 @@
 	<h1>{translate key="common.editorialHistory.page"}</h1>
 	<p>{translate key="common.editorialHistory.page.description"}</p>
 	{foreach from=$mastheadRoles item="mastheadRole"}
-		{if array_key_exists($mastheadRole->getId(), $mastheadUsers)}
-			<h2>{$mastheadRole->getLocalizedName()|escape}</h2>
+		{if array_key_exists($mastheadRole->id, $mastheadUsers)}
+			<h2>{$mastheadRole->getLocalizedData('name')|escape}</h2>
 			<ul class="user_listing" role="list">
-				{foreach from=$mastheadUsers[$mastheadRole->getId()] item="mastheadUser"}
+				{foreach from=$mastheadUsers[$mastheadRole->id] item="mastheadUser"}
 					<li>
 						{strip}
 						<span class="date_start">

diff --git a/templates/frontend/pages/editorialMasthead.tpl b/templates/frontend/pages/editorialMasthead.tpl
@@ -15,10 +15,10 @@
 
 	<h1>{translate key="common.editorialMasthead"}</h1>
 	{foreach from=$mastheadRoles item="mastheadRole"}
-		{if array_key_exists($mastheadRole->getId(), $mastheadUsers)}
-			<h2>{$mastheadRole->getLocalizedName()|escape}</h2>
+		{if array_key_exists($mastheadRole->id, $mastheadUsers)}
+			<h2>{$mastheadRole->getLocalizedData('name')|escape}</h2>
 			<ul class="user_listing" role="list">
-			{foreach from=$mastheadUsers[$mastheadRole->getId()] item="mastheadUser"}
+			{foreach from=$mastheadUsers[$mastheadRole->id] item="mastheadUser"}
 				<li>
 					{strip}
 						<span class="date_start">{translate key="common.fromUntil" from=$mastheadUser['dateStart'] until=""}</span>