From 80b40bb2c0a812b8ec276da788663bfcca2022b6 Mon Sep 17 00:00:00 2001
From: kolaente <k@knt.li>
Date: Mon, 4 Sep 2023 14:23:56 +0200
Subject: [PATCH] feat(notify): don't notify disabled users

---
 pkg/notifications/notification.go      | 14 ++++-
 pkg/notifications/notification_test.go | 87 ++++++++++++++++++--------
 pkg/user/user.go                       | 11 ++++
 3 files changed, 84 insertions(+), 28 deletions(-)

diff --git a/pkg/notifications/notification.go b/pkg/notifications/notification.go
index 3367b22a2..a17c89e96 100644
--- a/pkg/notifications/notification.go
+++ b/pkg/notifications/notification.go
@@ -17,6 +17,7 @@
 package notifications
 
 import (
+	"code.vikunja.io/api/pkg/log"
 	"encoding/json"
 
 	"code.vikunja.io/api/pkg/db"
@@ -40,10 +41,13 @@ type NotificationWithSubject interface {
 
 // Notifiable is an entity which can be notified. Usually a user.
 type Notifiable interface {
-	// Should return the email address this notifiable has.
+	// RouteForMail should return the email address this notifiable has.
 	RouteForMail() (string, error)
-	// Should return the id of the notifiable entity
+	// RouteForDB should return the id of the notifiable entity to save it in the database.
 	RouteForDB() int64
+	// ShouldNotify provides a last-minute way to cancel a notification. It will be called immediately before
+	// sending a notification.
+	ShouldNotify() (should bool, err error)
 }
 
 // Notify notifies a notifiable of a notification
@@ -53,6 +57,12 @@ func Notify(notifiable Notifiable, notification Notification) (err error) {
 		return nil
 	}
 
+	should, err := notifiable.ShouldNotify()
+	if err != nil || !should {
+		log.Debugf("Not notifying user %d because they are disabled", notifiable.RouteForDB())
+		return err
+	}
+
 	err = notifyMail(notifiable, notification)
 	if err != nil {
 		return
diff --git a/pkg/notifications/notification_test.go b/pkg/notifications/notification_test.go
index 1527a2029..826fb970c 100644
--- a/pkg/notifications/notification_test.go
+++ b/pkg/notifications/notification_test.go
@@ -50,6 +50,7 @@ func (n *testNotification) Name() string {
 }
 
 type testNotifiable struct {
+	ShouldSendNotification bool
 }
 
 // RouteForMail routes a test notification for mail
@@ -62,30 +63,64 @@ func (t *testNotifiable) RouteForDB() int64 {
 	return 42
 }
 
-func TestNotify(t *testing.T) {
-	tn := &testNotification{
-		Test:       "somethingsomething",
-		OtherValue: 42,
-	}
-	tnf := &testNotifiable{}
-
-	err := Notify(tnf, tn)
-
-	assert.NoError(t, err)
-	vals := map[string]interface{}{
-		"notifiable_id": 42,
-		"notification":  "'{\"other_value\":42,\"test\":\"somethingsomething\"}'",
-	}
-
-	if db.Type() == schemas.POSTGRES {
-		vals["notification::jsonb"] = vals["notification"].(string) + "::jsonb"
-		delete(vals, "notification")
-	}
-
-	if db.Type() == schemas.SQLITE {
-		vals["CAST(notification AS BLOB)"] = "CAST(" + vals["notification"].(string) + " AS BLOB)"
-		delete(vals, "notification")
-	}
-
-	db.AssertExists(t, "notifications", vals, true)
+func (t *testNotifiable) ShouldNotify() (should bool, err error) {
+	return t.ShouldSendNotification, nil
+}
+
+func TestNotify(t *testing.T) {
+	t.Run("normal", func(t *testing.T) {
+
+		s := db.NewSession()
+		defer s.Close()
+		_, err := s.Exec("delete from notifications")
+		assert.NoError(t, err)
+
+		tn := &testNotification{
+			Test:       "somethingsomething",
+			OtherValue: 42,
+		}
+		tnf := &testNotifiable{
+			ShouldSendNotification: true,
+		}
+
+		err = Notify(tnf, tn)
+		assert.NoError(t, err)
+		vals := map[string]interface{}{
+			"notifiable_id": 42,
+			"notification":  "'{\"other_value\":42,\"test\":\"somethingsomething\"}'",
+		}
+
+		if db.Type() == schemas.POSTGRES {
+			vals["notification::jsonb"] = vals["notification"].(string) + "::jsonb"
+			delete(vals, "notification")
+		}
+
+		if db.Type() == schemas.SQLITE {
+			vals["CAST(notification AS BLOB)"] = "CAST(" + vals["notification"].(string) + " AS BLOB)"
+			delete(vals, "notification")
+		}
+
+		db.AssertExists(t, "notifications", vals, true)
+	})
+	t.Run("disabled notifiable", func(t *testing.T) {
+
+		s := db.NewSession()
+		defer s.Close()
+		_, err := s.Exec("delete from notifications")
+		assert.NoError(t, err)
+
+		tn := &testNotification{
+			Test:       "somethingsomething",
+			OtherValue: 42,
+		}
+		tnf := &testNotifiable{
+			ShouldSendNotification: false,
+		}
+
+		err = Notify(tnf, tn)
+		assert.NoError(t, err)
+		db.AssertMissing(t, "notifications", map[string]interface{}{
+			"notifiable_id": 42,
+		})
+	})
 }
diff --git a/pkg/user/user.go b/pkg/user/user.go
index 69618b909..4a6881bd7 100644
--- a/pkg/user/user.go
+++ b/pkg/user/user.go
@@ -137,6 +137,17 @@ func (u *User) RouteForDB() int64 {
 	return u.ID
 }
 
+func (u *User) ShouldNotify() (bool, error) {
+	s := db.NewSession()
+	defer s.Close()
+	user, err := getUser(s, &User{ID: u.ID}, true)
+	if err != nil {
+		return false, err
+	}
+
+	return user.Status != StatusDisabled, err
+}
+
 // GetID implements the Auth interface
 func (u *User) GetID() int64 {
 	return u.ID