MM-53924 - Implement push notifications plugin hook and plugin api method (#24350)

* Revert "MM-52804 - Implement SendPushNotification plugin api method (#24273)"

This reverts commit 8418eefb75.

* Revert "MM-53924 - Implement NotificationWillBePushed plugin hook (#24263)"

This reverts commit f13a531bca.

* implement NotificationWillBePushed plugin hook

* implement SendPushNotification plugin api method

* move where we're setting post and channel type

* fix comment

---------

Co-authored-by: Mattermost Build <build@mattermost.com>

server/channels/app/notification_push.go

@@ -81,6 +81,25 @@ func (a *App) sendPushNotificationSync(c request.CTX, post *model.Post, user *mo
 }
 
 func (a *App) sendPushNotificationToAllSessions(msg *model.PushNotification, userID string, skipSessionId string) *model.AppError {
+	rejectionReason := ""
+	a.ch.RunMultiHook(func(hooks plugin.Hooks) bool {
+		var replacementNotification *model.PushNotification
+		replacementNotification, rejectionReason = hooks.NotificationWillBePushed(msg, userID)
+		if rejectionReason != "" {
+			mlog.Info("Notification cancelled by plugin.", mlog.String("rejection reason", rejectionReason))
+			return false
+		}
+		if replacementNotification != nil {
+			msg = replacementNotification
+		}
+		return true
+	}, plugin.NotificationWillBePushedID)
+
+	if rejectionReason != "" {
+		// Notifications rejected by a plugin should not be considered errors
+		return nil
+	}
+
 	sessions, err := a.getMobileAppSessions(userID)
 	if err != nil {
 		return err
@@ -140,27 +159,6 @@ func (a *App) sendPushNotificationToAllSessions(msg *model.PushNotification, use
 }
 
 func (a *App) sendPushNotification(notification *PostNotification, user *model.User, explicitMention, channelWideMention bool, replyToThreadType string) {
-	cancelled := false
-	a.ch.RunMultiHook(func(hooks plugin.Hooks) bool {
-		cancelled = hooks.NotificationWillBePushed(&model.PluginPushNotification{
-			Post:               notification.Post.ForPlugin(),
-			Channel:            notification.Channel,
-			UserID:             user.Id,
-			ExplicitMention:    explicitMention,
-			ChannelWideMention: channelWideMention,
-			ReplyToThreadType:  replyToThreadType,
-		})
-		if cancelled {
-			mlog.Info("Notification cancelled by plugin")
-			return false
-		}
-		return true
-	}, plugin.NotificationWillBePushedID)
-
-	if cancelled {
-		return
-	}
-
 	cfg := a.Config()
 	channel := notification.Channel
 	post := notification.Post
@@ -606,6 +604,10 @@ func (a *App) BuildPushNotificationMessage(c request.CTX, contentsConfig string,
 
 	msg.Badge = badgeCount
 
+	// Add post and channel types for plugins to use in the NotificationWillBePushed hook
+	msg.PostType = post.Type
+	msg.ChannelType = channel.Type
+
 	return msg, nil
 }
 

server/channels/app/notification_push_test.go

@@ -1468,6 +1468,7 @@ func TestPushNotificationRace(t *testing.T) {
 	require.NoError(t, err)
 	s.products["channels"] = ch
 
+	app := New(ServerConnector(s.Channels()))
 	require.NotPanics(t, func() {
 		s.createPushNotificationsHub(th.Context)
 
@@ -1475,9 +1476,9 @@ func TestPushNotificationRace(t *testing.T) {
 
 		// Now we start sending messages after the PN hub is shut down.
 		// We test all 3 notification types.
-		th.App.clearPushNotification("currentSessionId", "userId", "channelId", "")
+		app.clearPushNotification("currentSessionId", "userId", "channelId", "")
 
-		th.App.UpdateMobileAppBadge("userId")
+		app.UpdateMobileAppBadge("userId")
 
 		notification := &PostNotification{
 			Post:    &model.Post{},
@@ -1487,7 +1488,7 @@ func TestPushNotificationRace(t *testing.T) {
 			},
 			Sender: &model.User{},
 		}
-		th.App.sendPushNotification(notification, &model.User{}, true, false, model.CommentsNotifyAny)
+		app.sendPushNotification(notification, &model.User{}, true, false, model.CommentsNotifyAny)
 	})
 }
 

server/channels/app/plugin_api.go

@@ -1267,30 +1267,7 @@ func (api *PluginAPI) GetUploadSession(uploadID string) (*model.UploadSession, e
 	return fi, nil
 }
 
-func (api *PluginAPI) SendPluginPushNotification(notification *model.PluginPushNotification) error {
+func (api *PluginAPI) SendPushNotification(notification *model.PushNotification, userID string) *model.AppError {
-	var profiles map[string]*model.User
+	// Ignoring skipSessionId because it's only used internally to clear push notifications
-	var err error
+	return api.app.sendPushNotificationToAllSessions(notification, userID, "")
-	if notification.Channel.Type == model.ChannelTypeGroup {
-		if profiles, err = api.app.Srv().Store().User().GetAllProfilesInChannel(api.ctx.Context(), notification.Channel.Id, true); err != nil {
-			return err
-		}
-	}
-
-	sender, appErr := api.app.GetUser(notification.Post.UserId)
-	if appErr != nil {
-		return appErr
-	}
-	user, appErr := api.app.GetUser(notification.UserID)
-	if appErr != nil {
-		return appErr
-	}
-
-	postNotification := &PostNotification{
-		Post:       notification.Post,
-		Channel:    notification.Channel,
-		ProfileMap: profiles,
-		Sender:     sender,
-	}
-	api.app.sendPushNotification(postNotification, user, notification.ExplicitMention, notification.ChannelWideMention, notification.ReplyToThreadType)
-	return nil
 }

server/channels/app/plugin_api_test.go

@@ -2266,13 +2266,22 @@ func TestSendPushNotification(t *testing.T) {
 			defer wg.Done()
 			post := th.CreatePost(th.BasicChannel)
 			post.Message = "started a conversation"
-			notification := &model.PluginPushNotification{
+			notification := &model.PushNotification{
-				Post:    post,
+				Category:    model.CategoryCanReply,
-				Channel: th.BasicChannel,
+				Version:     model.PushMessageV2,
-				UserID:  user.Id,
+				Type:        model.PushTypeMessage,
+				TeamId:      th.BasicChannel.TeamId,
+				ChannelId:   th.BasicChannel.Id,
+				PostId:      post.Id,
+				RootId:      post.RootId,
+				SenderId:    post.UserId,
+				SenderName:  "Sender Name",
+				PostType:    post.Type,
+				ChannelType: th.BasicChannel.Type,
+				Message:     "Custom message",
 			}
-			appErr := api.SendPluginPushNotification(notification)
+			appErr := api.SendPushNotification(notification, user.Id)
-			require.NoError(t, appErr)
+			require.Nil(t, appErr)
 		}(*data.user)
 	}
 	wg.Wait()
@@ -2286,7 +2295,7 @@ func TestSendPushNotification(t *testing.T) {
 		case model.PushTypeMessage:
 			numMessages++
 			assert.Equal(t, th.BasicChannel.Id, n.ChannelId)
-			assert.Equal(t, fmt.Sprintf("@%s: started a conversation", th.BasicUser.GetDisplayName(model.ShowUsername)), n.Message)
+			assert.Equal(t, "Custom message", n.Message)
 		default:
 			assert.Fail(t, "should not receive any other push notification types")
 		}

server/channels/app/plugin_hooks_test.go

@@ -1347,17 +1347,25 @@ func TestHookNotificationWillBePushed(t *testing.T) {
 		name                        string
 		testCode                    string
 		expectedNotifications       int
+		expectedNotificationMessage string
 	}{
 		{
 			name:                  "successfully pushed",
-			testCode:              `return false`,
+			testCode:              `return nil, ""`,
 			expectedNotifications: 6,
 		},
 		{
 			name:                  "push notification rejected",
-			testCode:              `return true`,
+			testCode:              `return nil, "rejected"`,
 			expectedNotifications: 0,
 		},
+		{
+			name: "push notification modified",
+			testCode: `notification.Message = "brand new message"
+	return notification, ""`,
+			expectedNotifications:       6,
+			expectedNotificationMessage: "brand new message",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -1440,7 +1448,11 @@ func TestHookNotificationWillBePushed(t *testing.T) {
 				case model.PushTypeMessage:
 					numMessages++
 					assert.Equal(t, th.BasicChannel.Id, n.ChannelId)
+					if tt.expectedNotificationMessage != "" {
+						assert.Equal(t, tt.expectedNotificationMessage, n.Message)
+					} else {
 						assert.Contains(t, n.Message, "mentioned you")
+					}
 				default:
 					assert.Fail(t, "should not receive any other push notification types")
 				}

server/channels/app/test_templates/hook_notification_will_be_pushed.tmpl

@@ -1,15 +1,15 @@
 package main
 
 import (
-    "github.com/mattermost/mattermost/server/public/plugin"
 	"github.com/mattermost/mattermost/server/public/model"
+	"github.com/mattermost/mattermost/server/public/plugin"
 )
 
 type MyPlugin struct {
     plugin.MattermostPlugin
 }
 
-func (p *MyPlugin) NotificationWillBePushed(notification *model.PluginPushNotification) (cancel bool) {
+func (p *MyPlugin) NotificationWillBePushed(notification *model.PushNotification, userID string) (*model.PushNotification, string) {
     %s
 }
 

server/public/model/plugin_push_notification.go

@@ -1,21 +0,0 @@
-// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
-// See LICENSE.txt for license information.
-
-package model
-
-// PluginPushNotification is sent to the plugin when a push notification is going to be sent (via the
-// NotificationWillBePushed hook), and is used as the source data for the plugin api SendPluginPushNotification method.
-//
-// Note: Please keep in mind that Mattermost push notifications always refer to a specific post. Therefore, when used
-// as source data for `SendPluginPushNotification`, Post and Channel must be valid in order to create a correct
-// model.PushNotification.
-// Note: Post and Channel are pointers so that plugins using the NotificationWillBePushed hook will not need to query the
-// database on every push notification.
-type PluginPushNotification struct {
-	Post               *Post    // The post that will be used as the source of the push notification.
-	Channel            *Channel // The channel that the post appeared in.
-	UserID             string   // The receiver of the push notification.
-	ExplicitMention    bool     // Used to construct the generic "@sender mentioned you" msg when `cfg.EmailSettings.PushNotificationContents` is not set to `full`
-	ChannelWideMention bool     // Used to construct the generic "@sender notified the channel" msg when `cfg.EmailSettings.PushNotificationContents` is not set to `full`
-	ReplyToThreadType  string   // Used to construct the generic CRT msgs when `cfg.EmailSettings.PushNotificationContents` is not set to `full`; see `App.getPushNotificationMessage` for details.
-}

server/public/model/push_notification.go

@@ -67,6 +67,8 @@ type PushNotification struct {
 	Version          string      `json:"version,omitempty"`
 	IsCRTEnabled     bool        `json:"is_crt_enabled"`
 	IsIdLoaded       bool        `json:"is_id_loaded"`
+	PostType         string      `json:"-"`
+	ChannelType      ChannelType `json:"-"`
 }
 
 func (pn *PushNotification) DeepCopy() *PushNotification {

server/public/plugin/api.go

@@ -1181,14 +1181,17 @@ type API interface {
 	// Minimum server version: 7.6
 	GetUploadSession(uploadID string) (*model.UploadSession, error)
 
-	// SendPluginPushNotification will attempt to send a push notification to `notification.User`, using
+	// SendPushNotification will send a push notification to all of user's sessions.
-	// `notification.Post` as the source of the notification. The server will use the PluginPushNotification
+	//
-	// data to construct the final push notification according to the server's configuration and license. Refer
+	// It is the responsibility of the plugin to respect the server's configuration and licence,
-	// to `App.BuildPushNotificationMessage` for the logic used to construct the push notification.
+	// especially related to `cfg.EmailSettings.PushNotificationContents`, particularly
-	// Note: the NotificationWillBePushed hook will be run after SendPluginPushNotification is called.
+	// `model.IdLoadedNotification` and the generic settings.
+	// Refer to `app.sendPushNotificationSync` for the logic used to construct push notifications.
+	//
+	// Note: the NotificationWillBePushed hook will be run after SendPushNotification is called.
 	//
 	// Minimum server version: 9.0
-	SendPluginPushNotification(notification *model.PluginPushNotification) error
+	SendPushNotification(notification *model.PushNotification, userID string) *model.AppError
 }
 
 var handshake = plugin.HandshakeConfig{

server/public/plugin/api_timer_layer_generated.go

@@ -1267,9 +1267,9 @@ func (api *apiTimerLayer) GetUploadSession(uploadID string) (*model.UploadSessio
 	return _returnsA, _returnsB
 }
 
-func (api *apiTimerLayer) SendPluginPushNotification(notification *model.PluginPushNotification) error {
+func (api *apiTimerLayer) SendPushNotification(notification *model.PushNotification, userID string) *model.AppError {
 	startTime := timePkg.Now()
-	_returnsA := api.apiImpl.SendPluginPushNotification(notification)
+	_returnsA := api.apiImpl.SendPushNotification(notification, userID)
-	api.recordTime(startTime, "SendPluginPushNotification", _returnsA == nil)
+	api.recordTime(startTime, "SendPushNotification", _returnsA == nil)
 	return _returnsA
 }

server/public/plugin/client_rpc_generated.go

@@ -848,29 +848,31 @@ func init() {
 }
 
 type Z_NotificationWillBePushedArgs struct {
-	A *model.PluginPushNotification
+	A *model.PushNotification
+	B string
 }
 
 type Z_NotificationWillBePushedReturns struct {
-	A bool
+	A *model.PushNotification
+	B string
 }
 
-func (g *hooksRPCClient) NotificationWillBePushed(pushNotification *model.PluginPushNotification) (cancel bool) {
+func (g *hooksRPCClient) NotificationWillBePushed(pushNotification *model.PushNotification, userID string) (*model.PushNotification, string) {
-	_args := &Z_NotificationWillBePushedArgs{pushNotification}
+	_args := &Z_NotificationWillBePushedArgs{pushNotification, userID}
 	_returns := &Z_NotificationWillBePushedReturns{}
 	if g.implemented[NotificationWillBePushedID] {
 		if err := g.client.Call("Plugin.NotificationWillBePushed", _args, _returns); err != nil {
 			g.log.Error("RPC call NotificationWillBePushed to plugin failed.", mlog.Err(err))
 		}
 	}
-	return _returns.A
+	return _returns.A, _returns.B
 }
 
 func (s *hooksRPCServer) NotificationWillBePushed(args *Z_NotificationWillBePushedArgs, returns *Z_NotificationWillBePushedReturns) error {
 	if hook, ok := s.impl.(interface {
-		NotificationWillBePushed(pushNotification *model.PluginPushNotification) (cancel bool)
+		NotificationWillBePushed(pushNotification *model.PushNotification, userID string) (*model.PushNotification, string)
 	}); ok {
-		returns.A = hook.NotificationWillBePushed(args.A)
+		returns.A, returns.B = hook.NotificationWillBePushed(args.A, args.B)
 	} else {
 		return encodableError(fmt.Errorf("Hook NotificationWillBePushed called but not implemented."))
 	}
@@ -5869,31 +5871,31 @@ func (s *apiRPCServer) GetUploadSession(args *Z_GetUploadSessionArgs, returns *Z
 	return nil
 }
 
-type Z_SendPluginPushNotificationArgs struct {
+type Z_SendPushNotificationArgs struct {
-	A *model.PluginPushNotification
+	A *model.PushNotification
+	B string
 }
 
-type Z_SendPluginPushNotificationReturns struct {
+type Z_SendPushNotificationReturns struct {
-	A error
+	A *model.AppError
 }
 
-func (g *apiRPCClient) SendPluginPushNotification(notification *model.PluginPushNotification) error {
+func (g *apiRPCClient) SendPushNotification(notification *model.PushNotification, userID string) *model.AppError {
-	_args := &Z_SendPluginPushNotificationArgs{notification}
+	_args := &Z_SendPushNotificationArgs{notification, userID}
-	_returns := &Z_SendPluginPushNotificationReturns{}
+	_returns := &Z_SendPushNotificationReturns{}
-	if err := g.client.Call("Plugin.SendPluginPushNotification", _args, _returns); err != nil {
+	if err := g.client.Call("Plugin.SendPushNotification", _args, _returns); err != nil {
-		log.Printf("RPC call to SendPluginPushNotification API failed: %s", err.Error())
+		log.Printf("RPC call to SendPushNotification API failed: %s", err.Error())
 	}
 	return _returns.A
 }
 
-func (s *apiRPCServer) SendPluginPushNotification(args *Z_SendPluginPushNotificationArgs, returns *Z_SendPluginPushNotificationReturns) error {
+func (s *apiRPCServer) SendPushNotification(args *Z_SendPushNotificationArgs, returns *Z_SendPushNotificationReturns) error {
 	if hook, ok := s.impl.(interface {
-		SendPluginPushNotification(notification *model.PluginPushNotification) error
+		SendPushNotification(notification *model.PushNotification, userID string) *model.AppError
 	}); ok {
-		returns.A = hook.SendPluginPushNotification(args.A)
+		returns.A = hook.SendPushNotification(args.A, args.B)
-		returns.A = encodableError(returns.A)
 	} else {
-		return encodableError(fmt.Errorf("API SendPluginPushNotification called but not implemented."))
+		return encodableError(fmt.Errorf("API SendPushNotification called but not implemented."))
 	}
 	return nil
 }

server/public/plugin/hooks.go

@@ -287,13 +287,15 @@ type Hooks interface {
 	ConfigurationWillBeSaved(newCfg *model.Config) (*model.Config, error)
 
 	// NotificationWillBePushed is invoked before a push notification is sent to the push
-	// notification server. The intention is to allow plugins to cancel a push notification.
+	// notification server.
 	//
-	// To cancel a push notification, return true.
+	// To reject a notification, return an non-empty string describing why the notification was rejected.
+	// To modify the notification, return the replacement, non-nil *model.PushNotification and an empty string.
+	// To allow the notification without modification, return a nil *model.PushNotification and an empty string.
 	//
-	// Note that this method will be called for push notification sent by plugins, including
+	// Note that this method will be called for push notifications created by plugins, including the plugin that
-	// the plugin that created the push notification.
+	// created the notification.
 	//
 	// Minimum server version: 9.0
-	NotificationWillBePushed(pushNotification *model.PluginPushNotification) (cancel bool)
+	NotificationWillBePushed(pushNotification *model.PushNotification, userID string) (*model.PushNotification, string)
 }

server/public/plugin/hooks_timer_layer_generated.go

@@ -219,9 +219,9 @@ func (hooks *hooksTimerLayer) ConfigurationWillBeSaved(newCfg *model.Config) (*m
 	return _returnsA, _returnsB
 }
 
-func (hooks *hooksTimerLayer) NotificationWillBePushed(pushNotification *model.PluginPushNotification) (cancel bool) {
+func (hooks *hooksTimerLayer) NotificationWillBePushed(pushNotification *model.PushNotification, userID string) (*model.PushNotification, string) {
 	startTime := timePkg.Now()
-	_returnsA := hooks.hooksImpl.NotificationWillBePushed(pushNotification)
+	_returnsA, _returnsB := hooks.hooksImpl.NotificationWillBePushed(pushNotification, userID)
 	hooks.recordTime(startTime, "NotificationWillBePushed", true)
-	return _returnsA
+	return _returnsA, _returnsB
 }

server/public/plugin/plugintest/api.go

@@ -3639,15 +3639,17 @@ func (_m *API) SendMail(to string, subject string, htmlBody string) *model.AppEr
 	return r0
 }
 
-// SendPluginPushNotification provides a mock function with given fields: notification
+// SendPushNotification provides a mock function with given fields: notification, userID
-func (_m *API) SendPluginPushNotification(notification *model.PluginPushNotification) error {
+func (_m *API) SendPushNotification(notification *model.PushNotification, userID string) *model.AppError {
-	ret := _m.Called(notification)
+	ret := _m.Called(notification, userID)
 
-	var r0 error
+	var r0 *model.AppError
-	if rf, ok := ret.Get(0).(func(*model.PluginPushNotification) error); ok {
+	if rf, ok := ret.Get(0).(func(*model.PushNotification, string) *model.AppError); ok {
-		r0 = rf(notification)
+		r0 = rf(notification, userID)
 	} else {
-		r0 = ret.Error(0)
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*model.AppError)
+		}
 	}
 
 	return r0

server/public/plugin/plugintest/hooks.go

@@ -193,18 +193,30 @@ func (_m *Hooks) MessageWillBeUpdated(c *plugin.Context, newPost *model.Post, ol
 	return r0, r1
 }
 
-// NotificationWillBePushed provides a mock function with given fields: pushNotification
+// NotificationWillBePushed provides a mock function with given fields: pushNotification, userID
-func (_m *Hooks) NotificationWillBePushed(pushNotification *model.PluginPushNotification) bool {
+func (_m *Hooks) NotificationWillBePushed(pushNotification *model.PushNotification, userID string) (*model.PushNotification, string) {
-	ret := _m.Called(pushNotification)
+	ret := _m.Called(pushNotification, userID)
 
-	var r0 bool
+	var r0 *model.PushNotification
-	if rf, ok := ret.Get(0).(func(*model.PluginPushNotification) bool); ok {
+	var r1 string
-		r0 = rf(pushNotification)
+	if rf, ok := ret.Get(0).(func(*model.PushNotification, string) (*model.PushNotification, string)); ok {
+		return rf(pushNotification, userID)
+	}
+	if rf, ok := ret.Get(0).(func(*model.PushNotification, string) *model.PushNotification); ok {
+		r0 = rf(pushNotification, userID)
 	} else {
-		r0 = ret.Get(0).(bool)
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*model.PushNotification)
+		}
 	}
 
-	return r0
+	if rf, ok := ret.Get(1).(func(*model.PushNotification, string) string); ok {
+		r1 = rf(pushNotification, userID)
+	} else {
+		r1 = ret.Get(1).(string)
+	}
+
+	return r0, r1
 }
 
 // OnActivate provides a mock function with given fields:

server/public/plugin/product_hooks_generated.go

@@ -119,7 +119,7 @@ type ConfigurationWillBeSavedIFace interface {
 }
 
 type NotificationWillBePushedIFace interface {
-	NotificationWillBePushed(pushNotification *model.PluginPushNotification) (cancel bool)
+	NotificationWillBePushed(pushNotification *model.PushNotification, userID string) (*model.PushNotification, string)
 }
 
 type HooksAdapter struct {
@@ -615,11 +615,11 @@ func (a *HooksAdapter) ConfigurationWillBeSaved(newCfg *model.Config) (*model.Co
 
 }
 
-func (a *HooksAdapter) NotificationWillBePushed(pushNotification *model.PluginPushNotification) (cancel bool) {
+func (a *HooksAdapter) NotificationWillBePushed(pushNotification *model.PushNotification, userID string) (*model.PushNotification, string) {
 	if _, ok := a.implemented[NotificationWillBePushedID]; !ok {
 		panic("product hooks must implement NotificationWillBePushed")
 	}
 
-	return a.productHooks.(NotificationWillBePushedIFace).NotificationWillBePushed(pushNotification)
+	return a.productHooks.(NotificationWillBePushedIFace).NotificationWillBePushed(pushNotification, userID)
 
 }