Skip to content

Commit

Permalink
Add tests, fix PHPStan issues in tests, move misplaced integration te…
Browse files Browse the repository at this point in the history
…st. Some minor bugfixes
  • Loading branch information
chrieinv committed Oct 10, 2023
1 parent e05361b commit 7bac5ec
Show file tree
Hide file tree
Showing 56 changed files with 980 additions and 364 deletions.
2 changes: 1 addition & 1 deletion sourcecode/apis/contentauthor/app/Content.php
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ public function getOwnerData(): ResourceUserDataObject

if ($ownerData) {
$user->firstname = $ownerData->getFirstName() ?? '';
$user->lastName = $ownerData->getLastName() ?? '';
$user->lastname = $ownerData->getLastName() ?? '';
$user->email = $ownerData->getEmail() ?? '';
}

Expand Down
2 changes: 1 addition & 1 deletion sourcecode/apis/contentauthor/app/H5PContentsMetadata.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public function convertToMetadataObject($title = null): H5PMetadataObject
'licenseVersion' => $this->license_version,
'licenseExtras' => $this->license_extras,
'authorComments' => $this->author_comments,
'changes' => $this->changes,
'changes' => $this->getAttribute('changes'), // 'changes' conflicts with Eloquent variable that holds changed model attributes
'defaultLanguage' => $this->default_language,
]);
}
Expand Down
3 changes: 3 additions & 0 deletions sourcecode/apis/contentauthor/app/H5PLibraryLanguage.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@
namespace App;

use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\BelongsTo;

