Skip to content

Commit

Permalink
MM-55060 - Add PushSubTypeCalls for calls-specific push notifications (
Browse files Browse the repository at this point in the history
…#564)

* update server dep; use PushSubTypeCalls

* ripping out loadPushProxyVersion and tests

to merge with prev commit

* update server dep again (for correct calls sub_type)

* goodbye old friend

* update to master branch commit of mattermost server
  • Loading branch information
cpoile authored Nov 16, 2023
1 parent 47cd7a7 commit 685496a
Show file tree
Hide file tree
Showing 7 changed files with 5 additions and 156 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ require (
github.com/mattermost/calls-recorder v0.4.2
github.com/mattermost/logr/v2 v2.0.16
github.com/mattermost/mattermost-plugin-calls/server/public v0.0.1
github.com/mattermost/mattermost/server/public v0.0.10-0.20231109142142-8ffda2b73ea8
github.com/mattermost/mattermost/server/public v0.0.11-0.20231115180603-759bb70b2f44
github.com/mattermost/rtcd v0.12.0
github.com/mattermost/squirrel v0.2.0
github.com/pion/interceptor v0.1.25
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,8 @@ github.com/mattermost/ldap v3.0.4+incompatible h1:SOeNnz+JNR+foQ3yHkYqijb9MLPhXN
github.com/mattermost/ldap v3.0.4+incompatible/go.mod h1:b4reDCcGpBxJ4WX0f224KFY+OR0npin7or7EFpeIko4=
github.com/mattermost/logr/v2 v2.0.16 h1:jnePX4cPskC3WDFvUardh/xZfxNdsFXbEERJQ1kUEDE=
github.com/mattermost/logr/v2 v2.0.16/go.mod h1:1dm/YhTpozsqANXxo5Pi5zYLBsal2xY0pX+JZNbzYJY=
github.com/mattermost/mattermost/server/public v0.0.10-0.20231109142142-8ffda2b73ea8 h1:tZhcvX1k526YLHeO8lQpFa8J2Ejr7sHEYdzst5xrIjA=
github.com/mattermost/mattermost/server/public v0.0.10-0.20231109142142-8ffda2b73ea8/go.mod h1:AMuKlAabVFnLvf5bXg+69EdvtLIfty4ahFossB34LM4=
github.com/mattermost/mattermost/server/public v0.0.11-0.20231115180603-759bb70b2f44 h1:aTq8Vvdme0N8G2ZYEbKNlEBuXDr8MACPUmImIieKELc=
github.com/mattermost/mattermost/server/public v0.0.11-0.20231115180603-759bb70b2f44/go.mod h1:AMuKlAabVFnLvf5bXg+69EdvtLIfty4ahFossB34LM4=
github.com/mattermost/rtcd v0.12.0 h1:T/07JYExpwMmWq58QIp8KU08FWOzVmmatzLaEOIMP4A=
github.com/mattermost/rtcd v0.12.0/go.mod h1:s0J7u93n2e2YPATEWCiiJ19iQf2TTHJapXQR2POE8KA=
github.com/mattermost/squirrel v0.2.0 h1:8ZWeyf+MWQ2cL7hu9REZgLtz2IJi51qqZEovI3T3TT8=
Expand Down
2 changes: 0 additions & 2 deletions server/activate.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,6 @@ func (p *Plugin) OnActivate() error {
}()
}

p.loadPushProxyVersion()

if rtcdURL := cfg.getRTCDURL(); rtcdURL != "" && p.licenseChecker.RTCDAllowed() {
rtcdManager, err := p.newRTCDClientManager(rtcdURL)
if err != nil {
Expand Down
3 changes: 0 additions & 3 deletions server/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@ type Plugin struct {
// Database handle to the writer DB node
wDB *sql.DB
driverName string

// Push-proxy's version, refreshed when the plugin activates
pushProxyVersion string
}

func (p *Plugin) startSession(us *session, senderID string) {
Expand Down
10 changes: 2 additions & 8 deletions server/push_notifications.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import (
"github.com/mattermost/mattermost/server/public/model"
)

const minPushProxyVersionWithCalls = "5.27.0"

func (p *Plugin) NotificationWillBePushed(notification *model.PushNotification, userID string) (*model.PushNotification, string) {
// We will use our own notifications if:
// 1. This is a call start post
Expand Down Expand Up @@ -60,19 +58,15 @@ func (p *Plugin) sendPushNotifications(channelID, createdPostID, threadID string
return
}

pushType := model.PushTypeMessage
if err := checkMinVersion(minPushProxyVersionWithCalls, p.pushProxyVersion); err == nil {
pushType = model.PushTypeCalls
}

for _, member := range members {
if member.Id == sender.Id {
continue
}

msg := &model.PushNotification{
Version: model.PushMessageV2,
Type: pushType,
Type: model.PushTypeMessage,
SubType: model.PushSubTypeCalls,
TeamId: channel.TeamId,
ChannelId: channelID,
PostId: createdPostID,
Expand Down
61 changes: 0 additions & 61 deletions server/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package main
import (
"bytes"
"compress/zlib"
"encoding/json"
"errors"
"fmt"
"github.com/mattermost/mattermost/server/public/model"
Expand Down Expand Up @@ -65,58 +64,6 @@ func (p *Plugin) getNotificationNameFormat(userID string) string {
return *config.TeamSettings.TeammateNameDisplay
}

func (p *Plugin) loadPushProxyVersion() {
config := p.API.GetConfig()
if err := p.canSendPushNotifications(config, p.API.GetLicense()); err != nil {
p.LogWarn(err.Error())
return
}

client := NewClient()
var err error
p.pushProxyVersion, err = getPushProxyVersion(client, config)
if err != nil {
p.LogError(err.Error())
}
}

// getPushProxyVersion will return the version if the push proxy is reachable and version >= 5.27.0
// which is when the "/version" endpoint was added. Otherwise it will return "" for lower versions and for
// failed attempts to get the version (which could mean only that the push proxy was unavailable temporarily)
func getPushProxyVersion(client *http.Client, config *model.Config) (string, error) {
if config == nil ||
config.EmailSettings.PushNotificationServer == nil ||
*config.EmailSettings.PushNotificationServer == "" {
return "", nil
}
serverURL := strings.TrimRight(*config.EmailSettings.PushNotificationServer, "/") + "/version"
req, err := http.NewRequest("GET", serverURL, nil)
if err != nil {
return "", fmt.Errorf("failed to build request, err: %w", err)
}

resp, err := client.Do(req)
if err != nil {
return "", fmt.Errorf("http request failed, err: %w", err)
}
defer resp.Body.Close()

if resp.StatusCode == http.StatusOK {
var respData = struct {
Version string
Hash string
}{}
if err := json.NewDecoder(resp.Body).Decode(&respData); err != nil {
return "", fmt.Errorf("failed to decode http response, err: %w", err)
}

return respData.Version, nil
}

// Must not be newer version of push proxy
return "", nil
}

func (p *Plugin) canSendPushNotifications(config *model.Config, license *model.License) error {
if config == nil ||
config.EmailSettings.SendPushNotifications == nil ||
Expand Down Expand Up @@ -248,11 +195,3 @@ func mapKeys[K comparable, V any](m map[K]V) []K {
}
return keys
}

// NewClient creates a SimpleClient intended for one-off requests, like getPushProxyVersion.
// If we end up needing something more long term, we should consider storing it.
func NewClient() *http.Client {
return &http.Client{
Timeout: 10 * time.Second,
}
}
79 changes: 0 additions & 79 deletions server/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,82 +232,3 @@ func TestPlugin_canSendPushNotifications(t *testing.T) {
})
}
}

func TestPlugin_getPushProxyVersion(t *testing.T) {
config := &model.Config{
EmailSettings: model.EmailSettings{
PushNotificationServer: model.NewString(model.MHPNS),
},
}

tests := []struct {
name string
config *model.Config
want string
handler func(http.ResponseWriter)
}{
{
name: "config nil",
config: nil,
want: "",
},
{
name: "no push server",
config: &model.Config{
EmailSettings: model.EmailSettings{
PushNotificationServer: model.NewString(""),
},
},
want: "",
},
{
name: "nil push server",
config: &model.Config{
EmailSettings: model.EmailSettings{
PushNotificationServer: nil,
},
},
want: "",
},
{
name: "404 (old push proxy)",
config: config,
want: "",
handler: func(w http.ResponseWriter) {
w.WriteHeader(http.StatusNotFound)
},
},
{
name: "got a version",
config: config,
want: "5.37.0",
handler: func(w http.ResponseWriter) {
_, _ = w.Write([]byte(`{"version": "5.37.0", "hash": "abcde3445"}`))
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
assert.Equal(t, "/version", req.URL.String())
if tt.handler != nil {
tt.handler(w)
}
}))
defer server.Close()

// Do we have a push server url? if so, rewrite it for the test.
// Why are we doing it this way? We want to test the variety of different states the config
// can be in, as well as the responses the push proxy can give us.
if tt.config != nil &&
tt.config.EmailSettings.PushNotificationServer != nil &&
*tt.config.EmailSettings.PushNotificationServer != "" {
tt.config.EmailSettings.PushNotificationServer = model.NewString(server.URL)
}
ret, err := getPushProxyVersion(server.Client(), tt.config)

assert.NoError(t, err)
assert.Equalf(t, tt.want, ret, "test name: %s", tt.name)
})
}
}

0 comments on commit 685496a

Please sign in to comment.