Skip to content

Commit d256300

Browse files
voldymanAkshay
andauthored
/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 <akshay.shekher@gmail.com>
1 parent 0eebb64 commit d256300

File tree

2 files changed

+23
-9
lines changed

2 files changed

+23
-9
lines changed

chat/command.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -469,12 +469,13 @@ func InitCommands(c *Commands) {
469469
msg.From().SetAway(awayMsg)
470470
if awayMsg != "" {
471471
room.Send(message.NewEmoteMsg("has gone away: "+awayMsg, msg.From()))
472-
} else if !isAway {
473-
room.Send(message.NewSystemMsg("Not away. Append a reason message to set away.", msg.From()))
474-
} else {
472+
return nil
473+
}
474+
if isAway {
475475
room.Send(message.NewEmoteMsg("is back.", msg.From()))
476+
return nil
476477
}
477-
return nil
478+
return errors.New("not away. Append a reason message to set away")
478479
},
479480
})
480481

@@ -486,8 +487,9 @@ func InitCommands(c *Commands) {
486487
if isAway {
487488
msg.From().SetAway("")
488489
room.Send(message.NewEmoteMsg("is back.", msg.From()))
490+
return nil
489491
}
490-
return nil
492+
return errors.New("must be away to be back")
491493
},
492494
})
493495

chat/command_test.go

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,17 @@ func TestAwayCommands(t *testing.T) {
2525
// expected output
2626
IsUserAway bool
2727
AwayMessage string
28+
29+
// expected state change
30+
ExpectsError func(awayBefore bool) bool
2831
}
29-
awayStep := step{"/away snorkling", true, "snorkling"}
30-
notAwayStep := step{"/away", false, ""}
31-
backStep := step{"/back", false, ""}
32+
neverError := func(_ bool) bool { return false }
33+
// if the user was away before, then the error is expected
34+
errorIfAwayBefore := func(awayBefore bool) bool { return awayBefore }
35+
36+
awayStep := step{"/away snorkling", true, "snorkling", neverError}
37+
notAwayStep := step{"/away", false, "", errorIfAwayBefore}
38+
backStep := step{"/back", false, "", errorIfAwayBefore}
3239

3340
steps := []step{awayStep, notAwayStep, backStep}
3441
cases := [][]int{
@@ -42,7 +49,12 @@ func TestAwayCommands(t *testing.T) {
4249
for _, s := range []step{steps[c[0]], steps[c[1]], steps[c[2]]} {
4350
msg, _ := message.NewPublicMsg(s.Msg, u).ParseCommand()
4451

45-
cmds.Run(room, *msg)
52+
awayBeforeCommand, _, _ := u.GetAway()
53+
54+
err := cmds.Run(room, *msg)
55+
if err != nil && s.ExpectsError(awayBeforeCommand) {
56+
t.Fatalf("unexpected error running the command: %+v", err)
57+
}
4658

4759
isAway, _, awayMsg := u.GetAway()
4860
if isAway != s.IsUserAway {

0 commit comments

Comments
 (0)