class H5PLibraryLanguage extends Model
{
use HasFactory;

protected $table = 'h5p_libraries_languages';

protected $fillable = [
Expand Down
17 changes: 11 additions & 6 deletions sourcecode/apis/contentauthor/app/Http/Libraries/LtiTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ public function ltiShow($id)
if ($ltiRequest != null) {
return $this->doShow($id, $ltiRequest->generateContextKey(), $ltiRequest->isPreview());
} else {
Log::error(__METHOD__ . " Not a LTI request for showing H5P: $id.");
Log::error([
Log::error(__METHOD__ . " Not a LTI request for showing H5P: $id.", [

Check warning on line 17 in sourcecode/apis/contentauthor/app/Http/Libraries/LtiTrait.php

View check run for this annotation

Codecov / codecov/patch

sourcecode/apis/contentauthor/app/Http/Libraries/LtiTrait.php#L17

Added line #L17 was not covered by tests
'user' => Session::get('userId', 'not-logged-in-user'),
'url' => request()->url(),
'request' => request()->all()
Expand All @@ -30,8 +29,11 @@ public function ltiCreate(Request $request)
if ($ltiRequest != null) {
return $this->create($request);
} else {
Log::error(__METHOD__ . "Not a LTI request for H5P create.");
Log::error(['user' => Session::get('userId', 'not-logged-in-user'), 'url' => request()->url(), 'request' => request()->all()]);
Log::error(__METHOD__ . "Not a LTI request for H5P create.", [
'user' => Session::get('userId', 'not-logged-in-user'),
'url' => request()->url(),
'request' => request()->all()]
);

Check warning on line 36 in sourcecode/apis/contentauthor/app/Http/Libraries/LtiTrait.php

View check run for this annotation

Codecov / codecov/patch

sourcecode/apis/contentauthor/app/Http/Libraries/LtiTrait.php#L32-L36

Added lines #L32 - L36 were not covered by tests
throw new \Exception('No valid LTI request');
}
}
Expand All @@ -42,8 +44,11 @@ public function ltiEdit(Request $request, $id)
if ($ltiRequest != null) {
return $this->edit($request, $id);
} else {
Log::error(__METHOD__ . ": Not a LTI request for H5P: $id");
Log::error(['user' => Session::get('userId', 'not-logged-in-user'), 'url' => request()->url(), 'request' => request()->all()]);
Log::error(__METHOD__ . ": Not a LTI request for H5P: $id", [
'user' => Session::get('userId', 'not-logged-in-user'),
'url' => request()->url(),
'request' => request()->all()]
);

Check warning on line 51 in sourcecode/apis/contentauthor/app/Http/Libraries/LtiTrait.php

View check run for this annotation

Codecov / codecov/patch

sourcecode/apis/contentauthor/app/Http/Libraries/LtiTrait.php#L47-L51

Added lines #L47 - L51 were not covered by tests
throw new \Exception('No valid LTI request');
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,13 @@ public function handle($request, Closure $next)
}
}
Log::error(__METHOD__ . ': Access denied. H5P: ' . $this->request->h5p
. ' is not owned or shared with user:' . Session::get('authId', 'not-logged-in-user'));
Log::error([
'user' => Session::get('userId', 'not-logged-in-user'),
'url' => request()->url(),
'request' => request()->all()
]);
. ' is not owned or shared with user:' . Session::get('authId', 'not-logged-in-user'),
[
'user' => Session::get('userId', 'not-logged-in-user'),
'url' => request()->url(),
'request' => request()->all()
]
);
abort(403, 'Access denied, you are not the owner of the resource or it is not shared with you.');
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public function __construct()

public function setId($id, $hashId = true)
{
$this->id = $this->libraryId . '-' . $hashId ? md5($id) : $id;
$this->id = $this->libraryId . '-' . ($hashId ? md5($id) : $id);

return $this;
}
Expand Down
19 changes: 4 additions & 15 deletions sourcecode/apis/contentauthor/app/Libraries/H5P/Framework.php
Original file line number Diff line number Diff line change
Expand Up @@ -414,13 +414,8 @@ public function getWhitelist($isLibrary, $defaultContentWhitelist, $defaultLibra
/**
* Is the library a patched version of an existing library?
*
* @param object $library
* An associateve array containing:
* - machineName: The library machineName
* - majorVersion: The librarys majorVersion
* - minorVersion: The librarys minorVersion
* - patchVersion: The librarys patchVersion
* @return boolean
* @param array{machineName: string, majorVersion: int, minorVersion: int, patchVersion: int} $library
* @return bool
* TRUE if the library is a patched version of an existing library
* FALSE otherwise
* TODO: Implement this for real....
Expand All @@ -433,8 +428,7 @@ public function isPatchedLibrary($library)
$library['minorVersion']
])
->where('patch_version', "<", $library['patchVersion'])
->get()
->isNotEmpty();
->exists();
}

/**
Expand Down Expand Up @@ -1160,12 +1154,7 @@ public function getNumContent($libraryId, $skip = null)
*/
public function isContentSlugAvailable($slug)
{
$sql = "select slug from h5p_contents where slug=?";
$res = $this->db->prepare($sql)->execute([$slug])->fetch(PDO::FETCH_ASSOC);
if (sizeof($res) > 0) {
return false;
}
return true;
return !H5PContent::where('slug', $slug)->exists();
}

/**
Expand Down
1 change: 1 addition & 0 deletions sourcecode/apis/contentauthor/app/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
/**
* A dummy user that is not actually bound to a database table.
*
* @property string $auth_id
* @property string $name
* @property string $email
* @property string $password
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

declare(strict_types=1);

namespace Database\Factories;

use App\H5PLibraryLanguage;
use Illuminate\Database\Eloquent\Factories\Factory;

/**
* @template-extends Factory<H5PLibraryLanguage>
*/
class H5PLibraryLanguageFactory extends Factory
{
public function definition(): array
{
return [
'library_id' => $this->faker->numberBetween(),
'language_code' => $this->faker->unique()->languageCode(),
'translation' => '',
];
}
}
180 changes: 0 additions & 180 deletions sourcecode/apis/contentauthor/phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,6 @@ parameters:
count: 1
path: app/Content.php

-
message: "#^Access to an undefined property App\\\\Libraries\\\\DataObjects\\\\ResourceUserDataObject\\:\\:\\$lastName\\.$#"
count: 1
path: app/Content.php

-
message: "#^Access to an undefined property Illuminate\\\\Database\\\\Eloquent\\\\Model\\:\\:\\$email\\.$#"
count: 3
Expand Down Expand Up @@ -285,11 +280,6 @@ parameters:
count: 1
path: app/Http/Controllers/Admin/LibraryUpgradeController.php

-
message: "#^Parameter \\#1 \\$library of method H5PFrameworkInterface\\:\\:isPatchedLibrary\\(\\) expects object, array\\<string, mixed\\> given\\.$#"
count: 1
path: app/Http/Controllers/Admin/LibraryUpgradeController.php

-
message: "#^Expression on left side of \\?\\? is not nullable\\.$#"
count: 1
Expand Down Expand Up @@ -620,11 +610,6 @@ parameters:
count: 3
path: app/Libraries/H5P/AjaxRequest.php

-
message: "#^Ternary operator condition is always true\\.$#"
count: 1
path: app/Libraries/H5P/ContentType/BaseH5PContent.php

-
message: "#^Access to an undefined property App\\\\H5PLibrary\\:\\:\\$isOld\\.$#"
count: 1
Expand Down Expand Up @@ -680,16 +665,6 @@ parameters:
count: 3
path: app/Libraries/H5P/Framework.php

-
message: "#^Called 'isNotEmpty' on Laravel collection, but could have been retrieved as a query\\.$#"
count: 1
path: app/Libraries/H5P/Framework.php

-
message: "#^Cannot call method fetch\\(\\) on bool\\.$#"
count: 1
path: app/Libraries/H5P/Framework.php

-
message: "#^Expression on left side of \\?\\? is not nullable\\.$#"
count: 1
Expand Down Expand Up @@ -746,11 +721,6 @@ parameters:
count: 1
path: app/Libraries/H5P/Framework.php

-
message: "#^Call to method toArray\\(\\) on an unknown class Cerpus\\\\Helper\\\\Traits\\\\CreateTrait\\.$#"
count: 1
path: app/Libraries/H5P/H5PCopyright.php

-
message: "#^Left side of && is always true\\.$#"
count: 1
Expand Down Expand Up @@ -1136,157 +1106,7 @@ parameters:
count: 1
path: database/migrations/2020_12_07_133958_set_admin_password.php

-
message: "#^Access to an undefined property Illuminate\\\\Database\\\\Eloquent\\\\Model\\:\\:\\$auth_id\\.$#"
count: 10
path: tests/Integration/Article/ArticleVersioningTest.php

-
message: "#^Access to an undefined property Illuminate\\\\Database\\\\Eloquent\\\\Model\\:\\:\\$email\\.$#"
count: 11
path: tests/Integration/Article/ArticleVersioningTest.php

-
message: "#^Called 'count' on Laravel collection, but could have been retrieved as a query\\.$#"
count: 2
path: tests/Integration/Article/ArticleVersioningTest.php

-
message: "#^Access to an undefined property Illuminate\\\\Database\\\\Eloquent\\\\Model\\:\\:\\$auth_id\\.$#"
count: 2
path: tests/Integration/Content/ContentTest.php

-
message: "#^Access to an undefined property Illuminate\\\\Database\\\\Eloquent\\\\Model\\:\\:\\$auth_id\\.$#"
count: 10
path: tests/Integration/Content/LockStatusTest.php

-
message: "#^Access to an undefined property Illuminate\\\\Database\\\\Eloquent\\\\Model\\:\\:\\$auth_id\\.$#"
count: 5
path: tests/Integration/Content/UnlockTest.php

-
message: "#^Access to an undefined property App\\\\H5PLibrary\\:\\:\\$type\\.$#"
count: 1
path: tests/Integration/Http/Controllers/ArticleCopyrightControllerTest.php

-
message: "#^Parameter \\#1 \\$id of class App\\\\ApiModels\\\\User constructor expects string, int given\\.$#"
count: 2
path: tests/Integration/Http/Controllers/GameControllerTest.php

-
message: "#^Parameter \\#1 \\$id of class App\\\\ApiModels\\\\User constructor expects string, int given\\.$#"
count: 1
path: tests/Integration/Http/Controllers/H5PControllerTest.php

-
message: "#^Parameter \\#1 \\$id of class App\\\\ApiModels\\\\User constructor expects string, int given\\.$#"
count: 2
path: tests/Integration/Http/Controllers/LinkControllerTest.php

-
message: "#^Parameter \\#1 \\$id of class App\\\\ApiModels\\\\User constructor expects string, int given\\.$#"
count: 2
path: tests/Integration/Http/Controllers/QuestionSetControllerTest.php

-
message: "#^Variable \\$questionSetController in PHPDoc tag @var does not match assigned variable \\$questionsetController\\.$#"
count: 1
path: tests/Integration/Http/Controllers/QuestionSetControllerTest.php

-
message: "#^Property Tests\\\\Integration\\\\Jobs\\\\PingVideoApiTest\\:\\:\\$disk \\(Illuminate\\\\Filesystem\\\\FilesystemAdapter\\) does not accept Illuminate\\\\Contracts\\\\Filesystem\\\\Filesystem\\.$#"
count: 1
path: tests/Integration/Jobs/PingVideoApiTest.php

-
message: "#^Access to an undefined property Illuminate\\\\Database\\\\Eloquent\\\\Model\\:\\:\\$auth_id\\.$#"
count: 7
path: tests/Integration/Libraries/H5P/API/H5PImportControllerTest.php

-
message: "#^Access to an undefined property Illuminate\\\\Database\\\\Eloquent\\\\Model\\:\\:\\$auth_id\\.$#"
count: 18
path: tests/Integration/Libraries/H5P/CRUTest.php

-
message: "#^Access to an undefined property Illuminate\\\\Database\\\\Eloquent\\\\Model\\:\\:\\$email\\.$#"
count: 27
path: tests/Integration/Libraries/H5P/CRUTest.php

-
message: "#^Access to an undefined property Illuminate\\\\Database\\\\Eloquent\\\\Model\\:\\:\\$name\\.$#"
count: 14
path: tests/Integration/Libraries/H5P/CRUTest.php

-
message: "#^Access to an undefined property Tests\\\\Integration\\\\Libraries\\\\H5P\\\\CRUTest\\:\\:\\$editorFilesDirectory\\.$#"
count: 1
path: tests/Integration/Libraries/H5P/CRUTest.php

-
message: "#^Call to method toArray\\(\\) on an unknown class Cerpus\\\\Helper\\\\Traits\\\\CreateTrait\\.$#"
count: 4
path: tests/Integration/Libraries/H5P/H5PCopyrightTest.php

-
message: "#^Access to an undefined property Faker\\\\Generator\\:\\:\\$mimeType\\.$#"
count: 1
path: tests/Integration/Libraries/H5P/Package/ColumnTest.php

-
message: "#^Access to an undefined property Faker\\\\Generator\\:\\:\\$mimeType\\.$#"
count: 3
path: tests/Integration/Libraries/H5P/Package/CoursePresentationTest.php

-
message: "#^Unreachable statement \\- code above always terminates\\.$#"
count: 1
path: tests/Integration/Libraries/H5P/Package/MultiChoiceTest.php

-
message: "#^Access to an undefined property Faker\\\\Generator\\:\\:\\$mimeType\\.$#"
count: 1
path: tests/Integration/Libraries/H5P/Package/VideoTest.php

-
message: "#^Parameter \\$share of class App\\\\Libraries\\\\DataObjects\\\\ResourceMetadataDataObject constructor expects string\\|null, true given\\.$#"
count: 2
path: tests/Integration/Libraries/QuestionSetConverterTest.php

-
message: "#^Parameter \\#1 \\$attribution of method App\\\\Content\\:\\:setAttribution\\(\\) expects App\\\\Libraries\\\\DataObjects\\\\Attribution, string given\\.$#"
count: 1
path: tests/Integration/Models/ContentAttributionTest.php

-
message: "#^Access to an undefined property Illuminate\\\\Database\\\\Eloquent\\\\Model\\:\\:\\$id\\.$#"
count: 2
path: tests/Integration/Models/H5PContentRequestShouldBecomeNewVersionTest.php

-
message: "#^Access to an undefined property Illuminate\\\\Database\\\\Eloquent\\\\Model\\:\\:\\$title\\.$#"
count: 6
path: tests/Integration/Models/H5PContentRequestShouldBecomeNewVersionTest.php

-
message: "#^Call to an undefined method Illuminate\\\\Database\\\\Eloquent\\\\Model\\:\\:getContentLicense\\(\\)\\.$#"
count: 7
path: tests/Integration/Models/H5PContentRequestShouldBecomeNewVersionTest.php

-
message: "#^Call to an undefined method Illuminate\\\\Database\\\\Eloquent\\\\Model\\:\\:requestShouldBecomeNewVersion\\(\\)\\.$#"
count: 12
path: tests/Integration/Models/H5PContentRequestShouldBecomeNewVersionTest.php

-
message: "#^Call to an undefined method Tests\\\\TestCase\\:\\:setUpMockMQ\\(\\)\\.$#"
count: 1
path: tests/TestCase.php

-
message: "#^Method Tests\\\\TestCase\\:\\:setUpTraits\\(\\) should return array but return statement is missing\\.$#"
count: 1
path: tests/TestCase.php
Loading

0 comments on commit 7bac5ec

Please sign in to comment.