From 8c8df81d56fb30e4a48930a53213c3227d29cd42 Mon Sep 17 00:00:00 2001 From: kshitij katiyar Date: Mon, 4 Nov 2024 14:54:15 +0530 Subject: [PATCH 1/6] addede testcase for some functions in server/command.go --- server/plugin/command.go | 8 +- server/plugin/command_test.go | 322 ++++++++++++++++++++++++++++++++++ 2 files changed, 326 insertions(+), 4 deletions(-) diff --git a/server/plugin/command.go b/server/plugin/command.go index 6f632583d..c049007a7 100644 --- a/server/plugin/command.go +++ b/server/plugin/command.go @@ -151,7 +151,7 @@ func (p *Plugin) getMutedUsernames(userInfo *GitHubUserInfo) []string { return mutedUsers } -func (p *Plugin) handleMuteList(args *model.CommandArgs, userInfo *GitHubUserInfo) string { +func (p *Plugin) handleMuteList(_ *model.CommandArgs, userInfo *GitHubUserInfo) string { mutedUsernames := p.getMutedUsernames(userInfo) var mutedUsers string for _, user := range mutedUsernames { @@ -172,7 +172,7 @@ func contains(s []string, e string) bool { return false } -func (p *Plugin) handleMuteAdd(args *model.CommandArgs, username string, userInfo *GitHubUserInfo) string { +func (p *Plugin) handleMuteAdd(_ *model.CommandArgs, username string, userInfo *GitHubUserInfo) string { mutedUsernames := p.getMutedUsernames(userInfo) if contains(mutedUsernames, username) { return username + " is already muted" @@ -198,7 +198,7 @@ func (p *Plugin) handleMuteAdd(args *model.CommandArgs, username string, userInf return fmt.Sprintf("`%v`", username) + " is now muted. You'll no longer receive notifications for comments in your PRs and issues." } -func (p *Plugin) handleUnmute(args *model.CommandArgs, username string, userInfo *GitHubUserInfo) string { +func (p *Plugin) handleUnmute(_ *model.CommandArgs, username string, userInfo *GitHubUserInfo) string { mutedUsernames := p.getMutedUsernames(userInfo) userToMute := []string{username} newMutedList := arrayDifference(mutedUsernames, userToMute) @@ -211,7 +211,7 @@ func (p *Plugin) handleUnmute(args *model.CommandArgs, username string, userInfo return fmt.Sprintf("`%v`", username) + " is no longer muted" } -func (p *Plugin) handleUnmuteAll(args *model.CommandArgs, userInfo *GitHubUserInfo) string { +func (p *Plugin) handleUnmuteAll(_ *model.CommandArgs, userInfo *GitHubUserInfo) string { _, err := p.store.Set(userInfo.UserID+"-muted-users", []byte("")) if err != nil { return "Error occurred unmuting users" diff --git a/server/plugin/command_test.go b/server/plugin/command_test.go index bba55c982..ed41a21ed 100644 --- a/server/plugin/command_test.go +++ b/server/plugin/command_test.go @@ -9,6 +9,7 @@ import ( "github.com/mattermost/mattermost/server/public/plugin" "github.com/mattermost/mattermost/server/public/plugin/plugintest" "github.com/mattermost/mattermost/server/public/pluginapi" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -333,3 +334,324 @@ func TestExecuteCommand(t *testing.T) { }) } } + +func TestGetMutedUsernames(t *testing.T) { + mockKvStore, mockAPI, _, _, _ := GetTestSetup(t) + p := getPluginTest(mockAPI, mockKvStore) + userInfo, err := GetMockGHUserInfo(p) + assert.NoError(t, err) + + tests := []struct { + name string + setup func() + assertions func(t *testing.T, result []string) + }{ + { + name: "Error retrieving muted usernames", + setup: func() { + mockKvStore.EXPECT().Get("mockUserID-muted-users", gomock.Any()).Return(errors.New("error retrieving muted users")).Times(1) + }, + assertions: func(t *testing.T, result []string) { + assert.Nil(t, result) + }, + }, + { + name: "No muted usernames set for user", + setup: func() { + mockKvStore.EXPECT().Get("mockUserID-muted-users", gomock.Any()).DoAndReturn(func(key string, value *[]byte) error { + *value = []byte("") + return nil + }).Times(1) + }, + assertions: func(t *testing.T, result []string) { + assert.Equal(t, []string(nil), result) + }, + }, + { + name: "Successfully retrieves muted usernames", + setup: func() { + mutedUsernames := []byte("user1,user2,user3") + mockKvStore.EXPECT().Get("mockUserID-muted-users", gomock.Any()).DoAndReturn(func(key string, value *[]byte) error { + *value = mutedUsernames + return nil + }).Times(1) + }, + assertions: func(t *testing.T, result []string) { + assert.Equal(t, []string{"user1", "user2", "user3"}, result) + }, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + tc.setup() + + mutedUsernames := p.getMutedUsernames(userInfo) + + tc.assertions(t, mutedUsernames) + }) + } +} + +func TestHandleMuteList(t *testing.T) { + mockKvStore, mockAPI, _, _, _ := GetTestSetup(t) + p := getPluginTest(mockAPI, mockKvStore) + userInfo, err := GetMockGHUserInfo(p) + assert.NoError(t, err) + + tests := []struct { + name string + setup func() + assertions func(t *testing.T, result string) + }{ + { + name: "Error retrieving muted usernames", + setup: func() { + mockKvStore.EXPECT().Get("mockUserID-muted-users", gomock.Any()).Return(errors.New("error retrieving muted users")).Times(1) + }, + assertions: func(t *testing.T, result string) { + assert.Equal(t, "You have no muted users", result) + }, + }, + { + name: "No muted usernames set for user", + setup: func() { + mockKvStore.EXPECT().Get("mockUserID-muted-users", gomock.Any()).DoAndReturn(func(key string, value *[]byte) error { + *value = []byte("") + return nil + }).Times(1) + }, + assertions: func(t *testing.T, result string) { + assert.Equal(t, "You have no muted users", result) + }, + }, + { + name: "Successfully retrieves and formats muted usernames", + setup: func() { + mutedUsernames := []byte("user1,user2,user3") + mockKvStore.EXPECT().Get("mockUserID-muted-users", gomock.Any()).DoAndReturn(func(key string, value *[]byte) error { + *value = mutedUsernames + return nil + }).Times(1) + }, + assertions: func(t *testing.T, result string) { + expectedOutput := "Your muted users:\n- user1\n- user2\n- user3\n" + assert.Equal(t, expectedOutput, result) + }, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + tc.setup() + + result := p.handleMuteList(nil, userInfo) + + tc.assertions(t, result) + }) + } +} + +func TestContains(t *testing.T) { + tests := []struct { + name string + slice []string + element string + assertions func(t *testing.T, result bool) + }{ + { + name: "Element is present in slice", + slice: []string{"expectedElement1", "expectedElement2", "expectedElement3"}, + element: "expectedElement2", + assertions: func(t *testing.T, result bool) { + assert.True(t, result) + }, + }, + { + name: "Element is not present in slice", + slice: []string{"expectedElement1", "expectedElement2", "expectedElement3"}, + element: "expectedElement4", + assertions: func(t *testing.T, result bool) { + assert.False(t, result) + }, + }, + { + name: "Empty slice", + slice: []string{}, + element: "expectedElement1", + assertions: func(t *testing.T, result bool) { + assert.False(t, result) + }, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := contains(tc.slice, tc.element) + tc.assertions(t, result) + }) + } +} + +func TestHandleMuteAdd(t *testing.T) { + mockKvStore, mockAPI, _, _, _ := GetTestSetup(t) + p := getPluginTest(mockAPI, mockKvStore) + userInfo, err := GetMockGHUserInfo(p) + assert.NoError(t, err) + + tests := []struct { + name string + username string + setup func() + assertions func(t *testing.T, result string) + }{ + { + name: "Error saving the new muted username", + username: "errorUser", + setup: func() { + mockKvStore.EXPECT().Get(userInfo.UserID+"-muted-users", gomock.Any()).DoAndReturn(func(key string, value *[]byte) error { + *value = []byte("existingUser") + return nil + }).Times(1) + mockKvStore.EXPECT().Set(userInfo.UserID+"-muted-users", []byte("existingUser,errorUser")).Return(false, errors.New("store error")).Times(1) + }, + assertions: func(t *testing.T, result string) { + assert.Equal(t, "Error occurred saving list of muted users", result) + }, + }, + { + name: "Username is already muted", + username: "alreadyMutedUser", + setup: func() { + mockKvStore.EXPECT().Get(userInfo.UserID+"-muted-users", gomock.Any()).DoAndReturn(func(key string, value *[]byte) error { + *value = []byte("alreadyMutedUser") + return nil + }).Times(1) + }, + assertions: func(t *testing.T, result string) { + assert.Equal(t, "alreadyMutedUser is already muted", result) + }, + }, + { + name: "Invalid username with comma", + username: "invalid,user", + setup: func() { + mockKvStore.EXPECT().Get(userInfo.UserID+"-muted-users", gomock.Any()).DoAndReturn(func(key string, value *[]byte) error { + *value = []byte("") + return nil + }).Times(1) + }, + assertions: func(t *testing.T, result string) { + assert.Equal(t, "Invalid username provided", result) + }, + }, + { + name: "Successfully adds new muted username", + username: "newUser", + setup: func() { + mockKvStore.EXPECT().Get(userInfo.UserID+"-muted-users", gomock.Any()).DoAndReturn(func(key string, value *[]byte) error { + *value = []byte("existingUser") + return nil + }).Times(1) + mockKvStore.EXPECT().Set(userInfo.UserID+"-muted-users", []byte("existingUser,newUser")).Return(true, nil).Times(1) + }, + assertions: func(t *testing.T, result string) { + expectedMessage := "`newUser` is now muted. You'll no longer receive notifications for comments in your PRs and issues." + assert.Equal(t, expectedMessage, result) + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + tc.setup() + result := p.handleMuteAdd(nil, tc.username, userInfo) + tc.assertions(t, result) + }) + } +} + +func TestHandleUnmute(t *testing.T) { + mockKvStore, mockAPI, _, _, _ := GetTestSetup(t) + p := getPluginTest(mockAPI, mockKvStore) + userInfo, err := GetMockGHUserInfo(p) + assert.NoError(t, err) + + tests := []struct { + name string + username string + setup func() + expectedResult string + }{ + { + name: "Error occurred while unmuting the user", + username: "user1", + setup: func() { + mutedUsernames := []byte("user1,user2,user3") + mockKvStore.EXPECT().Get("mockUserID-muted-users", gomock.Any()).DoAndReturn(func(key string, value *[]byte) error { + *value = mutedUsernames + return nil + }).Times(1) + mockKvStore.EXPECT().Set(userInfo.UserID+"-muted-users", gomock.Any()).Return(false, errors.New("error saving muted users")).Times(1) + }, + expectedResult: "Error occurred unmuting users", + }, + { + name: "Successfully unmute a user", + username: "user1", + setup: func() { + mutedUsernames := []byte("user1,user2,user3") + mockKvStore.EXPECT().Get("mockUserID-muted-users", gomock.Any()).DoAndReturn(func(key string, value *[]byte) error { + *value = mutedUsernames + return nil + }).Times(1) + mockKvStore.EXPECT().Set(userInfo.UserID+"-muted-users", gomock.Any()).Return(true, nil).Times(1) + }, + expectedResult: "`user1` is no longer muted", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + tc.setup() + result := p.handleUnmute(nil, tc.username, userInfo) + assert.Equal(t, tc.expectedResult, result) + }) + } +} + +func TestHandleUnmuteAll(t *testing.T) { + mockKvStore, mockAPI, _, _, _ := GetTestSetup(t) + p := getPluginTest(mockAPI, mockKvStore) + userInfo, err := GetMockGHUserInfo(p) + assert.NoError(t, err) + + tests := []struct { + name string + setup func() + assertions func(string) + expectedResult string + }{ + { + name: "Error occurred while unmuting all users", + setup: func() { + mockKvStore.EXPECT().Set(userInfo.UserID+"-muted-users", []byte("")).Return(false, errors.New("error saving muted users")).Times(1) + }, + assertions: func(expectedResult string) { + assert.Equal(t, expectedResult, "Error occurred unmuting users") + }, + }, + { + name: "Successfully unmute all users", + setup: func() { + mockKvStore.EXPECT().Set(userInfo.UserID+"-muted-users", []byte("")).Return(true, nil).Times(1) + }, + assertions: func(expectedResult string) { + assert.Equal(t, expectedResult, "Unmuted all users") + }, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + tc.setup() + result := p.handleUnmuteAll(nil, userInfo) + tc.assertions(result) + }) + } +} From 1b56537add4ce1c0ffd982280eee6e6498858d82 Mon Sep 17 00:00:00 2001 From: kshitij katiyar Date: Thu, 7 Nov 2024 11:39:39 +0530 Subject: [PATCH 2/6] Updated the getMutedUser function to return proper response --- server/plugin/command.go | 30 +++++++++++++++++++++--------- server/plugin/command_test.go | 15 ++++++++------- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/server/plugin/command.go b/server/plugin/command.go index c049007a7..8b0ab55bc 100644 --- a/server/plugin/command.go +++ b/server/plugin/command.go @@ -136,23 +136,27 @@ func (p *Plugin) postCommandResponse(args *model.CommandArgs, text string) { p.client.Post.SendEphemeralPost(args.UserId, post) } -func (p *Plugin) getMutedUsernames(userInfo *GitHubUserInfo) []string { +func (p *Plugin) getMutedUsernames(userInfo *GitHubUserInfo) ([]string, error) { var mutedUsernameBytes []byte err := p.store.Get(userInfo.UserID+"-muted-users", &mutedUsernameBytes) if err != nil { - return nil + return nil, err } mutedUsernames := string(mutedUsernameBytes) var mutedUsers []string if len(mutedUsernames) == 0 { - return mutedUsers + return mutedUsers, nil } mutedUsers = strings.Split(mutedUsernames, ",") - return mutedUsers + return mutedUsers, nil } func (p *Plugin) handleMuteList(_ *model.CommandArgs, userInfo *GitHubUserInfo) string { - mutedUsernames := p.getMutedUsernames(userInfo) + mutedUsernames, err := p.getMutedUsernames(userInfo) + if err != nil { + return "Some error occurred getting muted users. Please try again later" + } + var mutedUsers string for _, user := range mutedUsernames { mutedUsers += fmt.Sprintf("- %v\n", user) @@ -173,7 +177,11 @@ func contains(s []string, e string) bool { } func (p *Plugin) handleMuteAdd(_ *model.CommandArgs, username string, userInfo *GitHubUserInfo) string { - mutedUsernames := p.getMutedUsernames(userInfo) + mutedUsernames, err := p.getMutedUsernames(userInfo) + if err != nil { + return "Some error occurred getting muted users. Please try again later" + } + if contains(mutedUsernames, username) { return username + " is already muted" } @@ -190,7 +198,7 @@ func (p *Plugin) handleMuteAdd(_ *model.CommandArgs, username string, userInfo * mutedUsers = username } - _, err := p.store.Set(userInfo.UserID+"-muted-users", []byte(mutedUsers)) + _, err = p.store.Set(userInfo.UserID+"-muted-users", []byte(mutedUsers)) if err != nil { return "Error occurred saving list of muted users" } @@ -199,11 +207,15 @@ func (p *Plugin) handleMuteAdd(_ *model.CommandArgs, username string, userInfo * } func (p *Plugin) handleUnmute(_ *model.CommandArgs, username string, userInfo *GitHubUserInfo) string { - mutedUsernames := p.getMutedUsernames(userInfo) + mutedUsernames, err := p.getMutedUsernames(userInfo) + if err != nil { + return "Some error occurred getting muted users. Please try again later" + } + userToMute := []string{username} newMutedList := arrayDifference(mutedUsernames, userToMute) - _, err := p.store.Set(userInfo.UserID+"-muted-users", []byte(strings.Join(newMutedList, ","))) + _, err = p.store.Set(userInfo.UserID+"-muted-users", []byte(strings.Join(newMutedList, ","))) if err != nil { return "Error occurred unmuting users" } diff --git a/server/plugin/command_test.go b/server/plugin/command_test.go index ed41a21ed..bb085fcae 100644 --- a/server/plugin/command_test.go +++ b/server/plugin/command_test.go @@ -344,15 +344,16 @@ func TestGetMutedUsernames(t *testing.T) { tests := []struct { name string setup func() - assertions func(t *testing.T, result []string) + assertions func(t *testing.T, result []string, err error) }{ { name: "Error retrieving muted usernames", setup: func() { mockKvStore.EXPECT().Get("mockUserID-muted-users", gomock.Any()).Return(errors.New("error retrieving muted users")).Times(1) }, - assertions: func(t *testing.T, result []string) { + assertions: func(t *testing.T, result []string, err error) { assert.Nil(t, result) + assert.ErrorContains(t, err, "error retrieving muted users") }, }, { @@ -363,7 +364,7 @@ func TestGetMutedUsernames(t *testing.T) { return nil }).Times(1) }, - assertions: func(t *testing.T, result []string) { + assertions: func(t *testing.T, result []string, _ error) { assert.Equal(t, []string(nil), result) }, }, @@ -376,7 +377,7 @@ func TestGetMutedUsernames(t *testing.T) { return nil }).Times(1) }, - assertions: func(t *testing.T, result []string) { + assertions: func(t *testing.T, result []string, _ error) { assert.Equal(t, []string{"user1", "user2", "user3"}, result) }, }, @@ -385,9 +386,9 @@ func TestGetMutedUsernames(t *testing.T) { t.Run(tc.name, func(t *testing.T) { tc.setup() - mutedUsernames := p.getMutedUsernames(userInfo) + mutedUsernames, err := p.getMutedUsernames(userInfo) - tc.assertions(t, mutedUsernames) + tc.assertions(t, mutedUsernames, err) }) } } @@ -409,7 +410,7 @@ func TestHandleMuteList(t *testing.T) { mockKvStore.EXPECT().Get("mockUserID-muted-users", gomock.Any()).Return(errors.New("error retrieving muted users")).Times(1) }, assertions: func(t *testing.T, result string) { - assert.Equal(t, "You have no muted users", result) + assert.Equal(t, "Some error occurred getting muted users. Please try again later", result) }, }, { From a4e92f0fbd9404fb52d81311b4d4e39363b0ef01 Mon Sep 17 00:00:00 2001 From: kshitij katiyar Date: Thu, 7 Nov 2024 13:12:39 +0530 Subject: [PATCH 3/6] added logs to the new error check --- server/plugin/command.go | 3 +++ server/plugin/command_test.go | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/server/plugin/command.go b/server/plugin/command.go index 8b0ab55bc..54493544c 100644 --- a/server/plugin/command.go +++ b/server/plugin/command.go @@ -154,6 +154,7 @@ func (p *Plugin) getMutedUsernames(userInfo *GitHubUserInfo) ([]string, error) { func (p *Plugin) handleMuteList(_ *model.CommandArgs, userInfo *GitHubUserInfo) string { mutedUsernames, err := p.getMutedUsernames(userInfo) if err != nil { + p.client.Log.Error("error occurred getting muted user.", "Error", err) return "Some error occurred getting muted users. Please try again later" } @@ -179,6 +180,7 @@ func contains(s []string, e string) bool { func (p *Plugin) handleMuteAdd(_ *model.CommandArgs, username string, userInfo *GitHubUserInfo) string { mutedUsernames, err := p.getMutedUsernames(userInfo) if err != nil { + p.client.Log.Error("error occurred getting muted user.", "Error", err) return "Some error occurred getting muted users. Please try again later" } @@ -209,6 +211,7 @@ func (p *Plugin) handleMuteAdd(_ *model.CommandArgs, username string, userInfo * func (p *Plugin) handleUnmute(_ *model.CommandArgs, username string, userInfo *GitHubUserInfo) string { mutedUsernames, err := p.getMutedUsernames(userInfo) if err != nil { + p.client.Log.Error("error occurred getting muted user.", "Error", err) return "Some error occurred getting muted users. Please try again later" } diff --git a/server/plugin/command_test.go b/server/plugin/command_test.go index bb085fcae..09b272edc 100644 --- a/server/plugin/command_test.go +++ b/server/plugin/command_test.go @@ -407,6 +407,7 @@ func TestHandleMuteList(t *testing.T) { { name: "Error retrieving muted usernames", setup: func() { + mockAPI.On("LogError", "error occurred getting muted user.", "Error", mock.Anything) mockKvStore.EXPECT().Get("mockUserID-muted-users", gomock.Any()).Return(errors.New("error retrieving muted users")).Times(1) }, assertions: func(t *testing.T, result string) { @@ -503,6 +504,16 @@ func TestHandleMuteAdd(t *testing.T) { setup func() assertions func(t *testing.T, result string) }{ + { + name: "Error retrieving muted usernames", + setup: func() { + mockAPI.On("LogError", "error occurred getting muted user.", "Error", mock.Anything) + mockKvStore.EXPECT().Get("mockUserID-muted-users", gomock.Any()).Return(errors.New("error retrieving muted users")).Times(1) + }, + assertions: func(t *testing.T, result string) { + assert.Equal(t, "Some error occurred getting muted users. Please try again later", result) + }, + }, { name: "Error saving the new muted username", username: "errorUser", @@ -581,6 +592,14 @@ func TestHandleUnmute(t *testing.T) { setup func() expectedResult string }{ + { + name: "Error retrieving muted usernames", + setup: func() { + mockAPI.On("LogError", "error occurred getting muted user.", "Error", mock.Anything) + mockKvStore.EXPECT().Get("mockUserID-muted-users", gomock.Any()).Return(errors.New("error retrieving muted users")).Times(1) + }, + expectedResult: "Some error occurred getting muted users. Please try again later", + }, { name: "Error occurred while unmuting the user", username: "user1", From f7abf21c62a8fd8bfe79268038f3ca14cab79f84 Mon Sep 17 00:00:00 2001 From: kshitij katiyar Date: Mon, 11 Nov 2024 16:27:53 +0530 Subject: [PATCH 4/6] fixed grammar of error msgs in command.go --- server/plugin/command.go | 6 +++--- server/plugin/command_test.go | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/server/plugin/command.go b/server/plugin/command.go index 54493544c..ab28e2b03 100644 --- a/server/plugin/command.go +++ b/server/plugin/command.go @@ -155,7 +155,7 @@ func (p *Plugin) handleMuteList(_ *model.CommandArgs, userInfo *GitHubUserInfo) mutedUsernames, err := p.getMutedUsernames(userInfo) if err != nil { p.client.Log.Error("error occurred getting muted user.", "Error", err) - return "Some error occurred getting muted users. Please try again later" + return "An error occurred getting muted users. Please try again later" } var mutedUsers string @@ -181,7 +181,7 @@ func (p *Plugin) handleMuteAdd(_ *model.CommandArgs, username string, userInfo * mutedUsernames, err := p.getMutedUsernames(userInfo) if err != nil { p.client.Log.Error("error occurred getting muted user.", "Error", err) - return "Some error occurred getting muted users. Please try again later" + return "An error occurred getting muted users. Please try again later" } if contains(mutedUsernames, username) { @@ -212,7 +212,7 @@ func (p *Plugin) handleUnmute(_ *model.CommandArgs, username string, userInfo *G mutedUsernames, err := p.getMutedUsernames(userInfo) if err != nil { p.client.Log.Error("error occurred getting muted user.", "Error", err) - return "Some error occurred getting muted users. Please try again later" + return "An error occurred getting muted users. Please try again later" } userToMute := []string{username} diff --git a/server/plugin/command_test.go b/server/plugin/command_test.go index 09b272edc..5e45bdd52 100644 --- a/server/plugin/command_test.go +++ b/server/plugin/command_test.go @@ -411,7 +411,7 @@ func TestHandleMuteList(t *testing.T) { mockKvStore.EXPECT().Get("mockUserID-muted-users", gomock.Any()).Return(errors.New("error retrieving muted users")).Times(1) }, assertions: func(t *testing.T, result string) { - assert.Equal(t, "Some error occurred getting muted users. Please try again later", result) + assert.Equal(t, "An error occurred getting muted users. Please try again later", result) }, }, { @@ -511,7 +511,7 @@ func TestHandleMuteAdd(t *testing.T) { mockKvStore.EXPECT().Get("mockUserID-muted-users", gomock.Any()).Return(errors.New("error retrieving muted users")).Times(1) }, assertions: func(t *testing.T, result string) { - assert.Equal(t, "Some error occurred getting muted users. Please try again later", result) + assert.Equal(t, "An error occurred getting muted users. Please try again later", result) }, }, { @@ -598,7 +598,7 @@ func TestHandleUnmute(t *testing.T) { mockAPI.On("LogError", "error occurred getting muted user.", "Error", mock.Anything) mockKvStore.EXPECT().Get("mockUserID-muted-users", gomock.Any()).Return(errors.New("error retrieving muted users")).Times(1) }, - expectedResult: "Some error occurred getting muted users. Please try again later", + expectedResult: "An error occurred getting muted users. Please try again later", }, { name: "Error occurred while unmuting the user", From 7c944a137f2857546749f439a89dde362c84059f Mon Sep 17 00:00:00 2001 From: kshitij katiyar Date: Thu, 16 Jan 2025 13:15:42 +0530 Subject: [PATCH 5/6] review fixes --- server/plugin/command.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/plugin/command.go b/server/plugin/command.go index 31f7530d0..adcfd0769 100644 --- a/server/plugin/command.go +++ b/server/plugin/command.go @@ -159,7 +159,7 @@ func (p *Plugin) getMutedUsernames(userInfo *GitHubUserInfo) ([]string, error) { func (p *Plugin) handleMuteList(_ *model.CommandArgs, userInfo *GitHubUserInfo) string { mutedUsernames, err := p.getMutedUsernames(userInfo) if err != nil { - p.client.Log.Error("error occurred getting muted user.", "Error", err) + p.client.Log.Error("error occurred getting muted users.", "UserID", userInfo.UserID, "Error", err) return "An error occurred getting muted users. Please try again later" } @@ -185,7 +185,7 @@ func contains(s []string, e string) bool { func (p *Plugin) handleMuteAdd(_ *model.CommandArgs, username string, userInfo *GitHubUserInfo) string { mutedUsernames, err := p.getMutedUsernames(userInfo) if err != nil { - p.client.Log.Error("error occurred getting muted user.", "Error", err) + p.client.Log.Error("error occurred getting muted users.", "UserID", userInfo.UserID, "Error", err) return "An error occurred getting muted users. Please try again later" } @@ -216,7 +216,7 @@ func (p *Plugin) handleMuteAdd(_ *model.CommandArgs, username string, userInfo * func (p *Plugin) handleUnmute(_ *model.CommandArgs, username string, userInfo *GitHubUserInfo) string { mutedUsernames, err := p.getMutedUsernames(userInfo) if err != nil { - p.client.Log.Error("error occurred getting muted user.", "Error", err) + p.client.Log.Error("error occurred getting muted users.", "UserID", userInfo.UserID, "Error", err) return "An error occurred getting muted users. Please try again later" } From cbce6ebcc8ae5289ed2283cb01b33fa1a70cef6a Mon Sep 17 00:00:00 2001 From: kshitij katiyar Date: Thu, 16 Jan 2025 13:37:38 +0530 Subject: [PATCH 6/6] updated muted list testcases --- server/plugin/command_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/plugin/command_test.go b/server/plugin/command_test.go index 2d33e3a0c..2db14e824 100644 --- a/server/plugin/command_test.go +++ b/server/plugin/command_test.go @@ -407,7 +407,7 @@ func TestHandleMuteList(t *testing.T) { { name: "Error retrieving muted usernames", setup: func() { - mockAPI.On("LogError", "error occurred getting muted user.", "Error", mock.Anything) + mockAPI.On("LogError", "error occurred getting muted users.", "UserID", userInfo.UserID, "Error", mock.Anything) mockKvStore.EXPECT().Get("mockUserID-muted-users", gomock.Any()).Return(errors.New("error retrieving muted users")).Times(1) }, assertions: func(t *testing.T, result string) { @@ -507,7 +507,7 @@ func TestHandleMuteAdd(t *testing.T) { { name: "Error retrieving muted usernames", setup: func() { - mockAPI.On("LogError", "error occurred getting muted user.", "Error", mock.Anything) + mockAPI.On("LogError", "error occurred getting muted users.", "UserID", userInfo.UserID, "Error", mock.Anything) mockKvStore.EXPECT().Get("mockUserID-muted-users", gomock.Any()).Return(errors.New("error retrieving muted users")).Times(1) }, assertions: func(t *testing.T, result string) { @@ -595,7 +595,7 @@ func TestHandleUnmute(t *testing.T) { { name: "Error retrieving muted usernames", setup: func() { - mockAPI.On("LogError", "error occurred getting muted user.", "Error", mock.Anything) + mockAPI.On("LogError", "error occurred getting muted users.", "UserID", userInfo.UserID, "Error", mock.Anything) mockKvStore.EXPECT().Get("mockUserID-muted-users", gomock.Any()).Return(errors.New("error retrieving muted users")).Times(1) }, expectedResult: "An error occurred getting muted users. Please try again later",