From c18d5778ea51d8ece48d440dd6169fc6eb6a98ad Mon Sep 17 00:00:00 2001 From: kshitij katiyar Date: Mon, 30 Sep 2024 18:04:20 +0530 Subject: [PATCH 01/13] [MM-820]: Added testcases for multiple server files --- server/command_test.go | 52 +++++++++ server/state_test.go | 236 +++++++++++++++++++++++++++++++++++++++++ server/user.go | 2 +- server/user_test.go | 103 ++++++++++++++++++ 4 files changed, 392 insertions(+), 1 deletion(-) create mode 100644 server/state_test.go diff --git a/server/command_test.go b/server/command_test.go index d602873..135473f 100644 --- a/server/command_test.go +++ b/server/command_test.go @@ -410,3 +410,55 @@ func TestGetHelpText(t *testing.T) { actual := p.getHelpText() require.Equal(t, expected, actual) } + +func TestExecuteCommand(t *testing.T) { + api := &plugintest.API{} + p := &Plugin{ + MattermostPlugin: plugin.MattermostPlugin{ + API: api, + }, + } + var dummyPluginContext plugin.Context + + tests := []struct { + name string + commandArgs model.CommandArgs + expectedMsg string + }{ + { + name: "Invalid Command", + commandArgs: model.CommandArgs{ + Command: "/dummyCommand start", + ChannelId: "dummyChannelID", + UserId: "dummyUserID", + }, + expectedMsg: "Command '/dummyCommand' is not /mstmeetings. Please try again.", + }, + { + name: "Successful execution of Help Command", + commandArgs: model.CommandArgs{ + Command: "/mstmeetings help", + ChannelId: "dummyChannelID", + UserId: "dummyUserID", + }, + expectedMsg: "###### Mattermost MS Teams Meetings Plugin - Slash Command Help\n* `/mstmeetings start` - Start an MS Teams meeting. \n* `/mstmeetings connect` - Connect to MS Teams meeting. \n* `/mstmeetings disconnect` - Disconnect your Mattermost account from MS Teams. \n* `/mstmeetings help` - Display this help text.", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + post := &model.Post{ + UserId: p.botUserID, + ChannelId: tt.commandArgs.ChannelId, + Message: tt.expectedMsg, + } + + api.On("SendEphemeralPost", tt.commandArgs.UserId, post).Return(&model.Post{}).Once() + + response, _ := p.ExecuteCommand(&dummyPluginContext, &tt.commandArgs) + + require.Equal(t, &model.CommandResponse{}, response) + api.AssertExpectations(t) + }) + } +} diff --git a/server/state_test.go b/server/state_test.go new file mode 100644 index 0000000..5bf95d0 --- /dev/null +++ b/server/state_test.go @@ -0,0 +1,236 @@ +package main + +import ( + "testing" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/plugin" + "github.com/mattermost/mattermost/server/public/plugin/plugintest" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestStoreState(t *testing.T) { + testCases := []struct { + name string + userID string + channelID string + justConnect bool + returnError error + expectError bool + expectedState string + expectedErrMsg string + }{ + { + name: "Store state successful", + userID: "userID1", + channelID: "channelID1", + justConnect: true, + returnError: nil, + expectError: false, + expectedState: "msteamsmeetinguserstate_userID1_channelID1_true", + }, + { + name: "Error occurred while storing state", + userID: "userID2", + channelID: "channelID2", + justConnect: false, + returnError: &model.AppError{Message: "error occurred while storing state"}, + expectError: true, + expectedErrMsg: "error occurred while storing state", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + api := &plugintest.API{} + p := &Plugin{ + MattermostPlugin: plugin.MattermostPlugin{ + API: api, + }, + } + + api.On("KVSet", mock.Anything, mock.Anything).Return(tc.returnError) + + state, err := p.StoreState(tc.userID, tc.channelID, tc.justConnect) + + if tc.expectError { + require.Error(t, err) + require.Equal(t, tc.expectedErrMsg, err.Error()) + } else { + require.NoError(t, err) + require.Equal(t, tc.expectedState, state) + } + }) + } +} + +func TestGetState(t *testing.T) { + testCases := []struct { + name string + key string + returnValue []byte + returnError error + expectedState string + expectError bool + expectedErrMsg string + }{ + { + name: "Valid state retrieved", + key: "dummyKey", + returnValue: []byte("dummyState"), + returnError: nil, + expectedState: "dummyState", + expectError: false, + }, + { + name: "Error occurred while getting stored state", + key: "dummyKey", + returnValue: []byte(""), + returnError: &model.AppError{Message: "error occurred while getting stored state"}, + expectedState: "", + expectError: true, + expectedErrMsg: "error occurred while getting stored state", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + api := &plugintest.API{} + p := &Plugin{ + MattermostPlugin: plugin.MattermostPlugin{ + API: api, + }, + } + + api.On("KVGet", tc.key).Return(tc.returnValue, tc.returnError) + + state, err := p.GetState(tc.key) + if tc.expectError { + require.Error(t, err) + require.Equal(t, tc.expectedState, state) + require.Equal(t, tc.expectedErrMsg, err.Error()) + } else { + require.NoError(t, err) + require.Equal(t, tc.expectedState, state) + } + }) + } +} + +func TestDeleteState(t *testing.T) { + testCases := []struct { + name string + key string + returnError error + expectError bool + expectedErrMsg string + }{ + { + name: "Delete state successful", + key: "dummyKey", + returnError: nil, + expectError: false, + }, + { + name: "Error occurred while deleting state", + key: "dummyKey", + returnError: &model.AppError{Message: "error occurred while deleting state"}, + expectError: true, + expectedErrMsg: "error occurred while deleting state", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + api := &plugintest.API{} + p := &Plugin{ + MattermostPlugin: plugin.MattermostPlugin{ + API: api, + }, + } + + api.On("KVDelete", tc.key).Return(tc.returnError) + + err := p.DeleteState(tc.key) + if tc.expectError { + require.Error(t, err) + require.Equal(t, tc.expectedErrMsg, err.Error()) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestParseState(t *testing.T) { + testCases := []struct { + name string + state string + expectedKey string + expectedUserID string + expectedChannelID string + expectedJustConnect bool + expectError bool + expectedErrMsg string + }{ + { + name: "Parse state successful", + state: "key1_userID1_channelID1_true", + expectedKey: "key1_userID1", + expectedUserID: "userID1", + expectedChannelID: "channelID1", + expectedJustConnect: true, + expectError: false, + }, + { + name: "State length mismatch", + state: "key1_userID1_channelID1", + expectedKey: "", + expectedUserID: "", + expectedChannelID: "", + expectedJustConnect: false, + expectError: true, + expectedErrMsg: "status mismatch", + }, + { + name: "Invalid state format", + state: "key1_userID1", + expectedKey: "", + expectedUserID: "", + expectedChannelID: "", + expectedJustConnect: false, + expectError: true, + expectedErrMsg: "status mismatch", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + api := &plugintest.API{} + p := &Plugin{ + MattermostPlugin: plugin.MattermostPlugin{ + API: api, + }, + } + + // Set up the mock to expect LogDebug calls for error cases + if tc.expectError { + api.On("LogDebug", "complete oauth, state mismatch", "stateComponents", mock.Anything, "state", tc.state).Return() + } + + key, userID, channelID, justConnect, err := p.ParseState(tc.state) + + if tc.expectError { + require.Error(t, err) + require.Equal(t, tc.expectedErrMsg, err.Error()) + } else { + require.NoError(t, err) + require.Equal(t, tc.expectedKey, key) + require.Equal(t, tc.expectedUserID, userID) + require.Equal(t, tc.expectedChannelID, channelID) + require.Equal(t, tc.expectedJustConnect, justConnect) + } + }) + } +} diff --git a/server/user.go b/server/user.go index c69c0dc..646ad8b 100644 --- a/server/user.go +++ b/server/user.go @@ -227,7 +227,7 @@ func (p *Plugin) resetAllOAuthTokens() { p.API.LogInfo("OAuth2 configuration changed. Resetting all users' tokens, everyone will need to reconnect to MS Teams") appErr := p.API.KVDeleteAll() if appErr != nil { - p.API.LogError("failed to reset users' OAuth2 tokens", "error", appErr.Error()) + p.API.LogError("failed to reset user's OAuth2 tokens", "error", appErr.Error()) return } } diff --git a/server/user_test.go b/server/user_test.go index 5b52af1..c879525 100644 --- a/server/user_test.go +++ b/server/user_test.go @@ -4,6 +4,11 @@ import ( "testing" "time" + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/plugin" + "github.com/mattermost/mattermost/server/public/plugin/plugintest" + + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "golang.org/x/oauth2" ) @@ -38,3 +43,101 @@ func TestEncryptUserData(t *testing.T) { require.NoError(t, err) require.EqualValues(t, &expected, decrypted) } + +func TestStoreUserInfo(t *testing.T) { + tests := []struct { + name string + kvSetUserErr error + kvSetRemoteErr error + expectedErr string + }{ + { + name: "No Error", + kvSetUserErr: nil, + kvSetRemoteErr: nil, + expectedErr: "", + }, + { + name: "Error Saving UserID", + kvSetUserErr: &model.AppError{Message: "some error occurred while saving the user id"}, + kvSetRemoteErr: nil, + expectedErr: "some error occurred while saving the user id", + }, + { + name: "Error Saving RemoteID", + kvSetUserErr: nil, + kvSetRemoteErr: &model.AppError{Message: "some error occurred while saving the remote id"}, + expectedErr: "some error occurred while saving the remote id", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + api := &plugintest.API{} + p := &Plugin{ + MattermostPlugin: plugin.MattermostPlugin{ + API: api, + }, + } + p.setConfiguration(&configuration{ + EncryptionKey: "demo_encrypt_key", + }) + + dummyInfo := &UserInfo{ + UserID: "dummyUserID", + RemoteID: "dummyRemoteID", + } + + api.On("KVSet", "token_"+dummyInfo.UserID, mock.Anything).Return(tt.kvSetUserErr) + api.On("KVSet", "tbyrid_"+dummyInfo.RemoteID, mock.Anything).Return(tt.kvSetRemoteErr) + + responseErr := p.StoreUserInfo(dummyInfo) + if tt.expectedErr == "" { + require.NoError(t, responseErr) + } else { + require.Error(t, responseErr) + require.Equal(t, tt.expectedErr, responseErr.Error()) + } + }) + } +} + +func TestResetAllOAuthTokens(t *testing.T) { + tests := []struct { + name string + kvDeleteAllErr error + expectLogError bool + }{ + { + name: "No Error", + kvDeleteAllErr: nil, + expectLogError: false, + }, + { + name: "Error Deleting Tokens", + kvDeleteAllErr: &model.AppError{Message: "error in deleting all oauth token"}, + expectLogError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + api := &plugintest.API{} + p := &Plugin{ + MattermostPlugin: plugin.MattermostPlugin{ + API: api, + }, + } + + api.On("LogInfo", "OAuth2 configuration changed. Resetting all users' tokens, everyone will need to reconnect to MS Teams").Return(nil) + api.On("KVDeleteAll").Return(tt.kvDeleteAllErr) + + if tt.expectLogError { + api.On("LogError", "failed to reset user's OAuth2 tokens", "error", tt.kvDeleteAllErr.Error()).Return(nil) + } + + p.resetAllOAuthTokens() + api.AssertExpectations(t) + }) + } +} From 0cb6c6278d059a35d8f3833fadbd4cd86dab9352 Mon Sep 17 00:00:00 2001 From: kshitij katiyar Date: Tue, 1 Oct 2024 14:05:17 +0530 Subject: [PATCH 02/13] [MM-822]: added testcases for http.go --- server/http.go | 2 +- server/http_test.go | 609 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 610 insertions(+), 1 deletion(-) create mode 100644 server/http_test.go diff --git a/server/http.go b/server/http.go index 46ed1a9..0c6e690 100644 --- a/server/http.go +++ b/server/http.go @@ -65,7 +65,7 @@ func (p *Plugin) connectUser(w http.ResponseWriter, r *http.Request) { state, err := p.GetState(getOAuthUserStateKey(userID)) if err != nil { - p.API.LogError("connectUser, failed to store user state", "UserID", userID, "Error", err.Error()) + p.API.LogError("connectUser, failed to get user state", "UserID", userID, "Error", err.Error()) http.Error(w, err.Error(), http.StatusInternalServerError) return } diff --git a/server/http_test.go b/server/http_test.go new file mode 100644 index 0000000..c1b3e53 --- /dev/null +++ b/server/http_test.go @@ -0,0 +1,609 @@ +package main + +import ( + "bytes" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/plugin" + "github.com/mattermost/mattermost/server/public/plugin/plugintest" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + msgraph "github.com/yaegashi/msgraph.go/beta" +) + +func TestConnectUser(t *testing.T) { + api := &plugintest.API{} + p := &Plugin{ + MattermostPlugin: plugin.MattermostPlugin{ + API: api, + }, + } + + tests := []struct { + name string + userID string + channelID string + expectedStatus int + expectedBody string + redirectExpected bool + expectedRedirectURL string + setup func() + }{ + { + name: "Unauthorized User", + userID: "", + channelID: "testChannelID", + expectedStatus: http.StatusUnauthorized, + expectedBody: "Not authorized\n", + redirectExpected: false, + expectedRedirectURL: "", + setup: func() { + api.On("LogError", "connectUser, unauthorized user").Return(nil) + }, + }, + { + name: "Missing Channel ID", + userID: "testUserID", + channelID: "", + expectedStatus: http.StatusBadRequest, + expectedBody: "channelID missing\n", + redirectExpected: false, + expectedRedirectURL: "", + setup: func() { + api.On("LogError", "connectUser, missing channelID in query params").Return(nil) + }, + }, + { + name: "Error getting OAuth Config", + userID: "testUserID", + channelID: "testChannelID", + expectedStatus: http.StatusInternalServerError, + expectedBody: "error fetching siteUrl\n", + redirectExpected: false, + expectedRedirectURL: "", + setup: func() { + p.setConfiguration(&configuration{ + OAuth2ClientID: "testOAuth2ClientID", + OAuth2ClientSecret: "testOAuth2ClientSecret", + OAuth2Authority: "testOAuth2Authority", + }) + + api.On("GetConfig").Return(&model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: nil, + }, + }) + + api.On("LogError", "connectUser, failed to get oauth config", "Error", "error fetching siteUrl").Return(nil) + }, + }, + { + name: "Error Getting User State", + userID: "testUserID", + channelID: "testChannelID", + expectedStatus: http.StatusInternalServerError, + expectedBody: "error occurred getting stored user state\n", + redirectExpected: false, + expectedRedirectURL: "", + setup: func() { + p.setConfiguration(&configuration{ + OAuth2ClientID: "testOAuth2ClientID", + OAuth2ClientSecret: "testOAuth2ClientSecret", + OAuth2Authority: "testOAuth2Authority", + }) + testSiteURL := "testSiteURL" + api.On("GetConfig").Return(&model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: &testSiteURL, + }, + }) + + api.On("KVGet", getOAuthUserStateKey("testUserID")).Return(nil, &model.AppError{Message: "error occurred getting stored user state"}) + api.On("LogError", "connectUser, failed to get user state", "UserID", "testUserID", "Error", "error occurred getting stored user state").Return(nil) + }, + }, + { + name: "Successful OAuth Redirect", + userID: "testUserID", + channelID: "testChannelID", + expectedStatus: http.StatusFound, + expectedBody: "Found.\n\n", + redirectExpected: true, + expectedRedirectURL: "https://login.microsoftonline.com/testOAuth2Authority/oauth2/v2.0/authorize?access_type=offline&client_id=testOAuth2ClientID&redirect_uri=testSiteURL%2Fplugins%2Fcom.mattermost.msteamsmeetings%2Foauth2%2Fcomplete&response_type=code&scope=offline_access+OnlineMeetings.ReadWrite&state=testOAuthState", + setup: func() { + p.setConfiguration(&configuration{ + OAuth2ClientID: "testOAuth2ClientID", + OAuth2ClientSecret: "testOAuth2ClientSecret", + OAuth2Authority: "testOAuth2Authority", + }) + + testSiteURL := "testSiteURL" + api.On("GetConfig").Return(&model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: &testSiteURL, + }, + }) + + mockState := "testOAuthState" + api.On("KVGet", getOAuthUserStateKey("testUserID")).Return([]byte(mockState), nil) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + api.ExpectedCalls = nil + + tt.setup() + + req := httptest.NewRequest(http.MethodGet, "/oauth2/connect?channelID="+tt.channelID, nil) + if tt.userID != "" { + req.Header.Set("Mattermost-User-ID", tt.userID) + } + w := httptest.NewRecorder() + + p.connectUser(w, req) + + resp := w.Result() + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + require.Equal(t, tt.expectedStatus, resp.StatusCode) + require.Equal(t, tt.expectedBody, string(body)) + + if tt.redirectExpected { + require.Equal(t, tt.expectedRedirectURL, resp.Header.Get("Location")) + } + + api.AssertExpectations(t) + }) + } +} + +func TestHandleStartMeeting(t *testing.T) { + api := &plugintest.API{} + tracker := &MockTracker{} + client := &MockClient{} + p := &Plugin{ + MattermostPlugin: plugin.MattermostPlugin{ + API: api, + }, + tracker: tracker, + botUserID: "botUserID", + client: client, + } + + testCases := []struct { + name string + userID string + channelID string + expectedStatus int + expectedBody string + setup func() + }{ + { + name: "Unauthorized User", + userID: "", + channelID: "testChannelID", + expectedStatus: http.StatusUnauthorized, + expectedBody: "Not authorized\n", + setup: func() { + api.On("LogError", "handleStartMeeting, unauthorized user").Return(nil) + }, + }, + { + name: "Invalid Request Body", + userID: "testUserID", + channelID: "testChannelID", + expectedStatus: http.StatusBadRequest, + expectedBody: "invalid character 'i' looking for beginning of value\n", + setup: func() { + api.On("LogError", "handleStartMeeting, failed to decode start meeting payload", "Error", "invalid character 'i' looking for beginning of value").Return(nil) + }, + }, + { + name: "Error Getting User", + userID: "testUserID", + channelID: "testChannelID", + expectedStatus: http.StatusInternalServerError, + expectedBody: "mock error\n", + setup: func() { + api.On("GetUser", "testUserID").Return(nil, &model.AppError{Message: "mock error", StatusCode: http.StatusInternalServerError}) + api.On("LogError", "handleStartMeeting, failed to get user", "UserID", "testUserID", "Error", "mock error").Return(nil) + }, + }, + { + name: "Error Getting Channel Member", + userID: "testUserID", + channelID: "testChannelID", + expectedStatus: http.StatusForbidden, + expectedBody: "Forbidden\n", + setup: func() { + user := &model.User{Id: "testUserID"} + api.On("GetUser", "testUserID").Return(user, nil) + api.On("GetChannelMember", "testChannelID", "testUserID").Return(nil, &model.AppError{Message: "mock error", StatusCode: http.StatusForbidden}) + api.On("LogError", "handleStartMeeting, failed to get channel member", "UserID", "testUserID", "Error", "mock error").Return(nil) + }, + }, + { + name: "Error Checking Previous Messages", + userID: "testUserID", + channelID: "testChannelID", + expectedStatus: http.StatusInternalServerError, + expectedBody: "mock error while checking previous messages\n", + setup: func() { + user := &model.User{Id: "testUserID"} + api.On("GetUser", "testUserID").Return(user, nil) + api.On("GetChannelMember", "testChannelID", "testUserID").Return(nil, nil) + api.On("GetPostsSince", "testChannelID", (time.Now().Unix()-30)*1000).Return(nil, &model.AppError{Message: "mock error while checking previous messages", StatusCode: http.StatusInternalServerError}) + api.On("LogError", "handleStartMeeting, error occurred while checking previous messages in channel", "ChannelID", "testChannelID", "Error", "mock error while checking previous messages").Return(nil) + }, + }, + { + name: "Authorization Error Writing Response", + userID: "testUserID", + channelID: "testChannelID", + expectedStatus: http.StatusOK, + expectedBody: "{\"meeting_url\": \"\"}error fetching siteUrl\n", + setup: func() { + api.On("GetConfig").Return(&model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: nil, + }, + }) + api.On("GetUser", "testUserID").Return(&model.User{Id: "testUserID"}, nil) + api.On("GetChannelMember", "testChannelID", "testUserID").Return(nil, nil) + api.On("GetPostsSince", "testChannelID", (time.Now().Unix()-30)*1000).Return(&model.PostList{}, nil) + api.On("LogError", "postConnect, cannot get oauth message", "error", "error fetching siteUrl").Return() + api.On("LogError", "authenticateAndFetchUser, cannot get oauth message", "error", "error fetching siteUrl").Return() + api.On("LogWarn", "failed to create connect post", "error", mock.Anything).Return(nil) + }, + }, + { + name: "Error creating connect post", + userID: "testUserID", + channelID: "testChannelID", + expectedStatus: http.StatusOK, + expectedBody: "{\"meeting_url\": \"\"}error fetching siteUrl\n", + setup: func() { + siteURL := "testSiteURL" + api.On("GetConfig").Return(&model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: &siteURL, + }, + }).Times(1) + api.On("GetConfig").Return(&model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: nil, + }, + }).Times(1) + p.setConfiguration(&configuration{ + EncryptionKey: "demo_encrypt_key", + }) + testUserInfo := &UserInfo{} + encryptedTestUserInfo, err := testUserInfo.EncryptedJSON([]byte("demo_encrypt_key")) + require.NoError(t, err) + api.On("KVGet", "token_testUserID").Return(encryptedTestUserInfo, nil) + api.On("GetUser", "testUserID").Return(&model.User{Id: "testUserID"}, nil) + api.On("GetChannelMember", "testChannelID", "testUserID").Return(nil, nil) + api.On("GetPostsSince", "testChannelID", (time.Now().Unix()-30)*1000).Return(&model.PostList{}, nil) + api.On("LogError", "postConnect, cannot get oauth message", "error", "error fetching siteUrl") + api.On("LogWarn", "failed to create connect post", "error", "error fetching siteUrl") + client.On("GetMe").Return(&msgraph.User{}, &authError{Message: "error occured in getting the msgraph user"}) + }, + }, + { + name: "Error posting meeting", + userID: "testUserID", + channelID: "testChannelID", + expectedStatus: http.StatusInternalServerError, + expectedBody: "cannot create post in this channel\n", + setup: func() { + siteURL := "testSiteURL" + api.On("GetConfig").Return(&model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: &siteURL, + }, + }) + p.setConfiguration(&configuration{ + EncryptionKey: "demo_encrypt_key", + }) + testUserInfo := &UserInfo{} + encryptedTestUserInfo, err := testUserInfo.EncryptedJSON([]byte("demo_encrypt_key")) + require.NoError(t, err) + api.On("KVGet", "token_testUserID").Return(encryptedTestUserInfo, nil) + api.On("GetUser", "testUserID").Return(&model.User{Id: "testUserID"}, nil) + api.On("GetChannelMember", "testChannelID", "testUserID").Return(nil, nil) + api.On("GetPostsSince", "testChannelID", (time.Now().Unix()-30)*1000).Return(&model.PostList{}, nil) + api.On("LogError", "handleStartMeeting, failed to post meeting", "UserID", "testUserID", "Error", "cannot create post in this channel") + api.On("HasPermissionToChannel", "testUserID", "testChannelID", model.PermissionCreatePost).Return(false) + client.On("GetMe").Return(&msgraph.User{}, nil) + // api.On("SendEphemeralPost", "testUserID", mock.Anything).Return(&model.Post{}) + }, + }, + { + name: "Start meeting successfully", + userID: "testUserID", + channelID: "testChannelID", + expectedStatus: http.StatusOK, + expectedBody: `{"meeting_url": "testJoinURL"}`, + setup: func() { + siteURL := "testSiteURL" + api.On("GetConfig").Return(&model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: &siteURL, + }, + }) + p.setConfiguration(&configuration{ + EncryptionKey: "demo_encrypt_key", + }) + testUserInfo := &UserInfo{} + encryptedTestUserInfo, err := testUserInfo.EncryptedJSON([]byte("demo_encrypt_key")) + require.NoError(t, err) + + testJoinURL := "testJoinURL" + api.On("KVGet", "token_testUserID").Return(encryptedTestUserInfo, nil) + api.On("GetUser", "testUserID").Return(&model.User{Id: "testUserID"}, nil) + api.On("GetChannel", "testChannelID").Return(&model.Channel{Id: "testChannelID", Type: model.ChannelTypeOpen}, nil) + api.On("GetChannelMember", "testChannelID", "testUserID").Return(nil, nil) + api.On("GetPostsSince", "testChannelID", (time.Now().Unix()-30)*1000).Return(&model.PostList{}, nil) + api.On("CreatePost", mock.Anything).Return(&model.Post{}, nil) + api.On("HasPermissionToChannel", "testUserID", "testChannelID", model.PermissionCreatePost).Return(true) + client.On("GetMe").Return(&msgraph.User{}, nil) + client.On("CreateMeeting", mock.Anything, mock.Anything, mock.Anything).Return(&msgraph.OnlineMeeting{JoinURL: &testJoinURL}, nil) + tracker.On("TrackUserEvent", "meeting_started", "testUserID", mock.Anything).Return(nil) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + api.ExpectedCalls = nil + client.ExpectedCalls = nil + + tc.setup() + + var reqBody []byte + if tc.name == "Invalid Request Body" { + reqBody = []byte("invalid-json-body") + } else { + reqBody, _ = json.Marshal(&startMeetingRequest{ + ChannelID: tc.channelID, + Personal: false, + Topic: "Test Meeting", + MeetingID: 123, + }) + } + + req := httptest.NewRequest(http.MethodPost, "/start_meeting", bytes.NewBuffer(reqBody)) + if tc.userID != "" { + req.Header.Set("Mattermost-User-ID", tc.userID) + } + w := httptest.NewRecorder() + + p.handleStartMeeting(w, req) + + resp := w.Result() + body := w.Body.String() + + require.Equal(t, tc.expectedStatus, resp.StatusCode) + require.Equal(t, tc.expectedBody, body) + + api.AssertExpectations(t) + tracker.AssertExpectations(t) + }) + } +} + +func TestCompleteUserOAuth(t *testing.T) { + api := &plugintest.API{} + client := &MockClient{} + p := &Plugin{ + MattermostPlugin: plugin.MattermostPlugin{ + API: api, + }, + client: client, + } + + tests := []struct { + name string + userID string + expectedStatus int + expectedBody string + state string + authorizationCode string + setup func() + }{ + { + name: "Unauthorized User", + userID: "", + expectedStatus: http.StatusUnauthorized, + expectedBody: "Not authorized, missing Mattermost user id\n", + state: "", + authorizationCode: "", + setup: func() { + api.On("LogError", "completeUserOAuth, unauthorized user").Return(nil) + }, + }, + { + name: "Error getting OAuth config", + userID: "testUserID", + expectedStatus: http.StatusInternalServerError, + expectedBody: "error in oauth config\n", + state: "", + authorizationCode: "", + setup: func() { + p.setConfiguration(&configuration{ + OAuth2ClientID: "testOAuth2ClientID", + OAuth2ClientSecret: "testOAuth2ClientSecret", + OAuth2Authority: "testOAuth2Authority", + }) + api.On("GetConfig").Return(&model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: nil, + }, + }) + api.On("LogError", "completeUserOAuth, failed to get oauth config", "Error", "error fetching siteUrl").Return(nil) + }, + }, + { + name: "Missing authorization code", + userID: "testUserID", + expectedStatus: http.StatusBadRequest, + expectedBody: "missing authorization code\n", + state: "", + authorizationCode: "", + setup: func() { + p.setConfiguration(&configuration{ + OAuth2ClientID: "testOAuth2ClientID", + OAuth2ClientSecret: "testOAuth2ClientSecret", + OAuth2Authority: "testOAuth2Authority", + }) + siteURL := "testSiteURL" + api.On("GetConfig").Return(&model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: &siteURL, + }, + }) + api.On("LogError", "completeUserOAuth, missing authorization code").Return(nil) + }, + }, + { + name: "Invalid state length", + userID: "testUserID", + expectedStatus: http.StatusBadRequest, + expectedBody: "invalid state\n", + state: "component1_component2", + authorizationCode: "validCode123", + setup: func() { + p.setConfiguration(&configuration{ + OAuth2ClientID: "testOAuth2ClientID", + OAuth2ClientSecret: "testOAuth2ClientSecret", + OAuth2Authority: "testOAuth2Authority", + }) + siteURL := "testSiteURL" + api.On("GetConfig").Return(&model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: &siteURL, + }, + }) + api.On("LogDebug", "complete oauth, state mismatch", "stateComponents", "[component1 component2]", "state", "component1_component2").Return(nil) + api.On("LogDebug", "complete oauth, cannot parse state", "error", "status mismatch").Return(nil) + }, + }, + { + name: "Missing stored state in KV store", + userID: "testUserID", + expectedStatus: http.StatusBadRequest, + expectedBody: "missing stored state\n", + state: "valid_state_userID_channelID", + authorizationCode: "testAuthCode", + setup: func() { + p.setConfiguration(&configuration{ + OAuth2ClientID: "testOAuth2ClientID", + OAuth2ClientSecret: "testOAuth2ClientSecret", + OAuth2Authority: "testOAuth2Authority", + }) + siteURL := "testSiteURL" + api.On("GetConfig").Return(&model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: &siteURL, + }, + }) + api.On("KVGet", "valid_state").Return(nil, &model.AppError{Message: "error getting state from store"}) + api.On("LogError", "completeUserOAuth, missing stored state").Return(nil) + }, + }, + { + name: "Stored state does not match provided state", + userID: "testUserID", + expectedStatus: http.StatusBadRequest, + expectedBody: "invalid state\n", + state: "valid_state_userID_channelID", + authorizationCode: "testAuthCode", + setup: func() { + p.setConfiguration(&configuration{ + OAuth2ClientID: "testOAuth2ClientID", + OAuth2ClientSecret: "testOAuth2ClientSecret", + OAuth2Authority: "testOAuth2Authority", + }) + siteURL := "testSiteURL" + api.On("GetConfig").Return(&model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: &siteURL, + }, + }) + api.On("KVGet", "valid_state").Return([]byte("different_stored_state"), nil) + api.On("LogError", "completeUserOAuth, invalid state").Return(nil) + }, + }, + { + name: "Incorrect User ID in State", + userID: "testUserID", + expectedStatus: http.StatusUnauthorized, + expectedBody: "Not authorized, incorrect user\n", + state: "valid_state_for_oauth", + authorizationCode: "testAuthCode", + setup: func() { + p.setConfiguration(&configuration{ + OAuth2ClientID: "testOAuth2ClientID", + OAuth2ClientSecret: "testOAuth2ClientSecret", + OAuth2Authority: "testOAuth2Authority", + }) + siteURL := "testSiteURL" + api.On("GetConfig").Return(&model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: &siteURL, + }, + }) + api.On("KVGet", "valid_state").Return([]byte("valid_state_for_oauth"), nil) + api.On("KVDelete", "valid_state").Return(nil) + api.On("LogError", "completeUserOAuth, unauthorized user", "UserID", "testUserID").Return(nil) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + api.ExpectedCalls = nil + + tt.setup() + + req := httptest.NewRequest(http.MethodGet, "/oauth2/complete", nil) + if tt.userID != "" { + req.Header.Set("Mattermost-User-ID", tt.userID) + } + + if tt.state != "" { + q := req.URL.Query() + q.Add("state", tt.state) + req.URL.RawQuery = q.Encode() + } + + if tt.authorizationCode != "" { + q := req.URL.Query() + q.Add("code", tt.authorizationCode) + req.URL.RawQuery = q.Encode() + } + + w := httptest.NewRecorder() + + p.completeUserOAuth(w, req) + + resp := w.Result() + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + require.Equal(t, tt.expectedStatus, resp.StatusCode) + require.Contains(t, tt.expectedBody, string(body)) + + api.AssertExpectations(t) + }) + } +} From 23cbecbec3b87d392b7ef03efc309db44d34d3c9 Mon Sep 17 00:00:00 2001 From: kshitij katiyar Date: Tue, 1 Oct 2024 14:59:17 +0530 Subject: [PATCH 03/13] [MM-823]: Added testcase for post.go --- server/post.go | 4 +- server/post_test.go | 340 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 342 insertions(+), 2 deletions(-) create mode 100644 server/post_test.go diff --git a/server/post.go b/server/post.go index fee9d07..98d3c1a 100644 --- a/server/post.go +++ b/server/post.go @@ -22,14 +22,14 @@ func (p *Plugin) postMeeting(creator *model.User, channelID string, topic string channel, appErr := p.API.GetChannel(channelID) if appErr != nil { - return nil, nil, err + return nil, nil, appErr } if channel.IsGroupOrDirect() { var members model.ChannelMembers members, appErr = p.API.GetChannelMembers(channelID, 0, 100) if appErr != nil { - return nil, nil, err + return nil, nil, appErr } if members == nil { return nil, nil, errors.New("returned members is nil") diff --git a/server/post_test.go b/server/post_test.go new file mode 100644 index 0000000..ddfb200 --- /dev/null +++ b/server/post_test.go @@ -0,0 +1,340 @@ +package main + +import ( + "errors" + "testing" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/plugin" + "github.com/mattermost/mattermost/server/public/plugin/plugintest" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + msgraph "github.com/yaegashi/msgraph.go/beta" +) + +func TestGetUserInfo(t *testing.T) { + api := &plugintest.API{} + client := &MockClient{} + p := &Plugin{ + MattermostPlugin: plugin.MattermostPlugin{ + API: api, + }, + client: client, + } + tests := []struct { + name string + creator *model.User + channelID string + topic string + expectedError string + setup func() + }{ + { + name: "User not connected", + creator: &model.User{Id: "testUserID"}, + channelID: "testChannelID", + topic: "testTopic", + expectedError: "Your Mattermost account is not connected to any Microsoft Teams account", + setup: func() { + api.On("KVGet", tokenKey+"testUserID").Return(nil, nil).Once() + }, + }, + { + name: "Unauthorized to create post in channel", + creator: &model.User{Id: "testUserID"}, + channelID: "testChannelID", + topic: "testTopic", + expectedError: "cannot create post in this channel", + setup: func() { + info := &UserInfo{ + Email: "testEmail", + } + p.setConfiguration(&configuration{ + EncryptionKey: "demo_encrypt_key", + }) + key := []byte(p.getConfiguration().EncryptionKey) + encryptedUserInfo, err := info.EncryptedJSON(key) + require.NoError(t, err) + api.On("KVGet", tokenKey+"testUserID").Return(encryptedUserInfo, nil).Once() + api.On("HasPermissionToChannel", "testUserID", "testChannelID", model.PermissionCreatePost).Return(false) + }, + }, + { + name: "Error getting the channel", + creator: &model.User{Id: "testUserID"}, + channelID: "testChannelID", + topic: "testTopic", + expectedError: "error occured getting the channel", + setup: func() { + info := &UserInfo{ + Email: "testEmail", + } + p.setConfiguration(&configuration{ + EncryptionKey: "demo_encrypt_key", + }) + key := []byte(p.getConfiguration().EncryptionKey) + encryptedUserInfo, err := info.EncryptedJSON(key) + require.NoError(t, err) + api.On("KVGet", tokenKey+"testUserID").Return(encryptedUserInfo, nil).Once() + api.On("HasPermissionToChannel", "testUserID", "testChannelID", model.PermissionCreatePost).Return(true) + api.On("GetChannel", "testChannelID").Return(nil, &model.AppError{Message: "error occured getting the channel"}) + }, + }, + { + name: "Error getting channel members", + creator: &model.User{Id: "testUserID"}, + channelID: "testChannelID", + topic: "testTopic", + expectedError: "error occurred getting channel members", + setup: func() { + info := &UserInfo{ + Email: "testEmail", + } + p.setConfiguration(&configuration{ + EncryptionKey: "demo_encrypt_key", + }) + key := []byte(p.getConfiguration().EncryptionKey) + encryptedUserInfo, err := info.EncryptedJSON(key) + require.NoError(t, err) + api.On("KVGet", tokenKey+"testUserID").Return(encryptedUserInfo, nil).Once() + api.On("HasPermissionToChannel", "testUserID", "testChannelID", model.PermissionCreatePost).Return(true) + api.On("GetChannel", "testChannelID").Return(&model.Channel{Id: "testChannelID", Type: model.ChannelTypeDirect}, nil) + api.On("GetChannelMembers", "testChannelID", 0, 100).Return(nil, &model.AppError{Message: "error occurred getting channel members"}) + }, + }, + { + name: "Channel members is nil", + creator: &model.User{Id: "testUserID"}, + channelID: "testChannelID", + topic: "testTopic", + expectedError: "returned members is nil", + setup: func() { + info := &UserInfo{ + Email: "testEmail", + } + p.setConfiguration(&configuration{ + EncryptionKey: "demo_encrypt_key", + }) + key := []byte(p.getConfiguration().EncryptionKey) + encryptedUserInfo, err := info.EncryptedJSON(key) + require.NoError(t, err) + api.On("KVGet", tokenKey+"testUserID").Return(encryptedUserInfo, nil).Once() + api.On("HasPermissionToChannel", "testUserID", "testChannelID", model.PermissionCreatePost).Return(true) + api.On("GetChannel", "testChannelID").Return(&model.Channel{Id: "testChannelID", Type: model.ChannelTypeDirect}, nil) + api.On("GetChannelMembers", "testChannelID", 0, 100).Return(nil, nil) + + }, + }, + { + name: "Error creating the meeting", + creator: &model.User{Id: "testUserID"}, + channelID: "testChannelID", + topic: "testTopic", + expectedError: "error creating the meeting", + setup: func() { + info := &UserInfo{ + Email: "testEmail", + } + p.setConfiguration(&configuration{ + EncryptionKey: "demo_encrypt_key", + }) + key := []byte(p.getConfiguration().EncryptionKey) + encryptedUserInfo, err := info.EncryptedJSON(key) + require.NoError(t, err) + api.On("KVGet", tokenKey+"testUserID").Return(encryptedUserInfo, nil).Once() + api.On("HasPermissionToChannel", "testUserID", "testChannelID", model.PermissionCreatePost).Return(true) + api.On("GetChannel", "testChannelID").Return(&model.Channel{Id: "testChannelID", Type: model.ChannelTypeDirect}, nil) + api.On("GetChannelMembers", "testChannelID", 0, 100).Return(model.ChannelMembers{}, nil) + client.On("CreateMeeting").Return(&msgraph.OnlineMeeting{}, errors.New("error creating the meeting")) + }, + }, + { + name: "Error creating the meeting post", + creator: &model.User{Id: "testUserID", Username: "testUsername"}, + channelID: "testChannelID", + topic: "testTopic", + expectedError: "error creating the post", + setup: func() { + info := &UserInfo{ + Email: "testEmail", + } + p.setConfiguration(&configuration{ + EncryptionKey: "demo_encrypt_key", + }) + key := []byte(p.getConfiguration().EncryptionKey) + encryptedUserInfo, err := info.EncryptedJSON(key) + require.NoError(t, err) + api.On("KVGet", tokenKey+"testUserID").Return(encryptedUserInfo, nil).Once() + api.On("HasPermissionToChannel", "testUserID", "testChannelID", model.PermissionCreatePost).Return(true) + api.On("GetChannel", "testChannelID").Return(&model.Channel{Id: "testChannelID", Type: model.ChannelTypeDirect}, nil) + api.On("GetChannelMembers", "testChannelID", 0, 100).Return(model.ChannelMembers{}, nil) + api.On("CreatePost", mock.Anything).Return(nil, &model.AppError{Message: "error creating the post"}) + testJoinURL := "testJoinURL" + client.On("CreateMeeting").Return(&msgraph.OnlineMeeting{JoinURL: &testJoinURL}, nil) + }, + }, + { + name: "Meeting posted successfully", + creator: &model.User{Id: "testUserID", Username: "testUsername"}, + channelID: "testChannelID", + topic: "testTopic", + expectedError: "", + setup: func() { + info := &UserInfo{ + Email: "testEmail", + } + p.setConfiguration(&configuration{ + EncryptionKey: "demo_encrypt_key", + }) + key := []byte(p.getConfiguration().EncryptionKey) + encryptedUserInfo, err := info.EncryptedJSON(key) + require.NoError(t, err) + api.On("KVGet", tokenKey+"testUserID").Return(encryptedUserInfo, nil).Once() + api.On("HasPermissionToChannel", "testUserID", "testChannelID", model.PermissionCreatePost).Return(true) + api.On("GetChannel", "testChannelID").Return(&model.Channel{Id: "testChannelID", Type: model.ChannelTypeDirect}, nil) + api.On("GetChannelMembers", "testChannelID", 0, 100).Return(model.ChannelMembers{}, nil) + api.On("CreatePost", mock.Anything).Return(&model.Post{}, nil) + testJoinURL := "testJoinURL" + client.On("CreateMeeting").Return(&msgraph.OnlineMeeting{JoinURL: &testJoinURL}, nil) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + api.ExpectedCalls = nil + client.ExpectedCalls = nil + + tt.setup() + + _, _, err := p.postMeeting(tt.creator, tt.channelID, tt.topic) + + if tt.expectedError != "" { + require.Error(t, err) + require.Equal(t, tt.expectedError, err.Error()) + } else { + require.NoError(t, err) + } + + api.AssertExpectations(t) + }) + } +} + +func TestPostConfirmCreateOrJoin(t *testing.T) { + testCases := []struct { + name string + provider string + expectedMessage string + }{ + { + name: "Provider is msteams", + provider: "msteams", + expectedMessage: "There is another recent meeting created on this channel.", + }, + { + name: "Provider is dummyProvider", + provider: "dummyProvider", + expectedMessage: "There is another recent meeting created on this channel with dummyProvider.", + }, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + channelID := "test_channel_id" + meetingURL := "https://example.com/meeting" + topic := "Test Meeting" + userID := "test_user_id" + creatorName := "test_creator" + + api := &plugintest.API{} + client := &MockClient{} + p := &Plugin{ + MattermostPlugin: plugin.MattermostPlugin{ + API: api, + }, + client: client, + botUserID: "testBotUserID", + } + expectedPost := &model.Post{ + UserId: p.botUserID, + ChannelId: channelID, + Message: tt.expectedMessage, + Type: "custom_mstmeetings", + Props: map[string]interface{}{ + "type": "custom_mstmeetings", + "meeting_link": meetingURL, + "meeting_status": postTypeConfirm, + "meeting_personal": true, + "meeting_topic": topic, + "meeting_creator_username": creatorName, + "meeting_provider": tt.provider, + }, + } + api.On("SendEphemeralPost", userID, mock.AnythingOfType("*model.Post")).Return(expectedPost, nil) + + post := p.postConfirmCreateOrJoin(meetingURL, channelID, topic, userID, creatorName, tt.provider) + require.Equal(t, tt.expectedMessage, post.Message) + }) + } +} + +func TestPostConnect(t *testing.T) { + api := &plugintest.API{} + p := &Plugin{ + MattermostPlugin: plugin.MattermostPlugin{ + API: api, + }, + botUserID: "testBostUserID", + } + tests := []struct { + name string + userID string + channelID string + expectedError string + setup func() + }{ + { + name: "Error getting oauth message", + channelID: "testChannelID", + userID: "testUserID", + expectedError: "error fetching siteUrl", + setup: func() { + api.On("GetConfig").Return(&model.Config{ServiceSettings: model.ServiceSettings{SiteURL: nil}}) + api.On("LogError", "postConnect, cannot get oauth message", "error", "error fetching siteUrl") + + }, + }, + { + name: "Error getting oauth message", + channelID: "testChannelID", + userID: "testUserID", + expectedError: "", + setup: func() { + testSiteURL := "testSiteURL" + api.On("GetConfig").Return(&model.Config{ServiceSettings: model.ServiceSettings{SiteURL: &testSiteURL}}) + api.On("SendEphemeralPost", "testUserID", mock.AnythingOfType("*model.Post")).Return(&model.Post{}, nil) + + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + api.ExpectedCalls = nil + + tt.setup() + + _, err := p.postConnect(tt.channelID, tt.userID) + + if tt.expectedError != "" { + require.Error(t, err) + require.Equal(t, tt.expectedError, err.Error()) + } else { + require.NoError(t, err) + } + + api.AssertExpectations(t) + }) + } +} From af48ff94720d6f12cd0b24354138d3889ccf699b Mon Sep 17 00:00:00 2001 From: kshitij katiyar Date: Tue, 1 Oct 2024 16:21:22 +0530 Subject: [PATCH 04/13] [MM-820]: review fixes --- server/state_test.go | 56 ++++++++++++++++++++------------------------ server/user_test.go | 17 ++++++++------ 2 files changed, 36 insertions(+), 37 deletions(-) diff --git a/server/state_test.go b/server/state_test.go index 5bf95d0..e5fbdea 100644 --- a/server/state_test.go +++ b/server/state_test.go @@ -12,31 +12,31 @@ import ( func TestStoreState(t *testing.T) { testCases := []struct { - name string + name string userID string channelID string - justConnect bool - returnError error - expectError bool - expectedState string - expectedErrMsg string + justConnect bool + returnError error + expectError bool + expectedState string + expectedErrMsg string }{ { name: "Store state successful", - userID: "userID1", - channelID: "channelID1", + userID: "userID1", + channelID: "channelID1", justConnect: true, returnError: nil, expectError: false, expectedState: "msteamsmeetinguserstate_userID1_channelID1_true", }, { - name: "Error occurred while storing state", - userID: "userID2", - channelID: "channelID2", - justConnect: false, - returnError: &model.AppError{Message: "error occurred while storing state"}, - expectError: true, + name: "Error occurred while storing state", + userID: "userID2", + channelID: "channelID2", + justConnect: false, + returnError: &model.AppError{Message: "error occurred while storing state"}, + expectError: true, expectedErrMsg: "error occurred while storing state", }, } @@ -61,6 +61,7 @@ func TestStoreState(t *testing.T) { require.NoError(t, err) require.Equal(t, tc.expectedState, state) } + api.AssertExpectations(t) }) } } @@ -75,23 +76,22 @@ func TestGetState(t *testing.T) { expectError bool expectedErrMsg string }{ - { - name: "Valid state retrieved", - key: "dummyKey", - returnValue: []byte("dummyState"), - returnError: nil, - expectedState: "dummyState", - expectError: false, - }, { name: "Error occurred while getting stored state", key: "dummyKey", returnValue: []byte(""), returnError: &model.AppError{Message: "error occurred while getting stored state"}, - expectedState: "", expectError: true, expectedErrMsg: "error occurred while getting stored state", }, + { + name: "Valid state retrieved", + key: "dummyKey", + returnValue: []byte("dummyState"), + returnError: nil, + expectedState: "dummyState", + expectError: false, + }, } for _, tc := range testCases { @@ -114,6 +114,7 @@ func TestGetState(t *testing.T) { require.NoError(t, err) require.Equal(t, tc.expectedState, state) } + api.AssertExpectations(t) }) } } @@ -159,6 +160,7 @@ func TestDeleteState(t *testing.T) { } else { require.NoError(t, err) } + api.AssertExpectations(t) }) } } @@ -186,9 +188,6 @@ func TestParseState(t *testing.T) { { name: "State length mismatch", state: "key1_userID1_channelID1", - expectedKey: "", - expectedUserID: "", - expectedChannelID: "", expectedJustConnect: false, expectError: true, expectedErrMsg: "status mismatch", @@ -196,9 +195,6 @@ func TestParseState(t *testing.T) { { name: "Invalid state format", state: "key1_userID1", - expectedKey: "", - expectedUserID: "", - expectedChannelID: "", expectedJustConnect: false, expectError: true, expectedErrMsg: "status mismatch", @@ -214,7 +210,6 @@ func TestParseState(t *testing.T) { }, } - // Set up the mock to expect LogDebug calls for error cases if tc.expectError { api.On("LogDebug", "complete oauth, state mismatch", "stateComponents", mock.Anything, "state", tc.state).Return() } @@ -231,6 +226,7 @@ func TestParseState(t *testing.T) { require.Equal(t, tc.expectedChannelID, channelID) require.Equal(t, tc.expectedJustConnect, justConnect) } + api.AssertExpectations(t) }) } } diff --git a/server/user_test.go b/server/user_test.go index c879525..6d455eb 100644 --- a/server/user_test.go +++ b/server/user_test.go @@ -51,12 +51,6 @@ func TestStoreUserInfo(t *testing.T) { kvSetRemoteErr error expectedErr string }{ - { - name: "No Error", - kvSetUserErr: nil, - kvSetRemoteErr: nil, - expectedErr: "", - }, { name: "Error Saving UserID", kvSetUserErr: &model.AppError{Message: "some error occurred while saving the user id"}, @@ -69,6 +63,12 @@ func TestStoreUserInfo(t *testing.T) { kvSetRemoteErr: &model.AppError{Message: "some error occurred while saving the remote id"}, expectedErr: "some error occurred while saving the remote id", }, + { + name: "User Info stored successfully", + kvSetUserErr: nil, + kvSetRemoteErr: nil, + expectedErr: "", + }, } for _, tt := range tests { @@ -89,7 +89,9 @@ func TestStoreUserInfo(t *testing.T) { } api.On("KVSet", "token_"+dummyInfo.UserID, mock.Anything).Return(tt.kvSetUserErr) - api.On("KVSet", "tbyrid_"+dummyInfo.RemoteID, mock.Anything).Return(tt.kvSetRemoteErr) + if tt.kvSetUserErr == nil { + api.On("KVSet", "tbyrid_"+dummyInfo.RemoteID, mock.Anything).Return(tt.kvSetRemoteErr) + } responseErr := p.StoreUserInfo(dummyInfo) if tt.expectedErr == "" { @@ -98,6 +100,7 @@ func TestStoreUserInfo(t *testing.T) { require.Error(t, responseErr) require.Equal(t, tt.expectedErr, responseErr.Error()) } + api.AssertExpectations(t) }) } } From 8bbd3b6504c5640c73c07ccb876ce794558a388a Mon Sep 17 00:00:00 2001 From: kshitij katiyar Date: Fri, 4 Oct 2024 11:31:21 +0530 Subject: [PATCH 05/13] [MM-820]: review fixes --- server/command_test.go | 56 +++++++++++++----------------------------- server/state_test.go | 52 +++++++++++++++++---------------------- server/user_test.go | 31 +++++++++-------------- 3 files changed, 51 insertions(+), 88 deletions(-) diff --git a/server/command_test.go b/server/command_test.go index 135473f..355af04 100644 --- a/server/command_test.go +++ b/server/command_test.go @@ -100,16 +100,10 @@ func TestHandleConnect(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - api := &plugintest.API{} + mockAPI := &plugintest.API{} mockClient := &MockClient{} - p := &Plugin{ - MattermostPlugin: plugin.MattermostPlugin{ - API: api, - }, - client: mockClient, - } - + p := SetupMockPlugin(mockAPI, nil, mockClient) p.setConfiguration(&configuration{ EncryptionKey: "demo_encrypt_key", OAuth2ClientID: "demo_oauth2_client_id", @@ -127,7 +121,7 @@ func TestHandleConnect(t *testing.T) { encryptedUserInfo, err := userInfo.EncryptedJSON([]byte("demo_encrypt_key")) require.NoError(t, err) - tt.mockSetup(api, encryptedUserInfo, mockClient) + tt.mockSetup(mockAPI, encryptedUserInfo, mockClient) resp, err := p.handleConnect(tt.args, tt.commandArgs) if tt.expectError { @@ -137,7 +131,7 @@ func TestHandleConnect(t *testing.T) { require.Contains(t, resp, tt.expectedOutput) } - api.AssertExpectations(t) + mockAPI.AssertExpectations(t) }) } } @@ -184,16 +178,10 @@ func TestHandleDisconnect(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - api := &plugintest.API{} + mockAPI := &plugintest.API{} mockTracker := &MockTracker{} - p := &Plugin{ - MattermostPlugin: plugin.MattermostPlugin{ - API: api, - }, - tracker: mockTracker, - } - + p := SetupMockPlugin(mockAPI, mockTracker, nil) p.setConfiguration(&configuration{ EncryptionKey: "demo_encrypt_key", }) @@ -208,13 +196,13 @@ func TestHandleDisconnect(t *testing.T) { encryptedUserInfo, err := userInfo.EncryptedJSON([]byte("demo_encrypt_key")) require.NoError(t, err) - tt.mockSetup(api, encryptedUserInfo, mockTracker) + tt.mockSetup(mockAPI, encryptedUserInfo, mockTracker) resp, err := p.handleDisconnect(tt.args, tt.commandArgs) require.NoError(t, err) require.Contains(t, resp, tt.expectedOutput) - api.AssertExpectations(t) + mockAPI.AssertExpectations(t) mockTracker.AssertExpectations(t) }) } @@ -357,18 +345,11 @@ func TestHandleStart(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - api := &plugintest.API{} + mockAPI := &plugintest.API{} mockTracker := &MockTracker{} mockClient := &MockClient{} - p := &Plugin{ - MattermostPlugin: plugin.MattermostPlugin{ - API: api, - }, - tracker: mockTracker, - client: mockClient, - } - + p := SetupMockPlugin(mockAPI, mockTracker, mockClient) p.setConfiguration(&configuration{ EncryptionKey: "demo_encrypt_key", }) @@ -383,7 +364,7 @@ func TestHandleStart(t *testing.T) { encryptedUserInfo, err := userInfo.EncryptedJSON([]byte("demo_encrypt_key")) require.NoError(t, err) - tt.mockSetup(api, encryptedUserInfo, mockTracker, mockClient) + tt.mockSetup(mockAPI, encryptedUserInfo, mockTracker, mockClient) resp, err := p.handleStart(tt.args, tt.commandArgs) if tt.expectError { @@ -393,7 +374,7 @@ func TestHandleStart(t *testing.T) { require.Contains(t, resp, tt.expectedOutput) } - api.AssertExpectations(t) + mockAPI.AssertExpectations(t) mockTracker.AssertExpectations(t) }) } @@ -412,12 +393,9 @@ func TestGetHelpText(t *testing.T) { } func TestExecuteCommand(t *testing.T) { - api := &plugintest.API{} - p := &Plugin{ - MattermostPlugin: plugin.MattermostPlugin{ - API: api, - }, - } + mockAPI := &plugintest.API{} + p := SetupMockPlugin(mockAPI, nil, nil) + var dummyPluginContext plugin.Context tests := []struct { @@ -453,12 +431,12 @@ func TestExecuteCommand(t *testing.T) { Message: tt.expectedMsg, } - api.On("SendEphemeralPost", tt.commandArgs.UserId, post).Return(&model.Post{}).Once() + mockAPI.On("SendEphemeralPost", tt.commandArgs.UserId, post).Return(&model.Post{}).Once() response, _ := p.ExecuteCommand(&dummyPluginContext, &tt.commandArgs) require.Equal(t, &model.CommandResponse{}, response) - api.AssertExpectations(t) + mockAPI.AssertExpectations(t) }) } } diff --git a/server/state_test.go b/server/state_test.go index e5fbdea..e77d14e 100644 --- a/server/state_test.go +++ b/server/state_test.go @@ -43,14 +43,10 @@ func TestStoreState(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - api := &plugintest.API{} - p := &Plugin{ - MattermostPlugin: plugin.MattermostPlugin{ - API: api, - }, - } + mockAPI := &plugintest.API{} + p := SetupMockPlugin(mockAPI, nil, nil) - api.On("KVSet", mock.Anything, mock.Anything).Return(tc.returnError) + mockAPI.On("KVSet", mock.Anything, mock.Anything).Return(tc.returnError) state, err := p.StoreState(tc.userID, tc.channelID, tc.justConnect) @@ -61,7 +57,7 @@ func TestStoreState(t *testing.T) { require.NoError(t, err) require.Equal(t, tc.expectedState, state) } - api.AssertExpectations(t) + mockAPI.AssertExpectations(t) }) } } @@ -97,11 +93,7 @@ func TestGetState(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { api := &plugintest.API{} - p := &Plugin{ - MattermostPlugin: plugin.MattermostPlugin{ - API: api, - }, - } + p := SetupMockPlugin(api, nil, nil) api.On("KVGet", tc.key).Return(tc.returnValue, tc.returnError) @@ -144,14 +136,10 @@ func TestDeleteState(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - api := &plugintest.API{} - p := &Plugin{ - MattermostPlugin: plugin.MattermostPlugin{ - API: api, - }, - } + mockAPI := &plugintest.API{} + p := SetupMockPlugin(mockAPI, nil, nil) - api.On("KVDelete", tc.key).Return(tc.returnError) + mockAPI.On("KVDelete", tc.key).Return(tc.returnError) err := p.DeleteState(tc.key) if tc.expectError { @@ -160,7 +148,7 @@ func TestDeleteState(t *testing.T) { } else { require.NoError(t, err) } - api.AssertExpectations(t) + mockAPI.AssertExpectations(t) }) } } @@ -203,15 +191,11 @@ func TestParseState(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - api := &plugintest.API{} - p := &Plugin{ - MattermostPlugin: plugin.MattermostPlugin{ - API: api, - }, - } + mockAPI := &plugintest.API{} + p := SetupMockPlugin(mockAPI, nil, nil) if tc.expectError { - api.On("LogDebug", "complete oauth, state mismatch", "stateComponents", mock.Anything, "state", tc.state).Return() + mockAPI.On("LogDebug", "complete oauth, state mismatch", "stateComponents", mock.Anything, "state", tc.state).Return() } key, userID, channelID, justConnect, err := p.ParseState(tc.state) @@ -226,7 +210,17 @@ func TestParseState(t *testing.T) { require.Equal(t, tc.expectedChannelID, channelID) require.Equal(t, tc.expectedJustConnect, justConnect) } - api.AssertExpectations(t) + mockAPI.AssertExpectations(t) }) } } + +func SetupMockPlugin(mockAPI *plugintest.API, mockTracker *MockTracker, mockClient *MockClient) *Plugin { + return &Plugin{ + MattermostPlugin: plugin.MattermostPlugin{ + API: mockAPI, + }, + tracker: mockTracker, + client: mockClient, + } +} diff --git a/server/user_test.go b/server/user_test.go index 6d455eb..c8767ff 100644 --- a/server/user_test.go +++ b/server/user_test.go @@ -5,7 +5,6 @@ import ( "time" "github.com/mattermost/mattermost/server/public/model" - "github.com/mattermost/mattermost/server/public/plugin" "github.com/mattermost/mattermost/server/public/plugin/plugintest" "github.com/stretchr/testify/mock" @@ -73,12 +72,8 @@ func TestStoreUserInfo(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - api := &plugintest.API{} - p := &Plugin{ - MattermostPlugin: plugin.MattermostPlugin{ - API: api, - }, - } + mockAPI := &plugintest.API{} + p := SetupMockPlugin(mockAPI, nil, nil) p.setConfiguration(&configuration{ EncryptionKey: "demo_encrypt_key", }) @@ -88,9 +83,9 @@ func TestStoreUserInfo(t *testing.T) { RemoteID: "dummyRemoteID", } - api.On("KVSet", "token_"+dummyInfo.UserID, mock.Anything).Return(tt.kvSetUserErr) + mockAPI.On("KVSet", "token_"+dummyInfo.UserID, mock.Anything).Return(tt.kvSetUserErr) if tt.kvSetUserErr == nil { - api.On("KVSet", "tbyrid_"+dummyInfo.RemoteID, mock.Anything).Return(tt.kvSetRemoteErr) + mockAPI.On("KVSet", "tbyrid_"+dummyInfo.RemoteID, mock.Anything).Return(tt.kvSetRemoteErr) } responseErr := p.StoreUserInfo(dummyInfo) @@ -100,7 +95,7 @@ func TestStoreUserInfo(t *testing.T) { require.Error(t, responseErr) require.Equal(t, tt.expectedErr, responseErr.Error()) } - api.AssertExpectations(t) + mockAPI.AssertExpectations(t) }) } } @@ -125,22 +120,18 @@ func TestResetAllOAuthTokens(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - api := &plugintest.API{} - p := &Plugin{ - MattermostPlugin: plugin.MattermostPlugin{ - API: api, - }, - } + mockAPI := &plugintest.API{} + p := SetupMockPlugin(mockAPI, nil, nil) - api.On("LogInfo", "OAuth2 configuration changed. Resetting all users' tokens, everyone will need to reconnect to MS Teams").Return(nil) - api.On("KVDeleteAll").Return(tt.kvDeleteAllErr) + mockAPI.On("LogInfo", "OAuth2 configuration changed. Resetting all users' tokens, everyone will need to reconnect to MS Teams").Return(nil) + mockAPI.On("KVDeleteAll").Return(tt.kvDeleteAllErr) if tt.expectLogError { - api.On("LogError", "failed to reset user's OAuth2 tokens", "error", tt.kvDeleteAllErr.Error()).Return(nil) + mockAPI.On("LogError", "failed to reset user's OAuth2 tokens", "error", tt.kvDeleteAllErr.Error()).Return(nil) } p.resetAllOAuthTokens() - api.AssertExpectations(t) + mockAPI.AssertExpectations(t) }) } } From 808bc46bb701845ce542e97717ec169196b26fbb Mon Sep 17 00:00:00 2001 From: kshitij katiyar Date: Fri, 4 Oct 2024 17:27:31 +0530 Subject: [PATCH 06/13] [MM-820]: review fixes --- server/command_test.go | 6 ---- server/state_test.go | 82 +++++++++++++++++++----------------------- server/user_test.go | 25 +++++-------- 3 files changed, 45 insertions(+), 68 deletions(-) diff --git a/server/command_test.go b/server/command_test.go index 355af04..c127b4d 100644 --- a/server/command_test.go +++ b/server/command_test.go @@ -68,7 +68,6 @@ func TestHandleConnect(t *testing.T) { mockSetup: func(_ *plugintest.API, _ []byte, _ *MockClient) { }, expectedOutput: tooManyParametersText, - expectError: false, }, { name: "Error connecting user", @@ -80,7 +79,6 @@ func TestHandleConnect(t *testing.T) { api.On("GetConfig").Return(&model.Config{ServiceSettings: model.ServiceSettings{SiteURL: model.NewString("https://example.com")}}) mockClient.On("GetMe").Return(&msgraph.User{}, errors.New("error getting user details")) }, - expectedOutput: "", expectError: true, expectedError: "error getting user details", }, @@ -93,7 +91,6 @@ func TestHandleConnect(t *testing.T) { api.On("GetConfig").Return(&model.Config{ServiceSettings: model.ServiceSettings{SiteURL: model.NewString("https://example.com")}}) mockClient.On("GetMe").Return(&msgraph.User{}, nil) }, - expectedOutput: "", expectError: false, }, } @@ -278,8 +275,6 @@ func TestHandleStart(t *testing.T) { api.On("SendEphemeralPost", "demoUserID", mock.Anything).Return(&model.Post{}) mockTracker.On("TrackUserEvent", mock.Anything, "demoUserID", mock.Anything).Return(nil) }, - expectError: false, - expectedOutput: "", }, { name: "Authentication error", @@ -339,7 +334,6 @@ func TestHandleStart(t *testing.T) { mockClient.On("CreateMeeting", mock.Anything, mock.Anything, mock.Anything).Return(&msgraph.OnlineMeeting{JoinURL: &joinURL}, nil) mockTracker.On("TrackUserEvent", "meeting_started", "demoUserID", mock.Anything).Return(nil) }, - expectError: false, }, } diff --git a/server/state_test.go b/server/state_test.go index e77d14e..be839b5 100644 --- a/server/state_test.go +++ b/server/state_test.go @@ -21,24 +21,21 @@ func TestStoreState(t *testing.T) { expectedState string expectedErrMsg string }{ - { - name: "Store state successful", - userID: "userID1", - channelID: "channelID1", - justConnect: true, - returnError: nil, - expectError: false, - expectedState: "msteamsmeetinguserstate_userID1_channelID1_true", - }, { name: "Error occurred while storing state", - userID: "userID2", - channelID: "channelID2", - justConnect: false, + userID: "mockUserID1", + channelID: "mockChannelID1", returnError: &model.AppError{Message: "error occurred while storing state"}, expectError: true, expectedErrMsg: "error occurred while storing state", }, + { + name: "Store state successful", + userID: "mockUserID2", + channelID: "mockChannelID2", + justConnect: true, + expectedState: "msteamsmeetinguserstate_mockUserID2_mockChannelID2_true", + }, } for _, tc := range testCases { @@ -66,27 +63,25 @@ func TestGetState(t *testing.T) { testCases := []struct { name string key string - returnValue []byte - returnError error + getStateValue []byte + getStateError error expectedState string expectError bool expectedErrMsg string }{ { name: "Error occurred while getting stored state", - key: "dummyKey", - returnValue: []byte(""), - returnError: &model.AppError{Message: "error occurred while getting stored state"}, + key: "mockKey", + getStateValue: []byte(""), + getStateError: &model.AppError{Message: "error occurred while getting stored state"}, expectError: true, expectedErrMsg: "error occurred while getting stored state", }, { name: "Valid state retrieved", - key: "dummyKey", - returnValue: []byte("dummyState"), - returnError: nil, - expectedState: "dummyState", - expectError: false, + key: "mockKey", + getStateValue: []byte("mockState"), + expectedState: "mockState", }, } @@ -95,7 +90,7 @@ func TestGetState(t *testing.T) { api := &plugintest.API{} p := SetupMockPlugin(api, nil, nil) - api.On("KVGet", tc.key).Return(tc.returnValue, tc.returnError) + api.On("KVGet", tc.key).Return(tc.getStateValue, tc.getStateError) state, err := p.GetState(tc.key) if tc.expectError { @@ -119,19 +114,17 @@ func TestDeleteState(t *testing.T) { expectError bool expectedErrMsg string }{ - { - name: "Delete state successful", - key: "dummyKey", - returnError: nil, - expectError: false, - }, { name: "Error occurred while deleting state", - key: "dummyKey", + key: "mockKey", returnError: &model.AppError{Message: "error occurred while deleting state"}, expectError: true, expectedErrMsg: "error occurred while deleting state", }, + { + name: "Delete state successful", + key: "mockKey", + }, } for _, tc := range testCases { @@ -164,6 +157,18 @@ func TestParseState(t *testing.T) { expectError bool expectedErrMsg string }{ + { + name: "State length mismatch", + state: "key1_userID1_channelID1", + expectError: true, + expectedErrMsg: "status mismatch", + }, + { + name: "Invalid state format", + state: "key1_userID1", + expectError: true, + expectedErrMsg: "status mismatch", + }, { name: "Parse state successful", state: "key1_userID1_channelID1_true", @@ -171,21 +176,6 @@ func TestParseState(t *testing.T) { expectedUserID: "userID1", expectedChannelID: "channelID1", expectedJustConnect: true, - expectError: false, - }, - { - name: "State length mismatch", - state: "key1_userID1_channelID1", - expectedJustConnect: false, - expectError: true, - expectedErrMsg: "status mismatch", - }, - { - name: "Invalid state format", - state: "key1_userID1", - expectedJustConnect: false, - expectError: true, - expectedErrMsg: "status mismatch", }, } @@ -221,6 +211,6 @@ func SetupMockPlugin(mockAPI *plugintest.API, mockTracker *MockTracker, mockClie API: mockAPI, }, tracker: mockTracker, - client: mockClient, + client: mockClient, } } diff --git a/server/user_test.go b/server/user_test.go index c8767ff..c428fb4 100644 --- a/server/user_test.go +++ b/server/user_test.go @@ -51,22 +51,17 @@ func TestStoreUserInfo(t *testing.T) { expectedErr string }{ { - name: "Error Saving UserID", - kvSetUserErr: &model.AppError{Message: "some error occurred while saving the user id"}, - kvSetRemoteErr: nil, - expectedErr: "some error occurred while saving the user id", + name: "Error Saving UserID", + kvSetUserErr: &model.AppError{Message: "some error occurred while saving the user id"}, + expectedErr: "some error occurred while saving the user id", }, { name: "Error Saving RemoteID", - kvSetUserErr: nil, kvSetRemoteErr: &model.AppError{Message: "some error occurred while saving the remote id"}, expectedErr: "some error occurred while saving the remote id", }, { - name: "User Info stored successfully", - kvSetUserErr: nil, - kvSetRemoteErr: nil, - expectedErr: "", + name: "User Info stored successfully", }, } @@ -79,8 +74,8 @@ func TestStoreUserInfo(t *testing.T) { }) dummyInfo := &UserInfo{ - UserID: "dummyUserID", - RemoteID: "dummyRemoteID", + UserID: "mockUserID", + RemoteID: "mockRemoteID", } mockAPI.On("KVSet", "token_"+dummyInfo.UserID, mock.Anything).Return(tt.kvSetUserErr) @@ -106,16 +101,14 @@ func TestResetAllOAuthTokens(t *testing.T) { kvDeleteAllErr error expectLogError bool }{ - { - name: "No Error", - kvDeleteAllErr: nil, - expectLogError: false, - }, { name: "Error Deleting Tokens", kvDeleteAllErr: &model.AppError{Message: "error in deleting all oauth token"}, expectLogError: true, }, + { + name: "No Error", + }, } for _, tt := range tests { From 91cb908163dccea6b83415495cf0e9d0c6505497 Mon Sep 17 00:00:00 2001 From: kshitij katiyar Date: Mon, 7 Oct 2024 14:06:25 +0530 Subject: [PATCH 07/13] [MM-823]: review fixes --- server/post_test.go | 152 +++++++++++--------------------------------- 1 file changed, 37 insertions(+), 115 deletions(-) diff --git a/server/post_test.go b/server/post_test.go index ddfb200..7d9d15f 100644 --- a/server/post_test.go +++ b/server/post_test.go @@ -13,27 +13,27 @@ import ( ) func TestGetUserInfo(t *testing.T) { - api := &plugintest.API{} - client := &MockClient{} - p := &Plugin{ - MattermostPlugin: plugin.MattermostPlugin{ - API: api, - }, - client: client, + p, api, client := SetupPluginMocks() + + info := &UserInfo{ + Email: "testEmail", } + p.setConfiguration(&configuration{ + EncryptionKey: "demo_encrypt_key", + }) + key := []byte(p.getConfiguration().EncryptionKey) + encryptedUserInfo, err := info.EncryptedJSON(key) + require.NoError(t, err) + tests := []struct { name string creator *model.User - channelID string - topic string expectedError string setup func() }{ { name: "User not connected", creator: &model.User{Id: "testUserID"}, - channelID: "testChannelID", - topic: "testTopic", expectedError: "Your Mattermost account is not connected to any Microsoft Teams account", setup: func() { api.On("KVGet", tokenKey+"testUserID").Return(nil, nil).Once() @@ -42,19 +42,8 @@ func TestGetUserInfo(t *testing.T) { { name: "Unauthorized to create post in channel", creator: &model.User{Id: "testUserID"}, - channelID: "testChannelID", - topic: "testTopic", expectedError: "cannot create post in this channel", setup: func() { - info := &UserInfo{ - Email: "testEmail", - } - p.setConfiguration(&configuration{ - EncryptionKey: "demo_encrypt_key", - }) - key := []byte(p.getConfiguration().EncryptionKey) - encryptedUserInfo, err := info.EncryptedJSON(key) - require.NoError(t, err) api.On("KVGet", tokenKey+"testUserID").Return(encryptedUserInfo, nil).Once() api.On("HasPermissionToChannel", "testUserID", "testChannelID", model.PermissionCreatePost).Return(false) }, @@ -62,19 +51,8 @@ func TestGetUserInfo(t *testing.T) { { name: "Error getting the channel", creator: &model.User{Id: "testUserID"}, - channelID: "testChannelID", - topic: "testTopic", expectedError: "error occured getting the channel", setup: func() { - info := &UserInfo{ - Email: "testEmail", - } - p.setConfiguration(&configuration{ - EncryptionKey: "demo_encrypt_key", - }) - key := []byte(p.getConfiguration().EncryptionKey) - encryptedUserInfo, err := info.EncryptedJSON(key) - require.NoError(t, err) api.On("KVGet", tokenKey+"testUserID").Return(encryptedUserInfo, nil).Once() api.On("HasPermissionToChannel", "testUserID", "testChannelID", model.PermissionCreatePost).Return(true) api.On("GetChannel", "testChannelID").Return(nil, &model.AppError{Message: "error occured getting the channel"}) @@ -83,19 +61,8 @@ func TestGetUserInfo(t *testing.T) { { name: "Error getting channel members", creator: &model.User{Id: "testUserID"}, - channelID: "testChannelID", - topic: "testTopic", expectedError: "error occurred getting channel members", setup: func() { - info := &UserInfo{ - Email: "testEmail", - } - p.setConfiguration(&configuration{ - EncryptionKey: "demo_encrypt_key", - }) - key := []byte(p.getConfiguration().EncryptionKey) - encryptedUserInfo, err := info.EncryptedJSON(key) - require.NoError(t, err) api.On("KVGet", tokenKey+"testUserID").Return(encryptedUserInfo, nil).Once() api.On("HasPermissionToChannel", "testUserID", "testChannelID", model.PermissionCreatePost).Return(true) api.On("GetChannel", "testChannelID").Return(&model.Channel{Id: "testChannelID", Type: model.ChannelTypeDirect}, nil) @@ -105,19 +72,8 @@ func TestGetUserInfo(t *testing.T) { { name: "Channel members is nil", creator: &model.User{Id: "testUserID"}, - channelID: "testChannelID", - topic: "testTopic", expectedError: "returned members is nil", setup: func() { - info := &UserInfo{ - Email: "testEmail", - } - p.setConfiguration(&configuration{ - EncryptionKey: "demo_encrypt_key", - }) - key := []byte(p.getConfiguration().EncryptionKey) - encryptedUserInfo, err := info.EncryptedJSON(key) - require.NoError(t, err) api.On("KVGet", tokenKey+"testUserID").Return(encryptedUserInfo, nil).Once() api.On("HasPermissionToChannel", "testUserID", "testChannelID", model.PermissionCreatePost).Return(true) api.On("GetChannel", "testChannelID").Return(&model.Channel{Id: "testChannelID", Type: model.ChannelTypeDirect}, nil) @@ -128,19 +84,8 @@ func TestGetUserInfo(t *testing.T) { { name: "Error creating the meeting", creator: &model.User{Id: "testUserID"}, - channelID: "testChannelID", - topic: "testTopic", expectedError: "error creating the meeting", setup: func() { - info := &UserInfo{ - Email: "testEmail", - } - p.setConfiguration(&configuration{ - EncryptionKey: "demo_encrypt_key", - }) - key := []byte(p.getConfiguration().EncryptionKey) - encryptedUserInfo, err := info.EncryptedJSON(key) - require.NoError(t, err) api.On("KVGet", tokenKey+"testUserID").Return(encryptedUserInfo, nil).Once() api.On("HasPermissionToChannel", "testUserID", "testChannelID", model.PermissionCreatePost).Return(true) api.On("GetChannel", "testChannelID").Return(&model.Channel{Id: "testChannelID", Type: model.ChannelTypeDirect}, nil) @@ -151,51 +96,27 @@ func TestGetUserInfo(t *testing.T) { { name: "Error creating the meeting post", creator: &model.User{Id: "testUserID", Username: "testUsername"}, - channelID: "testChannelID", - topic: "testTopic", expectedError: "error creating the post", setup: func() { - info := &UserInfo{ - Email: "testEmail", - } - p.setConfiguration(&configuration{ - EncryptionKey: "demo_encrypt_key", - }) - key := []byte(p.getConfiguration().EncryptionKey) - encryptedUserInfo, err := info.EncryptedJSON(key) require.NoError(t, err) api.On("KVGet", tokenKey+"testUserID").Return(encryptedUserInfo, nil).Once() api.On("HasPermissionToChannel", "testUserID", "testChannelID", model.PermissionCreatePost).Return(true) api.On("GetChannel", "testChannelID").Return(&model.Channel{Id: "testChannelID", Type: model.ChannelTypeDirect}, nil) api.On("GetChannelMembers", "testChannelID", 0, 100).Return(model.ChannelMembers{}, nil) api.On("CreatePost", mock.Anything).Return(nil, &model.AppError{Message: "error creating the post"}) - testJoinURL := "testJoinURL" - client.On("CreateMeeting").Return(&msgraph.OnlineMeeting{JoinURL: &testJoinURL}, nil) + client.On("CreateMeeting").Return(&msgraph.OnlineMeeting{JoinURL: model.NewString("testJoinURL")}, nil) }, }, { - name: "Meeting posted successfully", - creator: &model.User{Id: "testUserID", Username: "testUsername"}, - channelID: "testChannelID", - topic: "testTopic", - expectedError: "", + name: "Meeting posted successfully", + creator: &model.User{Id: "testUserID", Username: "testUsername"}, setup: func() { - info := &UserInfo{ - Email: "testEmail", - } - p.setConfiguration(&configuration{ - EncryptionKey: "demo_encrypt_key", - }) - key := []byte(p.getConfiguration().EncryptionKey) - encryptedUserInfo, err := info.EncryptedJSON(key) - require.NoError(t, err) api.On("KVGet", tokenKey+"testUserID").Return(encryptedUserInfo, nil).Once() api.On("HasPermissionToChannel", "testUserID", "testChannelID", model.PermissionCreatePost).Return(true) api.On("GetChannel", "testChannelID").Return(&model.Channel{Id: "testChannelID", Type: model.ChannelTypeDirect}, nil) api.On("GetChannelMembers", "testChannelID", 0, 100).Return(model.ChannelMembers{}, nil) api.On("CreatePost", mock.Anything).Return(&model.Post{}, nil) - testJoinURL := "testJoinURL" - client.On("CreateMeeting").Return(&msgraph.OnlineMeeting{JoinURL: &testJoinURL}, nil) + client.On("CreateMeeting").Return(&msgraph.OnlineMeeting{JoinURL: model.NewString("testJoinURL")}, nil) }, }, } @@ -207,7 +128,7 @@ func TestGetUserInfo(t *testing.T) { tt.setup() - _, _, err := p.postMeeting(tt.creator, tt.channelID, tt.topic) + _, _, err := p.postMeeting(tt.creator, "testChannelID", "testTopic") if tt.expectedError != "" { require.Error(t, err) @@ -222,6 +143,8 @@ func TestGetUserInfo(t *testing.T) { } func TestPostConfirmCreateOrJoin(t *testing.T) { + p, api, _ := SetupPluginMocks() + testCases := []struct { name string provider string @@ -247,15 +170,6 @@ func TestPostConfirmCreateOrJoin(t *testing.T) { userID := "test_user_id" creatorName := "test_creator" - api := &plugintest.API{} - client := &MockClient{} - p := &Plugin{ - MattermostPlugin: plugin.MattermostPlugin{ - API: api, - }, - client: client, - botUserID: "testBotUserID", - } expectedPost := &model.Post{ UserId: p.botUserID, ChannelId: channelID, @@ -271,6 +185,7 @@ func TestPostConfirmCreateOrJoin(t *testing.T) { "meeting_provider": tt.provider, }, } + api.ExpectedCalls = nil api.On("SendEphemeralPost", userID, mock.AnythingOfType("*model.Post")).Return(expectedPost, nil) post := p.postConfirmCreateOrJoin(meetingURL, channelID, topic, userID, creatorName, tt.provider) @@ -280,13 +195,8 @@ func TestPostConfirmCreateOrJoin(t *testing.T) { } func TestPostConnect(t *testing.T) { - api := &plugintest.API{} - p := &Plugin{ - MattermostPlugin: plugin.MattermostPlugin{ - API: api, - }, - botUserID: "testBostUserID", - } + p, api, _ := SetupPluginMocks() + tests := []struct { name string userID string @@ -306,10 +216,9 @@ func TestPostConnect(t *testing.T) { }, }, { - name: "Error getting oauth message", - channelID: "testChannelID", - userID: "testUserID", - expectedError: "", + name: "Error getting oauth message", + channelID: "testChannelID", + userID: "testUserID", setup: func() { testSiteURL := "testSiteURL" api.On("GetConfig").Return(&model.Config{ServiceSettings: model.ServiceSettings{SiteURL: &testSiteURL}}) @@ -338,3 +247,16 @@ func TestPostConnect(t *testing.T) { }) } } + +func SetupPluginMocks() (*Plugin, *plugintest.API, *MockClient) { + api := &plugintest.API{} + client := &MockClient{} + p := &Plugin{ + MattermostPlugin: plugin.MattermostPlugin{ + API: api, + }, + client: client, + } + + return p, api, client +} From 3cd2d77af51d404675fc9957f7849d51d97ab6bd Mon Sep 17 00:00:00 2001 From: kshitij katiyar Date: Mon, 7 Oct 2024 14:51:12 +0530 Subject: [PATCH 08/13] [MM-823]: fixed lint --- server/post_test.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/server/post_test.go b/server/post_test.go index 7d9d15f..316a292 100644 --- a/server/post_test.go +++ b/server/post_test.go @@ -51,11 +51,11 @@ func TestGetUserInfo(t *testing.T) { { name: "Error getting the channel", creator: &model.User{Id: "testUserID"}, - expectedError: "error occured getting the channel", + expectedError: "error occurred getting the channel", setup: func() { api.On("KVGet", tokenKey+"testUserID").Return(encryptedUserInfo, nil).Once() api.On("HasPermissionToChannel", "testUserID", "testChannelID", model.PermissionCreatePost).Return(true) - api.On("GetChannel", "testChannelID").Return(nil, &model.AppError{Message: "error occured getting the channel"}) + api.On("GetChannel", "testChannelID").Return(nil, &model.AppError{Message: "error occurred getting the channel"}) }, }, { @@ -78,7 +78,6 @@ func TestGetUserInfo(t *testing.T) { api.On("HasPermissionToChannel", "testUserID", "testChannelID", model.PermissionCreatePost).Return(true) api.On("GetChannel", "testChannelID").Return(&model.Channel{Id: "testChannelID", Type: model.ChannelTypeDirect}, nil) api.On("GetChannelMembers", "testChannelID", 0, 100).Return(nil, nil) - }, }, { @@ -212,7 +211,6 @@ func TestPostConnect(t *testing.T) { setup: func() { api.On("GetConfig").Return(&model.Config{ServiceSettings: model.ServiceSettings{SiteURL: nil}}) api.On("LogError", "postConnect, cannot get oauth message", "error", "error fetching siteUrl") - }, }, { @@ -223,7 +221,6 @@ func TestPostConnect(t *testing.T) { testSiteURL := "testSiteURL" api.On("GetConfig").Return(&model.Config{ServiceSettings: model.ServiceSettings{SiteURL: &testSiteURL}}) api.On("SendEphemeralPost", "testUserID", mock.AnythingOfType("*model.Post")).Return(&model.Post{}, nil) - }, }, } @@ -242,7 +239,6 @@ func TestPostConnect(t *testing.T) { } else { require.NoError(t, err) } - api.AssertExpectations(t) }) } From 8db5ba14c837bc98f22a408bd61373f61c8d219f Mon Sep 17 00:00:00 2001 From: kshitij katiyar Date: Mon, 7 Oct 2024 14:57:14 +0530 Subject: [PATCH 09/13] [MM-822]: review fixes --- server/http_test.go | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/server/http_test.go b/server/http_test.go index c1b3e53..d7f497f 100644 --- a/server/http_test.go +++ b/server/http_test.go @@ -37,12 +37,9 @@ func TestConnectUser(t *testing.T) { }{ { name: "Unauthorized User", - userID: "", channelID: "testChannelID", expectedStatus: http.StatusUnauthorized, expectedBody: "Not authorized\n", - redirectExpected: false, - expectedRedirectURL: "", setup: func() { api.On("LogError", "connectUser, unauthorized user").Return(nil) }, @@ -50,11 +47,8 @@ func TestConnectUser(t *testing.T) { { name: "Missing Channel ID", userID: "testUserID", - channelID: "", expectedStatus: http.StatusBadRequest, expectedBody: "channelID missing\n", - redirectExpected: false, - expectedRedirectURL: "", setup: func() { api.On("LogError", "connectUser, missing channelID in query params").Return(nil) }, @@ -65,8 +59,6 @@ func TestConnectUser(t *testing.T) { channelID: "testChannelID", expectedStatus: http.StatusInternalServerError, expectedBody: "error fetching siteUrl\n", - redirectExpected: false, - expectedRedirectURL: "", setup: func() { p.setConfiguration(&configuration{ OAuth2ClientID: "testOAuth2ClientID", @@ -89,8 +81,6 @@ func TestConnectUser(t *testing.T) { channelID: "testChannelID", expectedStatus: http.StatusInternalServerError, expectedBody: "error occurred getting stored user state\n", - redirectExpected: false, - expectedRedirectURL: "", setup: func() { p.setConfiguration(&configuration{ OAuth2ClientID: "testOAuth2ClientID", @@ -295,7 +285,7 @@ func TestHandleStartMeeting(t *testing.T) { api.On("GetPostsSince", "testChannelID", (time.Now().Unix()-30)*1000).Return(&model.PostList{}, nil) api.On("LogError", "postConnect, cannot get oauth message", "error", "error fetching siteUrl") api.On("LogWarn", "failed to create connect post", "error", "error fetching siteUrl") - client.On("GetMe").Return(&msgraph.User{}, &authError{Message: "error occured in getting the msgraph user"}) + client.On("GetMe").Return(&msgraph.User{}, &authError{Message: "error occurred in getting the msgraph user"}) }, }, { @@ -324,7 +314,6 @@ func TestHandleStartMeeting(t *testing.T) { api.On("LogError", "handleStartMeeting, failed to post meeting", "UserID", "testUserID", "Error", "cannot create post in this channel") api.On("HasPermissionToChannel", "testUserID", "testChannelID", model.PermissionCreatePost).Return(false) client.On("GetMe").Return(&msgraph.User{}, nil) - // api.On("SendEphemeralPost", "testUserID", mock.Anything).Return(&model.Post{}) }, }, { @@ -422,11 +411,8 @@ func TestCompleteUserOAuth(t *testing.T) { }{ { name: "Unauthorized User", - userID: "", expectedStatus: http.StatusUnauthorized, expectedBody: "Not authorized, missing Mattermost user id\n", - state: "", - authorizationCode: "", setup: func() { api.On("LogError", "completeUserOAuth, unauthorized user").Return(nil) }, @@ -436,8 +422,6 @@ func TestCompleteUserOAuth(t *testing.T) { userID: "testUserID", expectedStatus: http.StatusInternalServerError, expectedBody: "error in oauth config\n", - state: "", - authorizationCode: "", setup: func() { p.setConfiguration(&configuration{ OAuth2ClientID: "testOAuth2ClientID", @@ -457,8 +441,6 @@ func TestCompleteUserOAuth(t *testing.T) { userID: "testUserID", expectedStatus: http.StatusBadRequest, expectedBody: "missing authorization code\n", - state: "", - authorizationCode: "", setup: func() { p.setConfiguration(&configuration{ OAuth2ClientID: "testOAuth2ClientID", From 62d8c2b76e26813ddca6bdf7dc17b4d823800969 Mon Sep 17 00:00:00 2001 From: kshitij katiyar Date: Mon, 7 Oct 2024 15:14:53 +0530 Subject: [PATCH 10/13] [MM-822]: fixed lint --- server/command_test.go | 2 +- server/http_test.go | 58 +++++++++++++++++++++--------------------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/server/command_test.go b/server/command_test.go index 135473f..aea0551 100644 --- a/server/command_test.go +++ b/server/command_test.go @@ -455,7 +455,7 @@ func TestExecuteCommand(t *testing.T) { api.On("SendEphemeralPost", tt.commandArgs.UserId, post).Return(&model.Post{}).Once() - response, _ := p.ExecuteCommand(&dummyPluginContext, &tt.commandArgs) + response, _ := p.ExecuteCommand(&dummyPluginContext, &tt.commandArgs) // #nosec G601 require.Equal(t, &model.CommandResponse{}, response) api.AssertExpectations(t) diff --git a/server/http_test.go b/server/http_test.go index d7f497f..204aa54 100644 --- a/server/http_test.go +++ b/server/http_test.go @@ -36,29 +36,29 @@ func TestConnectUser(t *testing.T) { setup func() }{ { - name: "Unauthorized User", - channelID: "testChannelID", - expectedStatus: http.StatusUnauthorized, - expectedBody: "Not authorized\n", + name: "Unauthorized User", + channelID: "testChannelID", + expectedStatus: http.StatusUnauthorized, + expectedBody: "Not authorized\n", setup: func() { api.On("LogError", "connectUser, unauthorized user").Return(nil) }, }, { - name: "Missing Channel ID", - userID: "testUserID", - expectedStatus: http.StatusBadRequest, - expectedBody: "channelID missing\n", + name: "Missing Channel ID", + userID: "testUserID", + expectedStatus: http.StatusBadRequest, + expectedBody: "channelID missing\n", setup: func() { api.On("LogError", "connectUser, missing channelID in query params").Return(nil) }, }, { - name: "Error getting OAuth Config", - userID: "testUserID", - channelID: "testChannelID", - expectedStatus: http.StatusInternalServerError, - expectedBody: "error fetching siteUrl\n", + name: "Error getting OAuth Config", + userID: "testUserID", + channelID: "testChannelID", + expectedStatus: http.StatusInternalServerError, + expectedBody: "error fetching siteUrl\n", setup: func() { p.setConfiguration(&configuration{ OAuth2ClientID: "testOAuth2ClientID", @@ -76,11 +76,11 @@ func TestConnectUser(t *testing.T) { }, }, { - name: "Error Getting User State", - userID: "testUserID", - channelID: "testChannelID", - expectedStatus: http.StatusInternalServerError, - expectedBody: "error occurred getting stored user state\n", + name: "Error Getting User State", + userID: "testUserID", + channelID: "testChannelID", + expectedStatus: http.StatusInternalServerError, + expectedBody: "error occurred getting stored user state\n", setup: func() { p.setConfiguration(&configuration{ OAuth2ClientID: "testOAuth2ClientID", @@ -410,18 +410,18 @@ func TestCompleteUserOAuth(t *testing.T) { setup func() }{ { - name: "Unauthorized User", - expectedStatus: http.StatusUnauthorized, - expectedBody: "Not authorized, missing Mattermost user id\n", + name: "Unauthorized User", + expectedStatus: http.StatusUnauthorized, + expectedBody: "Not authorized, missing Mattermost user id\n", setup: func() { api.On("LogError", "completeUserOAuth, unauthorized user").Return(nil) }, }, { - name: "Error getting OAuth config", - userID: "testUserID", - expectedStatus: http.StatusInternalServerError, - expectedBody: "error in oauth config\n", + name: "Error getting OAuth config", + userID: "testUserID", + expectedStatus: http.StatusInternalServerError, + expectedBody: "error in oauth config\n", setup: func() { p.setConfiguration(&configuration{ OAuth2ClientID: "testOAuth2ClientID", @@ -437,10 +437,10 @@ func TestCompleteUserOAuth(t *testing.T) { }, }, { - name: "Missing authorization code", - userID: "testUserID", - expectedStatus: http.StatusBadRequest, - expectedBody: "missing authorization code\n", + name: "Missing authorization code", + userID: "testUserID", + expectedStatus: http.StatusBadRequest, + expectedBody: "missing authorization code\n", setup: func() { p.setConfiguration(&configuration{ OAuth2ClientID: "testOAuth2ClientID", From 4dd446f5377f9a3bc082ba2b40f21f7feef2efc1 Mon Sep 17 00:00:00 2001 From: kshitij katiyar Date: Mon, 7 Oct 2024 15:20:44 +0530 Subject: [PATCH 11/13] [MM-822]: removed default value variable declaration --- server/command_test.go | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/server/command_test.go b/server/command_test.go index aea0551..b335e4b 100644 --- a/server/command_test.go +++ b/server/command_test.go @@ -68,7 +68,6 @@ func TestHandleConnect(t *testing.T) { mockSetup: func(_ *plugintest.API, _ []byte, _ *MockClient) { }, expectedOutput: tooManyParametersText, - expectError: false, }, { name: "Error connecting user", @@ -80,9 +79,8 @@ func TestHandleConnect(t *testing.T) { api.On("GetConfig").Return(&model.Config{ServiceSettings: model.ServiceSettings{SiteURL: model.NewString("https://example.com")}}) mockClient.On("GetMe").Return(&msgraph.User{}, errors.New("error getting user details")) }, - expectedOutput: "", - expectError: true, - expectedError: "error getting user details", + expectError: true, + expectedError: "error getting user details", }, { name: "Successful connection", @@ -93,8 +91,6 @@ func TestHandleConnect(t *testing.T) { api.On("GetConfig").Return(&model.Config{ServiceSettings: model.ServiceSettings{SiteURL: model.NewString("https://example.com")}}) mockClient.On("GetMe").Return(&msgraph.User{}, nil) }, - expectedOutput: "", - expectError: false, }, } @@ -290,8 +286,6 @@ func TestHandleStart(t *testing.T) { api.On("SendEphemeralPost", "demoUserID", mock.Anything).Return(&model.Post{}) mockTracker.On("TrackUserEvent", mock.Anything, "demoUserID", mock.Anything).Return(nil) }, - expectError: false, - expectedOutput: "", }, { name: "Authentication error", @@ -351,7 +345,6 @@ func TestHandleStart(t *testing.T) { mockClient.On("CreateMeeting", mock.Anything, mock.Anything, mock.Anything).Return(&msgraph.OnlineMeeting{JoinURL: &joinURL}, nil) mockTracker.On("TrackUserEvent", "meeting_started", "demoUserID", mock.Anything).Return(nil) }, - expectError: false, }, } From 6ee794e2ac34e2915eefd1697eb882f8f1c8ddbd Mon Sep 17 00:00:00 2001 From: kshitij katiyar Date: Mon, 7 Oct 2024 15:42:03 +0530 Subject: [PATCH 12/13] [MM-822]: review fixes --- server/authorization_test.go | 2 +- server/command_test.go | 10 +++++----- server/http_test.go | 18 +++++++++--------- server/utils.go | 2 +- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/server/authorization_test.go b/server/authorization_test.go index 1ecbe26..4750f5c 100644 --- a/server/authorization_test.go +++ b/server/authorization_test.go @@ -28,7 +28,7 @@ func TestGetOauthMessage(t *testing.T) { siteURL: "", setupFunc: func(p *Plugin) { msg, err := p.getOauthMessage("mockChannelID") - require.EqualError(t, err, "error fetching siteUrl") + require.EqualError(t, err, "error fetching siteURL") require.EqualValues(t, "", msg) }, }, diff --git a/server/command_test.go b/server/command_test.go index f126c93..9913bdd 100644 --- a/server/command_test.go +++ b/server/command_test.go @@ -389,16 +389,16 @@ func TestExecuteCommand(t *testing.T) { mockAPI := &plugintest.API{} p := SetupMockPlugin(mockAPI, nil, nil) - var dummyPluginContext plugin.Context + var dummyPluginContext *plugin.Context tests := []struct { name string - commandArgs model.CommandArgs + commandArgs *model.CommandArgs expectedMsg string }{ { name: "Invalid Command", - commandArgs: model.CommandArgs{ + commandArgs: &model.CommandArgs{ Command: "/dummyCommand start", ChannelId: "dummyChannelID", UserId: "dummyUserID", @@ -407,7 +407,7 @@ func TestExecuteCommand(t *testing.T) { }, { name: "Successful execution of Help Command", - commandArgs: model.CommandArgs{ + commandArgs: &model.CommandArgs{ Command: "/mstmeetings help", ChannelId: "dummyChannelID", UserId: "dummyUserID", @@ -426,7 +426,7 @@ func TestExecuteCommand(t *testing.T) { mockAPI.On("SendEphemeralPost", tt.commandArgs.UserId, post).Return(&model.Post{}).Once() - response, _ := p.ExecuteCommand(&dummyPluginContext, &tt.commandArgs) // #nosec G601 + response, _ := p.ExecuteCommand(dummyPluginContext, tt.commandArgs) require.Equal(t, &model.CommandResponse{}, response) mockAPI.AssertExpectations(t) diff --git a/server/http_test.go b/server/http_test.go index 204aa54..0b2d9f4 100644 --- a/server/http_test.go +++ b/server/http_test.go @@ -58,7 +58,7 @@ func TestConnectUser(t *testing.T) { userID: "testUserID", channelID: "testChannelID", expectedStatus: http.StatusInternalServerError, - expectedBody: "error fetching siteUrl\n", + expectedBody: "error fetching siteURL\n", setup: func() { p.setConfiguration(&configuration{ OAuth2ClientID: "testOAuth2ClientID", @@ -72,7 +72,7 @@ func TestConnectUser(t *testing.T) { }, }) - api.On("LogError", "connectUser, failed to get oauth config", "Error", "error fetching siteUrl").Return(nil) + api.On("LogError", "connectUser, failed to get oauth config", "Error", "error fetching siteURL").Return(nil) }, }, { @@ -240,7 +240,7 @@ func TestHandleStartMeeting(t *testing.T) { userID: "testUserID", channelID: "testChannelID", expectedStatus: http.StatusOK, - expectedBody: "{\"meeting_url\": \"\"}error fetching siteUrl\n", + expectedBody: "{\"meeting_url\": \"\"}error fetching siteURL\n", setup: func() { api.On("GetConfig").Return(&model.Config{ ServiceSettings: model.ServiceSettings{ @@ -250,8 +250,8 @@ func TestHandleStartMeeting(t *testing.T) { api.On("GetUser", "testUserID").Return(&model.User{Id: "testUserID"}, nil) api.On("GetChannelMember", "testChannelID", "testUserID").Return(nil, nil) api.On("GetPostsSince", "testChannelID", (time.Now().Unix()-30)*1000).Return(&model.PostList{}, nil) - api.On("LogError", "postConnect, cannot get oauth message", "error", "error fetching siteUrl").Return() - api.On("LogError", "authenticateAndFetchUser, cannot get oauth message", "error", "error fetching siteUrl").Return() + api.On("LogError", "postConnect, cannot get oauth message", "error", "error fetching siteURL").Return() + api.On("LogError", "authenticateAndFetchUser, cannot get oauth message", "error", "error fetching siteURL").Return() api.On("LogWarn", "failed to create connect post", "error", mock.Anything).Return(nil) }, }, @@ -260,7 +260,7 @@ func TestHandleStartMeeting(t *testing.T) { userID: "testUserID", channelID: "testChannelID", expectedStatus: http.StatusOK, - expectedBody: "{\"meeting_url\": \"\"}error fetching siteUrl\n", + expectedBody: "{\"meeting_url\": \"\"}error fetching siteURL\n", setup: func() { siteURL := "testSiteURL" api.On("GetConfig").Return(&model.Config{ @@ -283,8 +283,8 @@ func TestHandleStartMeeting(t *testing.T) { api.On("GetUser", "testUserID").Return(&model.User{Id: "testUserID"}, nil) api.On("GetChannelMember", "testChannelID", "testUserID").Return(nil, nil) api.On("GetPostsSince", "testChannelID", (time.Now().Unix()-30)*1000).Return(&model.PostList{}, nil) - api.On("LogError", "postConnect, cannot get oauth message", "error", "error fetching siteUrl") - api.On("LogWarn", "failed to create connect post", "error", "error fetching siteUrl") + api.On("LogError", "postConnect, cannot get oauth message", "error", "error fetching siteURL") + api.On("LogWarn", "failed to create connect post", "error", "error fetching siteURL") client.On("GetMe").Return(&msgraph.User{}, &authError{Message: "error occurred in getting the msgraph user"}) }, }, @@ -433,7 +433,7 @@ func TestCompleteUserOAuth(t *testing.T) { SiteURL: nil, }, }) - api.On("LogError", "completeUserOAuth, failed to get oauth config", "Error", "error fetching siteUrl").Return(nil) + api.On("LogError", "completeUserOAuth, failed to get oauth config", "Error", "error fetching siteURL").Return(nil) }, }, { diff --git a/server/utils.go b/server/utils.go index 7f22735..ab6a0a9 100644 --- a/server/utils.go +++ b/server/utils.go @@ -12,7 +12,7 @@ import ( func (p *Plugin) getSiteURL() (string, error) { siteURLRef := p.API.GetConfig().ServiceSettings.SiteURL if siteURLRef == nil || *siteURLRef == "" { - return "", errors.New("error fetching siteUrl") + return "", errors.New("error fetching siteURL") } return *siteURLRef, nil From 1d809df5d732dc94c0a595f5c0305a9348934e1b Mon Sep 17 00:00:00 2001 From: kshitij katiyar Date: Mon, 7 Oct 2024 15:47:23 +0530 Subject: [PATCH 13/13] [MM-823]: review fixes --- server/post_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/post_test.go b/server/post_test.go index 316a292..bbecbc6 100644 --- a/server/post_test.go +++ b/server/post_test.go @@ -207,10 +207,10 @@ func TestPostConnect(t *testing.T) { name: "Error getting oauth message", channelID: "testChannelID", userID: "testUserID", - expectedError: "error fetching siteUrl", + expectedError: "error fetching siteURL", setup: func() { api.On("GetConfig").Return(&model.Config{ServiceSettings: model.ServiceSettings{SiteURL: nil}}) - api.On("LogError", "postConnect, cannot get oauth message", "error", "error fetching siteUrl") + api.On("LogError", "postConnect, cannot get oauth message", "error", "error fetching siteURL") }, }, {