Merge pull request #12972 from marcusmoore/bug/sc-23177

Fixes user name missing in accessory acceptance notifications

app/Http/Controllers/Account/AcceptanceController.php

@@ -121,7 +121,6 @@ class AcceptanceController extends Controller
         $pdf_filename = 'accepted-eula-'.date('Y-m-d-h-i-s').'.pdf';
         $sig_filename='';
 
-
         if ($request->input('asset_acceptance') == 'accepted') {
 
             /**
@@ -153,7 +152,6 @@ class AcceptanceController extends Controller
                 }
             }
 
-
             // this is horrible
             switch($acceptance->checkoutable_type){
                 case 'App\Models\Asset':
@@ -170,7 +168,7 @@ class AcceptanceController extends Controller
                         $pdf_view_route ='account.accept.accept-accessory-eula';
                         $accessory = Accessory::find($item->id);
                         $display_model = $accessory->name;
-                        $assigned_to = User::find($item->assignedTo);
+                        $assigned_to = User::find($acceptance->assigned_to_id)->present()->fullName;
                 break;
 
                 case 'App\Models\LicenseSeat':
@@ -253,11 +251,15 @@ class AcceptanceController extends Controller
             // This is the most horriblest
             switch($acceptance->checkoutable_type){
                 case 'App\Models\Asset':
+                    $asset_model = AssetModel::find($item->model_id);
+                    $display_model = $asset_model->name;
                     $assigned_to = User::find($acceptance->assigned_to_id)->present()->fullName;
                     break;
 
                 case 'App\Models\Accessory':
-                    $assigned_to = User::find($item->assignedTo);
+                    $accessory = Accessory::find($item->id);
+                    $display_model = $accessory->name;
+                    $assigned_to = User::find($acceptance->assigned_to_id)->present()->fullName;
                     break;
 
                 case 'App\Models\LicenseSeat':
@@ -269,6 +271,8 @@ class AcceptanceController extends Controller
                     break;
 
                 case 'App\Models\Consumable':
+                    $consumable = Consumable::find($item->id);
+                    $display_model = $consumable->name;
                     $assigned_to = User::find($acceptance->assigned_to_id)->present()->fullName;
                     break;
             }

app/Models/CheckoutAcceptance.php

@@ -3,13 +3,14 @@
 namespace App\Models;
 
 use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Eloquent\SoftDeletes;
 use Illuminate\Notifications\Notifiable;
 
 class CheckoutAcceptance extends Model
 {
-    use SoftDeletes, Notifiable;
+    use HasFactory, SoftDeletes, Notifiable;
 
     protected $casts = [
         'accepted_at' => 'datetime',

database/factories/CheckoutAcceptanceFactory.php

@@ -0,0 +1,41 @@
+<?php
+
+namespace Database\Factories;
+
+use App\Models\Accessory;
+use App\Models\Asset;
+use App\Models\User;
+use Illuminate\Database\Eloquent\Factories\Factory;
+
+class CheckoutAcceptanceFactory extends Factory
+{
+    /**
+     * Define the model's default state.
+     *
+     * @return array
+     */
+    public function definition()
+    {
+        return [
+            'checkoutable_type' => Asset::class,
+            'checkoutable_id' => Asset::factory(),
+            'assigned_to_id' => User::factory(),
+        ];
+    }
+
+    public function forAccessory()
+    {
+        return $this->state([
+            'checkoutable_type' => Accessory::class,
+            'checkoutable_id' => Accessory::factory(),
+        ]);
+    }
+
+    public function pending()
+    {
+        return $this->state([
+            'accepted_at' => null,
+            'declined_at' => null,
+        ]);
+    }
+}

resources/views/notifications/markdown/asset-acceptance.blade.php

@@ -11,7 +11,7 @@
 | **{{ ucfirst(trans('general.accepted')) }}** | {{ $accepted_date }} |
 @endif
 @if (isset($declined_date))
