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-LIB][main] #10630 Add support for add existing user to a context #10631

Merged
merged 5 commits into from
Dec 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
143 changes: 64 additions & 79 deletions api/v1/invitations/InvitationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,33 +30,35 @@
use PKP\invitation\core\Invitation;
use PKP\invitation\core\ReceiveInvitationController;
use PKP\invitation\core\traits\HasMailable;
use PKP\invitation\invitations\userRoleAssignment\resources\UserRoleAssignmentInviteResource;
use PKP\invitation\invitations\userRoleAssignment\rules\EmailMustNotExistRule;
use PKP\invitation\invitations\userRoleAssignment\rules\UserMustExistRule;
use PKP\invitation\models\InvitationModel;
use PKP\security\authorization\UserRolesRequiredPolicy;
use PKP\security\Role;
use PKP\validation\ValidatorFactory;
use Validator;

class InvitationController extends PKPBaseController
{
public const PARAM_TYPE = 'type';
public const PARAM_ID = 'invitationId';
public const PARAM_KEY = 'key';

public $notNeedAPIHandler = [
'getMany',
public $actionsInvite = [
'get',
'populate',
'invite',
'getMailable',
'cancel',
];

public $noParamRequired = [
'getMany',
public $actionsReceive = [
'receive',
'finalize',
'refine',
'decline',
'cancel',
];

public $requiresType = [
'add',
'getMany',
];

public $requiresOnlyId = [
Expand Down Expand Up @@ -124,7 +126,7 @@ public function getGroupRoutes(): void
]),
])->group(function () {

Route::get('', $this->getMany(...))
Route::get('{type}', $this->getMany(...))
Copy link
Member

Choose a reason for hiding this comment

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

@defstat this change is causing all tests break with latest pull of pkp-lib main branch . With the add of route param type, which probably has not been considered in some other places(from where AJAX request are made) cause the following exception

[29-Dec-2024 08:20:56 UTC] Symfony\Component\HttpKernel\Exception\NotFoundHttpException: The route publicknowledge/api/v1/invitations could not be found. in /Users/abir/Sites/code/ojs-main/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Routing/AbstractRouteCollection.php:44
Stack trace:
#0 /Users/abir/Sites/code/ojs-main/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Routing/RouteCollection.php(163): Illuminate\Routing\AbstractRouteCollection->handleMatchedRoute(Object(Illuminate\Http\Request), NULL)
#1 /Users/abir/Sites/code/ojs-main/lib/pkp/classes/core/PKPBaseController.php(104): Illuminate\Routing\RouteCollection->match(Object(Illuminate\Http\Request))
#2 /Users/abir/Sites/code/ojs-main/lib/pkp/classes/middleware/traits/HasRequiredMiddleware.php(53): PKP\core\PKPBaseController::getRequestedRoute(Object(Illuminate\Http\Request))
#3 /Users/abir/Sites/code/ojs-main/lib/pkp/classes/middleware/DecodeApiTokenWithValidation.php(64): PKP\middleware\DecodeApiTokenWithValidation->hasRequiredMiddleware(Object(Illuminate\Http\Request))
#4 /Users/abir/Sites/code/ojs-main/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): PKP\middleware\DecodeApiTokenWithValidation->handle(Object(Illuminate\Http\Request), Object(Closure))
#5 /Users/abir/Sites/code/ojs-main/lib/pkp/classes/middleware/SetupContextBasedOnRequestUrl.php(63): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}(Object(Illuminate\Http\Request))
#6 /Users/abir/Sites/code/ojs-main/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): PKP\middleware\SetupContextBasedOnRequestUrl->handle(Object(Illuminate\Http\Request), Object(Closure))
#7 /Users/abir/Sites/code/ojs-main/lib/pkp/classes/middleware/AllowCrossOrigin.php(34): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}(Object(Illuminate\Http\Request))
#8 /Users/abir/Sites/code/ojs-main/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): PKP\middleware\AllowCrossOrigin->handle(Object(Illuminate\Http\Request), Object(Closure))
#9 /Users/abir/Sites/code/ojs-main/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(119): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}(Object(Illuminate\Http\Request))
#10 /Users/abir/Sites/code/ojs-main/lib/pkp/classes/handler/APIHandler.php(102): Illuminate\Pipeline\Pipeline->then(Object(Closure))
#11 /Users/abir/Sites/code/ojs-main/lib/pkp/classes/core/APIRouter.php(116): PKP\handler\APIHandler->runRoutes()
#12 /Users/abir/Sites/code/ojs-main/lib/pkp/classes/core/Dispatcher.php(158): PKP\core\APIRouter->route(Object(APP\core\Request))
#13 /Users/abir/Sites/code/ojs-main/lib/pkp/classes/core/PKPApplication.php(395): PKP\core\Dispatcher->dispatch(Object(APP\core\Request))
#14 /Users/abir/Sites/code/ojs-main/index.php(30): PKP\core\PKPApplication->execute()
#15 /Users/abir/.composer/vendor/laravel/valet/server.php(110): require('/Users/abir/Sit...')
#16 {main}

As the route has changed with the addition of a new route param, route is not found by router .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@touhidurabir, sorry about that - I have made the change required and waiting for the tests to finish.

Copy link
Contributor Author

@defstat defstat Dec 30, 2024

Choose a reason for hiding this comment

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

@touhidurabir I don't see the error mentioned by you after the PR: pkp/ui-library#472 but those tests are still failing with other errors I think (see pkp/ojs#4573). Is there another problem that you know?

Copy link
Member

Choose a reason for hiding this comment

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

@defstat in the error log of failed tests , I can see the following exception

[Mon Dec 30 12:03:05 2024] Symfony\Component\HttpKernel\Exception\NotFoundHttpException: The route publicknowledge/api/v1/invitations/userRoleAssignment could not be found. in /home/runner/ojs/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Routing/AbstractRouteCollection.php:44

but I can tests with latest pkp-lib main checkout, this works for me .

However the current pkp-lib submodule reference to main app is not the latest main but few commits behind where the route in InvitationController set as

Route::get('', $this->getMany(...))
                ->name('invitation.getMany');

I think thats causing the issue . I think you need include the latest pkp-lib submodule update also in your PR .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @touhidurabir! again it seems that the tests are failing down the PHPUnit part of the tests this time.

Copy link
Member

Choose a reason for hiding this comment

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

Tests are failing because of this

Test Errored (functional\ArticleBodyTest::testCreate)
array_reduce(): Argument #1 ($array) must be of type array, null given
After Test Method Called (functional\ArticleBodyTest::tearDown)

it's a issue with jatsTemplate plugin's unit test and I have added a fix in the PR of #10292 at https://github.com/pkp/jatsTemplate/pull/54/files#diff-c87e6397a6dd5c2bf754d5b10da491be897899cf411d80e1dbcefedb20bd9960R114

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK @touhidurabir, thanks. Everything is merged for the first error.

->name('invitation.getMany');

// Get By Id Methods
Expand Down Expand Up @@ -207,45 +209,39 @@ public function authorize(PKPRequest $request, array &$args, array $roleAssignme
$invitation = Repo::invitation()->getByIdAndKey($invitationId, $invitationKey);
}

if ($actionName == 'getMany') {
$this->addPolicy(new UserRolesRequiredPolicy($request), true);
} else {
if (!isset($invitation)) {
throw new Exception('Invitation could not be created');
}

$this->invitation = $invitation;
if (!isset($invitation)) {
throw new Exception('Invitation could not be created');
}

if (!in_array($actionName, $this->notNeedAPIHandler)) {
if (!$this->invitation instanceof IApiHandleable) {
throw new Exception('This invitation does not support API handling');
}
$this->invitation = $invitation;

if (in_array($actionName, $this->requiresOnlyId) && $this->invitation->getStatus() != InvitationStatus::INITIALIZED) {
throw new Exception('This action is not allowed');
}
if (!$this->invitation instanceof IApiHandleable) {
throw new Exception('This invitation does not support API handling');
}

if (in_array($actionName, $this->requiresIdAndKey) && $this->invitation->getStatus() != InvitationStatus::PENDING) {
throw new Exception('This action is not allowed');
}
if (in_array($actionName, $this->actionsInvite) && $this->invitation->getStatus() != InvitationStatus::INITIALIZED) {
throw new Exception('This action is not allowed');
}

$this->createInvitationHandler = $invitation->getCreateInvitationController($this->invitation);
$this->receiveInvitationHandler = $invitation->getReceiveInvitationController($this->invitation);
if (in_array($actionName, $this->actionsReceive) && $this->invitation->getStatus() != InvitationStatus::PENDING) {
throw new Exception('This action is not allowed');
}

if (!isset($this->createInvitationHandler) || !isset($this->receiveInvitationHandler)) {
throw new Exception('This invitation should have defined its API handling code');
}
$this->createInvitationHandler = $invitation->getCreateInvitationController($this->invitation);
$this->receiveInvitationHandler = $invitation->getReceiveInvitationController($this->invitation);

$this->selectedHandler = $this->getHandlerForAction($actionName);
if (!isset($this->createInvitationHandler) || !isset($this->receiveInvitationHandler)) {
throw new Exception('This invitation should have defined its API handling code');
}

if (!method_exists($this->selectedHandler, $actionName)) {
throw new Exception("The handler does not support the method: {$actionName}");
}
$this->selectedHandler = $this->getHandlerForAction($actionName);

$this->selectedHandler->authorize($this, $request, $args, $roleAssignments);
}
if (!method_exists($this->selectedHandler, $actionName)) {
throw new Exception("The handler does not support the method: {$actionName}");
}

$this->selectedHandler->authorize($this, $request, $args, $roleAssignments);

return parent::authorize($request, $args, $roleAssignments);
}

Expand All @@ -269,7 +265,6 @@ public function add(Request $illuminateRequest): JsonResponse
'nullable',
'required_without:userId',
'email',
new EmailMustNotExistRule($payload['inviteeEmail']),
]
];

