Fix pull memberships action for local memberships

app/Actions/PullMemberAction.php

@@ -28,7 +28,7 @@ class PullMemberAction
         }
 
         $region = Region::firstWhere('nami_id', $member->regionId ?: -1);
-        $m = Member::updateOrCreate(['nami_id' => $member->id], [
+        Member::updateOrCreate(['nami_id' => $member->id], [
             'firstname' => $member->firstname,
             'lastname' => $member->lastname,
             'joined_at' => $member->joinedAt,

app/Actions/PullMembershipsAction.php

@@ -28,7 +28,7 @@ class PullMembershipsAction
         foreach ($memberships as $membership) {
             $existingMembership = Membership::where('nami_id', $membership->id)->first();
 
-            $group = Group::where('name', $membership->group)->first();
+            $group = Group::where('name', $membership->group)->whereNotNull('nami_id')->first();
             if (!$group) {
                 continue;
             }
@@ -39,6 +39,9 @@ class PullMembershipsAction
 
             $this->singleStrategy($member, $group, $membership);
         }
+
+        $membershipIds = $memberships->map(fn ($membership) => $membership->id)->toArray();
+        $member->memberships()->whereNotIn('nami_id', $membershipIds)->whereNotNull('nami_id')->delete();
     }
 
     private function overviewStrategy(Member $member, Group $group, NamiMembershipEntry $membership): ?Membership
@@ -64,6 +67,7 @@ class PullMembershipsAction
         return $member->memberships()->updateOrCreate(['nami_id' => $membership->id], [
             'nami_id' => $membership->id,
             'from' => $membership->startsAt,
+            'to' => $membership->endsAt,
             'group_id' => $group->id,
             'activity_id' => $activity->id,
             'subactivity_id' => $subactivity?->id,
@@ -93,6 +97,7 @@ class PullMembershipsAction
         return $member->memberships()->updateOrCreate(['nami_id' => $membership->id], [
             'nami_id' => $membership->id,
             'from' => $membership->startsAt,
+            'to' => $membership->endsAt,
             'group_id' => $group->id,
             'activity_id' => $activity->id,
             'subactivity_id' => $subactivity?->id,

app/Member/Membership.php

@@ -16,12 +16,14 @@ class Membership extends Model
 {
     use HasFactory;
 
-    public $fillable = ['subactivity_id', 'activity_id', 'group_id', 'member_id', 'nami_id', 'from', 'promised_at'];
+    public $fillable = ['subactivity_id', 'activity_id', 'group_id', 'member_id', 'nami_id', 'from', 'to', 'promised_at'];
 
     /**
      * @var array<string, string>
      */
     public $casts = [
+        'from' => 'date',
+        'to' => 'date',
         'promised_at' => 'date',
     ];
 

database/factories/ActivityFactory.php

@@ -46,7 +46,6 @@ class ActivityFactory extends Factory
     {
         return [
             'name' => $this->faker->words(5, true),
-            'nami_id' => $this->faker->randomNumber(),
         ];
     }
 

database/factories/Member/MembershipFactory.php

@@ -35,6 +35,22 @@ class MembershipFactory extends Factory
         return $this->state(['nami_id' => $namiId]);
     }
 
+    public function local(): self
+    {
+        return $this->state(['nami_id' => null]);
+    }
+
+    public function inLocal(string $activity, ?string $subactivity = null): self
+    {
+        $instance = $this->for(Activity::factory()->name($activity));
+
+        if ($subactivity) {
+            $instance = $instance->for(Subactivity::factory()->name($subactivity));
+        }
+
+        return $instance;
+    }
+
     public function in(string $activity, int $activityNamiId, ?string $subactivity = null, ?int $subactivityNamiId = null): self
     {
         $instance = $this->for(Activity::factory()->name($activity)->inNami($activityNamiId));

database/factories/SubactivityFactory.php

@@ -40,7 +40,6 @@ class SubactivityFactory extends Factory
     {
         return [
             'name' => $this->faker->words(5, true),
-            'nami_id' => $this->faker->randomNumber(),
         ];
     }
 

database/migrations/2023_02_05_233824_create_memberships_to_column.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('memberships', function (Blueprint $table) {
+            $table->datetime('to')->nullable();
+        });
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+    }
+};

database/migrations/2023_02_05_235353_edit_activities_nami_id_column.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('activities', function (Blueprint $table) {
+            $table->unsignedInteger('nami_id')->nullable(true)->change();
+        });
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+    }
+};

tests/Feature/Initialize/PullMembershipsActionTest.php

@@ -9,6 +9,7 @@ use App\Fee;
 use App\Gender;
 use App\Group;
 use App\Member\Member;
+use App\Member\Membership;
 use App\Nationality;
 use App\Payment\Subscription;
 use App\Subactivity;
@@ -139,4 +140,63 @@ class PullMembershipsActionTest extends TestCase
             'subactivity_id' => Subactivity::nami(55)->id,
         ]);
     }
+
+    public function testUpdateExistingMembership(): void
+    {
+        $member = Member::factory()
+            ->defaults()
+            ->for(Group::factory()->name('Gruppe')->inNami(90))
+            ->inNami(1001)
+            ->has(Membership::factory()->in('Leiter', 50, 'Rover', 60)->inNami(5060))
+            ->create();
+        app(MembershipFake::class)->fetches(1001, [
+            [
+                'id' => 5060,
+                'entries_aktivBis' => '2021-08-23 00:00:00',
+                'entries_aktivVon' => '2021-08-22 00:00:00',
+                'entries_taetigkeit' => 'Leiter (50)',
+                'entries_gruppierung' => 'Gruppe',
+                'entries_untergliederung' => 'Rover',
+            ],
+        ]);
+
+        app(PullMembershipsAction::class)->handle($member);
+
+        $this->assertDatabaseCount('memberships', 1);
+        $this->assertDatabaseHas('memberships', [
+            'nami_id' => 5060,
+            'from' => '2021-08-22 00:00:00',
+            'to' => '2021-08-23 00:00:00',
+        ]);
+    }
+
+    public function testDeleteExistingMembership(): void
+    {
+        $member = Member::factory()
+            ->defaults()
+            ->for(Group::factory()->name('Gruppe')->inNami(90))
+            ->inNami(1001)
+            ->has(Membership::factory()->in('Leiter', 50, 'Rover', 60)->inNami(5060))
+            ->create();
+        app(MembershipFake::class)->fetches(1001, []);
+
+        app(PullMembershipsAction::class)->handle($member);
+
+        $this->assertDatabaseCount('memberships', 0);
+    }
+
+    public function testDontDeleteLocalMemberships(): void
+    {
+        $member = Member::factory()
+            ->defaults()
+            ->for(Group::factory()->name('Gruppe')->inNami(90))
+            ->inNami(1001)
+            ->has(Membership::factory()->inLocal('Leiter', 'Rover')->local())
+            ->create();
+        app(MembershipFake::class)->fetches(1001, []);
+
+        app(PullMembershipsAction::class)->handle($member);
+
+        $this->assertDatabaseCount('memberships', 1);
+    }
 }