From 617aa2e5470a798cff97581a1404c9a6c84f7423 Mon Sep 17 00:00:00 2001 From: Nils Brinkmann Date: Fri, 7 Aug 2020 22:08:26 +0200 Subject: [PATCH] Added: Now using weightedRandom and a history of questions and users to avoid asking the same questions and same users over and over again --- go.mod | 1 + go.sum | 2 + plugin.json | 2 +- server/commands.go | 23 ++++++++- server/commands_test.go | 76 ++++++++++++++++++++++++++++-- server/helpers.go | 101 ++++++++++++++++++++++++++++++++-------- server/manifest.go | 2 +- server/plugin.go | 7 ++- 8 files changed, 187 insertions(+), 27 deletions(-) diff --git a/go.mod b/go.mod index 23668f3..2410fde 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/mattermost/mattermost-server v5.11.1+incompatible github.com/mattermost/mattermost-server/v5 v5.21.0 github.com/mholt/archiver/v3 v3.3.0 + github.com/mroth/weightedrand v0.3.0 github.com/nicksnyder/go-i18n v1.10.1 // indirect github.com/oleiade/lane v1.0.0 github.com/pkg/errors v0.9.1 diff --git a/go.sum b/go.sum index 674c9a8..9e22ba4 100644 --- a/go.sum +++ b/go.sum @@ -257,6 +257,8 @@ github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJ github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0= github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0= +github.com/mroth/weightedrand v0.3.0 h1:8dllRVd5Y1jIJsigUpQaL5DUlV4qBiM+SF82d+cMwew= +github.com/mroth/weightedrand v0.3.0/go.mod h1:3p2SIcC8al1YMzGhAIoXD+r9olo/g/cdJgAD905gyNE= github.com/muesli/smartcrop v0.2.1-0.20181030220600-548bbf0c0965/go.mod h1:i2fCI/UorTfgEpPPLWiFBv4pye+YAG78RwcQLUkocpI= github.com/muesli/smartcrop v0.3.0/go.mod h1:i2fCI/UorTfgEpPPLWiFBv4pye+YAG78RwcQLUkocpI= github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= diff --git a/plugin.json b/plugin.json index 2f53a1c..93e050c 100644 --- a/plugin.json +++ b/plugin.json @@ -2,7 +2,7 @@ "id": "com.nilsbrinkmann.icebreaker", "name": "Icebreaker Plugin", "description": "This plugin creates a bot which asks random questions", - "version": "2.0.0", + "version": "2.1.0", "min_server_version": "5.12.0", "server": { "executables": { diff --git a/server/commands.go b/server/commands.go index e415fba..6483e5f 100644 --- a/server/commands.go +++ b/server/commands.go @@ -2,7 +2,6 @@ package main import ( "fmt" - "math/rand" "strings" "github.com/mattermost/mattermost-server/v5/model" @@ -226,7 +225,14 @@ func (p *Plugin) executeCommandIcebreaker(args *model.CommandArgs) *model.Comman } //build the question and ask it - question := data.Questions[rand.Intn(len(data.Questions))] + question, err := p.GetRandomQuestion() + if err != nil { + return &model.CommandResponse{ + ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, + Text: "Error: There are no questions that I can ask. Be the first one to propose a question by using `/icebreaker add `", + } + } + message := fmt.Sprintf("Hey @%s! %s", user.GetDisplayName(""), question.Question) post := &model.Post{ ChannelId: args.ChannelId, @@ -235,6 +241,19 @@ func (p *Plugin) executeCommandIcebreaker(args *model.CommandArgs) *model.Comman Message: message, } + //store the user and question so we avoid asking the same users and same questions over and over + data.LastUsers = append(data.LastUsers, user.Id) + if len(data.LastUsers) > LenHistory { + index := 0 //remove the oldest element + data.LastUsers = append(data.LastUsers[:index], data.LastUsers[index+1:]...) + } + data.LastQuestions = append(data.LastQuestions, *question) + if len(data.LastQuestions) > LenHistory { + index := 0 //remove the oldest element + data.LastQuestions = append(data.LastQuestions[:index], data.LastQuestions[index+1:]...) + } + p.WriteToStorage(&data) + if _, err = p.API.CreatePost(post); err != nil { const errorMessage = "Error: Failed to create post" p.API.LogError(errorMessage, "err", err.Error()) diff --git a/server/commands_test.go b/server/commands_test.go index bf42ec0..ae9c0b8 100644 --- a/server/commands_test.go +++ b/server/commands_test.go @@ -168,7 +168,7 @@ func TestAskIcebreaker_fail(t *testing.T) { func TestAskIcebreaker_success(t *testing.T) { t.Run("Successful, first user", func(t *testing.T) { - rand.Seed(5) //seed guarantees that the loop goes through a few users before picking success_user + rand.Seed(1338) icebreakerData := &IceBreakerData{Questions: []Question{ Question{ Creator: "TestUser", Question: "How do you do?", @@ -189,6 +189,7 @@ func TestAskIcebreaker_success(t *testing.T) { api := &plugintest.API{} api.On("GetUser", mock.AnythingOfType("string")).Return(&model.User{Username: "TestUser"}, nil) api.On("KVGet", mock.AnythingOfType("string")).Return(reqBodyBytes.Bytes(), nil) + api.On("KVSet", mock.AnythingOfType("string"), mock.AnythingOfType("[]uint8")).Return(nil) api.On("GetUsersInChannel", mock.AnythingOfType("string"), mock.AnythingOfType("string"), mock.AnythingOfType("int"), mock.AnythingOfType("int")). Return(users, nil) api.On("GetUserStatus", "User1").Return(&model.Status{Status: "offline"}, nil) @@ -215,7 +216,7 @@ func TestAskIcebreaker_success(t *testing.T) { plugin.ExecuteCommand(nil, args) }) t.Run("Successful, other user", func(t *testing.T) { - rand.Seed(4) //seed guarantees that the loop goes through a few users before picking success_user2 + rand.Seed(1337) icebreakerData := &IceBreakerData{Questions: []Question{ Question{ Creator: "TestUser", Question: "How do you do?", @@ -236,6 +237,7 @@ func TestAskIcebreaker_success(t *testing.T) { api := &plugintest.API{} api.On("GetUser", mock.AnythingOfType("string")).Return(&model.User{Username: "TestUser"}, nil) api.On("KVGet", mock.AnythingOfType("string")).Return(reqBodyBytes.Bytes(), nil) + api.On("KVSet", mock.AnythingOfType("string"), mock.AnythingOfType("[]uint8")).Return(nil) api.On("GetUsersInChannel", mock.AnythingOfType("string"), mock.AnythingOfType("string"), mock.AnythingOfType("int"), mock.AnythingOfType("int")). Return(users, nil) api.On("GetUserStatus", "User1").Return(&model.Status{Status: "offline"}, nil) @@ -261,6 +263,74 @@ func TestAskIcebreaker_success(t *testing.T) { plugin.executeCommandIcebreaker(args) }) + t.Run("Successful, history", func(t *testing.T) { + rand.Seed(1338) + icebreakerData := &IceBreakerData{Questions: []Question{ + Question{ + Creator: "TestUser", Question: "First question", + }, + Question{ + Creator: "TestUser", Question: "Second question", + }, + Question{ + Creator: "TestUser", Question: "Third question", + }}, + LastUsers: []string{ + "SuccessUser1", + "SuccessUser2", + }, + LastQuestions: []Question{ + Question{ + Creator: "TestUser", Question: "First question", + }, + Question{ + Creator: "TestUser", Question: "Third question", + }, + }, + } + reqBodyBytes := new(bytes.Buffer) + json.NewEncoder(reqBodyBytes).Encode(icebreakerData) + + users := []*model.User{ + &model.User{IsBot: true}, + &model.User{Id: "TestUser"}, + &model.User{Id: "User1"}, + &model.User{Id: "User2"}, + &model.User{Id: "SuccessUser1", Username: "success_user1"}, + &model.User{Id: "SuccessUser2", Username: "success_user2"}, + &model.User{Id: "SuccessUser3", Username: "success_user3"}, + } + + args := &model.CommandArgs{ + Command: "/icebreaker ", + ChannelId: "TestChannel", + TeamId: "TestTeam", + RootId: "TestRoot", + UserId: "TestUser", + } + + plugin := &Plugin{} + api := &plugintest.API{} + api.On("GetUser", mock.AnythingOfType("string")).Return(&model.User{Username: "TestUser"}, nil) + api.On("KVGet", mock.AnythingOfType("string")).Return(reqBodyBytes.Bytes(), nil) + api.On("KVSet", mock.AnythingOfType("string"), mock.AnythingOfType("[]uint8")).Return(nil) + api.On("GetUsersInChannel", mock.AnythingOfType("string"), mock.AnythingOfType("string"), mock.AnythingOfType("int"), mock.AnythingOfType("int")). + Return(users, nil) + api.On("GetUserStatus", "User1").Return(&model.Status{Status: "offline"}, nil) + api.On("GetUserStatus", "User2").Return(&model.Status{Status: "dnd"}, nil) + api.On("GetUserStatus", "SuccessUser1").Return(&model.Status{Status: "online"}, nil) + api.On("GetUserStatus", "SuccessUser2").Return(&model.Status{Status: "online"}, nil) + api.On("GetUserStatus", "SuccessUser3").Return(&model.Status{Status: "online"}, nil) + api.On("CreatePost", &model.Post{ + ChannelId: "TestChannel", + RootId: "TestRoot", + UserId: "", + Message: "Hey @success_user3! Second question", + }).Return(nil, nil) + plugin.SetAPI(api) + + plugin.ExecuteCommand(nil, args) + }) } func TestAddIcebreaker(t *testing.T) { @@ -322,7 +392,7 @@ func TestAddIcebreaker(t *testing.T) { api := &plugintest.API{} api.On("GetUser", mock.AnythingOfType("string")).Return(&model.User{Username: "TestUser", Id: "TestUserId"}, nil) api.On("KVGet", mock.AnythingOfType("string")).Return(reqBodyBytes.Bytes(), nil) - api.On("KVSet", "IceBreakerData", bytesAfterAddingTheQuestion.Bytes()).Return(nil) + api.On("KVSet", "IceBreakerData_v2", bytesAfterAddingTheQuestion.Bytes()).Return(nil) plugin.SetAPI(api) args := &model.CommandArgs{ diff --git a/server/helpers.go b/server/helpers.go index c319652..e235c60 100644 --- a/server/helpers.go +++ b/server/helpers.go @@ -2,11 +2,12 @@ package main import ( "fmt" - "math/rand" + "math" "strconv" "strings" "github.com/mattermost/mattermost-server/v5/model" + "github.com/mroth/weightedrand" ) // GetRandomUser returns a random user that is found in the given channel and that is not a bot @@ -14,37 +15,99 @@ import ( func (p *Plugin) GetRandomUser(channelID string, userIDToIgnore string) (*model.User, *model.AppError) { //get a random user that is not a bot users, _ := p.API.GetUsersInChannel(channelID, "username", 0, 1000) + weightedUsers := []weightedrand.Choice{} //list of users, sorted by weight - targetuser := new(model.User) - hasUserBeenFound := false - for len(users) > 0 { //as long as there are users left continue to search for a good candidate - randomIndex := rand.Intn(len(users)) - currentUser := users[randomIndex] - if currentUser.IsBot { - users = append(users[:randomIndex], users[randomIndex+1:]...) //from https://stackoverflow.com/a/37335777/199513 + //read the users data for weightedrandom + data := p.ReadFromStorage() + + for _, user := range users { + if user.IsBot { continue } - if currentUser.Id == userIDToIgnore { - users = append(users[:randomIndex], users[randomIndex+1:]...) //from https://stackoverflow.com/a/37335777/199513 + if user.Id == userIDToIgnore { continue } - status, err := p.API.GetUserStatus(currentUser.Id) + status, err := p.API.GetUserStatus(user.Id) if (err != nil) || (status.Status == "offline") || (status.Status == "dnd") { - users = append(users[:randomIndex], users[randomIndex+1:]...) //from https://stackoverflow.com/a/37335777/199513 continue } - targetuser = currentUser - hasUserBeenFound = true - break + //check if the user has already been asked lately. Add him with a weight according to how recent the asking has been + //by iterating in reverse we make sure that users that appear multiple times in the list will not mess up the weights + isNewUser := true + if data.LastUsers != nil { + for index := len(data.LastUsers) - 1; index >= 0; index-- { + currentUserID := data.LastUsers[index] + if currentUserID == user.Id { + userWeight := uint(math.Abs(float64(index - len(data.LastUsers)))) + weightedUsers = append(weightedUsers, weightedrand.Choice{Weight: userWeight, Item: user}) + isNewUser = false + break + } + } + if !isNewUser { //if the user has been found within our data.LastUsers we can continue the loop + continue + } + } + + //Finally... this is a brand-new user that has never asked a question. Add him with a very high weight, so he'll be chosen with a high possibility + weightedUsers = append(weightedUsers, weightedrand.Choice{Weight: 1000, Item: user}) } - if !hasUserBeenFound { - return nil, &model.AppError{ - Message: "There is no user I can ask a question for...", + if len(weightedUsers) > 0 { + chooser := weightedrand.NewChooser(weightedUsers...) + user, ok := chooser.Pick().(*model.User) + if ok { + return user, nil } } - return targetuser, nil + + return nil, &model.AppError{ + Message: "There is no user I can ask a question for...", + } +} + +// GetRandomQuestion returns a random question that hasn't been asked recently +func (p *Plugin) GetRandomQuestion() (*Question, *model.AppError) { + weightedQuestions := []weightedrand.Choice{} //list of questions, sorted by weight + + //read the question data for weightedrandom + data := p.ReadFromStorage() + + for _, question := range data.Questions { + //check if the question has already been asked lately. Add it with a weight according to how recent the question has been asked + //by iterating in reverse we make sure that questions that appear multiple times in the list will not mess up the weights + isNewQuestion := true + if data.LastQuestions != nil { + for index := len(data.LastQuestions) - 1; index >= 0; index-- { + currentQuestion := data.LastQuestions[index] + if currentQuestion.Question == question.Question { + questionWeight := uint(math.Abs(float64(index - len(data.LastQuestions)))) + weightedQuestions = append(weightedQuestions, weightedrand.Choice{Weight: questionWeight, Item: currentQuestion}) + isNewQuestion = false + break + } + } + if !isNewQuestion { //if the question has been found within our data.LastQuestions we can continue the loop + continue + } + } + + //Finally... this is a brand-new question that has never been asked. Add it with a very high weight, so it'll be chosen with a high possibility + weightedQuestions = append(weightedQuestions, weightedrand.Choice{Weight: 1000, Item: question}) + } + + if len(weightedQuestions) > 0 { + chooser := weightedrand.NewChooser(weightedQuestions...) + question, ok := chooser.Pick().(Question) + if ok { + return &question, nil + } + } + + return nil, &model.AppError{ + Message: "There is no question to ask...", + } } func requireAdminUser(sourceUser *model.User) *model.CommandResponse { diff --git a/server/manifest.go b/server/manifest.go index 8a35a5c..abc42bb 100644 --- a/server/manifest.go +++ b/server/manifest.go @@ -15,7 +15,7 @@ const manifestStr = ` "id": "com.nilsbrinkmann.icebreaker", "name": "Icebreaker Plugin", "description": "This plugin creates a bot which asks random questions", - "version": "2.0.0", + "version": "2.1.0", "min_server_version": "5.12.0", "server": { "executables": { diff --git a/server/plugin.go b/server/plugin.go index df318f6..26282d4 100644 --- a/server/plugin.go +++ b/server/plugin.go @@ -32,9 +32,14 @@ type Question struct { //IceBreakerData contains all data necessary to be stored for the Icebreaker Plugin type IceBreakerData struct { - Questions []Question `json:"Questions"` + Questions []Question `json:"Questions"` + LastUsers []string `json:"LastUsers"` + LastQuestions []Question `json:"LastQuestions"` } +//LenHistory sets how many LastUsers/LastQuestions are stored to avoid asking the same users or same questions over and over +const LenHistory int = 50 + // OnActivate is invoked when the plugin is activated. // // This demo implementation logs a message to the demo channel whenever the plugin is activated.