Expand Down Expand Up @@ -338,63 +333,53 @@ public function getMany(Request $illuminateRequest): JsonResponse
$context = $illuminateRequest->attributes->get('context'); /** @var \PKP\context\Context $context */
$invitationType = $this->getParameter(self::PARAM_TYPE);

$count = $illuminateRequest->query('count', 10); // default count to 10 if not provided
$offset = $illuminateRequest->query('offset', 0); // default offset to 0 if not provided

// Build the common query
$query = InvitationModel::query()
->when($invitationType, function ($query, $invitationType) {
return $query->byType($invitationType);
})
->when($context, function ($query, $context) {
return $query->byContextId($context->getId());
})
->when($invitationType, fn($query) => $query->byType($invitationType))
->when($context, fn($query) => $query->byContextId($context->getId()))
->stillActive();

$maxCount = $query->count();

$invitations = $query->skip($offset)
->take($count)
->get();

$finalCollection = $invitations->map(function ($invitation) {
$specificInvitation = Repo::invitation()->getById($invitation->id);
return $specificInvitation;
});
// Delegate to the specific handler for additional logic
$specificData = $this->selectedHandler->getMany($illuminateRequest, $query);

return response()->json([
'itemsMax' => $maxCount,
'items' => (UserRoleAssignmentInviteResource::collection($finalCollection)),
'itemsMax' => $query->count(),
'items' => $specificData,
], Response::HTTP_OK);
}

