From 36b98e1e67a992dc6b9b82315c13b22a29e7dbb1 Mon Sep 17 00:00:00 2001 From: Adrian Hopek Date: Mon, 24 Jan 2022 11:21:00 +0100 Subject: [PATCH] #23 - cr fix --- app/Helpers/YearPeriodRetriever.php | 8 +++++++- app/Http/Requests/VacationLimitRequest.php | 14 +++++-------- app/Http/Resources/UserFormDataResource.php | 2 +- app/Http/Resources/VacationLimitResource.php | 4 ++-- app/Models/VacationLimit.php | 8 ++++---- app/Providers/AppServiceProvider.php | 7 ------- app/Providers/ObserverServiceProvider.php | 20 +++++++++++++++++++ config/app.php | 1 + ...19_140630_create_vacation_limits_table.php | 1 - resources/js/Pages/VacationLimits.vue | 1 - tests/Feature/SelectYearPeriodTest.php | 17 +++++++++++----- tests/Feature/VacationLimitTest.php | 6 ------ tests/Unit/YearPeriodRetrieverTest.php | 4 ++-- 13 files changed, 54 insertions(+), 39 deletions(-) create mode 100644 app/Providers/ObserverServiceProvider.php diff --git a/app/Helpers/YearPeriodRetriever.php b/app/Helpers/YearPeriodRetriever.php index 15ee6f2..b474865 100644 --- a/app/Helpers/YearPeriodRetriever.php +++ b/app/Helpers/YearPeriodRetriever.php @@ -4,16 +4,22 @@ declare(strict_types=1); namespace Toby\Helpers; +use Illuminate\Contracts\Session\Session; use Toby\Models\YearPeriod; class YearPeriodRetriever { public const SESSION_KEY = "selected_year_period"; + public function __construct( + protected Session $session, + ) { + } + public function selected(): YearPeriod { /** @var YearPeriod $yearPeriod */ - $yearPeriod = YearPeriod::query()->find(session()->get(static::SESSION_KEY)); + $yearPeriod = YearPeriod::query()->find($this->session->get(static::SESSION_KEY)); return $yearPeriod !== null ? $yearPeriod : $this->current(); } diff --git a/app/Http/Requests/VacationLimitRequest.php b/app/Http/Requests/VacationLimitRequest.php index a91ccfe..fa43bbc 100644 --- a/app/Http/Requests/VacationLimitRequest.php +++ b/app/Http/Requests/VacationLimitRequest.php @@ -15,8 +15,7 @@ class VacationLimitRequest extends FormRequest return [ "items" => ["required", "array"], "items.*.id" => ["required", "exists:vacation_limits,id"], - "items.*.hasVacation" => ["required", "boolean"], - "items.*.days" => ["exclude_if:items.*.hasVacation,false", "required", "integer", "min:0"], + "items.*.days" => ["nullable", "integer", "min:0"], ]; } @@ -25,13 +24,10 @@ class VacationLimitRequest extends FormRequest return VacationLimit::query()->find($this->collect("items")->pluck("id")); } - public function data(): Collection + public function data(): array { - return $this->collect("items")->mapWithKeys(fn(array $item): array => [ - $item["id"] => [ - "has_vacation" => $item["hasVacation"], - "days" => $item["days"], - ], - ]); + return $this->collect("items") + ->keyBy("id") + ->toArray(); } } diff --git a/app/Http/Resources/UserFormDataResource.php b/app/Http/Resources/UserFormDataResource.php index df7da1f..1ee44f3 100644 --- a/app/Http/Resources/UserFormDataResource.php +++ b/app/Http/Resources/UserFormDataResource.php @@ -8,7 +8,7 @@ use Illuminate\Http\Resources\Json\JsonResource; class UserFormDataResource extends JsonResource { - public static $wrap = false; + public static $wrap = null; public function toArray($request): array { diff --git a/app/Http/Resources/VacationLimitResource.php b/app/Http/Resources/VacationLimitResource.php index 21a2f3e..85692b9 100644 --- a/app/Http/Resources/VacationLimitResource.php +++ b/app/Http/Resources/VacationLimitResource.php @@ -8,14 +8,14 @@ use Illuminate\Http\Resources\Json\JsonResource; class VacationLimitResource extends JsonResource { - public static $wrap = false; + public static $wrap = null; public function toArray($request): array { return [ "id" => $this->id, "user" => new UserResource($this->user), - "hasVacation" => $this->has_vacation, + "hasVacation" => $this->hasVacation(), "days" => $this->days, ]; } diff --git a/app/Models/VacationLimit.php b/app/Models/VacationLimit.php index 2fe2433..6e6a361 100644 --- a/app/Models/VacationLimit.php +++ b/app/Models/VacationLimit.php @@ -12,7 +12,6 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo; * @property int $id * @property User $user * @property YearPeriod $yearPeriod - * @property bool $has_vacation * @property int $days */ class VacationLimit extends Model @@ -21,9 +20,10 @@ class VacationLimit extends Model protected $guarded = []; - protected $casts = [ - "has_vacation" => "boolean", - ]; + public function hasVacation(): bool + { + return $this->days !== null; + } public function user(): BelongsTo { diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 077ea67..cf3fe47 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -6,20 +6,13 @@ namespace Toby\Providers; use Illuminate\Support\Carbon; use Illuminate\Support\ServiceProvider; -use Toby\Models\User; use Toby\Models\VacationLimit; -use Toby\Models\YearPeriod; -use Toby\Observers\UserObserver; -use Toby\Observers\YearPeriodObserver; use Toby\Scopes\SelectedYearPeriodScope; class AppServiceProvider extends ServiceProvider { public function boot(): void { - User::observe(UserObserver::class); - YearPeriod::observe(YearPeriodObserver::class); - Carbon::macro("toDisplayString", fn() => $this->translatedFormat("j F Y")); VacationLimit::addGlobalScope($this->app->make(SelectedYearPeriodScope::class)); diff --git a/app/Providers/ObserverServiceProvider.php b/app/Providers/ObserverServiceProvider.php new file mode 100644 index 0000000..16badc4 --- /dev/null +++ b/app/Providers/ObserverServiceProvider.php @@ -0,0 +1,20 @@ +id(); $table->foreignIdFor(User::class)->constrained()->cascadeOnDelete(); $table->foreignIdFor(YearPeriod::class)->constrained()->cascadeOnDelete(); - $table->boolean("has_vacation")->default(false); $table->integer("days")->nullable(); $table->timestamps(); }); diff --git a/resources/js/Pages/VacationLimits.vue b/resources/js/Pages/VacationLimits.vue index a07dbde..04afda6 100644 --- a/resources/js/Pages/VacationLimits.vue +++ b/resources/js/Pages/VacationLimits.vue @@ -162,7 +162,6 @@ export default { .transform(data => ({ items: data.items.map(item => ({ id: item.id, - hasVacation: item.hasVacation, days: item.hasVacation ? item.days : null, })), })) diff --git a/tests/Feature/SelectYearPeriodTest.php b/tests/Feature/SelectYearPeriodTest.php index 7877be6..ef40190 100644 --- a/tests/Feature/SelectYearPeriodTest.php +++ b/tests/Feature/SelectYearPeriodTest.php @@ -14,17 +14,25 @@ class SelectYearPeriodTest extends FeatureTestCase { use DatabaseMigrations; + protected YearPeriodRetriever $yearPeriodRetriever; + + protected function setUp(): void + { + parent::setUp(); + + $this->yearPeriodRetriever = $this->app->make(YearPeriodRetriever::class); + } + public function testUserCanSelectNextYearPeriod(): void { $nextYearPeriod = $this->createYearPeriod(Carbon::now()->year + 1); $user = User::factory()->create(); - $yearPeriodRetriever = new YearPeriodRetriever(); $this->actingAs($user) ->post("/year-periods/{$nextYearPeriod->id}/select") ->assertRedirect(); - $this->assertSame($nextYearPeriod->id, $yearPeriodRetriever->selected()->id); + $this->assertSame($nextYearPeriod->id, $this->yearPeriodRetriever->selected()->id); } public function testUserCannotSelectNextYearPeriodIfDoesntExist(): void @@ -38,9 +46,8 @@ class SelectYearPeriodTest extends FeatureTestCase public function testIfUserDoesntSelectAnyYearPeriodCurrentActsAsSelected(): void { - $yearPeriodRetriever = new YearPeriodRetriever(); - $currentYearPeriod = $yearPeriodRetriever->current(); + $currentYearPeriod = $this->yearPeriodRetriever->current(); - $this->assertSame($currentYearPeriod->id, $yearPeriodRetriever->selected()->id); + $this->assertSame($currentYearPeriod->id, $this->yearPeriodRetriever->selected()->id); } } diff --git a/tests/Feature/VacationLimitTest.php b/tests/Feature/VacationLimitTest.php index e1d2e5e..7e62fa7 100644 --- a/tests/Feature/VacationLimitTest.php +++ b/tests/Feature/VacationLimitTest.php @@ -41,17 +41,14 @@ class VacationLimitTest extends FeatureTestCase $data = [ [ "id" => $limit1->id, - "hasVacation" => true, "days" => 25, ], [ "id" => $limit2->id, - "hasVacation" => false, "days" => null, ], [ "id" => $limit3->id, - "hasVacation" => true, "days" => 20, ], ]; @@ -64,19 +61,16 @@ class VacationLimitTest extends FeatureTestCase $this->assertDatabaseHas("vacation_limits", [ "id" => $limit1->id, - "has_vacation" => true, "days" => 25, ]); $this->assertDatabaseHas("vacation_limits", [ "id" => $limit2->id, - "has_vacation" => false, "days" => null, ]); $this->assertDatabaseHas("vacation_limits", [ "id" => $limit3->id, - "has_vacation" => true, "days" => 20, ]); } diff --git a/tests/Unit/YearPeriodRetrieverTest.php b/tests/Unit/YearPeriodRetrieverTest.php index 8d88104..a6a82c4 100644 --- a/tests/Unit/YearPeriodRetrieverTest.php +++ b/tests/Unit/YearPeriodRetrieverTest.php @@ -31,7 +31,7 @@ class YearPeriodRetrieverTest extends TestCase $this->current = Carbon::now(); Carbon::setTestNow($this->current); - $this->yearPeriodRetriever = new YearPeriodRetriever(); + $this->yearPeriodRetriever = $this->app->make(YearPeriodRetriever::class); $this->previousYearPeriod = $this->createYearPeriod($this->current->year - 1); $this->currentYearPeriod = $this->createCurrentYearPeriod(); @@ -43,7 +43,7 @@ class YearPeriodRetrieverTest extends TestCase $this->assertSame($this->currentYearPeriod->id, $this->yearPeriodRetriever->current()->id); } - public function testRetrievesCurrentYearPeriodWhenNoSelected(): void + public function testRetrievesCurrentYearPeriodWhenNoneIsSelected(): void { $this->clearSelectedYearPeriod();