From f65ff3b6b1b56ee5d8c5d826d97907a647d41056 Mon Sep 17 00:00:00 2001 From: Kamil Piech Date: Tue, 23 Jul 2024 08:43:50 +0200 Subject: [PATCH] #452 - user profile cleaning (#459) * #452 - refactor: user profile cleaning * Update app/Observers/UserHistoryObserver.php Co-authored-by: Krzysztof Rewak * #452 - fix: code review fixes * #452 - fix: rollback last update key name --------- Co-authored-by: Krzysztof Rewak --- app/Domain/DashboardAggregator.php | 4 +- app/Domain/UserVacationStatsRetriever.php | 4 +- .../Controllers/OvertimeRequestController.php | 8 +-- .../Controllers/VacationRequestController.php | 6 +- app/Http/Requests/UserRequest.php | 2 - app/Http/Resources/UserResource.php | 3 +- app/Models/Profile.php | 9 --- app/Models/User.php | 13 ++-- app/Observers/OvertimeRequestObserver.php | 2 +- app/Observers/UserHistoryObserver.php | 21 ++++++ app/Observers/VacationRequestObserver.php | 2 +- database/factories/ProfileFactory.php | 1 - ...0731_cleaning_fields_in_profiles_table.php | 31 +++++++++ database/seeders/DemoSeeder.php | 65 +++++++++++++++---- tests/Feature/ResumeTest.php | 1 - tests/Feature/UserTest.php | 6 -- 16 files changed, 129 insertions(+), 49 deletions(-) create mode 100644 app/Observers/UserHistoryObserver.php create mode 100644 database/migrations/2024_07_04_130731_cleaning_fields_in_profiles_table.php diff --git a/app/Domain/DashboardAggregator.php b/app/Domain/DashboardAggregator.php index adbd63a8..67493275 100644 --- a/app/Domain/DashboardAggregator.php +++ b/app/Domain/DashboardAggregator.php @@ -51,7 +51,7 @@ public function aggregateCalendarData(User $user, YearPeriod $yearPeriod): array ->vacations() ->with(["vacationRequest.vacations", "vacationRequest.user.profile"]) ->whereBelongsTo($yearPeriod) - ->cache("vacations{$user->id}") + ->cache("vacations:{$user->id}") ->approved() ->get() ->mapWithKeys( @@ -64,7 +64,7 @@ public function aggregateCalendarData(User $user, YearPeriod $yearPeriod): array ->vacations() ->with(["vacationRequest.vacations", "vacationRequest.user.profile"]) ->whereBelongsTo($yearPeriod) - ->cache("vacations{$user->id}") + ->cache("vacations:{$user->id}") ->pending() ->get() ->mapWithKeys( diff --git a/app/Domain/UserVacationStatsRetriever.php b/app/Domain/UserVacationStatsRetriever.php index 5d936ba7..2f666b6b 100644 --- a/app/Domain/UserVacationStatsRetriever.php +++ b/app/Domain/UserVacationStatsRetriever.php @@ -103,7 +103,7 @@ public function getVacationDaysLimit(User $user, YearPeriod $yearPeriod): int { return $user->vacationLimits() ->whereBelongsTo($yearPeriod) - ->cache("vacations{$user->id}") + ->cache("vacations:{$user->id}") ->first()?->limit ?? 0; } @@ -111,7 +111,7 @@ public function hasVacationDaysLimit(User $user, YearPeriod $yearPeriod): bool { return $user->vacationLimits() ->whereBelongsTo($yearPeriod) - ->cache("vacations{$user->id}") + ->cache("vacations:{$user->id}") ->first()?->hasVacation() ?? false; } diff --git a/app/Http/Controllers/OvertimeRequestController.php b/app/Http/Controllers/OvertimeRequestController.php index 54b8ac7c..ec3c8dde 100644 --- a/app/Http/Controllers/OvertimeRequestController.php +++ b/app/Http/Controllers/OvertimeRequestController.php @@ -48,28 +48,28 @@ public function index(Request $request, YearPeriodRetriever $yearPeriodRetriever ->overtimeRequests() ->whereBelongsTo($yearPeriodRetriever->selected()) ->states(OvertimeRequestStatesRetriever::pendingStates()) - ->cache(key: "overtime{$user->id}") + ->cache(key: "overtime:{$user->id}") ->count(); $success = $user ->overtimeRequests() ->whereBelongsTo($yearPeriodRetriever->selected()) ->states(OvertimeRequestStatesRetriever::successStates()) - ->cache(key: "overtime{$user->id}") + ->cache(key: "overtime:{$user->id}") ->count(); $failed = $user ->overtimeRequests() ->whereBelongsTo($yearPeriodRetriever->selected()) ->states(OvertimeRequestStatesRetriever::failedStates()) - ->cache(key: "overtime{$user->id}") + ->cache(key: "overtime:{$user->id}") ->count(); $settled = $user ->overtimeRequests() ->whereBelongsTo($yearPeriodRetriever->selected()) ->states(OvertimeRequestStatesRetriever::settledStates()) - ->cache(key: "overtime{$user->id}") + ->cache(key: "overtime:{$user->id}") ->count(); return inertia("OvertimeRequest/Index", [ diff --git a/app/Http/Controllers/VacationRequestController.php b/app/Http/Controllers/VacationRequestController.php index 2e2d73b7..693b0a1e 100644 --- a/app/Http/Controllers/VacationRequestController.php +++ b/app/Http/Controllers/VacationRequestController.php @@ -60,7 +60,7 @@ public function index(Request $request, YearPeriodRetriever $yearPeriodRetriever ->whereBelongsTo($yearPeriodRetriever->selected()) ->states(VacationRequestStatesRetriever::pendingStates()) ->when($withoutRemote, fn(Builder $query): Builder => $query->excludeType(VacationType::RemoteWork)) - ->cache(key: "vacations{$user->id}") + ->cache(key: "vacations:{$user->id}") ->count(); $success = $user @@ -68,7 +68,7 @@ public function index(Request $request, YearPeriodRetriever $yearPeriodRetriever ->whereBelongsTo($yearPeriodRetriever->selected()) ->states(VacationRequestStatesRetriever::successStates()) ->when($withoutRemote, fn(Builder $query): Builder => $query->excludeType(VacationType::RemoteWork)) - ->cache(key: "vacations{$user->id}") + ->cache(key: "vacations:{$user->id}") ->count(); $failed = $user @@ -76,7 +76,7 @@ public function index(Request $request, YearPeriodRetriever $yearPeriodRetriever ->whereBelongsTo($yearPeriodRetriever->selected()) ->states(VacationRequestStatesRetriever::failedStates()) ->when($withoutRemote, fn(Builder $query): Builder => $query->excludeType(VacationType::RemoteWork)) - ->cache(key: "vacations{$user->id}") + ->cache(key: "vacations:{$user->id}") ->count(); return inertia("VacationRequest/Index", [ diff --git a/app/Http/Requests/UserRequest.php b/app/Http/Requests/UserRequest.php index 81e05b3a..589eedc6 100644 --- a/app/Http/Requests/UserRequest.php +++ b/app/Http/Requests/UserRequest.php @@ -22,7 +22,6 @@ public function rules(): array "role" => ["required", new Enum(Role::class)], "position" => ["required"], "employmentForm" => ["required", new Enum(EmploymentForm::class)], - "employmentDate" => ["required", "date_format:" . DateFormats::DATE], "birthday" => ["required", "date_format:" . DateFormats::DATE, "before:today"], "slackId" => [], ]; @@ -43,7 +42,6 @@ public function profileData(): array "last_name" => $this->get("lastName"), "position" => $this->get("position"), "employment_form" => $this->get("employmentForm"), - "employment_date" => $this->get("employmentDate"), "birthday" => $this->get("birthday"), "slack_id" => $this->get("slackId"), ]; diff --git a/app/Http/Resources/UserResource.php b/app/Http/Resources/UserResource.php index 52b2f1ed..b251e162 100644 --- a/app/Http/Resources/UserResource.php +++ b/app/Http/Resources/UserResource.php @@ -14,6 +14,7 @@ public function toArray($request): array { $lastMedicalExam = $this->lastMedicalExam(); $lastOhsTraining = $this->lastOhsTraining(); + $startOfEmploymentInCurrentCompany = $this->startOfEmploymentInCurrentCompany(); return [ "id" => $this->id, @@ -25,7 +26,7 @@ public function toArray($request): array "deleted" => $this->trashed(), "lastActiveAt" => $this->last_active_at?->toDateTimeString(), "employmentForm" => $this->profile->employment_form->label(), - "employmentDate" => $this->profile->employment_date->toDisplayString(), + "employmentDate" => $startOfEmploymentInCurrentCompany?->from->toDisplayString(), "lastMedicalExamDate" => $lastMedicalExam?->from?->toDisplayString(), "nextMedicalExamDate" => $lastMedicalExam?->to?->toDisplayString(), "lastOhsTrainingDate" => $lastOhsTraining?->from?->toDisplayString(), diff --git a/app/Models/Profile.php b/app/Models/Profile.php index 191ae60e..1ffbf4c9 100644 --- a/app/Models/Profile.php +++ b/app/Models/Profile.php @@ -21,10 +21,6 @@ * @property EmploymentForm $employment_form * @property Carbon $employment_date * @property Carbon $birthday - * @property ?Carbon $last_medical_exam_date - * @property ?Carbon $next_medical_exam_date - * @property ?Carbon $last_ohs_training_date - * @property ?Carbon $next_ohs_training_date */ class Profile extends Model { @@ -34,12 +30,7 @@ class Profile extends Model protected $guarded = []; protected $casts = [ "employment_form" => EmploymentForm::class, - "employment_date" => "date", "birthday" => "date", - "last_medical_exam_date" => "date", - "next_medical_exam_date" => "date", - "last_ohs_training_date" => "date", - "next_ohs_training_date" => "date", ]; public function user(): BelongsTo diff --git a/app/Models/User.php b/app/Models/User.php index a02adc59..9bf6b9b9 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -99,6 +99,7 @@ public function lastMedicalExam(): ?UserHistory return $this->histories() ->where("type", UserHistoryType::MedicalExam) ->orderBy("to", "desc") + ->cache("user:history{$this->id}") ->first(); } @@ -107,6 +108,7 @@ public function lastOhsTraining(): ?UserHistory return $this->histories() ->where("type", UserHistoryType::OhsTraining) ->orderBy("to", "desc") + ->cache("user:history{$this->id}") ->first(); } @@ -115,7 +117,8 @@ public function startOfEmploymentInCurrentCompany(): ?UserHistory return $this->histories() ->where("type", UserHistoryType::Employment) ->where("is_employed_at_current_company", true) - ->orderBy("from", "asc") + ->orderBy("from") + ->cache("user:history{$this->id}") ->first(); } @@ -243,15 +246,15 @@ public function isWorkAnniversaryToday(): bool { $today = Carbon::now(); - $employmentDate = $this->profile->employment_date; + $employmentDate = $this->startOfEmploymentInCurrentCompany()?->from; - if ($employmentDate->isToday()) { + if ($employmentDate?->isToday()) { return false; } - $workAnniversary = $employmentDate->setYear($today->year); + $workAnniversary = $employmentDate?->setYear($today->year); - return $workAnniversary->isToday(); + return $workAnniversary?->isToday() ?? false; } protected static function newFactory(): UserFactory diff --git a/app/Observers/OvertimeRequestObserver.php b/app/Observers/OvertimeRequestObserver.php index 13ddccd0..700915d6 100644 --- a/app/Observers/OvertimeRequestObserver.php +++ b/app/Observers/OvertimeRequestObserver.php @@ -19,6 +19,6 @@ public function creating(OvertimeRequest $overtime): void public function updating(OvertimeRequest $overtime): void { - CacheQuery::forget("overtime{$overtime->user->id}"); + CacheQuery::forget("overtime:{$overtime->user->id}"); } } diff --git a/app/Observers/UserHistoryObserver.php b/app/Observers/UserHistoryObserver.php new file mode 100644 index 00000000..5a7984f8 --- /dev/null +++ b/app/Observers/UserHistoryObserver.php @@ -0,0 +1,21 @@ +user->id}"); + } + + public function updating(UserHistory $userHistory): void + { + CacheQuery::forget("user:history{$userHistory->user->id}"); + } +} diff --git a/app/Observers/VacationRequestObserver.php b/app/Observers/VacationRequestObserver.php index 634cd79f..414ba813 100644 --- a/app/Observers/VacationRequestObserver.php +++ b/app/Observers/VacationRequestObserver.php @@ -19,6 +19,6 @@ public function creating(VacationRequest $vacationRequest): void public function updating(VacationRequest $vacationRequest): void { - CacheQuery::forget("vacations{$vacationRequest->user->id}"); + CacheQuery::forget("vacations:{$vacationRequest->user->id}"); } } diff --git a/database/factories/ProfileFactory.php b/database/factories/ProfileFactory.php index e5190bba..179d67f6 100644 --- a/database/factories/ProfileFactory.php +++ b/database/factories/ProfileFactory.php @@ -22,7 +22,6 @@ public function definition(): array "last_name" => $this->faker->lastName(), "employment_form" => $this->faker->randomElement(EmploymentForm::cases()), "position" => $this->faker->jobTitle(), - "employment_date" => Carbon::createFromInterface($this->faker->dateTimeBetween("2020-10-27"))->toDateString(), "birthday" => Carbon::createFromInterface($this->faker->dateTimeBetween("1970-01-01", "1998-01-01"))->toDateString(), ]; } diff --git a/database/migrations/2024_07_04_130731_cleaning_fields_in_profiles_table.php b/database/migrations/2024_07_04_130731_cleaning_fields_in_profiles_table.php new file mode 100644 index 00000000..55397d7d --- /dev/null +++ b/database/migrations/2024_07_04_130731_cleaning_fields_in_profiles_table.php @@ -0,0 +1,31 @@ +dropColumn("last_medical_exam_date"); + $table->dropColumn("next_medical_exam_date"); + $table->dropColumn("last_ohs_training_date"); + $table->dropColumn("next_ohs_training_date"); + $table->dropColumn("employment_date"); + }); + } + + public function down(): void + { + Schema::table("profiles", function (Blueprint $table): void { + $table->date("last_medical_exam_date")->nullable()->default(null); + $table->date("next_medical_exam_date")->nullable()->default(null); + $table->date("last_ohs_training_date")->nullable()->default(null); + $table->date("next_ohs_training_date")->nullable()->default(null); + $table->date("employment_date")->nullable()->default(null); + }); + } +}; diff --git a/database/seeders/DemoSeeder.php b/database/seeders/DemoSeeder.php index 2e78f7be..18689544 100644 --- a/database/seeders/DemoSeeder.php +++ b/database/seeders/DemoSeeder.php @@ -11,6 +11,7 @@ use Toby\Domain\WorkDaysCalculator; use Toby\Enums\EmploymentForm; use Toby\Enums\Role; +use Toby\Enums\UserHistoryType; use Toby\Enums\VacationType; use Toby\Models\Benefit; use Toby\Models\BenefitsReport; @@ -48,11 +49,17 @@ public function run(): void "last_name" => "Kowalski", "employment_form" => EmploymentForm::EmploymentContract, "position" => "programista", - "employment_date" => Carbon::createFromDate(2021, 12, 31), ]) ->create(); + $user->histories()->create([ + "from" => Carbon::createFromDate(2021, 12, 31), + "to" => null, + "type" => UserHistoryType::Employment, + "employment_form" => EmploymentForm::EmploymentContract, + "is_employed_at_current_company" => true, + ]); - User::factory([ + $programmer = User::factory([ "email" => "jerzy.nowak@example.com", "remember_token" => Str::random(10), ]) @@ -62,11 +69,17 @@ public function run(): void "last_name" => "Nowak", "employment_form" => EmploymentForm::EmploymentContract, "position" => "programista", - "employment_date" => Carbon::createFromDate(2021, 5, 10), ]) ->create(); + $programmer->histories()->create([ + "from" => Carbon::createFromDate(2021, 5, 10), + "to" => null, + "type" => UserHistoryType::Employment, + "employment_form" => EmploymentForm::EmploymentContract, + "is_employed_at_current_company" => true, + ]); - User::factory([ + $tester = User::factory([ "email" => "anna.nowak@example.com", "remember_token" => Str::random(10), ]) @@ -76,11 +89,17 @@ public function run(): void "last_name" => "Nowak", "employment_form" => EmploymentForm::CommissionContract, "position" => "tester", - "employment_date" => Carbon::createFromDate(2021, 5, 10), ]) ->create(); + $tester->histories()->create([ + "from" => Carbon::createFromDate(2021, 5, 10), + "to" => null, + "type" => UserHistoryType::Employment, + "employment_form" => EmploymentForm::CommissionContract, + "is_employed_at_current_company" => true, + ]); - User::factory([ + $programmer = User::factory([ "email" => "tola.sawicka@example.com", "role" => Role::Employee, "remember_token" => Str::random(10), @@ -91,9 +110,15 @@ public function run(): void "last_name" => "Sawicka", "employment_form" => EmploymentForm::B2bContract, "position" => "programista", - "employment_date" => Carbon::createFromDate(2021, 1, 4), ]) ->create(); + $programmer->histories()->create([ + "from" => Carbon::createFromDate(2021, 1, 4), + "to" => null, + "type" => UserHistoryType::Employment, + "employment_form" => EmploymentForm::B2bContract, + "is_employed_at_current_company" => true, + ]); $technicalApprover = User::factory([ "email" => "maciej.ziolkowski@example.com", @@ -105,9 +130,15 @@ public function run(): void "last_name" => "Ziółkowski", "employment_form" => EmploymentForm::BoardMemberContract, "position" => "programista", - "employment_date" => Carbon::createFromDate(2021, 1, 4), ]) ->create(); + $technicalApprover->histories()->create([ + "from" => Carbon::createFromDate(2021, 1, 4), + "to" => null, + "type" => UserHistoryType::Employment, + "employment_form" => EmploymentForm::BoardMemberContract, + "is_employed_at_current_company" => true, + ]); $administrativeApprover = User::factory([ "email" => "katarzyna.zajac@example.com", @@ -119,11 +150,17 @@ public function run(): void "last_name" => "Zając", "employment_form" => EmploymentForm::EmploymentContract, "position" => "dyrektor", - "employment_date" => Carbon::createFromDate(2021, 1, 4), ]) ->create(); + $administrativeApprover->histories()->create([ + "from" => Carbon::createFromDate(2021, 1, 4), + "to" => null, + "type" => UserHistoryType::Employment, + "employment_form" => EmploymentForm::EmploymentContract, + "is_employed_at_current_company" => true, + ]); - User::factory([ + $administrator = User::factory([ "email" => "milosz.borowski@example.com", "remember_token" => Str::random(10), ]) @@ -133,9 +170,15 @@ public function run(): void "last_name" => "Borowski", "employment_form" => EmploymentForm::EmploymentContract, "position" => "administrator", - "employment_date" => Carbon::createFromDate(2021, 1, 4), ]) ->create(); + $administrator->histories()->create([ + "from" => Carbon::createFromDate(2021, 1, 4), + "to" => null, + "type" => UserHistoryType::Employment, + "employment_form" => EmploymentForm::EmploymentContract, + "is_employed_at_current_company" => true, + ]); $users = User::all(); diff --git a/tests/Feature/ResumeTest.php b/tests/Feature/ResumeTest.php index a608b603..1e5ee57d 100644 --- a/tests/Feature/ResumeTest.php +++ b/tests/Feature/ResumeTest.php @@ -55,7 +55,6 @@ public function testAdminCanCreateResumeForEmployee(): void "last_name" => "Kowalski", "employment_form" => EmploymentForm::EmploymentContract, "position" => "user", - "employment_date" => Carbon::createFromDate(2021, 1, 4), ])->create(); $this->actingAs($admin) diff --git a/tests/Feature/UserTest.php b/tests/Feature/UserTest.php index 19f7bcb3..5cc543ee 100644 --- a/tests/Feature/UserTest.php +++ b/tests/Feature/UserTest.php @@ -121,7 +121,6 @@ public function testAdminCanCreateUser(): void "last_name" => "Doe", "position" => "Test position", "employment_form" => EmploymentForm::B2bContract->value, - "employment_date" => Carbon::now()->toDateString(), "birthday" => Carbon::create(1990, 5, 31)->toDateString(), ]); } @@ -138,7 +137,6 @@ public function testItCreatesProperPermissionsWhileCreatingUser(): void "position" => "Test position", "email" => "john.doe@example.com", "employmentForm" => EmploymentForm::B2bContract->value, - "employmentDate" => Carbon::now()->toDateString(), "birthday" => Carbon::create(1990, 5, 31)->toDateString(), ]) ->assertSessionHasNoErrors(); @@ -167,7 +165,6 @@ public function testAdminCanEditUser(): void "first_name" => $user->profile->first_name, "last_name" => $user->profile->last_name, "employment_form" => $user->profile->employment_form->value, - "employment_date" => $user->profile->employment_date->toDateString(), "birthday" => $user->profile->birthday->toDateString(), ]); @@ -179,7 +176,6 @@ public function testAdminCanEditUser(): void "role" => Role::Employee->value, "position" => "Test position", "employmentForm" => EmploymentForm::B2bContract->value, - "employmentDate" => Carbon::now()->toDateString(), "birthday" => Carbon::create(1990, 5, 31)->toDateString(), ]) ->assertSessionHasNoErrors(); @@ -196,7 +192,6 @@ public function testAdminCanEditUser(): void "last_name" => "Doe", "position" => "Test position", "employment_form" => EmploymentForm::B2bContract->value, - "employment_date" => Carbon::now()->toDateString(), "birthday" => Carbon::create(1990, 5, 31)->toDateString(), ]); } @@ -244,7 +239,6 @@ public function testChangingUserRoleUpdatesPermissions(): void "role" => Role::TechnicalApprover->value, "position" => "Test position", "employmentForm" => EmploymentForm::B2bContract->value, - "employmentDate" => Carbon::now()->toDateString(), "birthday" => Carbon::create(1990, 5, 31)->toDateString(), ]) ->assertSessionHasNoErrors();