From cce6d2f51b6ac6c608b29e236cbba4711d85cf90 Mon Sep 17 00:00:00 2001 From: diegdar Date: Wed, 16 Oct 2024 11:26:15 +0200 Subject: [PATCH 1/5] refactor(routes): :recycle: rename studentId parameter to student in endpoint --- routes/api/v1.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routes/api/v1.php b/routes/api/v1.php index cacf9bc4..d444d5cf 100644 --- a/routes/api/v1.php +++ b/routes/api/v1.php @@ -57,11 +57,11 @@ Route::get('student/{student}/resume/additionaltraining', StudentAdditionalTrainingListController::class)->name('student.additionaltraining'); Route::get('student/{student}/resume/collaborations', StudentCollaborationDetailController::class)->name('student.collaborations'); Route::put('student/{student}/resume/collaborations', UpdateStudentCollaborationsController::class)->name('student.updateCollaborations'); +Route::get('student/{student}/resume/photo', GetStudentImageController::class)->middleware('auth:api', EnsureStudentOwner::class)->name('student.photo.get'); Route::prefix('student/{studentId}/resume')->group(function () { Route::put('languages', UpdateStudentLanguagesController::class)->name('student.languages.update'); Route::get('modality', StudentModalityController::class)->name('student.modality'); - Route::get('photo', GetStudentImageController::class)->middleware('auth:api', EnsureStudentOwner::class)->name('student.photo.get'); Route::post('photo', UpdateStudentImageController::class)->name('student.updatePhoto'); Route::delete('languages/{languageId}', DeleteStudentResumeLanguageController::class)->name('student.language.delete'); }); From 9a155d92ed632bbeecc8ff91aa256dab0badaf65 Mon Sep 17 00:00:00 2001 From: diegdar Date: Thu, 17 Oct 2024 10:45:20 +0200 Subject: [PATCH 2/5] refactor(test): :recycle: refactor GetStudentImageControllerTest for model binding --- .../Student/GetStudentImageControllerTest.php | 68 +++++++++---------- 1 file changed, 33 insertions(+), 35 deletions(-) diff --git a/tests/Feature/Controller/Student/GetStudentImageControllerTest.php b/tests/Feature/Controller/Student/GetStudentImageControllerTest.php index deea6e85..24e5a1be 100644 --- a/tests/Feature/Controller/Student/GetStudentImageControllerTest.php +++ b/tests/Feature/Controller/Student/GetStudentImageControllerTest.php @@ -9,32 +9,40 @@ }; use App\Service\Student\GetStudentImageService; use Illuminate\Foundation\Testing\DatabaseTransactions; -use Mockery\MockInterface; +use Illuminate\Support\Facades\Storage; use Tests\TestCase; class GetStudentImageControllerTest extends TestCase { use DatabaseTransactions; + private const PHOTOS_PATH = 'public/photos/'; + protected User $user; + protected Student $student; protected function setUp(): void { parent::setUp(); - } - private function createUser() - { - return User::factory()->create([ + $this->user = User::factory()->create([ 'dni'=>'27827083G', 'password'=>'Password%123' ]); + + $this->student =Student::factory()->for($this->user)->create(); + + } + + public function testCanInstantiateAnUser(): void + { + $this->assertInstanceOf(User::class, $this->user); } - private function createStudent(User $user):Student + public function testCanInstantiateAStudent(): void { - return Student::factory()->for($user)->create(); + $this->assertInstanceOf(Student::class, $this->student); } - private function getUserToken(User $user):string - { //hay que pasar lo datos manualmente porque el password es encriptado + private function getUserToken():string + { $singInUserData = ['dni'=>'27827083G', 'password'=>'Password%123']; $url = route('signin'); $response = $this->json('POST', $url, $singInUserData); @@ -42,56 +50,46 @@ private function getUserToken(User $user):string return $token; } - public function test_get_student_image():void + public function testCanGetStudentImage():void { - $user = $this->createUser(); - $student = $this->createStudent($user); - $token = $this->getUserToken($user); - $studentId = $student->id; + $student = $this->student; + $token = $this->getUserToken(); - $url = route('student.photo.get', $studentId); + $url = route('student.photo.get', $student->id); $response = $this->withHeaders([ 'Authorization' => 'Bearer ' . $token ])->get($url); $response->assertStatus(200); - } + $response->assertJson(['photo' => Storage::url(self::PHOTOS_PATH . $student->photo)]);} - public function test_get_student_image_not_found():void + public function testCanGetAnEmptyArrayWhenTheStudentHasNoPhoto():void { - $user = $this->createUser(); - $student = $this->createStudent($user); + $student = $this->student; $student->photo = null; $student->save(); - $token = $this->getUserToken($user); - $studentId = $student->id; + $token = $this->getUserToken(); - $url = route('student.photo.get', $studentId); + $url = route('student.photo.get', $student->id); $response = $this->withHeaders([ 'Authorization' => 'Bearer ' . $token ])->get($url); $response->assertStatus(200); - $response->assertJson([]); + $response->assertJson(['photo' => '']); } - public function test_can_return_a_500_on_internal_server_error(): void + public function testCanReturns_404WithInvalidStudentUuid(): void { - $this->mock(GetStudentImageService::class, function (MockInterface $mock) { - $mock->shouldReceive('execute') - ->andThrow(new \Exception('Internal Server Error')); - }); - $user = $this->createUser(); - $student = $this->createStudent($user); - $token = $this->getUserToken($user); - $studentId = $student->id; - - $url = route('student.photo.get', $studentId); + $invalidUuid = 'invalidUuid'; + $token = $this->getUserToken(); + + $url = route('student.photo.get', $invalidUuid); $response = $this->withHeaders([ 'Authorization' => 'Bearer ' . $token ])->get($url); - $response->assertStatus(500); + $response->assertStatus(404); } } From 123707181e07a66514c40407d7c0b3e2b0225e02 Mon Sep 17 00:00:00 2001 From: diegdar Date: Thu, 17 Oct 2024 10:59:51 +0200 Subject: [PATCH 3/5] refactor(controller/route): :recycle: refactor route and controller for model binding --- .../api/Student/GetStudentImageController.php | 21 +++++++------------ routes/api/v1.php | 2 +- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/app/Http/Controllers/api/Student/GetStudentImageController.php b/app/Http/Controllers/api/Student/GetStudentImageController.php index 27e3efa1..05ee4b8e 100644 --- a/app/Http/Controllers/api/Student/GetStudentImageController.php +++ b/app/Http/Controllers/api/Student/GetStudentImageController.php @@ -4,24 +4,17 @@ namespace App\Http\Controllers\api\Student; use App\Http\Controllers\Controller; -use App\Service\Student\GetStudentImageService; -use Illuminate\Support\Facades\Log; +use App\Models\Student; +use Illuminate\Support\Facades\Storage; class GetStudentImageController extends Controller { - public function __construct(public GetStudentImageService $getStudentImageService){ } + private const PHOTOS_PATH = 'public/photos/'; - public function __invoke(string $studentId) + public function __invoke(Student $student) { - try { - $studentPhoto = $this->getStudentImageService->execute($studentId); - return response()->json(['photo'=>$studentPhoto], 200); - } catch (\Exception $e) { - Log::error('Exception:', [ - 'exception' => $e->getMessage(), - 'trace' => $e->getTraceAsString(), - ]); - return response()->json('Error del servidor', 500); - } + $url = $student->photo ? Storage::url(self::PHOTOS_PATH . $student->photo): ''; + return response()->json(['photo'=>$url]); } + } diff --git a/routes/api/v1.php b/routes/api/v1.php index 9feb0bfd..ec0778c3 100644 --- a/routes/api/v1.php +++ b/routes/api/v1.php @@ -57,7 +57,7 @@ Route::get('student/{student}/resume/additionaltraining', StudentAdditionalTrainingListController::class)->name('student.additionaltraining'); Route::get('student/{student}/resume/collaborations', StudentCollaborationDetailController::class)->name('student.collaborations'); Route::put('student/{student}/resume/collaborations', UpdateStudentCollaborationsController::class)->name('student.updateCollaborations'); -Route::get('student/{student}/resume/photo', GetStudentImageController::class)->middleware('auth:api', EnsureStudentOwner::class)->name('student.photo.get'); +Route::get('student/{student}/resume/photo', GetStudentImageController::class)->name('student.photo.get'); Route::put('student/{student}/resume/photo', UpdateStudentImageController::class)->name('student.updatePhoto'); Route::prefix('student/{studentId}/resume')->group(function () { From 5d8b0b8ccec79b3c465da784c483aac045e532cc Mon Sep 17 00:00:00 2001 From: diegdar Date: Thu, 17 Oct 2024 11:07:11 +0200 Subject: [PATCH 4/5] refactor(service): :recycle: delete GetStudentImageService and its tests due to model binding --- .../Student/GetStudentImageService.php | 31 --------- .../Student/GetStudentImageServiceTest.php | 64 ------------------- 2 files changed, 95 deletions(-) delete mode 100644 app/Service/Student/GetStudentImageService.php delete mode 100644 tests/Feature/Service/Student/GetStudentImageServiceTest.php diff --git a/app/Service/Student/GetStudentImageService.php b/app/Service/Student/GetStudentImageService.php deleted file mode 100644 index bfd05063..00000000 --- a/app/Service/Student/GetStudentImageService.php +++ /dev/null @@ -1,31 +0,0 @@ -getStudent($studentId); - if ($student->photo != null && $student->photo != "") { - return Storage::url(self::PHOTOS_PATH . $student->photo); - } - return null; - } - - public function getStudent(string $studentId): Student - { - $student = Student::find($studentId); - if (!$student) { - throw new StudentNotFoundException($studentId); - } - return $student; - } -} diff --git a/tests/Feature/Service/Student/GetStudentImageServiceTest.php b/tests/Feature/Service/Student/GetStudentImageServiceTest.php deleted file mode 100644 index 4a9a8d9e..00000000 --- a/tests/Feature/Service/Student/GetStudentImageServiceTest.php +++ /dev/null @@ -1,64 +0,0 @@ -getStudentImageService = new GetStudentImageService(); - } - - private function createFakeData(): Student - { - return Student::factory()->create(); - } - - public function test_can_instantiate_service(): void - { - $this->assertInstanceOf(GetStudentImageService::class, $this->getStudentImageService); - } - - public function test_can_get_student_image(): void - { - $student = $this->createFakeData(); - $studentPhoto = $this->getStudentImageService->execute($student->id); - $this->assertIsString($studentPhoto); - } - - public function test_cannot_get_student_image_null_photo(): void - { - $student = $this->createFakeData(); - $student->photo = null; - $student->save(); - $studentPhoto = $this->getStudentImageService->execute($student->id); - $this->assertNull($studentPhoto); - } - public function test_cannot_get_student_image_empty_string_photo(): void - { - $student = $this->createFakeData(); - $student->photo = ""; - $student->save(); - $studentPhoto = $this->getStudentImageService->execute($student->id); - $this->assertNull($studentPhoto); - } - - public function test_cannot_get_student_image_throws_student_not_found_exception(): void - { - $this->expectException(StudentNotFoundException::class); - $this->getStudentImageService->execute("invalid-student-id"); - } - -} From 65cadbaa241850e10314aa5928526a89c081a157 Mon Sep 17 00:00:00 2001 From: diegdar Date: Thu, 17 Oct 2024 11:08:34 +0200 Subject: [PATCH 5/5] refactor(annotations): :recycle: refactor GetStudentImageAnnotation due to model binding --- .../Controllers/StudentResume/GetStudentImageAnnotation.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/Annotations/OpenApi/Controllers/StudentResume/GetStudentImageAnnotation.php b/app/Annotations/OpenApi/Controllers/StudentResume/GetStudentImageAnnotation.php index 9469a20c..bcb0055f 100644 --- a/app/Annotations/OpenApi/Controllers/StudentResume/GetStudentImageAnnotation.php +++ b/app/Annotations/OpenApi/Controllers/StudentResume/GetStudentImageAnnotation.php @@ -6,7 +6,7 @@ class GetStudentImageAnnotation { /** * @OA\Get ( - * path="/student/{studentId}/resume/photo", + * path="/student/{student}/resume/photo", * operationId="getStudentProfileImage", * tags={"Student -> Resume"}, * summary="Get the Student Profile Photo.", @@ -14,7 +14,7 @@ class GetStudentImageAnnotation * security={{"bearerAuth":{}}}, * * @OA\Parameter( - * name="studentId", + * name="student", * in="path", * description="Student ID", * required=true,