Merge pull request #691 from TheRestartProject/RES-1942_negative_atte…

…ndees

RES-1942 Negative attendee count

app/Console/Commands/FixVolunteerCount.php

@@ -0,0 +1,51 @@
+<?php
+
+namespace App\Console\Commands;
+
+use DB;
+use Illuminate\Console\Command;
+use App\Party;
+
+class FixVolunteerCount extends Command
+{
+    /**
+     * The name and signature of the console command.
+     *
+     * @var string
+     */
+    protected $signature = 'dev:fixvolunteercount';
+
+    /**
+     * The console command description.
+     *
+     * @var string
+     */
+    protected $description = 'Fix the volunteer count for all events';
+
+    /**
+     * Execute the console command.
+     *
+     * @return mixed
+     */
+    public function handle()
+    {
+        $events = Party::all();
+
+        foreach ($events as $event) {
+            $actual = DB::table('events_users')->where('event', $event->idevents)->where('status', 1)->count();
+
+            if ($actual > $event->volunteers) {
+                if ($event->volunteers < 0) {
+                    $this->info("Event {$event->idevents} has negative count {$event->volunteers}, $actual have confirmed");
+                } else {
+                    $this->info("Event {$event->idevents} has count {$event->volunteers}, but more ($actual) have confirmed");
+                }
+
+                $event->volunteers = $actual;
+                $event->save();
+            } else if ($event->volunteers < 0) {
+                $this->info("Event {$event->idevents} has negative count {$event->volunteers}, fewewr ($actual) have confirmed");
+            }
+        }
+    }
+}

app/Http/Controllers/PartyController.php

