fix controller missing import issue

Author: eugenefvdm

CHANGELOG.md

@@ -1,5 +1,13 @@
 # Changelog
 
+## v0.4 - 2025-06-09
+
+- Fixed `unsubscribe` controller logic by adding missing `Controller` import
+- Fixed documentation about which Laravel model to declare when using model specific notifications
+- Added documentation about what basic functionality the controller provides
+- Reorganised the readme a bit so that unsubscribe is nearer the end
+- Added more documentation for protected and public variables in the base class
+
 ## v0.3 - 2025-05-31
 
 ### Changed

README.md

@@ -70,7 +70,6 @@ In your notification class, add any or all of the following variables to do repe
 use Eugenefvdm\NotificationSubscriptions\Enums\RepeatFrequency;
 use Eugenefvdm\NotificationSubscriptions\Notifications\BaseNotification;
 
-
 class DailyReminder extends BaseNotification
 {
     use Queueable;
@@ -102,33 +101,6 @@ class DailyReminder extends BaseNotification
     }
 ```
 
-## Unsubscribe
-
-Any new notification will be automatically subscribed when used the first time.
-
-### Unsubscribe Links in Blades
-
-For unsubscribe links, modify the `toMail` method in the notification class:
-
-```php
-/**
- * Get the mail representation of the notification.
- */
-public function toMail(object $notifiable): MailMessage
-{
-    return (new MailMessage)
-        ->markdown('notification.reminders.max-count', [
-            'subscription' => $this->getSubscriptionFromNotifiable($notifiable)
-        ]);
-}
-```
-
-Then add this to your blade:
-
-```php
-<x-notification-subscriptions::unsubscribe :subscription="$subscription" />
-```
-
 ## Categorization
 
 All new messages without an explicit category assignment will be assigned to the `default` category in the database.
@@ -147,36 +119,68 @@ class DailyReminder extends BaseNotification
 
 ## Model Specific Subscriptions
 
-Notifications are typically tied to a user, but at times one wants to bind a notification to both a user and another model. For example, you might have a `products` table, and you want a user to be subscribed to a price notification for the `Product` model. Here's how:
+Notifications are typically tied to a user, but at times one wants to associate a notification to both a user and another model. For example, you might have a `products` table, and you want a user to be subscribed to a price notification for specific `Product` models. Here's how:
 
 ```php
 use Eugenefvdm\NotificationSubscriptions\Notifications\BaseNotification;
+use Illuminate\Database\Eloquent\Model;
 use App\Models\Product;
 
 class ProductPriceNotification extends BaseNotification
 {
     use Queueable;
 
-    public Product $customModel; // Override $customModel
+    public ?Model $customModel; // Override $customModel
 
-    public function __construct(Product $product)
+    public function __construct(Product $product) // Type-hint Product in constructor
     {
         $this->customModel = $product;
     }
 
     public function toMail(object $notifiable): MailMessage
     {
+        /** @var Product $product */
+        $product = $this->customModel;
+
         return (new MailMessage)
-            ->subject("Price Update for {$this->customModel->name}")
+            ->subject("Price Update for {$product->name}")
             ->markdown('notification.product-price-update', [
-                'product' => $this->customModel,
-                // Add line below and blade component for unsubscribe
+                'product' => $product,
                 'subscription' => $this->getSubscriptionFromNotifiable($notifiable)
             ]);
     }
 }
 ```
 
+## Unsubscribe
+
+Any new notification will be automatically subscribed when used the first time.
+
+### Unsubscribe Links in Blades
+
+For unsubscribe links, modify the `toMail` method in the notification class:
+
+```php
+/**
+ * Get the mail representation of the notification.
+ */
+public function toMail(object $notifiable): MailMessage
+{
+    return (new MailMessage)
+        ->markdown('notification.reminders.max-count', [
+            'subscription' => $this->getSubscriptionFromNotifiable($notifiable)
+        ]);
+}
+```
+
+Then add this to your blade:
+
+```php
+<x-notification-subscriptions::unsubscribe :subscription="$subscription" />
+```
+
+The default controller action for unsubscribe is to redirect back with a session variable value of `error` or `status`.
+
 ## Testing
 
 ```bash

src/Http/Controllers/NotificationController.php

@@ -5,6 +5,7 @@
 use Illuminate\Http\Request;
 use Illuminate\Support\Facades\Session;
 use Eugenefvdm\NotificationSubscriptions\Models\NotificationSubscription;
+use Illuminate\Routing\Controller;
 
 class NotificationController extends Controller
 {
@@ -16,20 +17,15 @@ public function unsubscribe(Request $request, string $uuid)
         $subscription = NotificationSubscription::where('uuid', $uuid)->first();
 
         if (!$subscription) {
-            abort(404, 'Subscription not found');
+            return redirect()->back()->with('error', 'Subscription not found');
         }
 
-        // Check if this notification can be unsubscribed
         if (!$subscription->canBeUnsubscribed()) {
-            return redirect()->route('home')->with('error', 'This notification cannot be unsubscribed.');
+            return redirect()->back()->with('error', 'This notification cannot be unsubscribed');
         }
 
-        // Unsubscribe
         $subscription->unsubscribe();
 
-        Session::flash('success', 'You have been successfully unsubscribed from this notification.');
-
-        // Return to previous page or home
-        return redirect()->back()->fallback(route('home'));
+        return redirect()->back()->with('success', 'Successfully unsubscribed');
     }    
 } 

