From 2a534a29980de29e58251fd115b44b5803320341 Mon Sep 17 00:00:00 2001 From: Mateusz Wojczal Date: Thu, 16 Jan 2025 12:14:11 +0100 Subject: [PATCH] =?UTF-8?q?Remove=20topic=20preivew=20from=20course=20deta?= =?UTF-8?q?ils=20as=20it=20super=20slow.=20Topic=20prev=E2=80=A6=20(#327)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove topic preivew from course details as it super slow. Topic preview available from endpoint * tests for preview * tests for preview * spelling --------- Co-authored-by: Mateusz Wojczal --- .github/workflows/test.yml | 5 +- docker-compose.yaml | 25 +++ phpunit.xml | 12 +- src/Http/Controllers/CourseAPIController.php | 22 ++- .../Controllers/Swagger/CourseAPISwagger.php | 52 +++++++ src/Http/Requests/GetCourseAPIRequest.php | 6 + src/Http/Resources/TopicSimpleResource.php | 33 ++-- src/Models/Topic.php | 1 + src/routes.php | 1 + testbench.yaml | 2 +- tests/APIs/CourseAnonymousApiTest.php | 147 +++++++++++------- 11 files changed, 230 insertions(+), 76 deletions(-) create mode 100644 docker-compose.yaml diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 88ec4be3..eaff4f08 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -3,10 +3,10 @@ name: Tests PHPUnit in environments on: [pull_request] jobs: - php8-laravel-latest-phpunit-mysql: + php83-laravel-latest-phpunit-mysql: runs-on: ubuntu-latest container: - image: escolalms/php:8 + image: escolalms/php:8.3-alpine services: mysql: @@ -144,5 +144,4 @@ jobs: - name: Run tests run: vendor/bin/phpunit - ### TODO add behat tests here diff --git a/docker-compose.yaml b/docker-compose.yaml new file mode 100644 index 00000000..b35fd466 --- /dev/null +++ b/docker-compose.yaml @@ -0,0 +1,25 @@ +networks: + escola_lms_tests: + name: escola_lms_tests + driver: bridge + +services: + courses_tests: + image: escolalms/php:8.3-alpine + environment: + - "COMPOSER_ROOT_VERSION=0.9.9" + volumes: + - ./:/var/www/html + command: bash -c "COMPOSER_ROOT_VERSION=0.9.9 composer install && vendor/bin/testbench migrate:fresh && vendor/bin/phpunit" + networks: + - escola_lms_tests + + postgres: + image: postgres:16 + networks: + - escola_lms_tests + environment: + - "POSTGRES_DB=${POSTGRES_DB:-default}" + - "POSTGRES_USER=${POSTGRES_USER:-default}" + - "POSTGRES_PASSWORD=${POSTGRES_PASSWORD:-secret}" + - TZ=Europe/Warsaw diff --git a/phpunit.xml b/phpunit.xml index 9f5fbb8b..a039be43 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,13 +1,5 @@ - + ./tests @@ -15,7 +7,7 @@ - + diff --git a/src/Http/Controllers/CourseAPIController.php b/src/Http/Controllers/CourseAPIController.php index ec5095d0..f890a054 100644 --- a/src/Http/Controllers/CourseAPIController.php +++ b/src/Http/Controllers/CourseAPIController.php @@ -2,6 +2,7 @@ namespace EscolaLms\Courses\Http\Controllers; +use EscolaLms\Courses\Http\Resources\TopicResource; use EscolaLms\Core\Dtos\OrderDto; use EscolaLms\Courses\Enum\CoursesPermissionsEnum; use EscolaLms\Courses\Enum\CourseStatusEnum; @@ -73,7 +74,9 @@ public function authoredCourses(ListAuthoredCourseAPIRequest $request): JsonResp return $this->sendResponseForResource(CourseListResource::collection( $this->courseRepository->getAuthoredCourses( - $user->getKey(), OrderDto::instantiateFromRequest($request))->paginate($request->get('per_page', 20)) + $user->getKey(), + OrderDto::instantiateFromRequest($request) + )->paginate($request->get('per_page', 20)) ), __('Courses retrieved successfully')); } @@ -176,4 +179,21 @@ public function uniqueTags(): JsonResponse $this->sendResponse($tags, 'Tags unique fetched successfully') : $this->sendError('Tags not found', 404); } + + public function preview($id, $topic_id, GetCourseAPIRequest $request): JsonResponse + { + $course = $request->getCourse(); + + if (empty($course)) { + return $this->sendError(__('Course not found')); + } + + $topic = $request->getTopic(); + + if (empty($topic) || !$topic->preview) { + return $this->sendError(__('Topic not found or not able to preview')); + } + + return $this->sendResponseForResource(TopicResource::make($topic), __('Topic preview retrieved successfully')); + } } diff --git a/src/Http/Controllers/Swagger/CourseAPISwagger.php b/src/Http/Controllers/Swagger/CourseAPISwagger.php index 2f924c57..4ee7b578 100644 --- a/src/Http/Controllers/Swagger/CourseAPISwagger.php +++ b/src/Http/Controllers/Swagger/CourseAPISwagger.php @@ -861,4 +861,56 @@ public function sort(SortAPIRequest $request); * ) */ public function uniqueTags(): JsonResponse; + + + /** + * @OA\Get( + * path="/api/courses/{id}/preview/{topic_id}", + * summary="Preview the specified Course Topic is set to preview", + * tags={"Courses"}, + * description="Get Topic with Topicable", + * @OA\Parameter( + * name="id", + * description="id of Course", + * @OA\Schema( + * type="integer", + * ), + * required=true, + * in="path" + * ), + * @OA\Parameter( + * name="topic_id", + * description="id of Topic", + * @OA\Schema( + * type="integer", + * ), + * required=true, + * in="path" + * ), + * @OA\Response( + * response=200, + * description="successful operation", + * @OA\MediaType( + * mediaType="application/json" + * ), + * @OA\Schema( + * type="object", + * @OA\Property( + * property="success", + * type="boolean" + * ), + * @OA\Property( + * property="data", + * ref="#/components/schemas/Topic" + * ), + * @OA\Property( + * property="message", + * type="string" + * ) + * ) + * ) + * ) + */ + + public function preview($id, $topic_id, GetCourseAPIRequest $request); } diff --git a/src/Http/Requests/GetCourseAPIRequest.php b/src/Http/Requests/GetCourseAPIRequest.php index fee4a633..c21981ce 100644 --- a/src/Http/Requests/GetCourseAPIRequest.php +++ b/src/Http/Requests/GetCourseAPIRequest.php @@ -3,6 +3,7 @@ namespace EscolaLms\Courses\Http\Requests; use EscolaLms\Courses\Models\Course; +use EscolaLms\Courses\Models\Topic; use Illuminate\Foundation\Http\FormRequest; use Illuminate\Support\Facades\Gate; @@ -37,4 +38,9 @@ public function getCourse(): ?Course { return Course::find($this->route('course')); } + + public function getTopic(): ?Topic + { + return Topic::find($this->route('topic_id')); + } } diff --git a/src/Http/Resources/TopicSimpleResource.php b/src/Http/Resources/TopicSimpleResource.php index 6899cfce..a7ef96d8 100644 --- a/src/Http/Resources/TopicSimpleResource.php +++ b/src/Http/Resources/TopicSimpleResource.php @@ -2,20 +2,35 @@ namespace EscolaLms\Courses\Http\Resources; -use EscolaLms\Auth\Traits\ResourceExtandable; use EscolaLms\Courses\Facades\Topic; use Illuminate\Http\Resources\Json\JsonResource; -class TopicSimpleResource extends TopicResource +class TopicSimpleResource extends JsonResource { + /** + * Transform the resource into an array. + * + * @param \Illuminate\Http\Request $request + * + * @return array + */ public function toArray($request) { - $response = parent::toArray($request); - if (!$this->resource->preview) { - unset($response['topicable']); - unset($response['resources']); - } - - return $response; + return [ + 'id' => $this->resource->id, + 'title' => $this->resource->title, + 'lesson_id' => $this->resource->lesson_id, + 'active' => $this->resource->active, + 'preview' => $this->resource->preview, + 'topicable_id' => $this->resource->topicable_id, + 'topicable_type' => $this->resource->topicable_type, + 'summary' => $this->resource->summary, + 'introduction' => $this->resource->introduction, + 'description' => $this->resource->description, + 'order' => $this->resource->order, + 'json' => $this->resource->json, + 'can_skip' => $this->resource->can_skip, + 'duration' => $this->resource->duration, + ]; } } diff --git a/src/Models/Topic.php b/src/Models/Topic.php index 637e7419..5feab3cc 100644 --- a/src/Models/Topic.php +++ b/src/Models/Topic.php @@ -101,6 +101,7 @@ * @property int $order * @property int $topicable_id * @property string $topicable_type + * @property bool $preview */ class Topic extends Model { diff --git a/src/routes.php b/src/routes.php index 1bb2eae7..9692c4fd 100644 --- a/src/routes.php +++ b/src/routes.php @@ -37,6 +37,7 @@ // user endpoints Route::group(['prefix' => 'api/courses'], function () { Route::get('/{course}/program', [CourseAPIController::class, 'program'])->middleware('cacheResponse'); + Route::get('/{course}/preview/{topic_id}', [CourseAPIController::class, 'preview']); Route::group(['middleware' => ['auth:api']], function () { Route::get('/{course}/scorm', [CourseAPIController::class, 'scorm']); diff --git a/testbench.yaml b/testbench.yaml index 9ee5010d..7bbb92a9 100644 --- a/testbench.yaml +++ b/testbench.yaml @@ -1,6 +1,6 @@ env: - DB_CONNECTION=pgsql - - DB_HOST=127.0.0.1 + - DB_HOST=postgres - DB_PORT=5432 - DB_DATABASE=default - DB_USERNAME=default diff --git a/tests/APIs/CourseAnonymousApiTest.php b/tests/APIs/CourseAnonymousApiTest.php index 5b5d93b1..9f1edde6 100644 --- a/tests/APIs/CourseAnonymousApiTest.php +++ b/tests/APIs/CourseAnonymousApiTest.php @@ -244,12 +244,13 @@ public function test_anonymous_read_free_course_program() 'course_id' => $course->getKey(), 'active' => true, ]) - ->has(Lesson::factory(['course_id' => $course->getKey()]) - ->count(2) - ->sequence( - ['active' => true], - ['active' => false], - ) + ->has( + Lesson::factory(['course_id' => $course->getKey()]) + ->count(2) + ->sequence( + ['active' => true], + ['active' => false], + ) ) ->create(); @@ -276,19 +277,21 @@ public function test_anonymous_can_attend_free_course_program() { $course = Course::factory() ->state(['status' => CourseStatusEnum::PUBLISHED, 'public' => true]) - ->has(Lesson::factory() - ->count(2) - ->sequence( - ['active' => true], - ['active' => false], - ) - ->has(Topic::factory()->count(2) + ->has( + Lesson::factory() + ->count(2) ->sequence( - ['active' => true, 'preview' => true], // return with topicable and resources - ['active' => true, 'preview' => false], // return without topicable and resources - ['active' => false], // not return + ['active' => true], + ['active' => false], + ) + ->has( + Topic::factory()->count(2) + ->sequence( + ['active' => true, 'preview' => true], // return with topicable and resources + ['active' => true, 'preview' => false], // return without topicable and resources + ['active' => false], // not return + ) ) - ) ) ->create(); @@ -298,28 +301,65 @@ public function test_anonymous_can_attend_free_course_program() ->assertJsonCount(2, 'data.lessons.0.topics'); $this->response->assertJson( - fn (AssertableJson $json) => $json->has( + fn(AssertableJson $json) => $json->has( 'data.lessons', - fn ($json) => $json->each( - fn (AssertableJson $lesson) => $lesson + fn($json) => $json->each( + fn(AssertableJson $lesson) => $lesson ->where('active', true) - ->has('topics', - fn (AssertableJson $topics) => $topics->each( - fn (AssertableJson $topic) => $topic - ->where('active', true) - ->where('preview', function (string $preview) use ($topic) { - $preview - ? $topic->hasAll(['topicable', 'resources'])->etc() - : $topic->missingAll(['topicable', 'resources'])->etc(); - return true; - })->etc() + ->has( + 'topics', + fn(AssertableJson $topics) => $topics->each( + fn(AssertableJson $topic) => $topic + ->where('active', true) + ->where('preview', function (string $preview) use ($topic) { + // $preview + // ? $topic->hasAll(['topicable', 'resources'])->etc() + // : $topic->missingAll(['topicable', 'resources'])->etc(); + return true; + })->etc() + )->etc() )->etc() - )->etc() ) )->etc() ); } + public function test_preview_topic_access() + { + $course = Course::factory() + ->state(['status' => CourseStatusEnum::PUBLISHED, 'public' => true]) + ->has( + Lesson::factory() + ->count(2) + ->sequence( + ['active' => true], + ['active' => false], + ) + ->has( + Topic::factory()->count(2) + ->sequence( + ['active' => true, 'preview' => true], // return with topicable and resources + ['active' => true, 'preview' => false], // return without topicable and resources + ['active' => false], // not return + ) + ) + ) + ->create(); + + $this->response = $this->getJson('/api/courses/' . $course->id); + $this->response->assertStatus(200); + + $lessons = $this->response->getData()->data->lessons; + + + + $this->response = $this->getJson('/api/courses/' . $course->id . '/preview/' . $lessons[0]->topics[0]->id); + $this->response->assertStatus($lessons[0]->topics[0]->preview ? 200 : 404); + + $this->response = $this->getJson('/api/courses/' . $course->id . '/preview/' . $lessons[0]->topics[1]->id); + $this->response->assertStatus($lessons[0]->topics[1]->preview ? 200 : 404); + } + public function test_anonymous_sorting() { $course1 = Course::factory()->create(['status' => CourseStatusEnum::PUBLISHED]); @@ -509,18 +549,20 @@ public function test_anonymous_read_course_without_inactive_lessons_and_topics() { $course = Course::factory() ->state(['status' => CourseStatusEnum::PUBLISHED, 'findable' => true, 'public' => true]) - ->has(Lesson::factory() - ->count(2) - ->sequence( - ['active' => true], - ['active' => false], - ) - ->has(Topic::factory()->count(2) + ->has( + Lesson::factory() + ->count(2) ->sequence( ['active' => true], ['active' => false], ) - ) + ->has( + Topic::factory()->count(2) + ->sequence( + ['active' => true], + ['active' => false], + ) + ) ) ->create(); @@ -530,16 +572,17 @@ public function test_anonymous_read_course_without_inactive_lessons_and_topics() ->assertJsonCount(1, 'data.lessons.0.topics'); $this->response->assertJson( - fn (AssertableJson $json) => $json->has( + fn(AssertableJson $json) => $json->has( 'data.lessons', - fn ($json) => $json->each( - fn (AssertableJson $lesson) => $lesson + fn($json) => $json->each( + fn(AssertableJson $lesson) => $lesson ->where('active', true)->etc() - ->has('topics', - fn (AssertableJson $topics) => $topics->each( - fn (AssertableJson $topic) => $topic + ->has( + 'topics', + fn(AssertableJson $topics) => $topics->each( + fn(AssertableJson $topic) => $topic ->where('active', true) - ->etc() + ->etc() )->etc() )->etc() )->etc() @@ -551,16 +594,16 @@ private function assertCourseAuthorFilterResponse(array $authorIds, int $count): { $this->response->assertJsonCount($count, 'data'); $this->response->assertJson( - fn (AssertableJson $json) => $json->has( + fn(AssertableJson $json) => $json->has( 'data', - fn ($json) => $json->each( - fn (AssertableJson $json) => + fn($json) => $json->each( + fn(AssertableJson $json) => $json->has( 'authors', - fn (AssertableJson $json) => + fn(AssertableJson $json) => $json->each( - fn (AssertableJson $json) => - $json->where('id', fn ($json) => in_array($json, $authorIds))->etc() + fn(AssertableJson $json) => + $json->where('id', fn($json) => in_array($json, $authorIds))->etc() )->etc() )->etc() )