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

@@ -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

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=

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": {

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 <question>`",
+		}
+	}
+
 	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())

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{

server/helpers.go

@@ -2,49 +2,112 @@ 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
 // This function is limited to 1000 users per channel
 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 {

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.