feat(notify): don't notify disabled users

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

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,15 +63,27 @@ func (t *testNotifiable) RouteForDB() int64 {
 	return 42
 }
 
+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{}
+		tnf := &testNotifiable{
-
+			ShouldSendNotification: true,
-	err := Notify(tnf, tn)
+		}
 
+		err = Notify(tnf, tn)
 		assert.NoError(t, err)
 		vals := map[string]interface{}{
 			"notifiable_id": 42,
@@ -88,4 +101,26 @@ func TestNotify(t *testing.T) {
 		}
 
 		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,
+		})
+	})
 }

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