src/Notifications/BaseNotification.php

@@ -16,16 +16,57 @@ abstract class BaseNotification extends Notification implements ShouldQueue
 {
     use Queueable;
 
-    protected ?NotificationSubscription $subscription = null;    
+    /**
+     * customModel may be overridden by child classes and used for model specific notifications.
+     * 
+     * @var null|Model
+     */
     public ?Model $customModel = null;
 
-    // Default repeat variables, can be overridden by child classes
+    /**
+     * The subscription that this notification belongs to.
+     * 
+     * @var null|NotificationSubscription
+     */
+    protected ?NotificationSubscription $subscription = null;    
+
+    /**
+     * The frequency at which this notification will be repeated.
+     * 
+     * See Enum RepeatFrequency for possible values.
+     * 
+     * @var null|RepeatFrequency
+     */
     protected static ?RepeatFrequency $repeatFrequency = null;
+
+    /**
+     * The interval at which this notification will be repeated.
+     * 
+     * @var null|int
+     */
     protected static ?int $repeatInterval = null;
+
+    /**
+     * The maximum number of times this notification can be repeated.
+     * 
+     * @var null|int
+     */
     protected static ?int $maxRepeats = null;
+
+    /**
+     * The initial delay for this notification.
+     * 
+     * @var null|Carbon
+     */
     protected static ?Carbon $initialDelay = null;
+
+    /**
+     * The category that will be stored in the database.
+     * 
+     * @var string
+     */
     protected static ?string $category = 'default';
-    
+
     // Get the repeat frequency from the base class
     public static function getRepeatFrequency(): ?RepeatFrequency
     {

tests/Feature/UnsubscribeLinkTest.php

@@ -1,6 +1,7 @@
 <?php
 
 use Eugenefvdm\NotificationSubscriptions\Tests\TestModels\User;
+use Eugenefvdm\NotificationSubscriptions\Tests\TestNotifications\SystemNotification;
 use Eugenefvdm\NotificationSubscriptions\Tests\TestNotifications\UnsubscribeLinkNotification;
 
 it('will show an unsubscribe link if the notification is not a system notification', function () {
@@ -20,4 +21,49 @@
 
     // Check the unsubscribe link is present in the mail
     $this->assertStringContainsString($unsubscribeUrl, $bladeView);
+});
+
+it('will allow you to unsubscribe from a notification if you click the unsubscribe link', function () {
+    $user = User::factory()->create();
+    $notification = new UnsubscribeLinkNotification();
+
+    $user->notify($notification);
+
+    $subscription = $user->notificationSubscriptions()
+    ->where('notification_template_id', UnsubscribeLinkNotification::getTemplate()->id)->first();
+
+    // Build the unsubscribe url
+    $unsubscribeUrl = url(route('notifications.unsubscribe', $subscription->uuid));
+
+    $mail = $notification->toMail($user);
+    $bladeView = $mail->render();
+
+    // Visit the unsubscribe url
+    $response = $this->get($unsubscribeUrl);
+    expect($response->status())->toBe(302);
+
+    // Check the success message is present
+    expect(session('success'))->not->toBeNull();
+
+    // Check if the subscription now has an unsubscribed_at timestamp
+    expect($subscription->fresh()->unsubscribed_at)->not->toBeNull();    
+});
+
+it('a system notification cannot be unsubscribed', function () {
+    $user = User::factory()->create();
+    $notification = new SystemNotification();
+
+    $user->notify($notification);
+
+    $subscription = $user->notificationSubscriptions()
+    ->where('notification_template_id', SystemNotification::getTemplate()->id)->first();
+
+    // Build the unsubscribe url
+    $unsubscribeUrl = url(route('notifications.unsubscribe', $subscription->uuid));
+
+    $response = $this->get($unsubscribeUrl);
+    expect($response->status())->toBe(302);
+
+    // Check the error message is present
+    expect(session('error'))->not->toBeNull();
 });

tests/TestNotifications/SystemNotification.php

@@ -0,0 +1,38 @@
+<?php
+
+namespace Eugenefvdm\NotificationSubscriptions\Tests\TestNotifications;
+
+use Eugenefvdm\NotificationSubscriptions\Notifications\BaseNotification;
+use Illuminate\Bus\Queueable;
+
+class SystemNotification extends BaseNotification
+{
+    use Queueable;
+
+    public static ?string $category = 'system';
+
+    /**
+     * Create a new notification instance.
+     */
+    public function __construct()
+    {
+        //
+    }
+
+    /**
+     * Get the notification's delivery channels.
+     *
+     * @return array<int, string>
+     */
+    public function via(object $notifiable): array
+    {
+        return ['mail'];
+    }
+
+    public function toMail(object $notifiable)
+    {
+        return (new \Illuminate\Notifications\Messages\MailMessage)
+            ->subject('System Category Test')
+            ->line('This is a test notification with system category.');
+    }
+}