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

Add tests, fix PHPStan issues in tests, move misplaced integration test. Some minor bugfixes #2569

Merged
merged 15 commits into from
Nov 21, 2023
Merged
Show file tree
Hide file tree
Changes from 7 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
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
21 changes: 15 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.", [
'user' => Session::get('userId', 'not-logged-in-user'),
'url' => request()->url(),
'request' => request()->all()
Expand All @@ -30,8 +29,13 @@ 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()]
);
throw new \Exception('No valid LTI request');
}
}
Expand All @@ -42,8 +46,13 @@ 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()]
);
throw new \Exception('No valid LTI request');
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,15 @@ public function handle($request, Closure $next)
return $next($request);
}
}
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()
]);
Log::error(
__METHOD__ . ': Access denied. H5P: ' . $this->request->h5p
. ' 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
chrieinv marked this conversation as resolved.
Show resolved Hide resolved
* 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();
chrieinv marked this conversation as resolved.
Show resolved Hide resolved
}

/**
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' => '',
];
}
}
Loading