public function getMailable(Request $illuminateRequest): JsonResponse
public function cancel(Request $illuminateRequest): JsonResponse
{
if (in_array(HasMailable::class, class_uses($this->invitation))) {
$mailable = $this->invitation->getMailable();

if (!$this->invitation->isPending()) {
return response()->json([
'mailable' => $mailable,
], Response::HTTP_OK);
'error' => __('invitation.api.error.invitationCantBeCanceled'),
], Response::HTTP_BAD_REQUEST);
}

return response()->json([
'error' => __('invitation.api.error.invitationTypeNotHasMailable'),
], Response::HTTP_BAD_REQUEST);
try {
return $this->selectedHandler->cancel();
} catch (\Exception $e) {
return response()->json([
'error' => $e->getMessage()
], Response::HTTP_INTERNAL_SERVER_ERROR);
}
}

public function cancel(Request $illuminateRequest): JsonResponse
public function getMailable(Request $illuminateRequest): JsonResponse
{
if (!$this->invitation->isPending()) {
// Ensure the invitation supports mailables
if (!in_array(HasMailable::class, class_uses($this->invitation))) {
return response()->json([
'error' => __('invitation.api.error.invitationCantBeCanceled'),
'error' => __('invitation.api.error.invitationTypeNotHasMailable'),
], Response::HTTP_BAD_REQUEST);
}

$this->invitation->updateStatus(InvitationStatus::CANCELLED);

return response()->json(
(new UserRoleAssignmentInviteResource($this->invitation))->toArray($illuminateRequest),
Response::HTTP_OK
);
try {
return $this->selectedHandler->getMailable();
} catch (\Exception $e) {
return response()->json([
'error' => $e->getMessage(),
], Response::HTTP_INTERNAL_SERVER_ERROR);
}
}
}
7 changes: 6 additions & 1 deletion classes/invitation/core/CreateInvitationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@

namespace PKP\invitation\core;

use Illuminate\Database\Eloquent\Builder;
use Illuminate\Http\JsonResponse;
use Illuminate\Http\Request;
use Illuminate\Routing\Controller;
use Illuminate\Support\Collection;
use PKP\core\PKPBaseController;
use PKP\core\PKPRequest;
use PKP\invitation\core\Invitation;

