From d25630020d29306cd509c74bc3ede1d076124841 Mon Sep 17 00:00:00 2001 From: Akshay Shekher Date: Wed, 13 Oct 2021 08:00:11 -0700 Subject: [PATCH] /back, /away: Change no-op to return err Fixes #402 When the user is not set as away, using the `/back` or `/away` command should return error. The previous behaviour was inconsistent, `/away` sent a message and `/back` ignored it. New behaviour is error for both cases. Co-authored-by: Akshay --- chat/command.go | 14 ++++++++------ chat/command_test.go | 20 ++++++++++++++++---- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/chat/command.go b/chat/command.go index 6420324..6a8fcd2 100644 --- a/chat/command.go +++ b/chat/command.go @@ -469,12 +469,13 @@ func InitCommands(c *Commands) { msg.From().SetAway(awayMsg) if awayMsg != "" { room.Send(message.NewEmoteMsg("has gone away: "+awayMsg, msg.From())) - } else if !isAway { - room.Send(message.NewSystemMsg("Not away. Append a reason message to set away.", msg.From())) - } else { - room.Send(message.NewEmoteMsg("is back.", msg.From())) + return nil } - return nil + if isAway { + room.Send(message.NewEmoteMsg("is back.", msg.From())) + return nil + } + return errors.New("not away. Append a reason message to set away") }, }) @@ -486,8 +487,9 @@ func InitCommands(c *Commands) { if isAway { msg.From().SetAway("") room.Send(message.NewEmoteMsg("is back.", msg.From())) + return nil } - return nil + return errors.New("must be away to be back") }, }) diff --git a/chat/command_test.go b/chat/command_test.go index 839d34c..37f5cb8 100644 --- a/chat/command_test.go +++ b/chat/command_test.go @@ -25,10 +25,17 @@ func TestAwayCommands(t *testing.T) { // expected output IsUserAway bool AwayMessage string + + // expected state change + ExpectsError func(awayBefore bool) bool } - awayStep := step{"/away snorkling", true, "snorkling"} - notAwayStep := step{"/away", false, ""} - backStep := step{"/back", false, ""} + neverError := func(_ bool) bool { return false } + // if the user was away before, then the error is expected + errorIfAwayBefore := func(awayBefore bool) bool { return awayBefore } + + awayStep := step{"/away snorkling", true, "snorkling", neverError} + notAwayStep := step{"/away", false, "", errorIfAwayBefore} + backStep := step{"/back", false, "", errorIfAwayBefore} steps := []step{awayStep, notAwayStep, backStep} cases := [][]int{ @@ -42,7 +49,12 @@ func TestAwayCommands(t *testing.T) { for _, s := range []step{steps[c[0]], steps[c[1]], steps[c[2]]} { msg, _ := message.NewPublicMsg(s.Msg, u).ParseCommand() - cmds.Run(room, *msg) + awayBeforeCommand, _, _ := u.GetAway() + + err := cmds.Run(room, *msg) + if err != nil && s.ExpectsError(awayBeforeCommand) { + t.Fatalf("unexpected error running the command: %+v", err) + } isAway, _, awayMsg := u.GetAway() if isAway != s.IsUserAway {