diff --git a/app/channel.go b/app/channel.go index 8a4cf3e7f6..11d8faf8c4 100644 --- a/app/channel.go +++ b/app/channel.go @@ -527,7 +527,16 @@ func (a *App) GetGroupChannel(userIds []string) (*model.Channel, *model.AppError func (a *App) UpdateChannel(channel *model.Channel) (*model.Channel, *model.AppError) { _, err := a.Srv().Store.Channel().Update(channel) if err != nil { - return nil, err + var appErr *model.AppError + var iErr *store.ErrInvalidInput + switch { + case errors.As(err, &iErr): + return nil, model.NewAppError("UpdateChannel", "app.channel.update.bad_id", nil, iErr.Error(), http.StatusBadRequest) + case errors.As(err, &appErr): + return nil, appErr + default: + return nil, model.NewAppError("UpdateChannel", "app.channel.update_channel.internal_error", nil, err.Error(), http.StatusInternalServerError) + } } a.invalidateCacheForChannel(channel) @@ -2326,7 +2335,16 @@ func (a *App) MoveChannel(team *model.Team, channel *model.Channel, user *model. channel.TeamId = team.Id if _, err := a.Srv().Store.Channel().Update(channel); err != nil { - return err + var appErr *model.AppError + var iErr *store.ErrInvalidInput + switch { + case errors.As(err, &iErr): + return model.NewAppError("MoveChannel", "app.channel.update.bad_id", nil, iErr.Error(), http.StatusBadRequest) + case errors.As(err, &appErr): + return appErr + default: + return model.NewAppError("MoveChannel", "app.channel.update_channel.internal_error", nil, err.Error(), http.StatusInternalServerError) + } } a.postChannelMoveMessage(user, channel, previousTeam) diff --git a/i18n/en.json b/i18n/en.json index a36e9c1c4c..8a341d2db4 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -2998,6 +2998,14 @@ "id": "app.channel.post_update_channel_purpose_message.updated_to", "translation": "%s updated the channel purpose to: %s" }, + { + "id": "app.channel.update.bad_id", + "translation": "Unable to update the channel." + }, + { + "id": "app.channel.update_channel.internal_error", + "translation": "Unable to update channel." + }, { "id": "app.cluster.404.app_error", "translation": "Cluster API endpoint not found." @@ -6266,38 +6274,6 @@ "id": "store.sql_channel.set_delete_at.update_public_channel.app_error", "translation": "Unable to update the materialized public channel." }, - { - "id": "store.sql_channel.update.app_error", - "translation": "Unable to update the channel." - }, - { - "id": "store.sql_channel.update.archived_channel.app_error", - "translation": "You can not modify an archived channel." - }, - { - "id": "store.sql_channel.update.commit_transaction.app_error", - "translation": "Unable to commit transaction." - }, - { - "id": "store.sql_channel.update.exists.app_error", - "translation": "A channel with that handle already exists." - }, - { - "id": "store.sql_channel.update.open_transaction.app_error", - "translation": "Unable to open transaction." - }, - { - "id": "store.sql_channel.update.previously.app_error", - "translation": "A channel with that handle was previously created." - }, - { - "id": "store.sql_channel.update.updating.app_error", - "translation": "We encountered an error updating the channel." - }, - { - "id": "store.sql_channel.update.upsert_public_channel.app_error", - "translation": "Unable to upsert materialized public channel." - }, { "id": "store.sql_channel.update_last_viewed_at.app_error", "translation": "Unable to update the last viewed at time." diff --git a/store/opentracing_layer.go b/store/opentracing_layer.go index 9523ad7984..1bde5af9a8 100644 --- a/store/opentracing_layer.go +++ b/store/opentracing_layer.go @@ -1863,7 +1863,7 @@ func (s *OpenTracingLayerChannelStore) SetDeleteAt(channelId string, deleteAt in return resultVar0 } -func (s *OpenTracingLayerChannelStore) Update(channel *model.Channel) (*model.Channel, *model.AppError) { +func (s *OpenTracingLayerChannelStore) Update(channel *model.Channel) (*model.Channel, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ChannelStore.Update") s.Root.Store.SetContext(newCtx) diff --git a/store/searchlayer/channel_layer.go b/store/searchlayer/channel_layer.go index bf1c2b924f..1a1484ffe1 100644 --- a/store/searchlayer/channel_layer.go +++ b/store/searchlayer/channel_layer.go @@ -53,7 +53,7 @@ func (c *SearchChannelStore) Save(channel *model.Channel, maxChannels int64) (*m return newChannel, err } -func (c *SearchChannelStore) Update(channel *model.Channel) (*model.Channel, *model.AppError) { +func (c *SearchChannelStore) Update(channel *model.Channel) (*model.Channel, error) { updatedChannel, err := c.ChannelStore.Update(channel) if err == nil { c.indexChannel(updatedChannel) diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index a1c7bc8eca..97c0a8ea9a 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -13,14 +13,14 @@ import ( "time" "github.com/mattermost/gorp" - "github.com/pkg/errors" - - sq "github.com/Masterminds/squirrel" "github.com/mattermost/mattermost-server/v5/einterfaces" "github.com/mattermost/mattermost-server/v5/mlog" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/services/cache2" "github.com/mattermost/mattermost-server/v5/store" + + sq "github.com/Masterminds/squirrel" + "github.com/pkg/errors" ) const ( @@ -638,10 +638,10 @@ func (s SqlChannelStore) saveChannelT(transaction *gorp.Transaction, channel *mo } // Update writes the updated channel to the database. -func (s SqlChannelStore) Update(channel *model.Channel) (*model.Channel, *model.AppError) { +func (s SqlChannelStore) Update(channel *model.Channel) (*model.Channel, error) { transaction, err := s.GetMaster().Begin() if err != nil { - return nil, model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "begin_transaction") } defer finalizeTransaction(transaction) @@ -652,20 +652,20 @@ func (s SqlChannelStore) Update(channel *model.Channel) (*model.Channel, *model. // Additionally propagate the write to the PublicChannels table. if err := s.upsertPublicChannelT(transaction, updatedChannel); err != nil { - return nil, model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.upsert_public_channel.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "upsertPublicChannelT: failed to upsert channel") } if err := transaction.Commit(); err != nil { - return nil, model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "commit_transaction") } return updatedChannel, nil } -func (s SqlChannelStore) updateChannelT(transaction *gorp.Transaction, channel *model.Channel) (*model.Channel, *model.AppError) { +func (s SqlChannelStore) updateChannelT(transaction *gorp.Transaction, channel *model.Channel) (*model.Channel, error) { channel.PreUpdate() if channel.DeleteAt != 0 { - return nil, model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.archived_channel.app_error", nil, "", http.StatusBadRequest) + return nil, store.NewErrInvalidInput("Channel", "DeleteAt", channel.DeleteAt) } if err := channel.IsValid(); err != nil { @@ -678,15 +678,15 @@ func (s SqlChannelStore) updateChannelT(transaction *gorp.Transaction, channel * dupChannel := model.Channel{} s.GetReplica().SelectOne(&dupChannel, "SELECT * FROM Channels WHERE TeamId = :TeamId AND Name= :Name AND DeleteAt > 0", map[string]interface{}{"TeamId": channel.TeamId, "Name": channel.Name}) if dupChannel.DeleteAt > 0 { - return nil, model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.previously.app_error", nil, "id="+channel.Id+", "+err.Error(), http.StatusBadRequest) + return nil, store.NewErrInvalidInput("Channel", "Id", channel.Id) } - return nil, model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.exists.app_error", nil, "id="+channel.Id+", "+err.Error(), http.StatusBadRequest) + return nil, store.NewErrInvalidInput("Channel", "Id", channel.Id) } - return nil, model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.updating.app_error", nil, "id="+channel.Id+", "+err.Error(), http.StatusInternalServerError) + return nil, errors.Wrapf(err, "failed to update channel with id=%s", channel.Id) } if count != 1 { - return nil, model.NewAppError("SqlChannelStore.Update", "store.sql_channel.update.app_error", nil, "id="+channel.Id, http.StatusInternalServerError) + return nil, fmt.Errorf("the expected number of channels to be updated is 1 but was %d", count) } return channel, nil diff --git a/store/store.go b/store/store.go index 5fd3ece1b6..1c08c145a1 100644 --- a/store/store.go +++ b/store/store.go @@ -130,7 +130,7 @@ type ChannelStore interface { Save(channel *model.Channel, maxChannelsPerTeam int64) (*model.Channel, error) CreateDirectChannel(userId *model.User, otherUserId *model.User) (*model.Channel, error) SaveDirectChannel(channel *model.Channel, member1 *model.ChannelMember, member2 *model.ChannelMember) (*model.Channel, error) - Update(channel *model.Channel) (*model.Channel, *model.AppError) + Update(channel *model.Channel) (*model.Channel, error) Get(id string, allowFromCache bool) (*model.Channel, *model.AppError) InvalidateChannel(id string) InvalidateChannelByName(teamId, name string) diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index 53816452d7..f6c124d97d 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -6499,16 +6499,16 @@ func testGroupSyncedChannelCount(t *testing.T, ss store.Store) { require.False(t, channel2.IsGroupConstrained()) defer ss.Channel().PermanentDelete(channel2.Id) - count, err := ss.Channel().GroupSyncedChannelCount() - require.Nil(t, err) + count, appErr := ss.Channel().GroupSyncedChannelCount() + require.Nil(t, appErr) require.GreaterOrEqual(t, count, int64(1)) channel2.GroupConstrained = model.NewBool(true) - channel2, err = ss.Channel().Update(channel2) + channel2, err := ss.Channel().Update(channel2) require.Nil(t, err) require.True(t, channel2.IsGroupConstrained()) - countAfter, err := ss.Channel().GroupSyncedChannelCount() - require.Nil(t, err) + countAfter, appErr := ss.Channel().GroupSyncedChannelCount() + require.Nil(t, appErr) require.GreaterOrEqual(t, countAfter, count+1) } diff --git a/store/storetest/group_store.go b/store/storetest/group_store.go index dadfb8c596..0d88873270 100644 --- a/store/storetest/group_store.go +++ b/store/storetest/group_store.go @@ -1672,8 +1672,8 @@ func testChannelMembersToAdd(t *testing.T, ss store.Store) { // reset state of channel and verify channel.DeleteAt = 0 - _, err = ss.Channel().Update(channel) - require.Nil(t, err) + _, nErr = ss.Channel().Update(channel) + require.Nil(t, nErr) channelMembers, err = ss.Group().ChannelMembersToAdd(0, nil) require.Nil(t, err) require.Len(t, channelMembers, 1) diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index 40675cc5c4..9551ecbf32 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -1691,7 +1691,7 @@ func (_m *ChannelStore) SetDeleteAt(channelId string, deleteAt int64, updateAt i } // Update provides a mock function with given fields: channel -func (_m *ChannelStore) Update(channel *model.Channel) (*model.Channel, *model.AppError) { +func (_m *ChannelStore) Update(channel *model.Channel) (*model.Channel, error) { ret := _m.Called(channel) var r0 *model.Channel @@ -1703,13 +1703,11 @@ func (_m *ChannelStore) Update(channel *model.Channel) (*model.Channel, *model.A } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(*model.Channel) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(*model.Channel) error); ok { r1 = rf(channel) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index 3339f869dd..6756f7583a 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -3851,8 +3851,8 @@ func testUserStoreGetChannelGroupUsers(t *testing.T, ss store.Store) { // update team to be group-constrained channel.GroupConstrained = model.NewBool(true) - _, err = ss.Channel().Update(channel) - require.Nil(t, err) + _, nErr = ss.Channel().Update(channel) + require.Nil(t, nErr) // still returns user (being group-constrained has no effect) requireNUsers(1) diff --git a/store/timer_layer.go b/store/timer_layer.go index 46aed5b8d1..dfd6349c35 100644 --- a/store/timer_layer.go +++ b/store/timer_layer.go @@ -1729,7 +1729,7 @@ func (s *TimerLayerChannelStore) SetDeleteAt(channelId string, deleteAt int64, u return resultVar0 } -func (s *TimerLayerChannelStore) Update(channel *model.Channel) (*model.Channel, *model.AppError) { +func (s *TimerLayerChannelStore) Update(channel *model.Channel) (*model.Channel, error) { start := timemodule.Now() resultVar0, resultVar1 := s.ChannelStore.Update(channel)