From f66ba396e8d6fe8cd7dc9194a44264e03f4b1940 Mon Sep 17 00:00:00 2001 From: Evandro Flores Date: Tue, 17 Sep 2019 23:52:30 +0100 Subject: [PATCH 01/15] Adding admin level commands --- cmd/base.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/cmd/base.go b/cmd/base.go index 8a301a7..08fa76d 100644 --- a/cmd/base.go +++ b/cmd/base.go @@ -3,6 +3,7 @@ package cmd import ( "encoding/json" "fmt" + "os" "strings" "github.com/evandroflores/pong/model" @@ -22,6 +23,15 @@ func Register(usage, description string, handler func(request slacker.Request, r slack.Client.Command(usage, &slacker.CommandDefinition{Description: description, Handler: handler}) } +func RegisterAdmin(usage, description string, handler func(request slacker.Request, response slacker.ResponseWriter)) { + log.Infof("Registering Admin %s - %s", usage, description) + slack.Client.Command(usage, &slacker.CommandDefinition{Description: description, Handler: handler, AuthorizationFunc: isAdmin}) +} + +func isAdmin(request slacker.Request) bool { + return strings.Contains(os.Getenv("PONG_ADMINS"), request.Event().User) +} + func cleanID(userID string) string { return replacer.Replace(userID) } From 5b5b9cd25fd9ba1fcecc0caafb798e61cd6a7824 Mon Sep 17 00:00:00 2001 From: Evandro Flores Date: Tue, 17 Sep 2019 23:52:47 +0100 Subject: [PATCH 02/15] Delete func --- model/player.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/model/player.go b/model/player.go index e55bb9e..af1601f 100644 --- a/model/player.go +++ b/model/player.go @@ -87,6 +87,11 @@ func (player *Player) Update() (int, error) { return (previousPosition - dbPlayer.GetPosition()), nil } +// Delete removes locally the user from the DB. +func (player *Player) Delete() { + database.Connection.Delete(&player) +} + // IngestData calls Slack API to get Users data func (player *Player) IngestData() { slackUser, err := slack.Client.GetUserInfo(player.SlackID) From 9849a2d33bafe4551913c6752bd68db6acbe4434 Mon Sep 17 00:00:00 2001 From: Evandro Flores Date: Tue, 17 Sep 2019 23:53:18 +0100 Subject: [PATCH 03/15] Function to sync disabled from Slack and remove from Pong --- cmd/sync_disabled.go | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 cmd/sync_disabled.go diff --git a/cmd/sync_disabled.go b/cmd/sync_disabled.go new file mode 100644 index 0000000..f5fd080 --- /dev/null +++ b/cmd/sync_disabled.go @@ -0,0 +1,40 @@ +package cmd + +import ( + "bytes" + "fmt" + + "github.com/evandroflores/pong/model" + "github.com/evandroflores/pong/slack" + "github.com/shomali11/slacker" +) + +func init() { + RegisterAdmin("sync-disabled", "Sync disabled Slack users and purge from Pong", syncDisabled) +} + +func syncDisabled(request slacker.Request, response slacker.ResponseWriter) { + response.Typing() + + teamID := cleanID(request.Event().Team) + channelID := cleanID(request.Event().Channel) + + players := model.GetAllPlayers(teamID, channelID) + + var message bytes.Buffer + + removed := 0 + for _, player := range players { + slackUser, _ := slack.Client.GetUserInfo(player.SlackID) + if slackUser.Deleted { + player.Delete() + message.WriteString(fmt.Sprintf("*%s* - Removed\n", player.Name)) + removed += 1 + } + } + if removed == 0 { + message.WriteString("No users removed.") + } + + response.Reply(message.String()) +} From 0b69f1db7332c413f50f9e6deb6d4b368407976b Mon Sep 17 00:00:00 2001 From: Evandro Flores Date: Wed, 18 Sep 2019 14:05:45 +0100 Subject: [PATCH 04/15] lint --- .codecov.yml | 2 ++ cmd/sync_disabled.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.codecov.yml b/.codecov.yml index 23744bd..3d566d0 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -7,3 +7,5 @@ coverage: patch: default: target: 50% +ignore: + - mock_response_writer.go diff --git a/cmd/sync_disabled.go b/cmd/sync_disabled.go index f5fd080..e74e0ec 100644 --- a/cmd/sync_disabled.go +++ b/cmd/sync_disabled.go @@ -29,7 +29,7 @@ func syncDisabled(request slacker.Request, response slacker.ResponseWriter) { if slackUser.Deleted { player.Delete() message.WriteString(fmt.Sprintf("*%s* - Removed\n", player.Name)) - removed += 1 + removed++ } } if removed == 0 { From 7a03661203d7045442df419cc0fa7bdfff0161cf Mon Sep 17 00:00:00 2001 From: Evandro Flores Date: Thu, 19 Sep 2019 15:03:27 +0100 Subject: [PATCH 05/15] Ignore mock coverage --- .codecov.yml | 4 ++-- cmd/base.go | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.codecov.yml b/.codecov.yml index 3d566d0..f3874c8 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -7,5 +7,5 @@ coverage: patch: default: target: 50% -ignore: - - mock_response_writer.go + ignore: + - "**/mock*.go" diff --git a/cmd/base.go b/cmd/base.go index 08fa76d..c641843 100644 --- a/cmd/base.go +++ b/cmd/base.go @@ -23,6 +23,7 @@ func Register(usage, description string, handler func(request slacker.Request, r slack.Client.Command(usage, &slacker.CommandDefinition{Description: description, Handler: handler}) } +// RegisterAdmin adds a command using an Admin authorization function func RegisterAdmin(usage, description string, handler func(request slacker.Request, response slacker.ResponseWriter)) { log.Infof("Registering Admin %s - %s", usage, description) slack.Client.Command(usage, &slacker.CommandDefinition{Description: description, Handler: handler, AuthorizationFunc: isAdmin}) From 0598b95d30c6aef6988926abc89b470e31a8b73f Mon Sep 17 00:00:00 2001 From: Evandro Flores Date: Thu, 19 Sep 2019 17:37:59 +0100 Subject: [PATCH 06/15] bouk mokey complaning on how we import --- cmd/base_test.go | 2 +- go.mod | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/base_test.go b/cmd/base_test.go index 6216b81..4b38f0e 100644 --- a/cmd/base_test.go +++ b/cmd/base_test.go @@ -8,7 +8,7 @@ import ( "testing" "time" - "github.com/bouk/monkey" + "bou.ke/monkey" "github.com/evandroflores/pong/model" sl "github.com/evandroflores/pong/slack" "github.com/nlopes/slack" diff --git a/go.mod b/go.mod index 93046a6..c5fc0f1 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/evandroflores/pong go 1.12 require ( - bou.ke/monkey v1.0.1 // indirect + bou.ke/monkey v1.0.1 github.com/bouk/monkey v1.0.1 github.com/jinzhu/gorm v1.9.10 github.com/nlopes/slack v0.5.1-0.20190605211732-a05dfd3f167d From 5d69729df5f990e3426be2f2c4de0eb3b117cc3c Mon Sep 17 00:00:00 2001 From: Evandro Flores Date: Thu, 19 Sep 2019 17:55:24 +0100 Subject: [PATCH 07/15] Testing IsAdmin check --- cmd/base_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/cmd/base_test.go b/cmd/base_test.go index 4b38f0e..90770ca 100644 --- a/cmd/base_test.go +++ b/cmd/base_test.go @@ -3,6 +3,7 @@ package cmd import ( "fmt" "math/rand" + "os" "reflect" "strings" "testing" @@ -159,3 +160,27 @@ func TestTrend(t *testing.T) { assert.Equal(t, getPosDiff(5), " ↑ 5 ") assert.Equal(t, getPosDiff(-5), " ↓ 5 ") } + +func TestIsAdmin(t *testing.T) { + props := proper.NewProperties(map[string]string{}) + evt := makeTestEvent() + + currentAdminEnv := os.Getenv("PONG_ADMINS") + os.Setenv("PONG_ADMINS", evt.User) + defer func() { os.Setenv("PONG_ADMINS", currentAdminEnv) }() + + request := &fakeRequest{event: evt, properties: props} + assert.True(t, isAdmin(request)) +} + +func TestNotAdmin(t *testing.T) { + props := proper.NewProperties(map[string]string{}) + evt := makeTestEvent() + + currentAdminEnv := os.Getenv("PONG_ADMINS") + os.Setenv("PONG_ADMINS", "") + defer func() { os.Setenv("PONG_ADMINS", currentAdminEnv) }() + + request := &fakeRequest{event: evt, properties: props} + assert.False(t, isAdmin(request)) +} From 8dc1bf01fc2c6d2743bc83ed330e5df411b9ce4d Mon Sep 17 00:00:00 2001 From: Evandro Flores Date: Thu, 19 Sep 2019 19:50:31 +0100 Subject: [PATCH 08/15] Testing LoadCommands --- cmd/base_test.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/cmd/base_test.go b/cmd/base_test.go index 90770ca..acba387 100644 --- a/cmd/base_test.go +++ b/cmd/base_test.go @@ -5,6 +5,7 @@ import ( "math/rand" "os" "reflect" + "runtime" "strings" "testing" "time" @@ -14,6 +15,8 @@ import ( sl "github.com/evandroflores/pong/slack" "github.com/nlopes/slack" "github.com/shomali11/proper" + "github.com/shomali11/slacker" + log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" ) @@ -91,6 +94,32 @@ func TestSayWhat(t *testing.T) { assert.Empty(t, response.GetErrors()) } +func TestLoadCommands(t *testing.T) { + + expectedMsg := fmt.Sprintf("[%d] commands loaded", len(sl.Client.BotCommands())) + + called := false + mockDefaultCommand := func(s *slacker.Slacker, handler func(slacker.Request, slacker.ResponseWriter)) { + fmt.Printf("DefaultCommand called") + assert.Equal(t, runtime.FuncForPC(reflect.ValueOf(sayWhat).Pointer()).Name(), + runtime.FuncForPC(reflect.ValueOf(handler).Pointer()).Name()) + called = true + } + + monkey.PatchInstanceMethod(reflect.TypeOf(sl.Client), "DefaultCommand", mockDefaultCommand) + + mockLogInfof := func(format string, args ...interface{}) { + assert.Equal(t, expectedMsg, fmt.Sprintf(format, args)) + } + + patchLog := monkey.Patch(log.Infof, mockLogInfof) + + LoadCommands() + + assert.True(t, called) + patchLog.Unpatch() +} + func TestInvalidWinner(t *testing.T) { var props = proper.NewProperties( map[string]string{ From bbad132cd12db5c132597b82f9b6f9caa56309ab Mon Sep 17 00:00:00 2001 From: Evandro Flores Date: Thu, 19 Sep 2019 22:14:37 +0100 Subject: [PATCH 09/15] Renaming sync command --- cmd/delete_disabled.go | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 cmd/delete_disabled.go diff --git a/cmd/delete_disabled.go b/cmd/delete_disabled.go new file mode 100644 index 0000000..c70fb0a --- /dev/null +++ b/cmd/delete_disabled.go @@ -0,0 +1,40 @@ +package cmd + +import ( + "bytes" + "fmt" + + "github.com/evandroflores/pong/model" + "github.com/evandroflores/pong/slack" + "github.com/shomali11/slacker" +) + +func init() { + RegisterAdmin("delete-disabled", "Delete disabled Slack users from Pong", deleteDisabled) +} + +func deleteDisabled(request slacker.Request, response slacker.ResponseWriter) { + response.Typing() + + teamID := cleanID(request.Event().Team) + channelID := cleanID(request.Event().Channel) + + players := model.GetAllPlayers(teamID, channelID) + + var message bytes.Buffer + + removed := 0 + for _, player := range players { + slackUser, _ := slack.Client.GetUserInfo(player.SlackID) + if slackUser.Deleted { + player.Delete() + message.WriteString(fmt.Sprintf("*%s* - Removed\n", player.Name)) + removed++ + } + } + if removed == 0 { + message.WriteString("No users removed.") + } + + response.Reply(message.String()) +} From a6c1e2a83d3a5eb94b0ac21b804ebd484dccf292 Mon Sep 17 00:00:00 2001 From: Evandro Flores Date: Thu, 19 Sep 2019 22:15:12 +0100 Subject: [PATCH 10/15] Adding parameters to makeTestPlayer --- cmd/base_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cmd/base_test.go b/cmd/base_test.go index acba387..6b4d06e 100644 --- a/cmd/base_test.go +++ b/cmd/base_test.go @@ -44,12 +44,16 @@ func makeTestPlayer() model.Player { name := fmt.Sprintf("Fake User - %08d", randomInt) slackID := fmt.Sprintf("U%08d", randomInt) + return makeTestPlayerWith("TTTTTTTTT", "CCCCCCCCC", slackID, name) +} + +func makeTestPlayerWith(teamID, channelID, slackID, name string) model.Player { mockIngest := func(player *model.Player) { fmt.Printf("Ingesting called for %s\n", player.ToStr()) } player := model.Player{ - TeamID: "TTTTTTTTT", - ChannelID: "CCCCCCCCC", + TeamID: teamID, + ChannelID: channelID, SlackID: slackID, Name: name, Image: "https://www.thispersondoesnotexist.com/image", From af9e442893eae276aedde357601b383d37a38551 Mon Sep 17 00:00:00 2001 From: Evandro Flores Date: Thu, 19 Sep 2019 22:15:46 +0100 Subject: [PATCH 11/15] Test suite for delete-disabled cmd --- cmd/delete_disabled_test.go | 69 +++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 cmd/delete_disabled_test.go diff --git a/cmd/delete_disabled_test.go b/cmd/delete_disabled_test.go new file mode 100644 index 0000000..e2fa98d --- /dev/null +++ b/cmd/delete_disabled_test.go @@ -0,0 +1,69 @@ +package cmd + +import ( + "fmt" + "reflect" + "testing" + + "bou.ke/monkey" + + "github.com/evandroflores/pong/database" + "github.com/evandroflores/pong/model" + "github.com/evandroflores/pong/slack" + ns "github.com/nlopes/slack" + "github.com/shomali11/slacker" + "github.com/stretchr/testify/suite" +) + +type DeleteDisabledTestSuite struct { + suite.Suite + channelID string + adminID string + nonAdminID string + players []model.Player + deactivated map[string]model.Player +} + +func (s *DeleteDisabledTestSuite) SetupSuite() { + s.channelID = "CTESTDEL" + s.adminID = "UIS0ADMIN" + s.nonAdminID = "UNOTADMIN" + s.players = []model.Player{} + s.deactivated = map[string]model.Player{} + + for i := 1; i <= 20; i++ { + player := makeTestPlayerWith("TTTTTTTX", s.channelID, fmt.Sprintf("U%08d", i), fmt.Sprintf("Fake User %08d", i)) + player.Points = 1000 - float64(i) + if i%2 == 0 { + s.deactivated[player.SlackID] = player + } + s.players = append(s.players, player) + + database.Connection.Create(&player) + } + + mockGetUserInfo := func(sl *slacker.Slacker, id string) (*ns.User, error) { + isDeactivated := false + if (s.deactivated[id] != model.Player{}) { + isDeactivated = true + } + + return &ns.User{ + ID: id, + Deleted: isDeactivated, + }, nil + } + + monkey.PatchInstanceMethod(reflect.TypeOf(slack.Client), "GetUserInfo", mockGetUserInfo) +} + +func (s *DeleteDisabledTestSuite) TearDownSuite() { + for i := 0; i < len(s.players); i++ { + database.Connection.Where(&s.players[i]).Delete(&model.Player{}) + } + database.Connection.Unscoped().Where("deleted_at is not null").Delete(&model.Player{}) +} + +func TestDeleteDisabledTestSuite(t *testing.T) { + suite.Run(t, new(DeleteDisabledTestSuite)) +} From b1c3a9270710f91d9f04173c8239b290d42dd423 Mon Sep 17 00:00:00 2001 From: Evandro Flores Date: Thu, 19 Sep 2019 22:52:38 +0100 Subject: [PATCH 12/15] adding parameters to makeTestEvent --- cmd/base_test.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/cmd/base_test.go b/cmd/base_test.go index 6b4d06e..3eb2084 100644 --- a/cmd/base_test.go +++ b/cmd/base_test.go @@ -31,11 +31,15 @@ func init() { } func makeTestEvent() *slack.MessageEvent { + return makeTestEventWith(teamID, channelID, userID) +} + +func makeTestEventWith(teamID, channelID, slackID string) *slack.MessageEvent { return &slack.MessageEvent{ Msg: slack.Msg{ - Team: "TTTTTTTTT", - Channel: "CCCCCCCCC", - User: "UUUUUUUUU", + Team: teamID, + Channel: channelID, + User: slackID, }} } @@ -44,7 +48,7 @@ func makeTestPlayer() model.Player { name := fmt.Sprintf("Fake User - %08d", randomInt) slackID := fmt.Sprintf("U%08d", randomInt) - return makeTestPlayerWith("TTTTTTTTT", "CCCCCCCCC", slackID, name) + return makeTestPlayerWith(teamID, channelID, slackID, name) } func makeTestPlayerWith(teamID, channelID, slackID, name string) model.Player { From e21eb107bd87b756084d507ea86dccfc427058a9 Mon Sep 17 00:00:00 2001 From: Evandro Flores Date: Thu, 19 Sep 2019 22:53:06 +0100 Subject: [PATCH 13/15] renaming sync_disabled --- cmd/sync_disabled.go | 40 ---------------------------------------- 1 file changed, 40 deletions(-) delete mode 100644 cmd/sync_disabled.go diff --git a/cmd/sync_disabled.go b/cmd/sync_disabled.go deleted file mode 100644 index e74e0ec..0000000 --- a/cmd/sync_disabled.go +++ /dev/null @@ -1,40 +0,0 @@ -package cmd - -import ( - "bytes" - "fmt" - - "github.com/evandroflores/pong/model" - "github.com/evandroflores/pong/slack" - "github.com/shomali11/slacker" -) - -func init() { - RegisterAdmin("sync-disabled", "Sync disabled Slack users and purge from Pong", syncDisabled) -} - -func syncDisabled(request slacker.Request, response slacker.ResponseWriter) { - response.Typing() - - teamID := cleanID(request.Event().Team) - channelID := cleanID(request.Event().Channel) - - players := model.GetAllPlayers(teamID, channelID) - - var message bytes.Buffer - - removed := 0 - for _, player := range players { - slackUser, _ := slack.Client.GetUserInfo(player.SlackID) - if slackUser.Deleted { - player.Delete() - message.WriteString(fmt.Sprintf("*%s* - Removed\n", player.Name)) - removed++ - } - } - if removed == 0 { - message.WriteString("No users removed.") - } - - response.Reply(message.String()) -} From dede0b62840c22c906d0c81f2691539dcfdef14d Mon Sep 17 00:00:00 2001 From: Evandro Flores Date: Thu, 19 Sep 2019 23:16:10 +0100 Subject: [PATCH 14/15] fixing tests --- cmd/rank_test.go | 2 +- cmd/top_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/rank_test.go b/cmd/rank_test.go index 7fd1e3b..13e4384 100644 --- a/cmd/rank_test.go +++ b/cmd/rank_test.go @@ -19,7 +19,7 @@ type RankTestSuite struct { } func (s *RankTestSuite) SetupSuite() { - s.rankChannelID = "CCCCCCCCC" + s.rankChannelID = "C12345678" s.noRankChannelID = "CAAAAAAAA" s.expectedRank = fmt.Sprintf("\n*Rank for * <#%s>\n\n", s.rankChannelID) diff --git a/cmd/top_test.go b/cmd/top_test.go index cc112d6..f3667ea 100644 --- a/cmd/top_test.go +++ b/cmd/top_test.go @@ -20,7 +20,7 @@ type TopTestSuite struct { } func (s *TopTestSuite) SetupSuite() { - s.rankChannelID = "CCCCCCCCC" + s.rankChannelID = "C12345678" s.noRankChannelID = "CAAAAAAAA" s.rankHeader = fmt.Sprintf("*Rank for * <#%s>", s.rankChannelID) From 06b92761765a30883d281a4a48767df7cc65f144 Mon Sep 17 00:00:00 2001 From: Evandro Flores Date: Thu, 19 Sep 2019 23:16:19 +0100 Subject: [PATCH 15/15] Delete disabled tests --- cmd/delete_disabled_test.go | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/cmd/delete_disabled_test.go b/cmd/delete_disabled_test.go index e2fa98d..4a3037b 100644 --- a/cmd/delete_disabled_test.go +++ b/cmd/delete_disabled_test.go @@ -3,6 +3,7 @@ package cmd import ( "fmt" "reflect" + "strings" "testing" "bou.ke/monkey" @@ -11,12 +12,14 @@ import ( "github.com/evandroflores/pong/model" "github.com/evandroflores/pong/slack" ns "github.com/nlopes/slack" + "github.com/shomali11/proper" "github.com/shomali11/slacker" "github.com/stretchr/testify/suite" ) type DeleteDisabledTestSuite struct { suite.Suite + teamID string channelID string adminID string nonAdminID string @@ -25,6 +28,7 @@ type DeleteDisabledTestSuite struct { } func (s *DeleteDisabledTestSuite) SetupSuite() { + s.teamID = "TDELETE" s.channelID = "CTESTDEL" s.adminID = "UIS0ADMIN" s.nonAdminID = "UNOTADMIN" @@ -32,7 +36,7 @@ func (s *DeleteDisabledTestSuite) SetupSuite() { s.deactivated = map[string]model.Player{} for i := 1; i <= 20; i++ { - player := makeTestPlayerWith("TTTTTTTX", s.channelID, fmt.Sprintf("U%08d", i), fmt.Sprintf("Fake User %08d", i)) + player := makeTestPlayerWith(s.teamID, s.channelID, fmt.Sprintf("U%08d", i), fmt.Sprintf("Fake User %08d", i)) player.Points = 1000 - float64(i) if i%2 == 0 { s.deactivated[player.SlackID] = player @@ -67,3 +71,31 @@ func (s *DeleteDisabledTestSuite) TearDownSuite() { func TestDeleteDisabledTestSuite(t *testing.T) { suite.Run(t, new(DeleteDisabledTestSuite)) } + +func (s *DeleteDisabledTestSuite) TestAdminRemoving() { + var props = proper.NewProperties(map[string]string{}) + + evt := makeTestEventWith(s.teamID, s.channelID, s.adminID) + + request := &fakeRequest{event: evt, properties: props} + response := &fakeResponse{} + + deleteDisabled(request, response) + s.Len(response.GetMessages(), 1) + s.Equal(strings.Count(response.GetMessages()[0], "Removed"), len(s.deactivated)) + s.Empty(response.GetErrors()) +} + +func (s *DeleteDisabledTestSuite) TestNoUsersRemoved() { + var props = proper.NewProperties(map[string]string{}) + + evt := makeTestEventWith(s.teamID, s.channelID, s.adminID) + + request := &fakeRequest{event: evt, properties: props} + response := &fakeResponse{} + + deleteDisabled(request, response) + s.Len(response.GetMessages(), 1) + s.Equal(response.GetMessages()[0], "No users removed.") + s.Empty(response.GetErrors()) +}