From 6f8e9aa5b1a4a51333810d5f985d3ba736fb0577 Mon Sep 17 00:00:00 2001 From: Christian Einvik Date: Wed, 22 Nov 2023 08:37:17 +0100 Subject: [PATCH 1/4] Remove create and edit for Link content type --- .../app/Http/Controllers/LinkController.php | 165 ------------------ .../app/Http/Libraries/LtiTrait.php | 18 +- sourcecode/apis/contentauthor/app/Link.php | 2 +- .../apis/contentauthor/phpstan-baseline.neon | 10 -- .../resources/views/link/create.blade.php | 31 ---- .../resources/views/link/edit.blade.php | 42 ----- sourcecode/apis/contentauthor/routes/web.php | 4 +- .../Http/Controllers/LinkControllerTest.php | 130 ++------------ 8 files changed, 34 insertions(+), 368 deletions(-) delete mode 100644 sourcecode/apis/contentauthor/resources/views/link/create.blade.php delete mode 100644 sourcecode/apis/contentauthor/resources/views/link/edit.blade.php diff --git a/sourcecode/apis/contentauthor/app/Http/Controllers/LinkController.php b/sourcecode/apis/contentauthor/app/Http/Controllers/LinkController.php index 04efa37b52..c2f2d56ee0 100644 --- a/sourcecode/apis/contentauthor/app/Http/Controllers/LinkController.php +++ b/sourcecode/apis/contentauthor/app/Http/Controllers/LinkController.php @@ -3,182 +3,18 @@ namespace App\Http\Controllers; use App\ACL\ArticleAccess; -use App\Events\LinkWasSaved; -use App\Http\Libraries\License; use App\Http\Libraries\LtiTrait; -use App\Http\Requests\LinksRequest; -use App\Libraries\H5P\Interfaces\H5PAdapterInterface; use App\Link; use App\Lti\Lti; -use App\Traits\ReturnToCore; -use Carbon\Carbon; -use Cerpus\VersionClient\VersionData; -use Illuminate\Http\JsonResponse; -use Illuminate\Http\Request; -use Illuminate\Http\Response; -use Illuminate\Support\Facades\Session; use Illuminate\View\View; class LinkController extends Controller { use LtiTrait; - use ReturnToCore; use ArticleAccess; public function __construct(private readonly Lti $lti) { - $this->middleware('core.return', ['only' => ['create', 'edit']]); - $this->middleware('lti.verify-auth', ['only' => ['create', 'edit', 'store', 'update']]); - $this->middleware('core.locale', ['only' => ['create', 'edit', 'store', 'update']]); - } - - public function create(Request $request) - { - if (!$this->canCreate()) { - abort(403); - } - - /** @var H5PAdapterInterface $adapter */ - $adapter = app(H5PAdapterInterface::class); - $ltiRequest = $this->lti->getRequest($request); - - $licenses = License::getLicenses($ltiRequest); - $license = License::getDefaultLicense($ltiRequest); - - $emails = ''; - $link = new Link(); - $redirectToken = $request->get('redirectToken'); - $userPublishEnabled = $adapter->isUserPublishEnabled(); - $canPublish = true; - $isPublished = false; - $canList = true; - - return view('link.create')->with(compact('licenses', 'license', 'emails', 'link', 'redirectToken', 'userPublishEnabled', 'canPublish', 'isPublished', 'canList')); - } - - /** - * Store a newly created resource in storage. - */ - public function store(LinksRequest $request): JsonResponse - { - if (!$this->canCreate()) { - abort(403); - } - - $inputs = $request->all(); - $metadata = json_decode($inputs['linkMetadata']); - - $link = new Link(); - $link->link_type = $inputs['linkType']; - $link->link_url = $inputs['linkUrl']; - $link->owner_id = Session::get('authId'); - $link->link_text = !empty($inputs['linkText']) ? $inputs['linkText'] : null; - $link->title = $metadata->title; - $link->metadata = !empty($inputs['linkMetadata']) ? $inputs['linkMetadata'] : null; - $link->is_published = $link::isUserPublishEnabled() ? $request->input('isPublished', 1) : 1; - $link->license = $inputs['license'] ?? ''; - $link->save(); - - event(new LinkWasSaved($link, VersionData::CREATE)); - - $url = $this->getRedirectToCoreUrl($link->toLtiContent(), $request->get('redirectToken')); - - return response()->json(['url' => $url], Response::HTTP_CREATED); - } - - public function edit(Request $request, $id) - { - $link = Link::findOrFail($id); - /** @var H5PAdapterInterface $adapter */ - $adapter = app(H5PAdapterInterface::class); - - $isOwner = $link->isOwner(Session::get('authId', 'qawsed')); - - if (!$link->shouldCreateFork(Session::get('authId', false))) { - $locked = $link->hasLock(); - if ($locked) { // Article is locked, add some info to the response - $now = Carbon::now(); - $expires = Carbon::createFromTimestamp($locked->updated_at->timestamp)->addHour(); - $lockHeadline = trans('lock.article-is-locked'); - $lockMessage = trans( - 'lock.article-will-expire', - [ - 'expires' => $expires->diffInMinutes($now), - 'editor' => $locked->getEditor(), - ] - ); - $editUrl = route('link.edit', $id); - $pollUrl = route('lock.status', $id); - - return view('content-lock.locked')->with(compact('lockHeadline', 'lockMessage', 'editUrl', 'pollUrl')); - } else { - $link->lock(); - } - } - - $emails = ""; //$this->getCollaboratorsEmails($link); - $ltiRequest = $this->lti->getRequest($request); - $licenses = License::getLicenses($ltiRequest); - $license = $link->license; - $redirectToken = $request->get('redirectToken'); - $userPublish = $adapter->isUserPublishEnabled(); - $canPublish = $link->canPublish($request); - $isPublished = $link->is_published; - $canList = $link->canList($request); - - return view('link.edit')->with(compact('link', 'isOwner', 'emails', 'license', 'licenses', 'id', 'redirectToken', 'userPublish', 'canPublish', 'canList', 'isPublished')); - } - - public function update(LinksRequest $request, $id) - { - $link = new Link(); - $oldLink = $link::findOrFail($id); - - if (!$this->canCreate()) { - abort(403); - } - - $inputs = $request->all(); - - $oldLicense = $oldLink->getContentLicense(); - $reason = $oldLink->shouldCreateFork(Session::get('authId', false)) ? VersionData::COPY : VersionData::UPDATE; - - if ($reason === VersionData::COPY && !$request->input("license", false)) { - $request->merge(["license" => $oldLicense]); - } - - // If you are a collaborator, use the old license - if ($oldLink->isCollaborator()) { - $request->merge(["license" => $oldLicense]); - } - - $link = $oldLink; - if ($oldLink->requestShouldBecomeNewVersion($request)) { - switch ($reason) { - case VersionData::UPDATE: - $link = $oldLink->makeCopy(); - break; - case VersionData::COPY: - $link = $oldLink->makeCopy(Session::get('authId')); - break; - } - } - - $metadata = json_decode($inputs['linkMetadata']); - $link->link_url = $inputs['linkUrl']; - $link->link_text = !empty($inputs['linkText']) ? $inputs['linkText'] : null; - $link->title = $metadata->title; - $link->metadata = !empty($inputs['linkMetadata']) ? $inputs['linkMetadata'] : null; - $link->is_published = $link::isUserPublishEnabled() ? $request->input('isPublished', 1) : 1; - $link->license = $inputs['license'] ?? $oldLink->license; - - $link->save(); - - event(new LinkWasSaved($link, $reason)); - - $url = $this->getRedirectToCoreUrl($link->toLtiContent(), $request->get('redirectToken')); - - return response()->json(['url' => $url], Response::HTTP_OK); } /** @@ -187,7 +23,6 @@ public function update(LinksRequest $request, $id) public function doShow($id, $context, $preview = false): View { $customCSS = $this->lti->getRequest(request())?->getLaunchPresentationCssUrl(); - /** @var Link $link */ $link = Link::findOrFail($id); if (!$link->canShow($preview)) { return view('layouts.draft-resource', [ diff --git a/sourcecode/apis/contentauthor/app/Http/Libraries/LtiTrait.php b/sourcecode/apis/contentauthor/app/Http/Libraries/LtiTrait.php index b73f89e65b..3579c6a883 100644 --- a/sourcecode/apis/contentauthor/app/Http/Libraries/LtiTrait.php +++ b/sourcecode/apis/contentauthor/app/Http/Libraries/LtiTrait.php @@ -18,7 +18,11 @@ public function ltiShow($id) ); } - return $this->doShow($id, $ltiRequest->generateContextKey(), $ltiRequest->isPreview()); + if (method_exists($this, 'doShow')) { + return $this->doShow($id, $ltiRequest->generateContextKey(), $ltiRequest->isPreview()); + } + + abort(500, 'Requested action is not available'); } public function ltiCreate(Request $request) @@ -32,7 +36,11 @@ public function ltiCreate(Request $request) ); } - return $this->create($request); + if (method_exists($this, 'create')) { + return $this->create($request); + } + + abort(500, 'Requested action is not available'); } public function ltiEdit(Request $request, $id) @@ -46,6 +54,10 @@ public function ltiEdit(Request $request, $id) ); } - return $this->edit($request, $id); + if (method_exists($this, 'edit')) { + return $this->edit($request, $id); + } + + abort(500, 'Requested action is not available'); } } diff --git a/sourcecode/apis/contentauthor/app/Link.php b/sourcecode/apis/contentauthor/app/Link.php index 6fa64b5953..ff0d043494 100644 --- a/sourcecode/apis/contentauthor/app/Link.php +++ b/sourcecode/apis/contentauthor/app/Link.php @@ -19,7 +19,7 @@ * @property string $owner_id * @property int $deleted_at * @property string $link_text - * @property string $metadata + * @property ?string $metadata * * @property Collection $collaborators * diff --git a/sourcecode/apis/contentauthor/phpstan-baseline.neon b/sourcecode/apis/contentauthor/phpstan-baseline.neon index 07fe1900ad..160855cbe4 100644 --- a/sourcecode/apis/contentauthor/phpstan-baseline.neon +++ b/sourcecode/apis/contentauthor/phpstan-baseline.neon @@ -300,11 +300,6 @@ parameters: count: 1 path: app/Http/Controllers/ArticleCopyrightController.php - - - message: "#^Call to an undefined method App\\\\Http\\\\Controllers\\\\GameController\\:\\:create\\(\\)\\.$#" - count: 1 - path: app/Http/Controllers/GameController.php - - message: "#^Called 'isNotEmpty' on Laravel collection, but could have been retrieved as a query\\.$#" count: 1 @@ -340,11 +335,6 @@ parameters: count: 1 path: app/Http/Controllers/HealthController.php - - - message: "#^Call to function is_null\\(\\) with string will always evaluate to false\\.$#" - count: 1 - path: app/Http/Controllers/LinkController.php - - message: "#^Call to function is_null\\(\\) with League\\\\Fractal\\\\Manager will always evaluate to false\\.$#" count: 1 diff --git a/sourcecode/apis/contentauthor/resources/views/link/create.blade.php b/sourcecode/apis/contentauthor/resources/views/link/create.blade.php deleted file mode 100644 index 652e390475..0000000000 --- a/sourcecode/apis/contentauthor/resources/views/link/create.blade.php +++ /dev/null @@ -1,31 +0,0 @@ -@extends('layouts.resource', ['formSurroundsMainContent' => true]) - -@section('title', trans('link.link')) -@section('header', trans('link.header')) -@section('headerInfo', trans('link.create-link')) - -@section('form_open') - {!! Form::open(['route' => 'link.store', 'method' => 'post', 'id' => 'content-form']) !!} - {!! Form::hidden("redirectToken", $redirectToken) !!} -@endsection - -@section("content") - @if (isset($errors) && count($errors) > 0) -
-
    - @foreach ($errors->all() as $error) -
  • {{ $error }}
    • - @endforeach -
-
- @endif -
-@endsection - -@push('js') - -@endpush - -@push('css') - -@endpush diff --git a/sourcecode/apis/contentauthor/resources/views/link/edit.blade.php b/sourcecode/apis/contentauthor/resources/views/link/edit.blade.php deleted file mode 100644 index f49083364b..0000000000 --- a/sourcecode/apis/contentauthor/resources/views/link/edit.blade.php +++ /dev/null @@ -1,42 +0,0 @@ -@extends('layouts.resource', ['formSurroundsMainContent' => true]) - -@section('title', trans('link.link')) -@section('header', trans('link.header')) -@section('headerInfo', trans('link.edit-link')) - -@section('form_open') - {!! Form::open(['route' => ['link.update', $link->id], 'method' => 'put', 'id' => 'content-form']) !!} - {!! Form::hidden("redirectToken", $redirectToken) !!} -@endsection - -@section("content") - @if (isset($errors) && count($errors) > 0) -
-
    - @foreach ($errors->all() as $error) -
  • {{ $error }}
    • - @endforeach -
-
- @endif - -
- - -@endsection - -@push("js") - - @include('fragments.js.edit-unlock', ['content' => $link]) -@endpush - -@push('css') - -@endpush diff --git a/sourcecode/apis/contentauthor/routes/web.php b/sourcecode/apis/contentauthor/routes/web.php index 60ac38ad55..818905b864 100644 --- a/sourcecode/apis/contentauthor/routes/web.php +++ b/sourcecode/apis/contentauthor/routes/web.php @@ -61,7 +61,6 @@ Route::post('/h5p/{id}', [H5PController::class, 'ltiShow'])->middleware(['core.behavior-settings:view', 'lti.redirect-to-editor'])->name('h5p.ltishow'); Route::post('/game/{id}', [GameController::class, 'ltiShow'])->middleware(['lti.redirect-to-editor']); - Route::post('/link/create', [LinkController::class, 'ltiCreate']); Route::post('/link/{id}', [LinkController::class, 'ltiShow'])->middleware(['lti.redirect-to-editor']); Route::post('questionset/create', [QuestionSetController::class, 'ltiCreate']); @@ -83,14 +82,13 @@ Route::group(['middleware' => ['core.ownership']], function () { Route::post('h5p/{id}/edit', [H5PController::class, 'ltiEdit'])->middleware(['core.behavior-settings:editor'])->name('h5p.ltiedit'); - Route::post('link/{id}/edit', [LinkController::class, 'ltiEdit']); }); }); Route::get('/slo', [SingleLogoutController::class, 'index'])->name('slo'); // Single logout route Route::resource('/article', ArticleController::class, ['except' => ['destroy']]); -Route::resource('/link', LinkController::class, ['except' => ['destroy']]); +Route::resource('/link', LinkController::class, ['only' => ['show']]); Route::post('/article/create/upload', [ArticleUploadController::class, 'uploadToNewArticle'])->name('article-upload.new'); Route::post('/article/{id}/upload', [ArticleUploadController::class, 'uploadToExistingArticle'])->name('article-upload.existing'); diff --git a/sourcecode/apis/contentauthor/tests/Integration/Http/Controllers/LinkControllerTest.php b/sourcecode/apis/contentauthor/tests/Integration/Http/Controllers/LinkControllerTest.php index 3631f9a5fb..37945c4bf0 100644 --- a/sourcecode/apis/contentauthor/tests/Integration/Http/Controllers/LinkControllerTest.php +++ b/sourcecode/apis/contentauthor/tests/Integration/Http/Controllers/LinkControllerTest.php @@ -3,14 +3,13 @@ namespace Tests\Integration\Http\Controllers; use App\ApiModels\User; -use App\Events\LinkWasSaved; -use App\Http\Controllers\LinkController; use App\Http\Libraries\License; use App\Link; -use Faker\Provider\Uuid; +use Cerpus\EdlibResourceKit\Oauth1\CredentialStoreInterface; +use Cerpus\EdlibResourceKit\Oauth1\Request as Oauth1Request; +use Cerpus\EdlibResourceKit\Oauth1\SignerInterface; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Foundation\Testing\WithFaker; -use Illuminate\Http\Request; use Illuminate\View\View; use Tests\Helpers\MockAuthApi; use Tests\TestCase; @@ -21,28 +20,7 @@ class LinkControllerTest extends TestCase use MockAuthApi; use WithFaker; - public function testCreate(): void - { - $this->session([ - 'authId' => Uuid::uuid(), - ]); - $request = Request::create('', parameters: [ - 'redirectToken' => 'UniqueToken', - ]); - $linkController = app(LinkController::class); - - $response = $linkController->create($request); - $this->assertNotEmpty($response); - $this->assertInstanceOf(View::class, $response); - - $data = $response->getData(); - - $this->assertCount(10, $data['licenses']); - $this->assertEquals(License::LICENSE_EDLIB, $data['license']); - $this->assertFalse($data['isPublished']); - } - - public function testEdit(): void + public function test_doShow(): void { $user = new User($this->faker->uuid, 'Emily', 'Quackfaster', 'emily.quackfaster@duckburg.quack'); $this->session([ @@ -53,101 +31,27 @@ public function testEdit(): void 'owner_id' => $user->getId(), ]); - $request = Request::create('', parameters: [ + $request = new Oauth1Request('POST', url('/link/' . $link->getId()), [ 'lti_version' => 'LTI-1p0', 'lti_message_type' => 'basic-lti-launch-request', 'resource_link_id' => 'random_link_9364f20a-a9b5-411a-8f60-8a4050f85d91', - 'launch_presentation_return_url' => "https://api.edlib.test/lti/v2/editors/contentauthor/return", 'ext_user_id' => "1", + 'launch_presentation_return_url' => "https://api.edlib.test/lti/v2/editors/contentauthor/return", 'launch_presentation_locale' => "nb", + 'launch_presentation_css_url' => 'my-styles.css', ]); + $request = $this->app->make(SignerInterface::class)->sign( + $request, + $this->app->make(CredentialStoreInterface::class), + ); - $linkController = app(LinkController::class); - $result = $linkController->edit($request, $link->getId()); + $result = $this->post('link/' . $link->getId(), $request->toArray()) + ->assertOk(); $this->assertNotEmpty($result); - $this->assertInstanceOf(View::class, $result); - $data = $result->getData(); - $this->assertArrayHasKey('licenses', $data); - $this->assertCount(10, $data['licenses']); - $this->assertArrayHasKey('license', $data); - $this->assertEquals(License::LICENSE_BY_NC_ND, $data['license']); - } - - public function testStore(): void - { - $this->withSession([ - 'authId' => Uuid::uuid(), - ]); - - $this->expectsEvents([ - LinkWasSaved::class, - ]); - - $response = $this->post(route('link.store'), [ - 'linkType' => 'external_link', - 'linkUrl' => $this->faker->url, - 'linkText' => 'The link', - 'linkMetadata' => json_encode(['title' => 'Another title']), - 'license' => License::LICENSE_BY_NC, - ]) - ->assertCreated(); - - $this->assertDatabaseHas('links', [ - 'title' => 'Another title', - 'link_text' => 'The link', - 'license' => License::LICENSE_BY_NC, - ]); - - /** @var Link $article */ - $article = Link::where('license', License::LICENSE_BY_NC)->first(); - $response->assertJson([ - 'url' => route('link.edit', $article->id), - ]); - } - - public function testUpdate(): void - { - $user = new User($this->faker->uuid, 'Emily', 'Quackfaster', 'emily.quackfaster@duckburg.quack'); - $this->session([ - 'authId' => $user->getId(), - ]); - - $this->expectsEvents([ - LinkWasSaved::class, - ]); - - $link = Link::factory()->create([ - 'link_type' => 'external_link', - 'link_url' => 'https://nowhere.not', - 'link_text' => 'The link', - 'title' => 'No title', - 'metadata' => json_encode(['title' => 'No title']), - 'license' => License::LICENSE_BY_NC, - 'owner_id' => $user->getId(), - ]); - - $response = $this->call('patch', '/link/' . $link->getId(), [ - 'linkType' => 'external_link', - 'linkUrl' => 'https://somewhere.not', - 'linkText' => 'Different link', - 'linkMetadata' => json_encode(['title' => 'Another title']), - 'license' => License::LICENSE_BY_NC_SA, - ]) - ->assertOk(); - - $this->assertDatabaseHas('links', [ - 'title' => 'Another title', - 'link_text' => 'Different link', - 'link_url' => 'https://somewhere.not', - 'license' => License::LICENSE_BY_NC_SA, - ]); - - /** @var Link $newLink */ - $newLink = Link::where('title', 'Another title')->first(); - - $response->assertJson([ - 'url' => route("link.edit", ['link' => $newLink->getId()]), - ]); + $this->assertInstanceOf(View::class, $result->getOriginalContent()); + $data = $result->getOriginalContent()->getData(); + $this->assertArrayHasKey('styles', $data); + $this->assertContains('my-styles.css', $data['styles']); } } From 691ead342e3f8f44df66bec0456b4415f538443d Mon Sep 17 00:00:00 2001 From: Christian Einvik Date: Wed, 22 Nov 2023 10:27:52 +0100 Subject: [PATCH 2/4] Update test --- .../app/Http/Libraries/LtiTrait.php | 30 ++++++------- .../Http/Libraries/LtiTraitTest.php | 44 ++++++++++++++++--- 2 files changed, 53 insertions(+), 21 deletions(-) diff --git a/sourcecode/apis/contentauthor/app/Http/Libraries/LtiTrait.php b/sourcecode/apis/contentauthor/app/Http/Libraries/LtiTrait.php index 3579c6a883..c4fa720a70 100644 --- a/sourcecode/apis/contentauthor/app/Http/Libraries/LtiTrait.php +++ b/sourcecode/apis/contentauthor/app/Http/Libraries/LtiTrait.php @@ -9,6 +9,10 @@ trait LtiTrait { public function ltiShow($id) { + if (!method_exists($this, 'doShow')) { + abort(500, 'Requested action is not available'); + } + $ltiRequest = $this->lti->getRequest(request()); if (!$ltiRequest) { @@ -18,15 +22,15 @@ public function ltiShow($id) ); } - if (method_exists($this, 'doShow')) { - return $this->doShow($id, $ltiRequest->generateContextKey(), $ltiRequest->isPreview()); - } - - abort(500, 'Requested action is not available'); + return $this->doShow($id, $ltiRequest->generateContextKey(), $ltiRequest->isPreview()); } public function ltiCreate(Request $request) { + if (!method_exists($this, 'create')) { + abort(500, 'Requested action is not available'); + } + $ltiRequest = $this->lti->getRequest($request); if (!$ltiRequest) { @@ -36,15 +40,15 @@ public function ltiCreate(Request $request) ); } - if (method_exists($this, 'create')) { - return $this->create($request); - } - - abort(500, 'Requested action is not available'); + return $this->create($request); } public function ltiEdit(Request $request, $id) { + if (!method_exists($this, 'edit')) { + abort(500, 'Requested action is not available'); + } + $ltiRequest = $this->lti->getRequest($request); if (!$ltiRequest) { @@ -54,10 +58,6 @@ public function ltiEdit(Request $request, $id) ); } - if (method_exists($this, 'edit')) { - return $this->edit($request, $id); - } - - abort(500, 'Requested action is not available'); + return $this->edit($request, $id); } } diff --git a/sourcecode/apis/contentauthor/tests/Integration/Http/Libraries/LtiTraitTest.php b/sourcecode/apis/contentauthor/tests/Integration/Http/Libraries/LtiTraitTest.php index 3592e2fb00..2fe6b012f4 100644 --- a/sourcecode/apis/contentauthor/tests/Integration/Http/Libraries/LtiTraitTest.php +++ b/sourcecode/apis/contentauthor/tests/Integration/Http/Libraries/LtiTraitTest.php @@ -4,10 +4,12 @@ namespace Tests\Integration\Http\Libraries; +use App\Http\Libraries\LtiTrait; use App\Lti\LtiRequest; use Cerpus\EdlibResourceKit\Oauth1\ValidatorInterface; use Exception; use Illuminate\Http\Request; +use Symfony\Component\HttpKernel\Exception\UnauthorizedHttpException; use Tests\Integration\Http\Libraries\Stubs\LtiTraitStubClass; use Tests\TestCase; @@ -23,15 +25,25 @@ public function setupLti(): void $this->instance(ValidatorInterface::class, $validator); } - public function test_ltiShow_exception(): void + public function test_ltiShow_unauthorized_exception(): void { - $this->expectException(Exception::class); + $this->expectException(UnauthorizedHttpException::class); $this->expectExceptionMessage('No valid LTI request'); $class = app(LtiTraitStubClass::class); $class->ltiShow(1); } + public function test_ltiShow_unavailable_exception(): void + { + $this->expectException(Exception::class); + $this->expectExceptionMessage('Requested action is not available'); + + $class = $this->getMockForTrait(LtiTrait::class); + /** @phpstan-ignore-next-line */ + $class->ltiShow(1); + } + public function test_ltiShow(): void { $this->setupLti(); @@ -45,15 +57,25 @@ public function test_ltiShow(): void $this->assertSame('doShow', $testClass->ltiShow(42)); } - public function test_ltiCreate_exception(): void + public function test_ltiCreate_unauthorized_exception(): void { - $this->expectException(Exception::class); + $this->expectException(UnauthorizedHttpException::class); $this->expectExceptionMessage('No valid LTI request'); $class = app(LtiTraitStubClass::class); $class->ltiCreate(Request::create('')); } + public function test_ltiCreate_unavailable_exception(): void + { + $this->expectException(Exception::class); + $this->expectExceptionMessage('Requested action is not available'); + + $class = $this->getMockForTrait(LtiTrait::class); + /** @phpstan-ignore-next-line */ + $class->ltiCreate(new Request()); + } + public function test_ltiCreate(): void { $this->setupLti(); @@ -63,15 +85,25 @@ public function test_ltiCreate(): void )); } - public function test_ltiEdit_exception(): void + public function test_ltiEdit_unauthorized_exception(): void { - $this->expectException(Exception::class); + $this->expectException(UnauthorizedHttpException::class); $this->expectExceptionMessage('No valid LTI request'); $class = app(LtiTraitStubClass::class); $class->ltiEdit(Request::create(''), 1); } + public function test_ltiEdit_unavailable_exception(): void + { + $this->expectException(Exception::class); + $this->expectExceptionMessage('Requested action is not available'); + + $class = $this->getMockForTrait(LtiTrait::class); + /** @phpstan-ignore-next-line */ + $class->ltiEdit(new Request(), 1); + } + public function test_ltiEdit(): void { $this->setupLti(); From 698d367ed934325efcfbca23c30d6e26a2d35003 Mon Sep 17 00:00:00 2001 From: Christian Einvik Date: Thu, 30 Nov 2023 10:51:23 +0100 Subject: [PATCH 3/4] merge tests for unavailable exception --- .../Http/Libraries/LtiTraitTest.php | 54 +++++++------------ 1 file changed, 20 insertions(+), 34 deletions(-) diff --git a/sourcecode/apis/contentauthor/tests/Integration/Http/Libraries/LtiTraitTest.php b/sourcecode/apis/contentauthor/tests/Integration/Http/Libraries/LtiTraitTest.php index 2fe6b012f4..53b1b04027 100644 --- a/sourcecode/apis/contentauthor/tests/Integration/Http/Libraries/LtiTraitTest.php +++ b/sourcecode/apis/contentauthor/tests/Integration/Http/Libraries/LtiTraitTest.php @@ -7,7 +7,7 @@ use App\Http\Libraries\LtiTrait; use App\Lti\LtiRequest; use Cerpus\EdlibResourceKit\Oauth1\ValidatorInterface; -use Exception; +use Generator; use Illuminate\Http\Request; use Symfony\Component\HttpKernel\Exception\UnauthorizedHttpException; use Tests\Integration\Http\Libraries\Stubs\LtiTraitStubClass; @@ -25,7 +25,7 @@ public function setupLti(): void $this->instance(ValidatorInterface::class, $validator); } - public function test_ltiShow_unauthorized_exception(): void + public function test_ltiShow_exception(): void { $this->expectException(UnauthorizedHttpException::class); $this->expectExceptionMessage('No valid LTI request'); @@ -34,16 +34,6 @@ public function test_ltiShow_unauthorized_exception(): void $class->ltiShow(1); } - public function test_ltiShow_unavailable_exception(): void - { - $this->expectException(Exception::class); - $this->expectExceptionMessage('Requested action is not available'); - - $class = $this->getMockForTrait(LtiTrait::class); - /** @phpstan-ignore-next-line */ - $class->ltiShow(1); - } - public function test_ltiShow(): void { $this->setupLti(); @@ -57,7 +47,7 @@ public function test_ltiShow(): void $this->assertSame('doShow', $testClass->ltiShow(42)); } - public function test_ltiCreate_unauthorized_exception(): void + public function test_ltiCreate_exception(): void { $this->expectException(UnauthorizedHttpException::class); $this->expectExceptionMessage('No valid LTI request'); @@ -66,16 +56,6 @@ public function test_ltiCreate_unauthorized_exception(): void $class->ltiCreate(Request::create('')); } - public function test_ltiCreate_unavailable_exception(): void - { - $this->expectException(Exception::class); - $this->expectExceptionMessage('Requested action is not available'); - - $class = $this->getMockForTrait(LtiTrait::class); - /** @phpstan-ignore-next-line */ - $class->ltiCreate(new Request()); - } - public function test_ltiCreate(): void { $this->setupLti(); @@ -85,7 +65,7 @@ public function test_ltiCreate(): void )); } - public function test_ltiEdit_unauthorized_exception(): void + public function test_ltiEdit_exception(): void { $this->expectException(UnauthorizedHttpException::class); $this->expectExceptionMessage('No valid LTI request'); @@ -94,16 +74,6 @@ public function test_ltiEdit_unauthorized_exception(): void $class->ltiEdit(Request::create(''), 1); } - public function test_ltiEdit_unavailable_exception(): void - { - $this->expectException(Exception::class); - $this->expectExceptionMessage('Requested action is not available'); - - $class = $this->getMockForTrait(LtiTrait::class); - /** @phpstan-ignore-next-line */ - $class->ltiEdit(new Request(), 1); - } - public function test_ltiEdit(): void { $this->setupLti(); @@ -113,4 +83,20 @@ public function test_ltiEdit(): void 42 )); } + + /** @dataProvider provider_unavailable_exception */ + public function test_unavailable_exception(string $function): void + { + $this->expectExceptionMessage('Requested action is not available'); + + $class = $this->getMockForTrait(LtiTrait::class); + $class->$function(new Request(), 1); + } + + public function provider_unavailable_exception(): Generator + { + yield 'missing create' => ['ltiCreate']; + yield 'missing edit' => ['ltiEdit']; + yield 'missing show' => ['ltiShow']; + } } From 11996c118a1a06003a9a1ccba5b84d02f5f4c7d8 Mon Sep 17 00:00:00 2001 From: Christian Einvik Date: Thu, 30 Nov 2023 15:58:36 +0100 Subject: [PATCH 4/4] Abort with 404 if method not available --- .../apis/contentauthor/app/Http/Libraries/LtiTrait.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sourcecode/apis/contentauthor/app/Http/Libraries/LtiTrait.php b/sourcecode/apis/contentauthor/app/Http/Libraries/LtiTrait.php index c4fa720a70..b22ed1da65 100644 --- a/sourcecode/apis/contentauthor/app/Http/Libraries/LtiTrait.php +++ b/sourcecode/apis/contentauthor/app/Http/Libraries/LtiTrait.php @@ -10,7 +10,7 @@ trait LtiTrait public function ltiShow($id) { if (!method_exists($this, 'doShow')) { - abort(500, 'Requested action is not available'); + abort(404, 'Requested action is not available'); } $ltiRequest = $this->lti->getRequest(request()); @@ -28,7 +28,7 @@ public function ltiShow($id) public function ltiCreate(Request $request) { if (!method_exists($this, 'create')) { - abort(500, 'Requested action is not available'); + abort(404, 'Requested action is not available'); } $ltiRequest = $this->lti->getRequest($request); @@ -46,7 +46,7 @@ public function ltiCreate(Request $request) public function ltiEdit(Request $request, $id) { if (!method_exists($this, 'edit')) { - abort(500, 'Requested action is not available'); + abort(404, 'Requested action is not available'); } $ltiRequest = $this->lti->getRequest($request);