Shift deletion: Simplify notification workflow

config/app.php

@@ -80,9 +80,9 @@ return [
 
         'oauth2.login' => \Engelsystem\Events\Listener\OAuth2::class . '@login',
 
-        'shift.entry.deleting' => [
+        'shift.deleting' => [
-            \Engelsystem\Events\Listener\Shift::class . '@deletedEntryCreateWorklog',
+            \Engelsystem\Events\Listener\Shift::class . '@shiftDeletingCreateWorklogs',
-            \Engelsystem\Events\Listener\Shift::class . '@deletedEntrySendEmail',
+            \Engelsystem\Events\Listener\Shift::class . '@shiftDeletingSendEmails',
         ],
 
         'shift.updating' => \Engelsystem\Events\Listener\Shift::class . '@updatedShiftSendEmail',

includes/controller/shifts_controller.php

@@ -243,18 +243,7 @@ function shift_delete_controller(): void
     $shift_id = $request->input('delete_shift');
     $shift = Shift::findOrFail($shift_id);
 
-    foreach ($shift->shiftEntries as $entry) {
+    event('shift.deleting', ['shift' => $shift]);
-        event('shift.entry.deleting', [
-            'user'       => $entry->user,
-            'start'      => $shift->start,
-            'end'        => $shift->end,
-            'name'       => $shift->shiftType->name,
-            'title'      => $shift->title,
-            'type'       => $entry->angelType->name,
-            'location'   => $shift->location,
-            'freeloaded' => $entry->freeloaded,
-        ]);
-    }
 
     $shift->delete();
 

includes/helper/shift_helper.php

@@ -5,10 +5,8 @@ namespace Engelsystem\Events\Listener;
 use Carbon\Carbon;
 use Engelsystem\Helpers\Shifts;
 use Engelsystem\Mail\EngelsystemMailer;
-use Engelsystem\Models\Location;
 use Engelsystem\Models\Shifts\Shift as ShiftModel;
 use Engelsystem\Models\Shifts\ShiftEntry;
-use Engelsystem\Models\User\User;
 use Engelsystem\Models\Worklog;
 use Illuminate\Database\Eloquent\Collection;
 use Psr\Log\LoggerInterface;
@@ -21,72 +19,56 @@ class Shift
     ) {
     }
 
-    public function deletedEntryCreateWorklog(
+    public function shiftDeletingCreateWorklogs(ShiftModel $shift): void
-        User $user,
+    {
-        Carbon $start,
+        foreach ($shift->shiftEntries as $entry) {
-        Carbon $end,
+            if ($entry->freeloaded || $shift->start > Carbon::now()) {
-        string $name,
+                continue;
-        string $title,
+            }
-        string $type,
+
-        Location $location,
+            $workLog = new Worklog();
-        bool $freeloaded
+            $workLog->user()->associate($entry->user);
-    ): void {
+            $workLog->creator()->associate(auth()->user());
-        if ($freeloaded || $start > Carbon::now()) {
+            $workLog->worked_at = $shift->start->copy()->startOfDay();
-            return;
+            $workLog->hours =
+                (($shift->end->timestamp - $shift->start->timestamp) / 60 / 60)
+                * Shifts::getNightShiftMultiplier($shift->start, $shift->end);
+            $workLog->comment = sprintf(
+                __('%s (%s as %s) in %s, %s - %s'),
+                $shift->shiftType->name,
+                $shift->title,
+                $entry->angelType->name,
+                $shift->location->name,
+                $shift->start->format(__('general.datetime')),
+                $shift->end->format(__('general.datetime'))
+            );
+            $workLog->save();
+
+            $this->log->info(
+                'Created worklog entry from shift for {user} ({uid}): {worklog})',
+                ['user' => $workLog->user->name, 'uid' => $workLog->user->id, 'worklog' => $workLog->comment]
+            );
         }
-
-        $workLog = new Worklog();
-        $workLog->user()->associate($user);
-        $workLog->creator()->associate(auth()->user());
-        $workLog->worked_at = $start->copy()->startOfDay();
-        $workLog->hours =
-            (($end->timestamp - $start->timestamp) / 60 / 60)
-            * Shifts::getNightShiftMultiplier($start, $end);
-        $workLog->comment = sprintf(
-            __('%s (%s as %s) in %s, %s - %s'),
-            $name,
-            $title,
-            $type,
-            $location->name,
-            $start->format(__('general.datetime')),
-            $end->format(__('general.datetime'))
-        );
-        $workLog->save();
-
-        $this->log->info(
-            'Created worklog entry from shift for {user} ({uid}): {worklog})',
-            ['user' => $workLog->user->name, 'uid' => $workLog->user->id, 'worklog' => $workLog->comment]
-        );
     }
 
-    public function deletedEntrySendEmail(
+    public function shiftDeletingSendEmails(ShiftModel $shift): void
-        User $user,
+    {
-        Carbon $start,
+        foreach ($shift->shiftEntries as $entry) {
-        Carbon $end,
+            if (!$entry->user->settings->email_shiftinfo) {
-        string $name,
+                continue;
-        string $title,
+            }
-        string $type,
-        Location $location,
-        bool $freeloaded
-    ): void {
-        if (!$user->settings->email_shiftinfo) {
-            return;
-        }
 
-        $this->mailer->sendViewTranslated(
+            $this->mailer->sendViewTranslated(
-            $user,
+                $entry->user,
-            'notification.shift.deleted',
+                'notification.shift.deleted',
-            'emails/worklog-from-shift',
+                'emails/worklog-from-shift',
-            [
+                [
-                'name'       => $name,
+                    'shift' => $shift,
-                'title'      => $title,
+                    'entry' => $entry,
-                'start'      => $start,
+                    'username' => $entry->user->displayName,
-                'end'        => $end,
+                ]
-                'location'   => $location,
+            );
-                'freeloaded' => $freeloaded,
+        }
-                'username'   => $user->displayName,
-            ]
-        );
     }
 
     public function updatedShiftSendEmail(

resources/views/emails/worklog-from-shift.twig

@@ -5,12 +5,12 @@
 {% endblock %}
 
 {% block message %}
-{{ name }}
+{{ shift.shiftType.name }}
-{{ title }}
+{{ shift.title }}
-{{ start.format(__('general.datetime')) }} - {{ end.format(__('general.datetime')) }}
+{{ shift.start.format(__('general.datetime')) }} - {{ shift.end.format(__('general.datetime')) }}
-{{ location.name }}
+{{ shift.location.name }}
 
-{% if start <= date() and not freeloaded %}
+{% if shift.start <= date() and not entry.freeloaded %}
 {{ __('notification.shift.deleted.worklog') }}
 {% endif %}
 {% endblock %}

src/Controllers/Admin/LocationsController.php

@@ -140,18 +140,7 @@ class LocationsController extends BaseController
 
         $shifts = $location->shifts;
         foreach ($shifts as $shift) {
-            foreach ($shift->shiftEntries as $entry) {
+            event('shift.deleting', ['shift' => $shift]);
-                event('shift.entry.deleting', [
-                    'user'       => $entry->user,
-                    'start'      => $shift->start,
-                    'end'        => $shift->end,
-                    'name'       => $shift->shiftType->name,
-                    'title'      => $shift->title,
-                    'type'       => $entry->angelType->name,
-                    'location'   => $location,
-                    'freeloaded' => $entry->freeloaded,
-                ]);
-            }
         }
         $location->delete();
 

src/Controllers/Admin/ScheduleController.php

@@ -22,12 +22,12 @@ use Engelsystem\Models\Shifts\Schedule as ScheduleModel;
 use Engelsystem\Models\Shifts\ScheduleShift;
 use Engelsystem\Models\Shifts\Shift;
 use Engelsystem\Models\Shifts\ShiftType;
-use Engelsystem\Models\User\User;
 use ErrorException;
 use GuzzleHttp\Client as GuzzleClient;
 use GuzzleHttp\Exception\ConnectException;
 use GuzzleHttp\Exception\GuzzleException;
 use Illuminate\Database\Connection as DatabaseConnection;
+use Illuminate\Database\Eloquent\Collection as DatabaseCollection;
 use Illuminate\Support\Collection;
 use Psr\Log\LoggerInterface;
 
@@ -289,34 +289,15 @@ class ScheduleController extends BaseController
         $this->log->info('Created schedule location "{location}"', ['location' => $room->getName()]);
     }
 
-    protected function fireDeleteShiftEntryEvents(Event $event, ScheduleModel $schedule): void
+    protected function fireDeleteShiftEvents(Event $event, ScheduleModel $schedule): void
     {
-        $shiftEntries = $this->db
+        /** @var DatabaseCollection|ScheduleShift[] $scheduleShifts */
-            ->table('shift_entries')
+        $scheduleShifts = ScheduleShift::where('guid', $event->getGuid())
-            ->select([
+            ->where('schedule_id', $schedule->id)
-                'shift_types.name', 'shifts.title', 'angel_types.name AS type', 'locations.id AS location_id',
-                'shifts.start', 'shifts.end', 'shift_entries.user_id', 'shift_entries.freeloaded',
-            ])
-            ->join('shifts', 'shifts.id', 'shift_entries.shift_id')
-            ->join('schedule_shift', 'shifts.id', 'schedule_shift.shift_id')
-            ->join('locations', 'locations.id', 'shifts.location_id')
-            ->join('angel_types', 'angel_types.id', 'shift_entries.angel_type_id')
-            ->join('shift_types', 'shift_types.id', 'shifts.shift_type_id')
-            ->where('schedule_shift.guid', $event->getGuid())
-            ->where('schedule_shift.schedule_id', $schedule->id)
             ->get();
 
-        foreach ($shiftEntries as $shiftEntry) {
+        foreach ($scheduleShifts as $scheduleShift) {
-            event('shift.entry.deleting', [
+            event('shift.deleting', ['shift' => $scheduleShift->shift]);
-                'user' => User::find($shiftEntry->user_id),
-                'start' => Carbon::make($shiftEntry->start),
-                'end' => Carbon::make($shiftEntry->end),
-                'name' => $shiftEntry->name,
-                'title' => $shiftEntry->title,
-                'type' => $shiftEntry->type,
-                'location' => Location::find($shiftEntry->location_id),
-                'freeloaded' => $shiftEntry->freeloaded,
-            ]);
         }
     }
 
@@ -391,7 +372,7 @@ class ScheduleController extends BaseController
         $scheduleShift = ScheduleShift::whereGuid($event->getGuid())->where('schedule_id', $schedule->id)->first();
         $shift = $scheduleShift->shift;
 
-        $this->fireDeleteShiftEntryEvents($event, $schedule);
+        $this->fireDeleteShiftEvents($event, $schedule);
         $shift->delete();
         $scheduleShift->delete();
 

src/Controllers/Admin/ShiftTypesController.php

@@ -157,18 +157,7 @@ class ShiftTypesController extends BaseController
 
         $shifts = $shiftType->shifts;
         foreach ($shifts as $shift) {
-            foreach ($shift->shiftEntries as $entry) {
+            event('shift.deleting', ['shift' => $shift]);
-                event('shift.entry.deleting', [
-                    'user' => $entry->user,
-                    'start' => $shift->start,
-                    'end' => $shift->end,
-                    'name' => $shift->shiftType->name,
-                    'title' => $shift->title,
-                    'type' => $entry->angelType->name,
-                    'location' => $shift->location,
-                    'freeloaded' => $entry->freeloaded,
-                ]);
-            }
         }
         $shiftType->delete();
 

src/Controllers/Admin/ShiftsController.php

@@ -65,19 +65,7 @@ class ShiftsController extends BaseController
         );
 
         foreach ($shifts as $shift) {
-            foreach ($shift->shiftEntries as $entry) {
+            event('shift.deleting', ['shift' => $shift]);
-                event('shift.entry.deleting', [
-                    'user' => $entry->user,
-                    'start' => $shift->start,
-                    'end' => $shift->end,
-                    'name' => $shift->shiftType->name,
-                    'title' => $shift->title,
-                    'type' => $entry->angelType->name,
-                    'location' => $shift->location,
-                    'freeloaded' => $entry->freeloaded,
-                ]);
-            }
-
             $shift->delete();
 
             $this->log->info(

tests/Unit/Controllers/Admin/LocationsControllerTest.php

@@ -198,9 +198,9 @@ class LocationsControllerTest extends ControllerTest
         $dispatcher->expects($this->once())
             ->method('dispatch')
             ->willReturnCallback(function (string $event, array $data) use ($location, $user) {
-                $this->assertEquals('shift.entry.deleting', $event);
+                $this->assertEquals('shift.deleting', $event);
-                $this->assertEquals($location->id, $data['location']->id);
+                $this->assertEquals($location->id, $data['shift']->location->id);
-                $this->assertEquals($user->id, $data['user']->id);
+                $this->assertEquals($user->id, $data['shift']->shiftEntries[0]->user->id);
 
                 return [];
             });

tests/Unit/Controllers/Admin/ScheduleControllerTest.php

@@ -206,9 +206,9 @@ class ScheduleControllerTest extends ControllerTest
     {
         $this->setExpects($this->redirect, 'to', ['/admin/schedule'], $this->response);
 
-        $this->event->expects($this->exactly(2))
+        $this->event->expects($this->exactly(3))
             ->method('dispatch')
-            ->with('shift.entry.deleting')
+            ->with('shift.deleting')
             ->willReturn([]);
 
         $request = Request::create('', 'POST', ['delete' => 'yes'])
@@ -348,9 +348,9 @@ class ScheduleControllerTest extends ControllerTest
         $request = Request::create('', 'POST')
             ->withAttribute('schedule_id', $this->schedule->id);
 
-        $this->event->expects($this->exactly(2))
+        $this->event->expects($this->exactly(3))
             ->method('dispatch')
-            ->withConsecutive(['shift.updating'], ['shift.entry.deleting'])
+            ->withConsecutive(['shift.updating'], ['shift.deleting'])
             ->willReturn([]);
 
         /** @var ScheduleController $controller */

tests/Unit/Controllers/Admin/ShiftTypesControllerTest.php

@@ -205,9 +205,9 @@ class ShiftTypesControllerTest extends ControllerTest
         $dispatcher->expects($this->once())
             ->method('dispatch')
             ->willReturnCallback(function (string $event, array $data) use ($shifttype, $user) {
-                $this->assertEquals('shift.entry.deleting', $event);
+                $this->assertEquals('shift.deleting', $event);
-                $this->assertEquals($shifttype->name, $data['name']);
+                $this->assertEquals($shifttype->name, $data['shift']->shiftType->name);
-                $this->assertEquals($user->id, $data['user']->id);
+                $this->assertEquals($user->id, $data['shift']->shiftEntries[0]->user->id);
 
                 return [];
             });

tests/Unit/Controllers/Admin/ShiftsControllerTest.php

@@ -54,7 +54,7 @@ class ShiftsControllerTest extends ControllerTest
         /** @var EventDispatcher|MockObject $event */
         $event = $this->createMock(EventDispatcher::class);
         $this->app->instance('events.dispatcher', $event);
-        $this->setExpects($event, 'dispatch', ['shift.entry.deleting'], [], $this->exactly(2));
+        $this->setExpects($event, 'dispatch', ['shift.deleting'], [], $this->exactly(3));
 
         /** @var ShiftsController $controller */
         $controller = $this->app->make(ShiftsController::class);