Migrate Update method from ChannelStore to return idiomatic plain error (#14687)
* Partial advances * Update migrated * Fix imports * Suggestions * Suggestions * Updating i18n Co-authored-by: Jesús Espino <jespinog@gmail.com> Co-authored-by: mattermod <mattermod@users.noreply.github.com>
Этот коммит содержится в:
коммит произвёл
GitHub
родитель
6a5dd550c8
Коммит
60cc775cf6
@@ -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)
|
||||
|
||||
|
||||
40
i18n/en.json
40
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."
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
Ссылка в новой задаче
Block a user