@@ -714,13 +714,10 @@ public function confirmInvite($event_id, $hash)
 
         if (! empty($user_event)) {
             // Update event invite
-            EventsUsers::where('status', $hash)->where('event', $event_id)->update([
+            EventsUsers::where('status', $hash)->where('event', $event_id)->first()->update([
                 'status' => 1,
             ]);
 
-            // Increment volunteers column to include latest invite
-            $event = Party::find($event_id);
-
             $this->notifyHostsOfRsvp($user_event, $event_id);
 
             return redirect('/party/view/'.$user_event->event);

app/Observers/EventsUsersObserver.php

@@ -39,10 +39,11 @@ public function created(EventsUsers $eu)
 
         if ($eu->status == 1) {
             // Confirmed.  Make sure they are on the thread.
-            $this->confirmed($event, $user);
+            $this->confirmed($event, $user, true);
         } else {
-            // Not confirmed.  Make sure they are not on the thread.
-            $this->removed($event, $user);
+            // Not confirmed.  Make sure they are not on the thread.  Don't change the count, as they shouldn't
+            // be on it anyway.
+            $this->removed($event, $user, false);
         }
     }
 
@@ -52,18 +53,22 @@ public function created(EventsUsers $eu)
      * @param  \App\EventsUsers  $eu
      * @return void
      */
-    public function updated(EventsUsers $eu) {
+    public function updating(EventsUsers $eu) {
         $idevents = $eu->event;
         $event = \App\Party::find($idevents);
         $iduser = $eu->user;
         $user = $iduser ? User::find($iduser) : null;
 
-        if ($eu->status == 1) {
-            // Confirmed.  Make sure they are on the thread.
-            $this->confirmed($event, $user);
-        } else {
-            // Not confirmed.  Make sure they are not on the thread.
-            $this->removed($event, $user);
+        if ($eu->isDirty('status')) {
+            // The confirmed status has changed, so we need to update the thread.
+
+            if ($eu->status == 1) {
+                // Confirmed.  Make sure they are on the thread.
+                $this->confirmed($event, $user, true);
+            } else {
+                // Not confirmed.  Make sure they are not on the thread.
+                $this->removed($event, $user, true);
+            }
         }
     }
 
@@ -80,18 +85,21 @@ public function deleted(EventsUsers $eu)
         $iduser = $eu->user;
         $user = $iduser ? User::find($iduser) : null;
 
-        // Make sure they are not on the thread.
-        $this->removed($event, $user);
+        // Make sure they are not on the thread.  If they were confirmed, we need to update the volunteer count.
+        $this->removed($event, $user, true, $eu->status == 1);
     }
 
     /**
      * @param Party $event
      * @param User $user
      * @return void
      */
-    private function confirmed($event, $user): void
+    private function confirmed($event, $user, $count): void
     {
-        $event->increment('volunteers');
+        if ($count) {
+            $event->increment('volunteers');
+        }
+
         event(new UserConfirmedEvent($event->idevents, $user ? $user->id : null));
     }
 
@@ -100,9 +108,12 @@ private function confirmed($event, $user): void
      * @param User $user
      * @return void
      */
-    private function removed($event, $user): void
+    private function removed($event, $user, $count): void
     {
-        $event->decrement('volunteers');
+        if ($count) {
+            $event->decrement('volunteers');
+        }
+
         event(new UserLeftEvent($event->idevents, $user ? $user->id : null));
     }
 }

resources/js/components/EventAttendanceCount.vue

@@ -42,7 +42,7 @@ export default {
     }
   },
   mounted() {
-    this.current = this.count !== null ? this.count : 0
+    this.current = this.count !== null ? Math.max(this.count, 0) : 0
   },
   methods: {
     inc() {
@@ -57,7 +57,14 @@ export default {
     },
     set() {
       // This is triggered on keyup as the change even doesn't fire while you're still in the field.
-      this.$emit('change', this.current)
+      this.current = parseInt(this.current)
+
+      if (this.current > 0) {
+        this.$emit('change', this.current)
+      } else {
+        // Don't allow them to type a negative value.
+        this.current = 0
+      }
     }
   }
 }

tests/Feature/Events/AddRemoveVolunteerTest.php

@@ -19,6 +19,7 @@
 use Symfony\Component\DomCrawler\Crawler;
 use Tests\TestCase;
 use Illuminate\Support\Facades\Queue;
+use function PHPUnit\Framework\assertEquals;
 
 class AddRemoveVolunteerTest extends TestCase
 {
@@ -150,6 +151,10 @@ public function testAddRemove($role, $addrole, $shouldBeHost)
         $response = $this->get('/party/view/'.$event->idevents);
         $response->assertSee('Invites sent!');
 
+        // Invited volunteers shouldn't affect the count.
+        $event->refresh();
+        assertEquals(0, $event->volunteers);
+
         $response = $this->put('/api/events/' . $event->idevents . '/volunteers', [
             'volunteer_email_address' => $restarter->email,
             'full_name' => $restarter->name,
@@ -165,6 +170,10 @@ public function testAddRemove($role, $addrole, $shouldBeHost)
             'id' => $volunteer->idevents_users,
         ])->assertSee('true');
 
+        // Invited volunteers shouldn't affect the count.
+        $event->refresh();
+        assertEquals(0, $event->volunteers);
+
         // Add by name only
         $response = $this->put('/api/events/' . $event->idevents . '/volunteers', [
             'full_name' => 'Jo Bloggins',
@@ -179,6 +188,10 @@ public function testAddRemove($role, $addrole, $shouldBeHost)
             'id' => $volunteer->idevents_users,
         ])->assertSee('true');
 
+        // Invited volunteers shouldn't affect the count.
+        $event->refresh();
+        assertEquals(0, $event->volunteers);
+
         // Add anonymous.
         $response = $this->put('/api/events/' . $event->idevents . '/volunteers', []);
 

tests/Feature/Events/InviteEventTest.php

@@ -16,6 +16,7 @@
 use Tests\TestCase;
 use App\Notifications\JoinEvent;
 use Illuminate\Support\Facades\Queue;
+use function PHPUnit\Framework\assertEquals;
 
 class InviteEventTest extends TestCase
 {
@@ -70,6 +71,10 @@ function ($notification, $channels, $user) use ($group, $event, $host) {
                 return true;
             }
         );
+
+        // Invited volunteers shouldn't update the count.
+        $event->refresh();
+        assertEquals(0, $event->volunteers);
     }
 
     public function testInviteReal()
@@ -93,6 +98,8 @@ public function testInviteReal()
         $group = Group::findOrFail($idgroups);
         $idevents = $this->createEvent($idgroups, 'tomorrow');
         $event = Party::findOrFail($idevents);
+        $event->refresh();
+        assertEquals(1, $event->volunteers);
 
         // We want the event handler to kick in and synchronise to Discourse.
         $this->artisan("queue:work --stop-when-empty");
@@ -118,6 +125,10 @@ public function testInviteReal()
             'role' => 4,
         ]);
 
