From 1ebf19825bc735424f7278a3c749f83d24ab80f1 Mon Sep 17 00:00:00 2001 From: Elias Nahum Date: Thu, 12 Apr 2018 10:12:59 -0300 Subject: Fix mute channel when not a member and adding unit tests (#8609) --- app/channel.go | 8 +- app/command_mute.go | 5 +- app/command_mute_test.go | 204 +++++++++++++++++++++++++++++++++++++++++++++++ i18n/en.json | 8 ++ 4 files changed, 223 insertions(+), 2 deletions(-) create mode 100644 app/command_mute_test.go diff --git a/app/channel.go b/app/channel.go index 40f21c3a9..7c14538f4 100644 --- a/app/channel.go +++ b/app/channel.go @@ -1491,7 +1491,13 @@ func (a *App) GetDirectChannel(userId1, userId2 string) (*model.Channel, *model. } func (a *App) ToggleMuteChannel(channelId string, userId string) *model.ChannelMember { - member := (<-a.Srv.Store.Channel().GetMember(channelId, userId)).Data.(*model.ChannelMember) + result := <-a.Srv.Store.Channel().GetMember(channelId, userId) + + if result.Err != nil { + return nil + } + + member := result.Data.(*model.ChannelMember) if member.NotifyProps[model.MARK_UNREAD_NOTIFY_PROP] == model.CHANNEL_NOTIFY_MENTION { member.NotifyProps[model.MARK_UNREAD_NOTIFY_PROP] = model.CHANNEL_MARK_UNREAD_ALL diff --git a/app/command_mute.go b/app/command_mute.go index fdc698cd9..d3a2fa5f7 100644 --- a/app/command_mute.go +++ b/app/command_mute.go @@ -40,7 +40,7 @@ func (me *MuteProvider) DoCommand(a *App, args *model.CommandArgs, message strin var noChannelErr *model.AppError if channel, noChannelErr = a.GetChannel(args.ChannelId); noChannelErr != nil { - return &model.CommandResponse{Text: args.T("api.command_mute.error", map[string]interface{}{"Channel": channel.DisplayName}), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + return &model.CommandResponse{Text: args.T("api.command_mute.no_channel.error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} } channelName := "" @@ -63,6 +63,9 @@ func (me *MuteProvider) DoCommand(a *App, args *model.CommandArgs, message strin } channelMember := a.ToggleMuteChannel(channel.Id, args.UserId) + if channelMember == nil { + return &model.CommandResponse{Text: args.T("api.command_mute.not_member.error", map[string]interface{}{"Channel": channelName}), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL} + } // Invalidate cache to allow cache lookups while sending notifications a.Srv.Store.Channel().InvalidateCacheForChannelMembersNotifyProps(channel.Id) diff --git a/app/command_mute_test.go b/app/command_mute_test.go new file mode 100644 index 000000000..9f82c48cc --- /dev/null +++ b/app/command_mute_test.go @@ -0,0 +1,204 @@ +// Copyright (c) 2018-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package app + +import ( + "github.com/mattermost/mattermost-server/model" + "github.com/nicksnyder/go-i18n/i18n" + "github.com/stretchr/testify/assert" + "testing" + "time" +) + +func TestMuteCommandNoChannel(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + if testing.Short() { + t.SkipNow() + } + + channel1 := th.BasicChannel + channel1M, channel1MError := th.App.GetChannelMember(channel1.Id, th.BasicUser.Id) + + if channel1MError != nil { + t.Fatal("User is not a member of channel 1") + } + + if channel1M.NotifyProps[model.MARK_UNREAD_NOTIFY_PROP] == model.CHANNEL_NOTIFY_MENTION { + t.Fatal("channel shouldn't be muted on initial setup") + } + + cmd := &MuteProvider{} + resp := cmd.DoCommand(th.App, &model.CommandArgs{ + T: i18n.IdentityTfunc(), + UserId: th.BasicUser.Id, + }, "") + assert.Equal(t, "api.command_mute.no_channel.error", resp.Text) +} + +func TestMuteCommandNoArgs(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + channel1 := th.BasicChannel + channel1M, _ := th.App.GetChannelMember(channel1.Id, th.BasicUser.Id) + + assert.Equal(t, model.CHANNEL_NOTIFY_ALL, channel1M.NotifyProps[model.MARK_UNREAD_NOTIFY_PROP]) + + cmd := &MuteProvider{} + + // First mute the channel + resp := cmd.DoCommand(th.App, &model.CommandArgs{ + T: i18n.IdentityTfunc(), + ChannelId: channel1.Id, + UserId: th.BasicUser.Id, + }, "") + assert.Equal(t, "api.command_mute.success_mute", resp.Text) + + // Now unmute the channel + time.Sleep(time.Millisecond) + resp = cmd.DoCommand(th.App, &model.CommandArgs{ + T: i18n.IdentityTfunc(), + ChannelId: channel1.Id, + UserId: th.BasicUser.Id, + }, "") + + assert.Equal(t, "api.command_mute.success_unmute", resp.Text) +} + +func TestMuteCommandSpecificChannel(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + if testing.Short() { + t.SkipNow() + } + + channel1 := th.BasicChannel + channel2, _ := th.App.CreateChannel(&model.Channel{ + DisplayName: "AA", + Name: "aa" + model.NewId() + "a", + Type: model.CHANNEL_OPEN, + TeamId: th.BasicTeam.Id, + CreatorId: th.BasicUser.Id, + }, true) + + channel2M, _ := th.App.GetChannelMember(channel2.Id, th.BasicUser.Id) + + assert.Equal(t, model.CHANNEL_NOTIFY_ALL, channel2M.NotifyProps[model.MARK_UNREAD_NOTIFY_PROP]) + + cmd := &MuteProvider{} + + // First mute the channel + resp := cmd.DoCommand(th.App, &model.CommandArgs{ + T: i18n.IdentityTfunc(), + ChannelId: channel1.Id, + UserId: th.BasicUser.Id, + }, channel2.Name) + assert.Equal(t, "api.command_mute.success_mute", resp.Text) + time.Sleep(time.Millisecond) + channel2M, _ = th.App.GetChannelMember(channel2.Id, th.BasicUser.Id) + assert.Equal(t, model.CHANNEL_NOTIFY_MENTION, channel2M.NotifyProps[model.MARK_UNREAD_NOTIFY_PROP]) + + // Now unmute the channel + resp = cmd.DoCommand(th.App, &model.CommandArgs{ + T: i18n.IdentityTfunc(), + ChannelId: channel1.Id, + UserId: th.BasicUser.Id, + }, "~"+channel2.Name) + + assert.Equal(t, "api.command_mute.success_unmute", resp.Text) + time.Sleep(time.Millisecond) + channel2M, _ = th.App.GetChannelMember(channel2.Id, th.BasicUser.Id) + assert.Equal(t, model.CHANNEL_NOTIFY_ALL, channel2M.NotifyProps[model.MARK_UNREAD_NOTIFY_PROP]) +} + +func TestMuteCommandNotMember(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + if testing.Short() { + t.SkipNow() + } + + channel1 := th.BasicChannel + channel2, _ := th.App.CreateChannel(&model.Channel{ + DisplayName: "AA", + Name: "aa" + model.NewId() + "a", + Type: model.CHANNEL_OPEN, + TeamId: th.BasicTeam.Id, + CreatorId: th.BasicUser.Id, + }, false) + + cmd := &MuteProvider{} + + // First mute the channel + resp := cmd.DoCommand(th.App, &model.CommandArgs{ + T: i18n.IdentityTfunc(), + ChannelId: channel1.Id, + UserId: th.BasicUser.Id, + }, channel2.Name) + assert.Equal(t, "api.command_mute.not_member.error", resp.Text) +} + +func TestMuteCommandNotChannel(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + if testing.Short() { + t.SkipNow() + } + + channel1 := th.BasicChannel + + cmd := &MuteProvider{} + + // First mute the channel + resp := cmd.DoCommand(th.App, &model.CommandArgs{ + T: i18n.IdentityTfunc(), + ChannelId: channel1.Id, + UserId: th.BasicUser.Id, + }, "~noexists") + assert.Equal(t, "api.command_mute.error", resp.Text) +} + +func TestMuteCommandDMChannel(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + if testing.Short() { + t.SkipNow() + } + + channel2, _ := th.App.CreateDirectChannel(th.BasicUser.Id, th.BasicUser2.Id) + channel2M, _ := th.App.GetChannelMember(channel2.Id, th.BasicUser.Id) + + assert.Equal(t, model.CHANNEL_NOTIFY_ALL, channel2M.NotifyProps[model.MARK_UNREAD_NOTIFY_PROP]) + + cmd := &MuteProvider{} + + // First mute the channel + resp := cmd.DoCommand(th.App, &model.CommandArgs{ + T: i18n.IdentityTfunc(), + ChannelId: channel2.Id, + UserId: th.BasicUser.Id, + }, "") + assert.Equal(t, "api.command_mute.success_mute_direct_msg", resp.Text) + time.Sleep(time.Millisecond) + channel2M, _ = th.App.GetChannelMember(channel2.Id, th.BasicUser.Id) + assert.Equal(t, model.CHANNEL_NOTIFY_MENTION, channel2M.NotifyProps[model.MARK_UNREAD_NOTIFY_PROP]) + + // Now unmute the channel + resp = cmd.DoCommand(th.App, &model.CommandArgs{ + T: i18n.IdentityTfunc(), + ChannelId: channel2.Id, + UserId: th.BasicUser.Id, + }, "") + + assert.Equal(t, "api.command_mute.success_unmute_direct_msg", resp.Text) + time.Sleep(time.Millisecond) + channel2M, _ = th.App.GetChannelMember(channel2.Id, th.BasicUser.Id) + assert.Equal(t, model.CHANNEL_NOTIFY_ALL, channel2M.NotifyProps[model.MARK_UNREAD_NOTIFY_PROP]) +} diff --git a/i18n/en.json b/i18n/en.json index bf93e5b1c..dec619dec 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -906,6 +906,14 @@ "id": "api.command_mute.error", "translation": "Could not find the channel {{.Channel}}. Please use the [channel handle](https://about.mattermost.com/default-channel-handle-documentation) to identify channels." }, + { + "id": "api.command_mute.no_channel.error", + "translation": "Could not find the specified channel. Please use the [channel handle](https://about.mattermost.com/default-channel-handle-documentation) to identify channels." + }, + { + "id": "api.command_mute.not_member.error", + "translation": "Could not mute channel {{.Channel}} as you are not a member." + }, { "id": "api.command_mute.hint", "translation": "~[channel]" -- cgit v1.2.3-1-g7c22