Skip to content

Commit

Permalink
[MI-3132] Fixed the issue of infinite loop when editing messages and …
Browse files Browse the repository at this point in the history 
…a potential bug related to no. of channel members (#154)

* [MI-3132] Fixed the issue of infinite loop when editing messages and fixed a potential bug related to no. of channel members
Added the logic to continue the flow if we get PaymentRequired error from Graph API when updating a message
Increased the no. of channel members that we are fetching from the Mattermost API
Modified the unit tests

* [MI-3132] Added a comment
  • Loading branch information
@manojmalik20
manojmalik20 authored Jun 1, 2023
1 parent abd6df6 commit 5b68612
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 23 deletions.
2 changes: 1 addition & 1 deletion server/handlers/handlers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ func (ah *ActivityHandler) handleUpdatedActivity(activityIds msteams.ActivityIds
return
}

// Ingnore if the change is already applied in the database
// Ignore if the change is already applied in the database
if postInfo.MSTeamsLastUpdateAt.UnixMicro() == msg.LastUpdateAt.UnixMicro() {
return
}
Expand Down
15 changes: 10 additions & 5 deletions server/message_hooks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"bytes"
"fmt"
"math"
"regexp"
"strings"
"time"
Expand All @@ -24,7 +25,7 @@ func (p *Plugin) MessageWillBePosted(_ *plugin.Context, post *model.Post) (*mode
return post, ""
}
if channel.Type == model.ChannelTypeDirect || channel.Type == model.ChannelTypeGroup {
members, err := p.API.GetChannelMembers(channel.Id, 0, 10)
members, err := p.API.GetChannelMembers(channel.Id, 0, math.MaxInt32)
if err != nil {
return post, ""
}
Expand Down Expand Up @@ -68,7 +69,7 @@ func (p *Plugin) MessageHasBeenPosted(_ *plugin.Context, post *model.Post) {
return
}
if (channel.Type == model.ChannelTypeDirect || channel.Type == model.ChannelTypeGroup) && p.getConfiguration().SyncDirectMessages {
members, appErr := p.API.GetChannelMembers(post.ChannelId, 0, 10)
members, appErr := p.API.GetChannelMembers(post.ChannelId, 0, math.MaxInt32)
if appErr != nil {
return
}
Expand Down Expand Up @@ -184,7 +185,7 @@ func (p *Plugin) MessageHasBeenUpdated(_ *plugin.Context, newPost, oldPost *mode
return
}

members, appErr := p.API.GetChannelMembers(newPost.ChannelId, 0, 10)
members, appErr := p.API.GetChannelMembers(newPost.ChannelId, 0, math.MaxInt32)
if appErr != nil {
return
}
Expand Down Expand Up @@ -586,7 +587,11 @@ func (p *Plugin) Update(teamID, channelID string, user *model.User, newPost, old

if err = client.UpdateMessage(teamID, channelID, parentID, postInfo.MSTeamsID, content, mentions); err != nil {
p.API.LogWarn("Error updating the post", "error", err)
return err
// If the error is regarding payment required for metered APIs, ignore it and continue because
// the post is updated regardless
if !strings.Contains(err.Error(), "code: PaymentRequired") {
return err
}
}

var updatedMessage *msteams.Message
Expand Down Expand Up @@ -663,7 +668,7 @@ func (p *Plugin) GetChatIDForChannel(clientUserID string, channelID string) (str
return "", errors.New("invalid channel type, chatID is only available for direct messages and group messages")
}

members, appErr := p.API.GetChannelMembers(channelID, 0, 10)
members, appErr := p.API.GetChannelMembers(channelID, 0, math.MaxInt32)
if appErr != nil {
return "", appErr
}
Expand Down
35 changes: 18 additions & 17 deletions server/message_hooks_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"bytes"
"errors"
"math"
"testing"
"time"

Expand Down Expand Up @@ -39,15 +40,15 @@ func TestMessageWillBePosted(t *testing.T) {
Name: "MessageWillBePosted: Unable to get the channel members",
SetupAPI: func(api *plugintest.API) {
api.On("GetChannel", testutils.GetChannelID()).Return(testutils.GetChannel(model.ChannelTypeDirect), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, 10).Return(nil, testutils.GetInternalServerAppError("unable to get the channel members")).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, math.MaxInt32).Return(nil, testutils.GetInternalServerAppError("unable to get the channel members")).Times(1)
},
ExpectedPost: testutils.GetPost(testutils.GetChannelID(), testutils.GetUserID()),
},
{
Name: "MessageWillBePosted: Unable to get the user",
SetupAPI: func(api *plugintest.API) {
api.On("GetChannel", testutils.GetChannelID()).Return(testutils.GetChannel(model.ChannelTypeDirect), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, 10).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, math.MaxInt32).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetUser", testutils.GetID()).Return(nil, testutils.GetInternalServerAppError("unable to get the user")).Times(1)
},
ExpectedPost: testutils.GetPost(testutils.GetChannelID(), testutils.GetUserID()),
Expand All @@ -56,7 +57,7 @@ func TestMessageWillBePosted(t *testing.T) {
Name: "MessageWillBePosted: User is ms teams user",
SetupAPI: func(api *plugintest.API) {
api.On("GetChannel", testutils.GetChannelID()).Return(testutils.GetChannel(model.ChannelTypeDirect), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, 10).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, math.MaxInt32).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetUser", testutils.GetID()).Return(testutils.GetUser(model.SystemAdminRoleId, "test@msteamssync"), nil).Times(1)
api.On("GetUser", testutils.GetID()).Return(&model.User{
Username: "msteams_mockUser",
Expand All @@ -70,7 +71,7 @@ func TestMessageWillBePosted(t *testing.T) {
Name: "MessageWillBePosted: User is not ms teams user",
SetupAPI: func(api *plugintest.API) {
api.On("GetChannel", testutils.GetChannelID()).Return(testutils.GetChannel(model.ChannelTypeDirect), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, 10).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, math.MaxInt32).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetUser", testutils.GetID()).Return(testutils.GetUser(model.SystemAdminRoleId, "[email protected]"), nil).Times(2)
},
ExpectedPost: testutils.GetPost(testutils.GetChannelID(), testutils.GetUserID()),
Expand Down Expand Up @@ -323,7 +324,7 @@ func TestMessageHasBeenUpdated(t *testing.T) {
SetupAPI: func(api *plugintest.API) {
api.On("GetUser", testutils.GetID()).Return(testutils.GetUser(model.SystemAdminRoleId, "[email protected]"), nil).Times(1)
api.On("GetChannel", testutils.GetChannelID()).Return(testutils.GetChannel(model.ChannelTypeDirect), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, 10).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, math.MaxInt32).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetConfig").Return(&model.Config{ServiceSettings: model.ServiceSettings{SiteURL: model.NewString("/")}}, nil).Times(2)
},
SetupStore: func(store *storemocks.Store) {
Expand Down Expand Up @@ -375,7 +376,7 @@ func TestMessageHasBeenUpdated(t *testing.T) {
SetupAPI: func(api *plugintest.API) {
api.On("GetUser", testutils.GetID()).Return(testutils.GetUser(model.SystemAdminRoleId, "[email protected]"), nil).Times(1)
api.On("GetChannel", testutils.GetChannelID()).Return(testutils.GetChannel(model.ChannelTypeDirect), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, 10).Return(nil, testutils.GetInternalServerAppError("unable to get channel members")).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, math.MaxInt32).Return(nil, testutils.GetInternalServerAppError("unable to get channel members")).Times(1)
},
SetupStore: func(store *storemocks.Store) {
store.On("GetLinkByChannelID", testutils.GetChannelID()).Return(nil, nil).Times(1)
Expand All @@ -390,7 +391,7 @@ func TestMessageHasBeenUpdated(t *testing.T) {
api.On("LogError", "Unable to handle message update", "error", mock.Anything).Times(1)
api.On("GetUser", testutils.GetID()).Return(testutils.GetUser(model.SystemAdminRoleId, "[email protected]"), nil).Times(1)
api.On("GetChannel", testutils.GetChannelID()).Return(testutils.GetChannel(model.ChannelTypeDirect), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, 10).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, math.MaxInt32).Return(testutils.GetChannelMembers(2), nil).Times(1)
},
SetupStore: func(store *storemocks.Store) {
store.On("GetLinkByChannelID", testutils.GetChannelID()).Return(nil, nil).Times(1)
Expand All @@ -407,7 +408,7 @@ func TestMessageHasBeenUpdated(t *testing.T) {
SetupAPI: func(api *plugintest.API) {
api.On("GetUser", testutils.GetID()).Return(testutils.GetUser(model.SystemAdminRoleId, "[email protected]"), nil).Times(1)
api.On("GetChannel", testutils.GetChannelID()).Return(testutils.GetChannel(model.ChannelTypeDirect), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, 10).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, math.MaxInt32).Return(testutils.GetChannelMembers(2), nil).Times(1)
},
SetupStore: func(store *storemocks.Store) {
store.On("GetLinkByChannelID", testutils.GetChannelID()).Return(nil, nil).Times(1)
Expand Down Expand Up @@ -536,7 +537,7 @@ func TestSetChatReaction(t *testing.T) {
SetupAPI: func(api *plugintest.API) {
api.On("LogWarn", "Error creating post reaction", "error", mock.Anything)
api.On("GetChannel", testutils.GetChannelID()).Return(testutils.GetChannel(model.ChannelTypeDirect), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, 10).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, math.MaxInt32).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetConfig").Return(&model.Config{ServiceSettings: model.ServiceSettings{SiteURL: model.NewString("/")}}, nil).Times(2)
},
SetupStore: func(store *storemocks.Store) {
Expand All @@ -553,7 +554,7 @@ func TestSetChatReaction(t *testing.T) {
Name: "SetChatReaction: Valid",
SetupAPI: func(api *plugintest.API) {
api.On("GetChannel", testutils.GetChannelID()).Return(testutils.GetChannel(model.ChannelTypeDirect), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, 10).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, math.MaxInt32).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetConfig").Return(&model.Config{ServiceSettings: model.ServiceSettings{SiteURL: model.NewString("/")}}, nil).Times(2)
},
SetupStore: func(store *storemocks.Store) {
Expand Down Expand Up @@ -712,7 +713,7 @@ func TestUnsetChatReaction(t *testing.T) {
SetupAPI: func(api *plugintest.API) {
api.On("LogWarn", "Error creating post", "error", mock.Anything)
api.On("GetChannel", testutils.GetChannelID()).Return(testutils.GetChannel(model.ChannelTypeDirect), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, 10).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, math.MaxInt32).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetConfig").Return(&model.Config{ServiceSettings: model.ServiceSettings{SiteURL: model.NewString("/")}}, nil).Times(2)
},
SetupStore: func(store *storemocks.Store) {
Expand All @@ -729,7 +730,7 @@ func TestUnsetChatReaction(t *testing.T) {
Name: "UnsetChatReaction: Valid",
SetupAPI: func(api *plugintest.API) {
api.On("GetChannel", testutils.GetChannelID()).Return(testutils.GetChannel(model.ChannelTypeDirect), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, 10).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, math.MaxInt32).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetConfig").Return(&model.Config{ServiceSettings: model.ServiceSettings{SiteURL: model.NewString("/")}}, nil).Times(2)
},
SetupStore: func(store *storemocks.Store) {
Expand Down Expand Up @@ -1604,7 +1605,7 @@ func TestGetChatIDForChannel(t *testing.T) {
Name: "GetChatIDForChannel: Unable to get the channel members",
SetupAPI: func(api *plugintest.API) {
api.On("GetChannel", testutils.GetChannelID()).Return(testutils.GetChannel(model.ChannelTypeGroup), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, 10).Return(nil, testutils.GetInternalServerAppError("unable to get the channel members")).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, math.MaxInt32).Return(nil, testutils.GetInternalServerAppError("unable to get the channel members")).Times(1)
},
SetupStore: func(store *storemocks.Store) {},
SetupClient: func(client *clientmocks.Client, uclient *clientmocks.Client) {},
Expand All @@ -1614,7 +1615,7 @@ func TestGetChatIDForChannel(t *testing.T) {
Name: "GetChatIDForChannel: Unable to store users",
SetupAPI: func(api *plugintest.API) {
api.On("GetChannel", testutils.GetChannelID()).Return(testutils.GetChannel(model.ChannelTypeGroup), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, 10).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, math.MaxInt32).Return(testutils.GetChannelMembers(2), nil).Times(1)
},
SetupStore: func(store *storemocks.Store) {
store.On("MattermostToTeamsUserID", testutils.GetID()).Return("", errors.New("unable to store the user")).Times(1)
Expand All @@ -1626,7 +1627,7 @@ func TestGetChatIDForChannel(t *testing.T) {
Name: "GetChatIDForChannel: Unable to get the client",
SetupAPI: func(api *plugintest.API) {
api.On("GetChannel", testutils.GetChannelID()).Return(testutils.GetChannel(model.ChannelTypeGroup), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, 10).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, math.MaxInt32).Return(testutils.GetChannelMembers(2), nil).Times(1)
},
SetupStore: func(store *storemocks.Store) {
store.On("MattermostToTeamsUserID", testutils.GetID()).Return("mockTeamsUserID", nil).Times(2)
Expand All @@ -1639,7 +1640,7 @@ func TestGetChatIDForChannel(t *testing.T) {
Name: "GetChatIDForChannel: Unable to create or get chat for users",
SetupAPI: func(api *plugintest.API) {
api.On("GetChannel", testutils.GetChannelID()).Return(testutils.GetChannel(model.ChannelTypeGroup), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, 10).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, math.MaxInt32).Return(testutils.GetChannelMembers(2), nil).Times(1)
},
SetupStore: func(store *storemocks.Store) {
store.On("MattermostToTeamsUserID", testutils.GetID()).Return("mockTeamsUserID", nil).Times(2)
Expand All @@ -1654,7 +1655,7 @@ func TestGetChatIDForChannel(t *testing.T) {
Name: "GetChatIDForChannel: Valid",
SetupAPI: func(api *plugintest.API) {
api.On("GetChannel", testutils.GetChannelID()).Return(testutils.GetChannel(model.ChannelTypeGroup), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, 10).Return(testutils.GetChannelMembers(2), nil).Times(1)
api.On("GetChannelMembers", testutils.GetChannelID(), 0, math.MaxInt32).Return(testutils.GetChannelMembers(2), nil).Times(1)
},
SetupStore: func(store *storemocks.Store) {
store.On("MattermostToTeamsUserID", testutils.GetID()).Return("mockTeamsUserID", nil).Times(2)
Expand Down

0 comments on commit 5b68612

Please sign in to comment.