Merge pull request #256 from ECFMP/expired-flow-measures

feat: expired flow measures

app/Discord/FlowMeasure/Webhook/Filter/ExpiredWebhookFilter.php

@@ -3,17 +3,61 @@
 namespace App\Discord\FlowMeasure\Webhook\Filter;
 
 use App\Discord\Webhook\WebhookInterface;
+use App\Helpers\FlowMeasureIdentifierGenerator;
+use App\Models\DiscordNotification;
 use App\Models\FlowMeasure;
+use App\Repository\FlowMeasureRepository;
+use Carbon\Carbon;
 
 class ExpiredWebhookFilter implements FilterInterface
 {
     use ChecksForDiscordNotificationsToWebhook;
 
+    private readonly FlowMeasureRepository $flowMeasureRepository;
+
+    public function __construct(FlowMeasureRepository $flowMeasureRepository)
+    {
+        $this->flowMeasureRepository = $flowMeasureRepository;
+    }
+
     public function shouldUseWebhook(FlowMeasure $flowMeasure, WebhookInterface $webhook): bool
     {
+        // A division webhook
+        if (!is_null($webhook->id())) {
+            return false;
+        }
+
+        // Doesnt meet conditions
+        if (
+            $this->notManyWebhooksRecentlySent() &&
+            $this->notRevisedMoreThanOnce($flowMeasure) &&
+            $this->lessThanThreeActiveMeasures($flowMeasure)
+        ) {
+            return false;
+        }
+
         return $this->existingNotificationDoesntExist(
             $flowMeasure->withdrawnAndExpiredDiscordNotifications(),
             $webhook
         );
     }
+
+    private function notManyWebhooksRecentlySent(): bool
+    {
+        return DiscordNotification::where('created_at', '>=', Carbon::now()->subHours(2))
+                ->whereNull('division_discord_webhook_id')
+                ->count() <= 5;
+    }
+
+    private function notRevisedMoreThanOnce(FlowMeasure $flowMeasure): bool
+    {
+        return FlowMeasureIdentifierGenerator::timesRevised($flowMeasure) < 2;
+    }
+
+    private function lessThanThreeActiveMeasures(FlowMeasure $measure): bool
+    {
+        return $this->flowMeasureRepository->getFlowMeasuresActiveDuringPeriod($measure->start_time, $measure->end_time)
+                ->reject(fn (FlowMeasure $activeMeasure) => $activeMeasure->id === $measure->id)
+                ->count() < 3;
+    }
 }

app/Helpers/FlowMeasureIdentifierGenerator.php