-| **{{ trans('general.declined') }}** | {{ $declined_date }} |
+| **{{ ucfirst(trans('general.declined')) }}** | {{ $declined_date }} |
 @endif
 @if ((isset($item_tag)) && ($item_tag!=''))
 | **{{ trans('mail.asset_tag') }}** | {{ $item_tag }} |

routes/web.php

@@ -291,7 +291,8 @@ Route::group(['prefix' => 'account', 'middleware' => ['auth']], function () {
     Route::get('accept/{id}', [Account\AcceptanceController::class, 'create'])
         ->name('account.accept.item');
 
-    Route::post('accept/{id}', [Account\AcceptanceController::class, 'store']);
+    Route::post('accept/{id}', [Account\AcceptanceController::class, 'store'])
+        ->name('account.store-acceptance');
 
     Route::get(
         'print',

tests/Feature/CheckoutAcceptances/AccessoryAcceptanceTest.php

@@ -0,0 +1,82 @@
+<?php
+
+namespace Tests\Feature\CheckoutAcceptances;
+
+use App\Models\Accessory;
+use App\Models\CheckoutAcceptance;
+use App\Notifications\AcceptanceAssetAcceptedNotification;
+use App\Notifications\AcceptanceAssetDeclinedNotification;
+use Notification;
+use Tests\Support\InteractsWithSettings;
+use Tests\TestCase;
+
+class AccessoryAcceptanceTest extends TestCase
+{
+    use InteractsWithSettings;
+
+    /**
+     * This can be absorbed into a bigger test
+     */
+    public function testUsersNameIsIncludedInAccessoryAcceptedNotification()
+    {
+        Notification::fake();
+
+        $this->settings->enableAlertEmail();
+
+        $acceptance = CheckoutAcceptance::factory()
+            ->pending()
+            ->for(Accessory::factory()->appleMouse(), 'checkoutable')
+            ->create();
+
+        $this->actingAs($acceptance->assignedTo)
+            ->post(route('account.store-acceptance', $acceptance), ['asset_acceptance' => 'accepted'])
+            ->assertSessionHasNoErrors();
+
+        $this->assertNotNull($acceptance->fresh()->accepted_at);
+
+        Notification::assertSentTo(
+            $acceptance,
+            function (AcceptanceAssetAcceptedNotification $notification) use ($acceptance) {
+                $this->assertStringContainsString(
+                    $acceptance->assignedTo->present()->fullName,
+                    $notification->toMail()->render()
+                );
+
+                return true;
+            }
+        );
+    }
+
+    /**
+     * This can be absorbed into a bigger test
+     */
+    public function testUsersNameIsIncludedInAccessoryDeclinedNotification()
+    {
+        Notification::fake();
+
+        $this->settings->enableAlertEmail();
+
+        $acceptance = CheckoutAcceptance::factory()
+            ->pending()
+            ->for(Accessory::factory()->appleMouse(), 'checkoutable')
+            ->create();
+
+        $this->actingAs($acceptance->assignedTo)
+            ->post(route('account.store-acceptance', $acceptance), ['asset_acceptance' => 'declined'])
+            ->assertSessionHasNoErrors();
+
+        $this->assertNotNull($acceptance->fresh()->declined_at);
+
+        Notification::assertSentTo(
+            $acceptance,
+            function (AcceptanceAssetDeclinedNotification $notification) use ($acceptance) {
+                $this->assertStringContainsString(
+                    $acceptance->assignedTo->present()->fullName,
+                    $notification->toMail($acceptance)->render()
+                );
+
+                return true;
+            }
+        );
+    }
+}

tests/Support/Settings.php

@@ -18,6 +18,16 @@ class Settings
         return new self();
     }
 
+    public function enableAlertEmail(string $email = 'notifications@afcrichmond.com'): Settings
+    {
+        return $this->update(['alert_email' => $email]);
+    }
+
+    public function disableAlertEmail(): Settings
+    {
+        return $this->update(['alert_email' => null]);
+    }
+
     public function enableMultipleFullCompanySupport(): Settings
     {
         return $this->update(['full_multiple_companies_support' => 1]);