abstract class CreateInvitationController extends Controller
{
Expand All @@ -30,4 +31,8 @@ abstract public function add(Request $illuminateRequest): JsonResponse;
abstract public function populate(Request $illuminateRequest): JsonResponse;
abstract public function invite(Request $illuminateRequest): JsonResponse;
abstract public function get(Request $illuminateRequest): JsonResponse;

abstract public function cancel(): JsonResponse;
abstract public function getMailable(): JsonResponse;
abstract public function getMany(Request $illuminateRequest, Builder $query): Collection;
}
19 changes: 12 additions & 7 deletions classes/invitation/core/Invitation.php
Original file line number Diff line number Diff line change
Expand Up @@ -236,11 +236,7 @@ public function updatePayload(?ValidationContext $validationContext = null): ?bo
}
}

// Update the payload attribute on the invitation model
$this->invitationModel->setAttribute('payload', $currentPayloadArray);

// Save the updated invitation model to the database
return $this->invitationModel->save();
return $this->invitationModel->update(['payload' => $currentPayloadArray]);
}

public function getNotAccessibleBeforeInvite(): array
Expand Down Expand Up @@ -351,6 +347,15 @@ public function getExistingUser(): ?User
return Repo::user()->get($this->invitationModel->userId);
}

public function getExistingUserByEmail(): ?User
{
if (!isset($this->invitationModel->email)) {
return null;
}

return Repo::user()->getByEmail($this->invitationModel->email);
}

public function getContext(): ?Context
{
if (!isset($this->invitationModel->contextId)) {
Expand Down Expand Up @@ -480,10 +485,10 @@ protected function array_diff_assoc_recursive($array1, $array2)
return $difference;
}

public function updateStatus(InvitationStatus $status): void
public function updateStatus(InvitationStatus $status): bool
{
$this->invitationModel->status = $status;
$this->invitationModel->save();
return $this->invitationModel->save();
}

public function isPending(): bool
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,20 @@
use Illuminate\Routing\Controller;
use PKP\invitation\core\enums\InvitationAction;

/**
* @template TInvitation of Invitation
*/
abstract class InvitationActionRedirectController extends Controller
{
/** @var TInvitation */
protected Invitation $invitation;

public function __construct(Invitation $invitation)
{
$this->invitation = $invitation;
}

abstract public function preRedirectActions(InvitationAction $action);
abstract public function preRedirectActions(InvitationAction $action): void;

abstract public function acceptHandle(Request $request): void;
abstract public function declineHandle(Request $request): void;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public function declineHandle(Request $request): void
$request->redirectUrl($url);
}

public function preRedirectActions(InvitationAction $action)
public function preRedirectActions(InvitationAction $action): void
{
if ($action == InvitationAction::ACCEPT) {
$this->getInvitation()->finalize();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public function declineHandle(Request $request): void
$request->redirectUrl($url);
}

public function preRedirectActions(InvitationAction $action)
public function preRedirectActions(InvitationAction $action): void
{
if ($action == InvitationAction::ACCEPT) {
$this->getInvitation()->finalize();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public function declineHandle(Request $request): void
$request->redirectUrl($url);
}

public function preRedirectActions(InvitationAction $action)
public function preRedirectActions(InvitationAction $action): void
{
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,11 @@ public function getValidationRules(ValidationContext $validationContext = Valida
$this->getPayload()->userGroupsToAdd
);
$invitationValidationRules[Invitation::VALIDATION_RULE_GENERIC][] = new UserMustExistRule($this->getUserId());
}

if (
$validationContext === ValidationContext::VALIDATION_CONTEXT_FINALIZE
) {
$invitationValidationRules[Invitation::VALIDATION_RULE_GENERIC][] = new EmailMustNotExistRule($this->getEmail());
}

Expand Down Expand Up @@ -207,5 +212,25 @@ public function updatePayload(?ValidationContext $validationContext = null): ?bo
// Call the parent updatePayload method to continue the normal update process
return parent::updatePayload($validationContext);
}


public function changeInvitationUserIdUsingUserEmail(): ?bool
{
$invitationUserByEmail = $this->getExistingUserByEmail();

if (!isset($this->invitationModel->userId) && isset($invitationUserByEmail)) {
$this->invitationModel->userId = $invitationUserByEmail->getId();
$this->invitationModel->email = null;

$result = $this->invitationModel->save();

if ($result) {
$this->getPayload()->shouldUseInviteData = true;

return $this->updatePayload();
}
}

return null;
}

}
Loading