@@ -26,7 +26,7 @@ public static function generateRevisedIdentifier(FlowMeasure $measure): string
     }
 
     public static function generateIdentifier(
-        Carbon $startTime,
+        Carbon                  $startTime,
         FlightInformationRegion $flightInformationRegion
     ): string {
         return sprintf(
@@ -37,6 +37,14 @@ public static function generateIdentifier(
         );
     }
 
+    public static function timesRevised(FlowMeasure $flowMeasure)
+    {
+        $identifierParts = [];
+        preg_match(self::IDENTIFIER_REGEX, $flowMeasure->identifier, $identifierParts);
+
+        return isset($identifierParts[5]) ? ((int)$identifierParts[5]) - 1 : 0;
+    }
+
     private static function designator(Carbon $startTime, FlightInformationRegion $flightInformationRegion): string
     {
         $flowMeasuresToday = FlowMeasure::where('start_time', '>=', $startTime->copy()->startOfDay())

app/Models/FlowMeasure.php

@@ -66,6 +66,24 @@ public function event(): BelongsTo
         return $this->belongsTo(Event::class);
     }
 
+    public function scopeStartsBetween(Builder $query, Carbon $periodStart, Carbon $periodEnd): Builder
+    {
+        return $query->where('start_time', '>=', $periodStart)
+            ->where('start_time', '<=', $periodEnd);
+    }
+
+    public function scopeEndsBetween(Builder $query, Carbon $periodStart, Carbon $periodEnd): Builder
+    {
+        return $query->where('end_time', '>=', $periodStart)
+            ->where('end_time', '<=', $periodEnd);
+    }
+
+    public function scopeActiveThroughout(Builder $query, Carbon $periodStart, Carbon $periodEnd): Builder
+    {
+        return $query->where('start_time', '<=', $periodStart)
+            ->where('end_time', '>=', $periodEnd);
+    }
+
     public function scopeActive(Builder $query): Builder
     {
         $now = Carbon::now();
@@ -81,7 +99,7 @@ public function scopeExpired(Builder $query): Builder
     public function scopeExpiredRecently(Builder $query): Builder
     {
         return $query->where('end_time', '<', Carbon::now())
-            ->where('end_time', '>', Carbon::now()->subHours(2));
+            ->where('end_time', '>', Carbon::now()->subMinutes(15));
     }
 
     public function isActive(): bool

app/Repository/FlowMeasureRepository.php

@@ -3,11 +3,22 @@
 namespace App\Repository;
 
 use App\Models\FlowMeasure;
+use Carbon\Carbon;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Support\Collection;
 
 class FlowMeasureRepository
 {
+    public function getFlowMeasuresActiveDuringPeriod(Carbon $start, Carbon $end): Collection
+    {
+        return $this->baseQuery(false)
+            ->startsBetween($start, $end)
+            ->union($this->baseQuery(false)->endsBetween($start, $end))
+            ->union($this->baseQuery(false)->activeThroughout($start, $end))
+            ->orderBy('id')
+            ->get();
+    }
+
     public function getApiRelevantFlowMeasures(bool $includeDeleted): Collection
     {
         return $this->baseQuery($includeDeleted)

database/factories/FlowMeasureFactory.php

@@ -74,7 +74,7 @@ public function finishedRecently(): static
     {
         return $this->state(fn (array $attributes) => [
             'start_time' => $this->faker->dateTimeBetween('-3 hour', '-2 hour'),
-            'end_time' => $this->faker->dateTimeBetween('-59 minute', 'now'),
+            'end_time' => $this->faker->dateTimeBetween('-14 minute', 'now'),
         ]);
     }
 

database/migrations/2022_08_18_162121_add_index_to_discord_notifications_table.php

@@ -0,0 +1,28 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+
+return new class () extends Migration {
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        Schema::table('discord_notifications', function (Blueprint $table) {
+            $table->index('created_at');
+        });
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+    }
+};

tests/Discord/FlowMeasure/Webhook/Filter/ExpiredWebhookFilterTest.php

@@ -9,6 +9,7 @@
 use App\Models\DiscordNotificationType;
 use App\Models\DivisionDiscordWebhook;
 use App\Models\FlowMeasure;
+use Carbon\Carbon;
 use Tests\TestCase;
 
 class ExpiredWebhookFilterTest extends TestCase
@@ -25,9 +26,11 @@ public function setUp(): void
         $this->divisionDiscordWebhook = DivisionDiscordWebhook::factory()->create();
     }
 
-    public function testItShouldUseWebhookIfFlowMeasureHasNeverBeenExpiredOrWithdrawnToEcfmpWebhook()
+    public function testItShouldUseEcfmpWebhookIfLotsOfNotificationsHaveBeenSentRecently()
     {
         $measure = FlowMeasure::factory()->create();
+        DiscordNotification::factory()->count(6)->create(['division_discord_webhook_id' => null]);
+
         $this->assertTrue(
             $this->filter->shouldUseWebhook(
                 $measure,
@@ -36,9 +39,38 @@ public function testItShouldUseWebhookIfFlowMeasureHasNeverBeenExpiredOrWithdraw
         );
     }
 
-    public function testItShouldUseWebhookIfFlowMeasureHasOnlyBeenNotifiedToEcfmpWebhook()
+    public function testItShouldUseEcfmpWebhookIfTheFlowMeasureHasBeenModifiedTwice()
     {
+        $measure = FlowMeasure::factory()->create(['identifier' => 'EGTT23A-3']);
+
+        $this->assertTrue(
+            $this->filter->shouldUseWebhook(
+                $measure,
+                $this->ecfmpWebhook
+            )
+        );
+    }
+
+    public function testItShouldUseEcfmpWebhookIfThereAreOtherMeasuresActiveAroundTheTime()
+    {
+        FlowMeasure::factory()
+            ->withTimes(Carbon::now()->subMinutes(30), Carbon::now()->addMinutes(45))
+            ->count(3)
+            ->create();
+
         $measure = FlowMeasure::factory()->create();
+
+        $this->assertTrue(
+            $this->filter->shouldUseWebhook(
+                $measure,
+                $this->ecfmpWebhook
+            )
+        );
+    }
+
+    public function testItShouldUseWebhookIfFlowMeasureHasOnlyBeenNotifiedToEcfmpWebhook()
+    {
+        $measure = FlowMeasure::factory()->create(['identifier' => 'EGTT23A-3']);
         $discordNotification = DiscordNotification::factory()->create();
         $measure->discordNotifications()->attach(
             [
@@ -61,7 +93,7 @@ public function testItShouldUseWebhookIfFlowMeasureHasOnlyBeenNotifiedToEcfmpWeb
 
     public function testItShouldUseWebhookIfFlowMeasureHasOnlyBeenActivatedToEcfmpWebhook()
     {
-        $measure = FlowMeasure::factory()->create();
+        $measure = FlowMeasure::factory()->create(['identifier' => 'EGTT23A-3']);
         $discordNotification = DiscordNotification::factory()->create();
         $measure->discordNotifications()->attach(
             [
@@ -84,7 +116,7 @@ public function testItShouldUseWebhookIfFlowMeasureHasOnlyBeenActivatedToEcfmpWe
 
     public function testItShouldNotUseWebhookIfFlowMeasureHasBeenWithdrawnToEcfmpWebhook()
     {
-        $measure = FlowMeasure::factory()->create();
+        $measure = FlowMeasure::factory()->create(['identifier' => 'EGTT23A-3']);
         $discordNotification = DiscordNotification::factory()->create();
         $measure->discordNotifications()->attach(
             [
@@ -107,7 +139,7 @@ public function testItShouldNotUseWebhookIfFlowMeasureHasBeenWithdrawnToEcfmpWeb
 
     public function testItShouldNotUseWebhookIfFlowMeasureHasBeenExpiredToEcfmpWebhook()
     {
-        $measure = FlowMeasure::factory()->create();
+        $measure = FlowMeasure::factory()->create(['identifier' => 'EGTT23A-3']);
         $discordNotification = DiscordNotification::factory()->create();
         $measure->discordNotifications()->attach(
             [
@@ -128,18 +160,32 @@ public function testItShouldNotUseWebhookIfFlowMeasureHasBeenExpiredToEcfmpWebho
         );
     }
 
-    public function testItShouldUseWebhookIfFlowMeasureHasNeverBeenExpiredToDivisionWebhook()
+    public function testItShouldNotUseWebhookIfDoesntMeetConditionsForEcfmpWebhook()
     {
-        $measure = FlowMeasure::factory()->create();
-        $this->assertTrue(
+        // Once revised
+        $measure = FlowMeasure::factory()->create(['identifier' => 'EGTT23A-2']);
+
+        // Two other measures
+        FlowMeasure::factory()
+            ->withTimes($measure->start_time->clone()->subMinutes(2), $measure->start_time->clone()->addMinutes(2))
+            ->count(2)
+            ->create();
+
+        // Not too many recently sent
+        DiscordNotification::factory()->count(5)->create(['division_discord_webhook_id' => null]);
+        DiscordNotification::factory()->count(5)
+            ->toDivisionWebhook(DivisionDiscordWebhook::factory()->create())
+            ->create();
+
+        $this->assertFalse(
             $this->filter->shouldUseWebhook(
                 $measure,
-                $this->divisionDiscordWebhook
+                $this->ecfmpWebhook
             )
         );
     }
 
-    public function testItShouldUseWebhookIfFlowMeasureHasOnlyBeenNotifiedToDivisionWebhook()
+    public function testItShouldNotUseWebhookIfFlowMeasureHasOnlyBeenNotifiedToDivisionWebhook()
     {
         $measure = FlowMeasure::factory()->create();
         $discordNotification = DiscordNotification::factory()
@@ -156,15 +202,15 @@ public function testItShouldUseWebhookIfFlowMeasureHasOnlyBeenNotifiedToDivision
             ]
         );
 
-        $this->assertTrue(
+        $this->assertFalse(
             $this->filter->shouldUseWebhook(
                 $measure,
                 $this->divisionDiscordWebhook
             )
         );
     }
 
-    public function testItShouldUseWebhookIfFlowMeasureHasOnlyBeenActivatedToDivisionWebhook()
+    public function testItShouldNotUseWebhookIfFlowMeasureHasOnlyBeenActivatedToDivisionWebhook()
     {
         $measure = FlowMeasure::factory()->create();
         $discordNotification = DiscordNotification::factory()
@@ -181,7 +227,7 @@ public function testItShouldUseWebhookIfFlowMeasureHasOnlyBeenActivatedToDivisio
             ]
         );
 
-        $this->assertTrue(
+        $this->assertFalse(
             $this->filter->shouldUseWebhook(
                 $measure,
                 $this->divisionDiscordWebhook

tests/Helpers/FlowMeasureIdentifierGeneratorTest.php

@@ -28,11 +28,11 @@ public function setUp(): void
      * @dataProvider dataProvider
      */
     public function testItGeneratesFlowMeasures(
-        int $numberExisting,
+        int    $numberExisting,
         string $date,
         string $expectedDayNumber,
         string $expectedDesignator,
-        bool $isDeleted = false
+        bool   $isDeleted = false
     ) {
         $fir = FlightInformationRegion::factory()->create(['identifier' => 'EGTT']);
         $otherFir = FlightInformationRegion::factory()->create(['identifier' => 'EGPX']);
@@ -84,7 +84,7 @@ public function testItGeneratesFlowMeasures(
         );
     }
 
-    public function dataProvider()
+    public function dataProvider(): array
     {
         return [
             'First of the day' => [0, '2022-04-30 12:00:00', '30', 'A'],
@@ -120,7 +120,7 @@ public function testItGeneratesRevisedFlowMeasures(
         $this->assertEquals($expected, FlowMeasureIdentifierGenerator::generateRevisedIdentifier($measure));
     }
 
-    public function revisionProvider()
+    public function revisionProvider(): array
     {
         return [
             'First revision' => ['EGTT01A', 'EGTT01A-2'],
@@ -131,4 +131,29 @@ public function revisionProvider()
             'Someone really messed up revision' => ['EGTT25A-9999', 'EGTT25A-10000'],
         ];
     }
+
+    /**
+     * @dataProvider revisionIdentifierProvider
+     */
+    public function testItCountsRevisions(
+        string $identifier,
+        int    $expected
+    ) {
+        $this->assertEquals(
+            $expected,
+            FlowMeasureIdentifierGenerator::timesRevised(
+                FlowMeasure::factory()->make(['identifier' => $identifier])
+            )
+        );
+    }
+
+    public function revisionIdentifierProvider(): array
+    {
+        return [
+            'First revision' => ['EGTT01A', 0],
+            'Second revision' => ['EGTT01A-2', 1],
+            'Ninth revision' => ['EGTT01A-10', 9],
+            'Tenth revision' => ['EGTT01A-11', 10],
+        ];
+    }
 }