+        // Invited volunteers shouldn't update the count.
+        $event->refresh();
+        assertEquals(1, $event->volunteers);
+
         // Admin approves the event.
         $admin = User::factory()->administrator()->create();
         $this->get('/logout');
@@ -159,6 +170,10 @@ public function testInviteReal()
         $events = $this->getVueProperties($response5)[1][':initial-events'];
         $this->assertNotFalse(strpos($events, '"attending":true'));
 
+        // Count should now include them.
+        $event->refresh();
+        assertEquals(2, $event->volunteers);
+
         // Invite again - different code path when they're already there.
         $response = $this->post('/party/invite', [
             'group_name' => $group->name,
@@ -193,6 +208,9 @@ public function testInvitableUserPOV()
                            ]);
         $this->actingAs($host);
 
+        $event->refresh();
+        assertEquals(1, $event->volunteers);
+
         // Should have no group members and therefore no invitable members.
         $response = $this->get('/party/get-group-emails-with-names/'.$event->idevents);
         $members = json_decode($response->getContent());
@@ -227,6 +245,10 @@ public function testInvitableUserPOV()
 
         $response5->assertSessionHas('success');
 
+        // Invited volunteers shouldn't update the count.
+        $event->refresh();
+        assertEquals(1, $event->volunteers);
+
         // Invited member should not show up as invitable.
         $response6 = $this->get('/party/get-group-emails-with-names/'.$event->idevents);
         $members = json_decode($response6->getContent());
@@ -253,6 +275,10 @@ public function testInvitableUserPOV()
         $redirectTo = $response8->getTargetUrl();
         $this->assertNotFalse(strpos($redirectTo, '/party/view/'.$event->idevents));
 
+        // Should now show.
+        $event->refresh();
+        assertEquals(2, $event->volunteers);
+
         // Now a group member and confirmed so should not show as invitable.
         $this->get('/logout');
         $this->actingAs($host);
@@ -385,12 +411,18 @@ public function testInviteViaLink() {
         $group = Group::findOrFail($idgroups);
         $idevents = $this->createEvent($idgroups, 'tomorrow');
         $event = Party::findOrFail($idevents);
+        assertEquals(1, $event->volunteers);
 
         $unique_shareable_code = Fixometer::generateUniqueShareableCode(\App\Party::class, 'shareable_code');
         $event->update([
                            'shareable_code' => $unique_shareable_code,
                        ]);
 
+
+        // Invited volunteers shouldn't update the count.
+        $event->refresh();
+        assertEquals(1, $event->volunteers);
+
         // Accept the invite via the code.
         $this->actingAs($user);
         $this->followingRedirects();
@@ -402,6 +434,11 @@ public function testInviteViaLink() {
             'name' => $event->venue
         ]), false);
 
+
+        // Should now show.
+        $event->refresh();
+        assertEquals(2, $event->volunteers);
+
         // Try with invalid code.
         $this->expectException(NotFoundHttpException::class);
         $rsp = $this->get('/party/invite/' . $unique_shareable_code . '1');

tests/Feature/Events/JoinEventTest.php

@@ -10,6 +10,7 @@
 use DB;
 use Illuminate\Support\Facades\Notification;
 use Tests\TestCase;
+use function PHPUnit\Framework\assertEquals;
 
 class JoinEventTest extends TestCase
 {
@@ -30,6 +31,7 @@ public function testJoin()
         // Joining should trigger adding to the Discourse thread.  Fake one.
         $event = \App\Party::find($idevents);
         $event->discourse_thread = 123;
+        assertEquals(1, $event->volunteers);
         $event->save();
 
         Queue::assertPushed(\Illuminate\Events\CallQueuedListener::class, function ($job) use ($event, $user) {
@@ -56,6 +58,10 @@ public function testJoin()
             ],
         ]);
 
+        // Should show in count
+        $event->refresh();
+        assertEquals(2, $event->volunteers);
+
         // Say we can't attend.
         $this->followingRedirects();
         $response = $this->get('/party/cancel-invite/'.$event->idevents);
@@ -66,6 +72,9 @@ public function testJoin()
             ],
         ]);
 
+        $event->refresh();
+        assertEquals(1, $event->volunteers);
+
         Queue::assertPushed(\Illuminate\Events\CallQueuedListener::class, function ($job) use ($event, $user) {
             if ($job->class == RemoveUserFromDiscourseThreadForEvent::class) {
                 return true;