diff --git a/app/Listeners/CreateActivity.php b/app/Listeners/CreateActivity.php index 736aea1..bc59e76 100644 --- a/app/Listeners/CreateActivity.php +++ b/app/Listeners/CreateActivity.php @@ -34,7 +34,7 @@ public function handle(EventCreated $eventCreated): void $event = $eventCreated->getEvent(); $adjacentActivity = $this->getAdjacentActivity($event); - if ($adjacentActivity && $this->canBeMergedWithAdjacentActivity($adjacentActivity, $event)) { + if ($adjacentActivity && $this->canAbsorbEvent($adjacentActivity, $event)) { $startedAt = $adjacentActivity->started_at; if ($event->started_at) { $startedAt = $adjacentActivity->started_at->min($event->started_at); @@ -49,7 +49,7 @@ public function handle(EventCreated $eventCreated): void } } - private function canBeMergedWithAdjacentActivity(Activity $lastActivity, Event $event): bool + private function canAbsorbEvent(Activity $lastActivity, Event $event): bool { $suggestion = $lastActivity->entrySuggestion; @@ -98,21 +98,52 @@ private function createActivityFromEvent(Event $event): ?Activity $activity->is_internal = $event->is_internal; $activity->event_type_id = $event->eventType->id ?? null; + $absorbedActivities = collect(); if (config('timatic.feature.activity_overlap_detection')) { - $activity = $this->handleOverlappingActivities($activity); + $absorbedActivities = $this->trimOverlappingActivities($activity); + + if (! $activity->ended_at->isAfter($activity->started_at)) { + $this->attachEventToCoveringActivity($event); + + return null; + } } - if ($activity) { - $this->db->transaction(function () use ($activity, $event) { - $activity->save(); - $activity->events()->save($event); + $this->db->transaction(function () use ($activity, $event, $absorbedActivities) { + $activity->save(); + $activity->events()->save($event); + + $absorbedActivities->each(function (Activity $absorbedActivity) use ($activity) { + $absorbedActivity->events()->update(['activity_id' => $activity->id]); + $absorbedActivity->delete(); }); - } + }); return $activity; } - private function handleOverlappingActivities(Activity $activity): ?Activity + private function attachEventToCoveringActivity(Event $event): void + { + /** @var ?Activity $coveringActivity */ + $coveringActivity = Activity::query() + ->where('user_id', $event->user_id) + ->where('started_at', '<=', $event->ended_at) + ->where('ended_at', '>=', $event->ended_at) + ->first(); + + if ($coveringActivity && $this->canAbsorbEvent($coveringActivity, $event)) { + $coveringActivity->events()->save($event); + } + } + + /** + * Trims activities overlapping the new activity's period. An existing activity whose + * trimmed period would collapse is returned for absorption: the new activity takes + * over its events and the empty activity is deleted. + * + * @return Collection + */ + private function trimOverlappingActivities(Activity $activity): Collection { /** @var Collection|Activity[] $overlappingActivities */ $overlappingActivities = Activity::query() @@ -137,7 +168,9 @@ private function handleOverlappingActivities(Activity $activity): ?Activity }) ->get(); - $overlappingActivities->each(function ($overlappingActivity) use ($activity) { + $absorbedActivities = collect(); + + $overlappingActivities->each(function ($overlappingActivity) use ($activity, $absorbedActivities) { /** @var Activity $overlappingActivity */ if (! is_null($overlappingActivity->eventType) && $overlappingActivity->eventType->weight >= (int) $activity->eventType?->weight) { @@ -150,14 +183,15 @@ private function handleOverlappingActivities(Activity $activity): ?Activity } else { $overlappingActivity->started_at = $activity->ended_at; } - $overlappingActivity->save(); + + if ($overlappingActivity->ended_at->isAfter($overlappingActivity->started_at)) { + $overlappingActivity->save(); + } else { + $absorbedActivities->push($overlappingActivity); + } } }); - if ($activity->ended_at->isAfter($activity->started_at)) { - return $activity; - } else { - return null; - } + return $absorbedActivities; } } diff --git a/app/Models/Activity.php b/app/Models/Activity.php index 9c64d85..2b4f717 100644 --- a/app/Models/Activity.php +++ b/app/Models/Activity.php @@ -10,7 +10,6 @@ use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\HasMany; -use Illuminate\Database\Eloquent\Relations\HasOne; use Illuminate\Support\Collection; /** @@ -85,10 +84,10 @@ public function entrySuggestion(): BelongsTo } /** - * @return HasOne + * @return BelongsTo */ - public function eventType(): HasOne + public function eventType(): BelongsTo { - return $this->hasOne(EventType::class); + return $this->belongsTo(EventType::class); } } diff --git a/tests/Integration/Activity/CreateActivityTest.php b/tests/Integration/Activity/CreateActivityTest.php index b563783..aca97cd 100644 --- a/tests/Integration/Activity/CreateActivityTest.php +++ b/tests/Integration/Activity/CreateActivityTest.php @@ -270,3 +270,115 @@ expect(Activity::query()->count())->toEqual(2); }); + +test('a fully covered lower-weight activity is absorbed instead of getting a negative duration', function () { + config()->set('timatic.feature.activity_overlap_detection', true); + Illuminate\Support\Facades\Event::fake(); + + $eventTypeLight = EventType::factory()->state(['weight' => 1])->create(); + $eventTypeHeavy = EventType::factory()->state(['weight' => 999])->create(); + $user = User::factory()->create(); + + /** @var Event $coveredEvent */ + $coveredEvent = Event::factory()->create([ + 'user_id' => $user->id, + 'event_type_id' => $eventTypeLight->id, + 'started_at' => Carbon::now()->subWeek()->setTime(hour: 10, minute: 5, second: 0), + 'ended_at' => Carbon::now()->subWeek()->setTime(hour: 10, minute: 10, second: 0), + ]); + + /** @var CreateActivity $listener */ + $listener = app(CreateActivity::class); + $listener->handle(new EventCreated($coveredEvent)); + + /** @var Event $coveringEvent */ + $coveringEvent = Event::factory()->create([ + 'user_id' => $user->id, + 'event_type_id' => $eventTypeHeavy->id, + 'started_at' => Carbon::now()->subWeek()->setTime(hour: 10, minute: 0, second: 0), + 'ended_at' => Carbon::now()->subWeek()->setTime(hour: 10, minute: 15, second: 0), + ]); + $listener->handle(new EventCreated($coveringEvent)); + + expect(Activity::count())->toBe(1) + ->and(Activity::whereColumn('started_at', '>=', 'ended_at')->count())->toBe(0) + ->and($coveredEvent->fresh()->activity_id)->toBe($coveringEvent->fresh()->activity_id); +}); + +test('an event fully covered by a matching activity attaches to that activity', function () { + config()->set('timatic.feature.activity_overlap_detection', true); + Illuminate\Support\Facades\Event::fake(); + + $sameState = [ + 'event_type_id' => EventType::factory()->state(['weight' => 1])->create()->id, + 'customer_id' => 'customerX', + 'ticket_number' => 'TIC-1', + 'user_id' => User::factory()->create()->id, + ]; + + /** @var Event $coveringEvent */ + $coveringEvent = Event::factory()->create(array_merge($sameState, [ + 'started_at' => Carbon::now()->subWeek()->setTime(hour: 10, minute: 0, second: 0), + 'ended_at' => Carbon::now()->subWeek()->setTime(hour: 10, minute: 30, second: 0), + ])); + + /** @var CreateActivity $listener */ + $listener = app(CreateActivity::class); + $listener->handle(new EventCreated($coveringEvent)); + + /** @var Event $coveredEvent */ + $coveredEvent = Event::factory()->create(array_merge($sameState, [ + 'started_at' => Carbon::now()->subWeek()->setTime(hour: 10, minute: 5, second: 0), + 'ended_at' => Carbon::now()->subWeek()->setTime(hour: 10, minute: 10, second: 0), + ])); + $listener->handle(new EventCreated($coveredEvent)); + + expect(Activity::count())->toBe(1) + ->and($coveredEvent->fresh()->activity_id)->toBe($coveringEvent->fresh()->activity_id); +}); + +test('a covered event of another customer stays unattached instead of mixing customers', function () { + config()->set('timatic.feature.activity_overlap_detection', true); + Illuminate\Support\Facades\Event::fake(); + + $eventTypeId = EventType::factory()->state(['weight' => 1])->create()->id; + $user = User::factory()->create(); + + /** @var Event $coveringEvent */ + $coveringEvent = Event::factory()->create([ + 'event_type_id' => $eventTypeId, + 'customer_id' => 'customerX', + 'ticket_number' => 'TIC-1', + 'user_id' => $user->id, + 'started_at' => Carbon::now()->subWeek()->setTime(hour: 10, minute: 0, second: 0), + 'ended_at' => Carbon::now()->subWeek()->setTime(hour: 10, minute: 30, second: 0), + ]); + + /** @var CreateActivity $listener */ + $listener = app(CreateActivity::class); + $listener->handle(new EventCreated($coveringEvent)); + + /** @var Event $coveredEvent */ + $coveredEvent = Event::factory()->create([ + 'event_type_id' => $eventTypeId, + 'customer_id' => 'customerY', + 'ticket_number' => 'TIC-2', + 'user_id' => $user->id, + 'started_at' => Carbon::now()->subWeek()->setTime(hour: 10, minute: 5, second: 0), + 'ended_at' => Carbon::now()->subWeek()->setTime(hour: 10, minute: 10, second: 0), + ]); + $listener->handle(new EventCreated($coveredEvent)); + + expect(Activity::count())->toBe(1) + ->and($coveredEvent->fresh()->activity_id)->toBeNull(); +}); + +it('loads the event type of an activity', function () { + Illuminate\Support\Facades\Event::fake(); + $eventType = EventType::firstOrCreate(['id' => 'ticket_saved'], ['weight' => 1]); + + $activity = Activity::factory()->create(['event_type_id' => $eventType->id]); + + expect($activity->eventType)->toBeInstanceOf(EventType::class) + ->and($activity->eventType->id)->toBe('ticket_saved'); +});