diff --git a/server/channels/api4/channel.go b/server/channels/api4/channel.go index c7c0806995..dc74959b98 100644 --- a/server/channels/api4/channel.go +++ b/server/channels/api4/channel.go @@ -377,6 +377,11 @@ func patchChannel(c *Context, w http.ResponseWriter, r *http.Request) { } } + if patch.BannerInfo != nil && (originalOldChannel.Type != model.ChannelTypeOpen && originalOldChannel.Type != model.ChannelTypePrivate) { + c.Err = model.NewAppError("patchChannel", "api.channel.update_channel.banner_info.channel_type.not_allowed", nil, "", http.StatusBadRequest) + return + } + rchannel, appErr := c.App.PatchChannel(c.AppContext, oldChannel, patch, c.AppContext.Session().UserId) if appErr != nil { c.Err = appErr diff --git a/server/channels/api4/channel_test.go b/server/channels/api4/channel_test.go index 4b4987fa3a..876a5177f8 100644 --- a/server/channels/api4/channel_test.go +++ b/server/channels/api4/channel_test.go @@ -160,6 +160,44 @@ func TestCreateChannel(t *testing.T) { CheckErrorID(t, err, "api.context.invalid_body_param.app_error") CheckBadRequestStatus(t, resp) }) + + t.Run("Can create channel with banner info", func(t *testing.T) { + channel := &model.Channel{ + DisplayName: GenerateTestChannelName(), + Name: GenerateTestChannelName(), + Type: model.ChannelTypeOpen, + TeamId: team.Id, + BannerInfo: &model.ChannelBannerInfo{ + Enabled: model.NewPointer(true), + Text: model.NewPointer("banner text"), + BackgroundColor: model.NewPointer("color"), + }, + } + + createdChannel, resp, err := client.CreateChannel(context.Background(), channel) + require.NoError(t, err) + CheckCreatedStatus(t, resp) + + require.True(t, *createdChannel.BannerInfo.Enabled) + require.Equal(t, "banner text", *createdChannel.BannerInfo.Text) + require.Equal(t, "color", *createdChannel.BannerInfo.BackgroundColor) + }) + + t.Run("Cannot create channel with banner enabled but not configured", func(t *testing.T) { + channel := &model.Channel{ + DisplayName: "", + Name: GenerateTestChannelName(), + Type: model.ChannelTypeOpen, + TeamId: team.Id, + BannerInfo: &model.ChannelBannerInfo{ + Enabled: model.NewPointer(true), + }, + } + + _, resp, err := client.CreateChannel(context.Background(), channel) + CheckErrorID(t, err, "api.context.invalid_body_param.app_error") + CheckBadRequestStatus(t, resp) + }) } func TestUpdateChannel(t *testing.T) { @@ -343,110 +381,137 @@ func TestPatchChannel(t *testing.T) { client := th.Client team := th.BasicTeam - var nullPatch *model.ChannelPatch + t.Run("should be unable to apply a null patch", func(t *testing.T) { + var nullPatch *model.ChannelPatch - _, nullResp, err := client.PatchChannel(context.Background(), th.BasicChannel.Id, nullPatch) - require.Error(t, err) - CheckBadRequestStatus(t, nullResp) - - patch := &model.ChannelPatch{ - Name: new(string), - DisplayName: new(string), - Header: new(string), - Purpose: new(string), - } - *patch.Name = model.NewId() - *patch.DisplayName = model.NewId() - *patch.Header = model.NewId() - *patch.Purpose = model.NewId() - - channel, _, err := client.PatchChannel(context.Background(), th.BasicChannel.Id, patch) - require.NoError(t, err) - - require.Equal(t, *patch.Name, channel.Name, "do not match") - require.Equal(t, *patch.DisplayName, channel.DisplayName, "do not match") - require.Equal(t, *patch.Header, channel.Header, "do not match") - require.Equal(t, *patch.Purpose, channel.Purpose, "do not match") - - patch.Name = nil - oldName := channel.Name - channel, _, err = client.PatchChannel(context.Background(), th.BasicChannel.Id, patch) - require.NoError(t, err) - - require.Equal(t, oldName, channel.Name, "should not have updated") - - // Test updating default channel's name and returns error - defaultChannel, _ := th.App.GetChannelByName(th.Context, model.DefaultChannelName, team.Id, false) - defaultChannelPatch := &model.ChannelPatch{ - Name: new(string), - } - *defaultChannelPatch.Name = "testing" - _, resp, err := client.PatchChannel(context.Background(), defaultChannel.Id, defaultChannelPatch) - require.Error(t, err) - CheckBadRequestStatus(t, resp) - - // Test GroupConstrained flag - patch.GroupConstrained = model.NewPointer(true) - rchannel, resp, err := client.PatchChannel(context.Background(), th.BasicChannel.Id, patch) - require.NoError(t, err) - CheckOKStatus(t, resp) - - require.Equal(t, *rchannel.GroupConstrained, *patch.GroupConstrained, "GroupConstrained flags do not match") - patch.GroupConstrained = nil - - _, resp, err = client.PatchChannel(context.Background(), "junk", patch) - require.Error(t, err) - CheckBadRequestStatus(t, resp) - - _, resp, err = client.PatchChannel(context.Background(), model.NewId(), patch) - require.Error(t, err) - CheckNotFoundStatus(t, resp) - - user := th.CreateUser() - client.Login(context.Background(), user.Email, user.Password) - _, resp, err = client.PatchChannel(context.Background(), th.BasicChannel.Id, patch) - require.Error(t, err) - CheckForbiddenStatus(t, resp) - - th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { - _, _, err = client.PatchChannel(context.Background(), th.BasicChannel.Id, patch) - require.NoError(t, err) - - _, _, err = client.PatchChannel(context.Background(), th.BasicPrivateChannel.Id, patch) - require.NoError(t, err) + _, nullResp, err := client.PatchChannel(context.Background(), th.BasicChannel.Id, nullPatch) + require.Error(t, err) + CheckBadRequestStatus(t, nullResp) }) - // Test updating the header of someone else's GM channel. - user1 := th.CreateUser() - user2 := th.CreateUser() - user3 := th.CreateUser() + t.Run("should be able to patch values", func(t *testing.T) { + patch := &model.ChannelPatch{ + Name: new(string), + DisplayName: new(string), + Header: new(string), + Purpose: new(string), + } + *patch.Name = model.NewId() + *patch.DisplayName = model.NewId() + *patch.Header = model.NewId() + *patch.Purpose = model.NewId() - groupChannel, _, err := client.CreateGroupChannel(context.Background(), []string{user1.Id, user2.Id}) - require.NoError(t, err) + channel, _, err := client.PatchChannel(context.Background(), th.BasicChannel.Id, patch) + require.NoError(t, err) - client.Logout(context.Background()) - client.Login(context.Background(), user3.Email, user3.Password) + require.Equal(t, *patch.Name, channel.Name, "do not match") + require.Equal(t, *patch.DisplayName, channel.DisplayName, "do not match") + require.Equal(t, *patch.Header, channel.Header, "do not match") + require.Equal(t, *patch.Purpose, channel.Purpose, "do not match") + }) - channelPatch := &model.ChannelPatch{} - channelPatch.Header = new(string) - *channelPatch.Header = "lolololol" + t.Run("should be able to patch with no name", func(t *testing.T) { + channel := &model.Channel{ + DisplayName: GenerateTestChannelName(), + Name: GenerateTestChannelName(), + Type: model.ChannelTypeOpen, + TeamId: team.Id, + } + var err error + channel, _, err = client.CreateChannel(context.Background(), channel) + require.NoError(t, err) - _, resp, err = client.PatchChannel(context.Background(), groupChannel.Id, channelPatch) - require.Error(t, err) - CheckForbiddenStatus(t, resp) + patch := &model.ChannelPatch{ + Header: new(string), + Purpose: new(string), + } - // Test updating the header of someone else's GM channel. - client.Logout(context.Background()) - client.Login(context.Background(), user.Email, user.Password) + oldName := channel.Name + patchedChannel, _, err := client.PatchChannel(context.Background(), channel.Id, patch) + require.NoError(t, err) - directChannel, _, err := client.CreateDirectChannel(context.Background(), user.Id, user1.Id) - require.NoError(t, err) + require.Equal(t, oldName, patchedChannel.Name, "should not have updated") + }) - client.Logout(context.Background()) - client.Login(context.Background(), user3.Email, user3.Password) - _, resp, err = client.PatchChannel(context.Background(), directChannel.Id, channelPatch) - require.Error(t, err) - CheckForbiddenStatus(t, resp) + t.Run("Test updating default channel's name and returns error", func(t *testing.T) { + // Test updating default channel's name and returns error + defaultChannel, _ := th.App.GetChannelByName(th.Context, model.DefaultChannelName, team.Id, false) + defaultChannelPatch := &model.ChannelPatch{ + Name: new(string), + } + *defaultChannelPatch.Name = "testing" + _, resp, err := client.PatchChannel(context.Background(), defaultChannel.Id, defaultChannelPatch) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + }) + + t.Run("Test GroupConstrained flag", func(t *testing.T) { + // Test GroupConstrained flag + patch := &model.ChannelPatch{} + patch.GroupConstrained = model.NewPointer(true) + rchannel, resp, err := client.PatchChannel(context.Background(), th.BasicChannel.Id, patch) + require.NoError(t, err) + CheckOKStatus(t, resp) + + require.Equal(t, *rchannel.GroupConstrained, *patch.GroupConstrained, "GroupConstrained flags do not match") + patch.GroupConstrained = nil + + _, resp, err = client.PatchChannel(context.Background(), "junk", patch) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + + _, resp, err = client.PatchChannel(context.Background(), model.NewId(), patch) + require.Error(t, err) + CheckNotFoundStatus(t, resp) + + user := th.CreateUser() + client.Login(context.Background(), user.Email, user.Password) + _, resp, err = client.PatchChannel(context.Background(), th.BasicChannel.Id, patch) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + + th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { + _, _, err = client.PatchChannel(context.Background(), th.BasicChannel.Id, patch) + require.NoError(t, err) + + _, _, err = client.PatchChannel(context.Background(), th.BasicPrivateChannel.Id, patch) + require.NoError(t, err) + }) + }) + + t.Run("Test updating the header of someone else's GM channel", func(t *testing.T) { + // Test updating the header of someone else's GM channel. + user := th.CreateUser() + user1 := th.CreateUser() + user2 := th.CreateUser() + user3 := th.CreateUser() + + groupChannel, _, err := client.CreateGroupChannel(context.Background(), []string{user1.Id, user2.Id}) + require.NoError(t, err) + + client.Logout(context.Background()) + client.Login(context.Background(), user3.Email, user3.Password) + + channelPatch := &model.ChannelPatch{} + channelPatch.Header = new(string) + *channelPatch.Header = "lolololol" + + _, resp, err := client.PatchChannel(context.Background(), groupChannel.Id, channelPatch) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + + client.Logout(context.Background()) + client.Login(context.Background(), user.Email, user.Password) + + directChannel, _, err := client.CreateDirectChannel(context.Background(), user.Id, user1.Id) + require.NoError(t, err) + + client.Logout(context.Background()) + client.Login(context.Background(), user3.Email, user3.Password) + _, resp, err = client.PatchChannel(context.Background(), directChannel.Id, channelPatch) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) t.Run("Should block changes to name, display name or purpose for group messages", func(t *testing.T) { user1 := th.CreateUser() @@ -518,6 +583,140 @@ func TestPatchChannel(t *testing.T) { require.Error(t, err) CheckBadRequestStatus(t, resp) }) + + t.Run("Should be able to configure channel banner on a channel", func(t *testing.T) { + client.Logout(context.Background()) + th.LoginBasic() + + channel := &model.Channel{ + DisplayName: GenerateTestChannelName(), + Name: GenerateTestChannelName(), + Type: model.ChannelTypeOpen, + TeamId: team.Id, + } + var err error + channel, _, err = client.CreateChannel(context.Background(), channel) + require.NoError(t, err) + + patch := &model.ChannelPatch{ + BannerInfo: &model.ChannelBannerInfo{ + Enabled: model.NewPointer(true), + Text: model.NewPointer("banner text"), + BackgroundColor: model.NewPointer("color"), + }, + } + + patchedChannel, resp, err := client.PatchChannel(context.Background(), channel.Id, patch) + require.NoError(t, err) + CheckOKStatus(t, resp) + require.NotNil(t, patchedChannel.BannerInfo) + require.True(t, *patchedChannel.BannerInfo.Enabled) + require.Equal(t, "banner text", *patchedChannel.BannerInfo.Text) + require.Equal(t, "color", *patchedChannel.BannerInfo.BackgroundColor) + }) + + t.Run("Cannot enable channel banner without configuring it", func(t *testing.T) { + client.Logout(context.Background()) + th.LoginBasic() + + channel := &model.Channel{ + DisplayName: GenerateTestChannelName(), + Name: GenerateTestChannelName(), + Type: model.ChannelTypeOpen, + TeamId: team.Id, + } + var err error + channel, _, err = client.CreateChannel(context.Background(), channel) + require.NoError(t, err) + + patch := &model.ChannelPatch{ + BannerInfo: &model.ChannelBannerInfo{ + Enabled: model.NewPointer(true), + }, + } + + _, resp, err := client.PatchChannel(context.Background(), channel.Id, patch) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + + // now we will configure it first, then enable it + patch = &model.ChannelPatch{ + BannerInfo: &model.ChannelBannerInfo{ + Enabled: nil, + Text: model.NewPointer("banner text"), + BackgroundColor: model.NewPointer("color"), + }, + } + + patchedChannel, resp, err := client.PatchChannel(context.Background(), channel.Id, patch) + require.NoError(t, err) + CheckOKStatus(t, resp) + require.NotNil(t, patchedChannel.BannerInfo) + require.Nil(t, patchedChannel.BannerInfo.Enabled) + require.Equal(t, "banner text", *patchedChannel.BannerInfo.Text) + require.Equal(t, "color", *patchedChannel.BannerInfo.BackgroundColor) + + patch = &model.ChannelPatch{ + BannerInfo: &model.ChannelBannerInfo{ + Enabled: model.NewPointer(true), + }, + } + + patchedChannel, resp, err = client.PatchChannel(context.Background(), channel.Id, patch) + require.NoError(t, err) + CheckOKStatus(t, resp) + require.NotNil(t, patchedChannel.BannerInfo) + require.True(t, *patchedChannel.BannerInfo.Enabled) + require.Equal(t, "banner text", *patchedChannel.BannerInfo.Text) + require.Equal(t, "color", *patchedChannel.BannerInfo.BackgroundColor) + }) + + t.Run("Cannot configure channel banner on a DM channel", func(t *testing.T) { + client.Logout(context.Background()) + th.LoginBasic() + + dmChannel, resp, err := client.CreateDirectChannel(context.Background(), th.BasicUser.Id, th.BasicUser2.Id) + require.NoError(t, err) + CheckCreatedStatus(t, resp) + + patch := &model.ChannelPatch{ + BannerInfo: &model.ChannelBannerInfo{ + Enabled: model.NewPointer(true), + Text: model.NewPointer("banner text"), + BackgroundColor: model.NewPointer("color"), + }, + } + + patchedChannel, resp, err := client.PatchChannel(context.Background(), dmChannel.Id, patch) + require.Error(t, err) + require.Equal(t, "Channel banner can only be configured on Public and Private channels.", err.Error()) + CheckBadRequestStatus(t, resp) + require.Nil(t, patchedChannel) + }) + + t.Run("Cannot configure channel banner on a GM channel", func(t *testing.T) { + client.Logout(context.Background()) + th.LoginBasic() + + user3 := th.CreateUser() + gmChannel, resp, err := client.CreateGroupChannel(context.Background(), []string{th.BasicUser.Id, th.BasicUser2.Id, user3.Id}) + require.NoError(t, err) + CheckCreatedStatus(t, resp) + + patch := &model.ChannelPatch{ + BannerInfo: &model.ChannelBannerInfo{ + Enabled: model.NewPointer(true), + Text: model.NewPointer("banner text"), + BackgroundColor: model.NewPointer("color"), + }, + } + + patchedChannel, resp, err := client.PatchChannel(context.Background(), gmChannel.Id, patch) + require.Error(t, err) + require.Equal(t, "Channel banner can only be configured on Public and Private channels.", err.Error()) + CheckBadRequestStatus(t, resp) + require.Nil(t, patchedChannel) + }) } func TestChannelUnicodeNames(t *testing.T) { diff --git a/server/channels/app/channel_test.go b/server/channels/app/channel_test.go index 340722ec2a..09d8a3ff5d 100644 --- a/server/channels/app/channel_test.go +++ b/server/channels/app/channel_test.go @@ -7,6 +7,7 @@ import ( "context" "errors" "fmt" + "net/http" "sort" "strings" "sync" @@ -3137,6 +3138,7 @@ func TestPatchChannelMembersNotifyProps(t *testing.T) { assert.NotNil(t, appErr) }) } + func TestGetChannelFileCount(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() @@ -3189,3 +3191,145 @@ func TestGetChannelFileCount(t *testing.T) { require.Nil(t, appErr) require.Equal(t, int64(2), count) } + +func TestUpdateChannel(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + t.Run("should be able to update banner info", func(t *testing.T) { + channel := th.createChannel(th.Context, th.BasicTeam, model.ChannelTypeOpen) + + channel.BannerInfo = &model.ChannelBannerInfo{ + Enabled: model.NewPointer(true), + Text: model.NewPointer("banner text"), + BackgroundColor: model.NewPointer("#000000"), + } + + updatedChannel, appErr := th.App.UpdateChannel(th.Context, channel) + require.Nil(t, appErr) + require.NotNil(t, updatedChannel.BannerInfo) + require.True(t, *updatedChannel.BannerInfo.Enabled) + require.Equal(t, "banner text", *updatedChannel.BannerInfo.Text) + require.Equal(t, "#000000", *updatedChannel.BannerInfo.BackgroundColor) + + channel.BannerInfo.Enabled = model.NewPointer(false) + updatedChannel, appErr = th.App.UpdateChannel(th.Context, channel) + require.Nil(t, appErr) + require.NotNil(t, updatedChannel.BannerInfo) + require.False(t, *updatedChannel.BannerInfo.Enabled) + }) +} + +func TestPatchChannel(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + t.Run("should be able to patch banner info", func(t *testing.T) { + channel := th.createChannel(th.Context, th.BasicTeam, model.ChannelTypeOpen) + + patch := &model.ChannelPatch{ + BannerInfo: &model.ChannelBannerInfo{ + Enabled: model.NewPointer(true), + Text: model.NewPointer("banner text"), + BackgroundColor: model.NewPointer("#000000"), + }, + } + + patchedChannel, appErr := th.App.PatchChannel(th.Context, channel, patch, channel.CreatorId) + require.Nil(t, appErr) + require.NotNil(t, patchedChannel.BannerInfo) + require.True(t, *patchedChannel.BannerInfo.Enabled) + require.Equal(t, "banner text", *patchedChannel.BannerInfo.Text) + require.Equal(t, "#000000", *patchedChannel.BannerInfo.BackgroundColor) + + patch = &model.ChannelPatch{ + BannerInfo: &model.ChannelBannerInfo{ + Text: model.NewPointer("text 1"), + }, + } + + patchedChannel, appErr = th.App.PatchChannel(th.Context, channel, patch, channel.CreatorId) + require.Nil(t, appErr) + require.NotNil(t, patchedChannel.BannerInfo) + require.True(t, *patchedChannel.BannerInfo.Enabled) + require.Equal(t, "text 1", *patchedChannel.BannerInfo.Text) + require.Equal(t, "#000000", *patchedChannel.BannerInfo.BackgroundColor) + + patch = &model.ChannelPatch{ + BannerInfo: &model.ChannelBannerInfo{ + BackgroundColor: model.NewPointer("#FF00FF"), + }, + } + + patchedChannel, appErr = th.App.PatchChannel(th.Context, channel, patch, channel.CreatorId) + require.Nil(t, appErr) + require.NotNil(t, patchedChannel.BannerInfo) + require.True(t, *patchedChannel.BannerInfo.Enabled) + require.Equal(t, "text 1", *patchedChannel.BannerInfo.Text) + require.Equal(t, "#FF00FF", *patchedChannel.BannerInfo.BackgroundColor) + + // should be able to unset fields as well + patch = &model.ChannelPatch{ + BannerInfo: &model.ChannelBannerInfo{ + Enabled: model.NewPointer(false), + }, + } + + patchedChannel, appErr = th.App.PatchChannel(th.Context, channel, patch, channel.CreatorId) + require.Nil(t, appErr) + require.NotNil(t, patchedChannel.BannerInfo) + require.False(t, *patchedChannel.BannerInfo.Enabled) + }) + + t.Run("should not allow saving channel with invalid background info", func(t *testing.T) { + channel := th.createChannel(th.Context, th.BasicTeam, model.ChannelTypeOpen) + + // enabling banner without data is invalid + patch := &model.ChannelPatch{ + BannerInfo: &model.ChannelBannerInfo{ + Enabled: model.NewPointer(true), + }, + } + + patchedChannel, appErr := th.App.PatchChannel(th.Context, channel, patch, channel.CreatorId) + require.Nil(t, patchedChannel) + require.NotNil(t, appErr) + require.Equal(t, http.StatusBadRequest, appErr.StatusCode) + require.Equal(t, "model.channel.is_valid.banner_info.text.empty.app_error", appErr.Id) + }) + + t.Run("cannot configure channel banner on DMs", func(t *testing.T) { + dmChannel := th.CreateDmChannel(th.BasicUser2) + + // enabling banner without data is invalid + patch := &model.ChannelPatch{ + BannerInfo: &model.ChannelBannerInfo{ + Enabled: model.NewPointer(true), + }, + } + + patchedChannel, appErr := th.App.PatchChannel(th.Context, dmChannel, patch, dmChannel.CreatorId) + require.Nil(t, patchedChannel) + require.NotNil(t, appErr) + require.Equal(t, appErr.StatusCode, http.StatusBadRequest) + require.Equal(t, "model.channel.is_valid.banner_info.channel_type.app_error", appErr.Id) + }) + + t.Run("cannot configure channel banner on GMs", func(t *testing.T) { + user3 := th.CreateUser() + gmChannel := th.CreateGroupChannel(th.Context, th.BasicUser2, user3) + + // enabling banner without data is invalid + patch := &model.ChannelPatch{ + BannerInfo: &model.ChannelBannerInfo{ + Enabled: model.NewPointer(true), + }, + } + + patchedChannel, appErr := th.App.PatchChannel(th.Context, gmChannel, patch, gmChannel.CreatorId) + require.Nil(t, patchedChannel) + require.NotNil(t, appErr) + require.Equal(t, appErr.StatusCode, http.StatusBadRequest) + require.Equal(t, "model.channel.is_valid.banner_info.channel_type.app_error", appErr.Id) + }) +} diff --git a/server/channels/db/migrations/migrations.list b/server/channels/db/migrations/migrations.list index 0888404443..7e342bcded 100644 --- a/server/channels/db/migrations/migrations.list +++ b/server/channels/db/migrations/migrations.list @@ -261,6 +261,8 @@ channels/db/migrations/mysql/000131_create_index_pagination_on_property_values.d channels/db/migrations/mysql/000131_create_index_pagination_on_property_values.up.sql channels/db/migrations/mysql/000132_create_index_pagination_on_property_fields.down.sql channels/db/migrations/mysql/000132_create_index_pagination_on_property_fields.up.sql +channels/db/migrations/mysql/000133_add_channel_banner_fields.down.sql +channels/db/migrations/mysql/000133_add_channel_banner_fields.up.sql channels/db/migrations/postgres/000001_create_teams.down.sql channels/db/migrations/postgres/000001_create_teams.up.sql channels/db/migrations/postgres/000002_create_team_members.down.sql @@ -523,3 +525,5 @@ channels/db/migrations/postgres/000131_create_index_pagination_on_property_value channels/db/migrations/postgres/000131_create_index_pagination_on_property_values.up.sql channels/db/migrations/postgres/000132_create_index_pagination_on_property_fields.down.sql channels/db/migrations/postgres/000132_create_index_pagination_on_property_fields.up.sql +channels/db/migrations/postgres/000133_add_channel_banner_fields.down.sql +channels/db/migrations/postgres/000133_add_channel_banner_fields.up.sql diff --git a/server/channels/db/migrations/mysql/000133_add_channel_banner_fields.down.sql b/server/channels/db/migrations/mysql/000133_add_channel_banner_fields.down.sql new file mode 100644 index 0000000000..af29e75514 --- /dev/null +++ b/server/channels/db/migrations/mysql/000133_add_channel_banner_fields.down.sql @@ -0,0 +1,14 @@ +SET @preparedStatement = (SELECT IF( + EXISTS( + SELECT 1 FROM INFORMATION_SCHEMA.STATISTICS + WHERE table_name = 'Channels' + AND table_schema = DATABASE() + AND column_name = 'BannerInfo' + ), + 'ALTER TABLE Channels DROP COLUMN BannerInfo;', + 'SELECT 1;' +)); + +PREPARE removeColumnIfExists FROM @preparedStatement; +EXECUTE removeColumnIfExists; +DEALLOCATE PREPARE removeColumnIfExists; diff --git a/server/channels/db/migrations/mysql/000133_add_channel_banner_fields.up.sql b/server/channels/db/migrations/mysql/000133_add_channel_banner_fields.up.sql new file mode 100644 index 0000000000..1dee9c496a --- /dev/null +++ b/server/channels/db/migrations/mysql/000133_add_channel_banner_fields.up.sql @@ -0,0 +1,14 @@ +SET @preparedStatement = (SELECT IF( + NOT EXISTS( + SELECT 1 FROM INFORMATION_SCHEMA.COLUMNS + WHERE table_name = 'Channels' + AND table_schema = DATABASE() + AND column_name = 'BannerInfo' + ), + 'ALTER TABLE Channels ADD COLUMN BannerInfo json;', + 'SELECT 1;' +)); + +PREPARE addColumnIfNotExists FROM @preparedStatement; +EXECUTE addColumnIfNotExists; +DEALLOCATE PREPARE addColumnIfNotExists; diff --git a/server/channels/db/migrations/postgres/000133_add_channel_banner_fields.down.sql b/server/channels/db/migrations/postgres/000133_add_channel_banner_fields.down.sql new file mode 100644 index 0000000000..e531ec37a2 --- /dev/null +++ b/server/channels/db/migrations/postgres/000133_add_channel_banner_fields.down.sql @@ -0,0 +1 @@ +ALTER TABLE channels DROP COLUMN IF EXISTS bannerinfo; diff --git a/server/channels/db/migrations/postgres/000133_add_channel_banner_fields.up.sql b/server/channels/db/migrations/postgres/000133_add_channel_banner_fields.up.sql new file mode 100644 index 0000000000..7b37755c93 --- /dev/null +++ b/server/channels/db/migrations/postgres/000133_add_channel_banner_fields.up.sql @@ -0,0 +1 @@ +ALTER TABLE channels ADD COLUMN IF NOT EXISTS bannerinfo jsonb; diff --git a/server/channels/store/sqlstore/channel_store.go b/server/channels/store/sqlstore/channel_store.go index 8b730a19f0..420f18de02 100644 --- a/server/channels/store/sqlstore/channel_store.go +++ b/server/channels/store/sqlstore/channel_store.go @@ -135,28 +135,10 @@ func channelSliceColumns(prefix ...string) []string { p + "Shared", p + "TotalMsgCountRoot", p + "LastRootPostAt", + p + "BannerInfo", } } -func channelMemberToSlice(member *model.ChannelMember) []any { - resultSlice := []any{} - resultSlice = append(resultSlice, member.ChannelId) - resultSlice = append(resultSlice, member.UserId) - resultSlice = append(resultSlice, member.ExplicitRoles) - resultSlice = append(resultSlice, member.LastViewedAt) - resultSlice = append(resultSlice, member.MsgCount) - resultSlice = append(resultSlice, member.MsgCountRoot) - resultSlice = append(resultSlice, member.MentionCount) - resultSlice = append(resultSlice, member.MentionCountRoot) - resultSlice = append(resultSlice, member.UrgentMentionCount) - resultSlice = append(resultSlice, model.MapToJSON(member.NotifyProps)) - resultSlice = append(resultSlice, member.LastUpdateAt) - resultSlice = append(resultSlice, member.SchemeUser) - resultSlice = append(resultSlice, member.SchemeAdmin) - resultSlice = append(resultSlice, member.SchemeGuest) - return resultSlice -} - func channelToSlice(channel *model.Channel) []interface{} { return []interface{}{ channel.Id, @@ -178,9 +160,29 @@ func channelToSlice(channel *model.Channel) []interface{} { channel.Shared, channel.TotalMsgCountRoot, channel.LastRootPostAt, + channel.BannerInfo, } } +func channelMemberToSlice(member *model.ChannelMember) []any { + resultSlice := []any{} + resultSlice = append(resultSlice, member.ChannelId) + resultSlice = append(resultSlice, member.UserId) + resultSlice = append(resultSlice, member.ExplicitRoles) + resultSlice = append(resultSlice, member.LastViewedAt) + resultSlice = append(resultSlice, member.MsgCount) + resultSlice = append(resultSlice, member.MsgCountRoot) + resultSlice = append(resultSlice, member.MentionCount) + resultSlice = append(resultSlice, member.MentionCountRoot) + resultSlice = append(resultSlice, member.UrgentMentionCount) + resultSlice = append(resultSlice, model.MapToJSON(member.NotifyProps)) + resultSlice = append(resultSlice, member.LastUpdateAt) + resultSlice = append(resultSlice, member.SchemeUser) + resultSlice = append(resultSlice, member.SchemeAdmin) + resultSlice = append(resultSlice, member.SchemeGuest) + return resultSlice +} + type channelMemberWithSchemeRolesList []channelMemberWithSchemeRoles func getChannelRoles(schemeGuest, schemeUser, schemeAdmin bool, defaultTeamGuestRole, defaultTeamUserRole, defaultTeamAdminRole, defaultChannelGuestRole, defaultChannelUserRole, defaultChannelAdminRole string, @@ -834,7 +836,8 @@ func (s SqlChannelStore) updateChannelT(transaction *sqlxTxWrapper, channel *mod GroupConstrained=:GroupConstrained, Shared=:Shared, TotalMsgCountRoot=:TotalMsgCountRoot, - LastRootPostAt=:LastRootPostAt + LastRootPostAt=:LastRootPostAt, + BannerInfo=:BannerInfo WHERE Id=:Id`, channel) if err != nil { if IsUniqueConstraintError(err, []string{"Name", "channels_name_teamid_key"}) { diff --git a/server/channels/store/storetest/channel_store.go b/server/channels/store/storetest/channel_store.go index 521f61c26c..13fca2c0a4 100644 --- a/server/channels/store/storetest/channel_store.go +++ b/server/channels/store/storetest/channel_store.go @@ -206,6 +206,24 @@ func testChannelStoreSave(t *testing.T, rctx request.CTX, ss store.Store) { _, nErr = ss.Channel().Save(rctx, &o2, -1) require.Error(t, nErr, "should have failed to save a duplicate of an archived channel") require.True(t, errors.As(nErr, &cErr)) + + o1 = model.Channel{} + o1.TeamId = teamID + o1.DisplayName = "Name" + o1.Name = NewTestID() + o1.Type = model.ChannelTypeOpen + o1.BannerInfo = &model.ChannelBannerInfo{ + Enabled: model.NewPointer(true), + Text: model.NewPointer("banner text"), + BackgroundColor: model.NewPointer("#000000"), + } + + savedChannel, nErr := ss.Channel().Save(rctx, &o1, -1) + require.NoError(t, nErr, "should have saved channel") + require.NotNil(t, savedChannel.BannerInfo) + require.True(t, *savedChannel.BannerInfo.Enabled) + require.Equal(t, "banner text", *savedChannel.BannerInfo.Text) + require.Equal(t, "#000000", *savedChannel.BannerInfo.BackgroundColor) } func testChannelStoreSaveDirectChannel(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) { @@ -369,6 +387,46 @@ func testChannelStoreUpdate(t *testing.T, rctx request.CTX, ss store.Store) { var uniqueConstraintErr *store.ErrUniqueConstraint require.ErrorAs(t, err, &uniqueConstraintErr) require.Contains(t, uniqueConstraintErr.Columns, "Name") + + channel := model.Channel{} + channel.TeamId = model.NewId() + channel.DisplayName = "Name" + channel.Name = NewTestID() + channel.Type = model.ChannelTypeOpen + + _, nErr = ss.Channel().Save(rctx, &channel, -1) + require.NoError(t, nErr) + + channel.BannerInfo = &model.ChannelBannerInfo{ + Enabled: model.NewPointer(true), + Text: model.NewPointer("banner text"), + BackgroundColor: model.NewPointer("#000000"), + } + + updatedChannel, err := ss.Channel().Update(rctx, &channel) + require.NoError(t, err, err) + require.NotNil(t, updatedChannel.BannerInfo) + require.True(t, *updatedChannel.BannerInfo.Enabled) + require.Equal(t, "banner text", *updatedChannel.BannerInfo.Text) + require.Equal(t, "#000000", *updatedChannel.BannerInfo.BackgroundColor) + + // can turn off channel banners + channel.BannerInfo.Enabled = model.NewPointer(false) + + updatedChannel, err = ss.Channel().Update(rctx, &channel) + require.NoError(t, err, err) + require.NotNil(t, updatedChannel.BannerInfo) + require.False(t, *updatedChannel.BannerInfo.Enabled) + + // can update text and color of channel banners + channel.BannerInfo.Text = model.NewPointer("updated text") + channel.BannerInfo.BackgroundColor = model.NewPointer("#FFFFFF") + + updatedChannel, err = ss.Channel().Update(rctx, &channel) + require.NoError(t, err, err) + require.NotNil(t, updatedChannel.BannerInfo) + require.Equal(t, "updated text", *updatedChannel.BannerInfo.Text) + require.Equal(t, "#FFFFFF", *updatedChannel.BannerInfo.BackgroundColor) } func testGetChannelUnread(t *testing.T, rctx request.CTX, ss store.Store) { @@ -7504,54 +7562,17 @@ func testMaterializedPublicChannels(t *testing.T, rctx request.CTX, ss store.Sto // o3 is a public channel on the team that already existed in the PublicChannels table. o3 := model.Channel{ - Id: model.NewId(), TeamId: teamID, DisplayName: "Open Channel 3", Name: model.NewId(), Type: model.ChannelTypeOpen, } - - _, execerr := s.GetMaster().NamedExec(` - INSERT INTO - PublicChannels(Id, DeleteAt, TeamId, DisplayName, Name, Header, Purpose) - VALUES - (:id, :deleteat, :teamid, :displayname, :name, :header, :purpose); - `, map[string]any{ - "id": o3.Id, - "deleteat": o3.DeleteAt, - "teamid": o3.TeamId, - "displayname": o3.DisplayName, - "name": o3.Name, - "header": o3.Header, - "purpose": o3.Purpose, - }) - require.NoError(t, execerr) + _, nErr = ss.Channel().Save(rctx, &o3, -1) + require.NoError(t, nErr) o3.DisplayName = "Open Channel 3 - Modified" - - _, execerr = s.GetMaster().NamedExec(` - INSERT INTO - Channels(Id, CreateAt, UpdateAt, DeleteAt, TeamId, Type, DisplayName, Name, Header, Purpose, LastPostAt, LastRootPostAt, TotalMsgCount, ExtraUpdateAt, CreatorId, TotalMsgCountRoot) - VALUES - (:id, :createat, :updateat, :deleteat, :teamid, :type, :displayname, :name, :header, :purpose, :lastpostat, :lastrootpostat, :totalmsgcount, :extraupdateat, :creatorid, 0); - `, map[string]any{ - "id": o3.Id, - "createat": o3.CreateAt, - "updateat": o3.UpdateAt, - "deleteat": o3.DeleteAt, - "teamid": o3.TeamId, - "type": o3.Type, - "displayname": o3.DisplayName, - "name": o3.Name, - "header": o3.Header, - "purpose": o3.Purpose, - "lastpostat": o3.LastPostAt, - "lastrootpostat": o3.LastRootPostAt, - "totalmsgcount": o3.TotalMsgCount, - "extraupdateat": o3.ExtraUpdateAt, - "creatorid": o3.CreatorId, - }) - require.NoError(t, execerr) + _, err = ss.Channel().Update(rctx, &o3) + require.NoError(t, err) t.Run("verify o3 INSERT converted to UPDATE", func(t *testing.T) { channels, channelErr := ss.Channel().SearchInTeam(teamID, "", true) @@ -7570,7 +7591,7 @@ func testMaterializedPublicChannels(t *testing.T, rctx request.CTX, ss store.Sto _, nErr = ss.Channel().Save(rctx, &o4, -1) require.NoError(t, nErr) - _, execerr = s.GetMaster().Exec(` + _, execerr := s.GetMaster().Exec(` DELETE FROM PublicChannels WHERE diff --git a/server/i18n/en.json b/server/i18n/en.json index 7e48fa7714..3d77afe6cb 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -515,6 +515,10 @@ "id": "api.channel.restore_channel.unarchived", "translation": "{{.Username}} unarchived the channel." }, + { + "id": "api.channel.update_channel.banner_info.channel_type.not_allowed", + "translation": "Channel banner can only be configured on Public and Private channels." + }, { "id": "api.channel.update_channel.deleted.app_error", "translation": "The channel has been archived or deleted." @@ -8492,6 +8496,22 @@ "id": "model.channel.is_valid.1_or_more.app_error", "translation": "Name must be 1 or more lowercase alphanumeric character." }, + { + "id": "model.channel.is_valid.banner_info.background_color.empty.app_error", + "translation": "Channel banner color cannot be empty when channel banner is enabled" + }, + { + "id": "model.channel.is_valid.banner_info.channel_type.app_error", + "translation": "Channel banner can only be configured on Public and Private channels" + }, + { + "id": "model.channel.is_valid.banner_info.text.empty.app_error", + "translation": "Channel banner info text cannot be empty when channel banner is enabled" + }, + { + "id": "model.channel.is_valid.banner_info.text.invalid_length.app_error", + "translation": "Channel banner info text is too long. Max allowed length is {{.maxLength}} characters." + }, { "id": "model.channel.is_valid.create_at.app_error", "translation": "Create at must be a valid time." diff --git a/server/public/model/channel.go b/server/public/model/channel.go index 786a4f281d..536bc1236c 100644 --- a/server/public/model/channel.go +++ b/server/public/model/channel.go @@ -5,8 +5,10 @@ package model import ( "crypto/sha1" + "database/sql/driver" "encoding/hex" "encoding/json" + "fmt" "io" "net/http" "regexp" @@ -32,33 +34,66 @@ const ( ChannelHeaderMaxRunes = 1024 ChannelPurposeMaxRunes = 250 ChannelCacheSize = 25000 + ChannelBannerInfoMaxLength = 1024 ChannelSortByUsername = "username" ChannelSortByStatus = "status" ) +type ChannelBannerInfo struct { + Enabled *bool `json:"enabled"` + Text *string `json:"text"` + BackgroundColor *string `json:"background_color"` +} + +func (c *ChannelBannerInfo) Scan(value interface{}) error { + if value == nil { + return nil + } + + b, ok := value.([]byte) + if !ok { + return fmt.Errorf("expected []byte, got %T", value) + } + + return json.Unmarshal(b, c) +} + +func (c ChannelBannerInfo) Value() (driver.Value, error) { + if c == (ChannelBannerInfo{}) { + return nil, nil + } + + j, err := json.Marshal(c) + if err != nil { + return nil, err + } + return string(j), nil +} + type Channel struct { - Id string `json:"id"` - CreateAt int64 `json:"create_at"` - UpdateAt int64 `json:"update_at"` - DeleteAt int64 `json:"delete_at"` - TeamId string `json:"team_id"` - Type ChannelType `json:"type"` - DisplayName string `json:"display_name"` - Name string `json:"name"` - Header string `json:"header"` - Purpose string `json:"purpose"` - LastPostAt int64 `json:"last_post_at"` - TotalMsgCount int64 `json:"total_msg_count"` - ExtraUpdateAt int64 `json:"extra_update_at"` - CreatorId string `json:"creator_id"` - SchemeId *string `json:"scheme_id"` - Props map[string]any `json:"props"` - GroupConstrained *bool `json:"group_constrained"` - Shared *bool `json:"shared"` - TotalMsgCountRoot int64 `json:"total_msg_count_root"` - PolicyID *string `json:"policy_id"` - LastRootPostAt int64 `json:"last_root_post_at"` + Id string `json:"id"` + CreateAt int64 `json:"create_at"` + UpdateAt int64 `json:"update_at"` + DeleteAt int64 `json:"delete_at"` + TeamId string `json:"team_id"` + Type ChannelType `json:"type"` + DisplayName string `json:"display_name"` + Name string `json:"name"` + Header string `json:"header"` + Purpose string `json:"purpose"` + LastPostAt int64 `json:"last_post_at"` + TotalMsgCount int64 `json:"total_msg_count"` + ExtraUpdateAt int64 `json:"extra_update_at"` + CreatorId string `json:"creator_id"` + SchemeId *string `json:"scheme_id"` + Props map[string]any `json:"props"` + GroupConstrained *bool `json:"group_constrained"` + Shared *bool `json:"shared"` + TotalMsgCountRoot int64 `json:"total_msg_count_root"` + PolicyID *string `json:"policy_id"` + LastRootPostAt int64 `json:"last_root_post_at"` + BannerInfo *ChannelBannerInfo `json:"banner_info"` } func (o *Channel) Auditable() map[string]interface{} { @@ -99,11 +134,12 @@ type ChannelsWithCount struct { } type ChannelPatch struct { - DisplayName *string `json:"display_name"` - Name *string `json:"name"` - Header *string `json:"header"` - Purpose *string `json:"purpose"` - GroupConstrained *bool `json:"group_constrained"` + DisplayName *string `json:"display_name"` + Name *string `json:"name"` + Header *string `json:"header"` + Purpose *string `json:"purpose"` + GroupConstrained *bool `json:"group_constrained"` + BannerInfo *ChannelBannerInfo `json:"banner_info"` } func (c *ChannelPatch) Auditable() map[string]interface{} { @@ -261,6 +297,22 @@ func (o *Channel) IsValid() *AppError { } } + if o.BannerInfo != nil && o.BannerInfo.Enabled != nil && *o.BannerInfo.Enabled { + if o.Type != ChannelTypeOpen && o.Type != ChannelTypePrivate { + return NewAppError("Channel.IsValid", "model.channel.is_valid.banner_info.channel_type.app_error", nil, "", http.StatusBadRequest) + } + + if o.BannerInfo.Text == nil || len(*o.BannerInfo.Text) == 0 { + return NewAppError("Channel.IsValid", "model.channel.is_valid.banner_info.text.empty.app_error", nil, "", http.StatusBadRequest) + } else if len(*o.BannerInfo.Text) > ChannelBannerInfoMaxLength { + return NewAppError("Channel.IsValid", "model.channel.is_valid.banner_info.text.invalid_length.app_error", map[string]any{"maxLength": ChannelBannerInfoMaxLength}, "", http.StatusBadRequest) + } + + if o.BannerInfo.BackgroundColor == nil || len(*o.BannerInfo.BackgroundColor) == 0 { + return NewAppError("Channel.IsValid", "model.channel.is_valid.banner_info.background_color.empty.app_error", nil, "", http.StatusBadRequest) + } + } + return nil } @@ -312,6 +364,25 @@ func (o *Channel) Patch(patch *ChannelPatch) { if patch.GroupConstrained != nil { o.GroupConstrained = patch.GroupConstrained } + + // patching channel banner info + if patch.BannerInfo != nil { + if o.BannerInfo == nil { + o.BannerInfo = &ChannelBannerInfo{} + } + + if patch.BannerInfo.Enabled != nil { + o.BannerInfo.Enabled = patch.BannerInfo.Enabled + } + + if patch.BannerInfo.Text != nil { + o.BannerInfo.Text = patch.BannerInfo.Text + } + + if patch.BannerInfo.BackgroundColor != nil { + o.BannerInfo.BackgroundColor = patch.BannerInfo.BackgroundColor + } + } } func (o *Channel) MakeNonNil() {