diff --git a/app/webhook.go b/app/webhook.go index b156317284..7aa6cd2a33 100644 --- a/app/webhook.go +++ b/app/webhook.go @@ -673,11 +673,7 @@ func (a *App) CreateCommandWebhook(commandId string, args *model.CommandArgs) (* ParentId: args.ParentId, } - if result := <-a.Srv.Store.CommandWebhook().Save(hook); result.Err != nil { - return nil, result.Err - } else { - return result.Data.(*model.CommandWebhook), nil - } + return a.Srv.Store.CommandWebhook().Save(hook) } func (a *App) HandleCommandWebhook(hookId string, response *model.CommandResponse) *model.AppError { diff --git a/store/sqlstore/command_webhook_store.go b/store/sqlstore/command_webhook_store.go index 1ea4f73289..123e527481 100644 --- a/store/sqlstore/command_webhook_store.go +++ b/store/sqlstore/command_webhook_store.go @@ -36,24 +36,21 @@ func (s SqlCommandWebhookStore) CreateIndexesIfNotExists() { s.CreateIndexIfNotExists("idx_command_webhook_create_at", "CommandWebhooks", "CreateAt") } -func (s SqlCommandWebhookStore) Save(webhook *model.CommandWebhook) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - if len(webhook.Id) > 0 { - result.Err = model.NewAppError("SqlCommandWebhookStore.Save", "store.sql_command_webhooks.save.existing.app_error", nil, "id="+webhook.Id, http.StatusBadRequest) - return - } +func (s SqlCommandWebhookStore) Save(webhook *model.CommandWebhook) (*model.CommandWebhook, *model.AppError) { + if len(webhook.Id) > 0 { + return nil, model.NewAppError("SqlCommandWebhookStore.Save", "store.sql_command_webhooks.save.existing.app_error", nil, "id="+webhook.Id, http.StatusBadRequest) + } - webhook.PreSave() - if result.Err = webhook.IsValid(); result.Err != nil { - return - } + webhook.PreSave() + if err := webhook.IsValid(); err != nil { + return nil, err + } - if err := s.GetMaster().Insert(webhook); err != nil { - result.Err = model.NewAppError("SqlCommandWebhookStore.Save", "store.sql_command_webhooks.save.app_error", nil, "id="+webhook.Id+", "+err.Error(), http.StatusInternalServerError) - } else { - result.Data = webhook - } - }) + if err := s.GetMaster().Insert(webhook); err != nil { + return nil, model.NewAppError("SqlCommandWebhookStore.Save", "store.sql_command_webhooks.save.app_error", nil, "id="+webhook.Id+", "+err.Error(), http.StatusInternalServerError) + } + + return webhook, nil } func (s SqlCommandWebhookStore) Get(id string) store.StoreChannel { diff --git a/store/store.go b/store/store.go index 68d1768f56..b2eb1f63e7 100644 --- a/store/store.go +++ b/store/store.go @@ -428,7 +428,7 @@ type CommandStore interface { } type CommandWebhookStore interface { - Save(webhook *model.CommandWebhook) StoreChannel + Save(webhook *model.CommandWebhook) (*model.CommandWebhook, *model.AppError) Get(id string) StoreChannel TryUse(id string, limit int) StoreChannel Cleanup() diff --git a/store/storetest/command_webhook_store.go b/store/storetest/command_webhook_store.go index a1f97b979a..661d701843 100644 --- a/store/storetest/command_webhook_store.go +++ b/store/storetest/command_webhook_store.go @@ -6,6 +6,8 @@ package storetest import ( "testing" + "github.com/stretchr/testify/require" + "net/http" "github.com/mattermost/mattermost-server/model" @@ -23,7 +25,8 @@ func testCommandWebhookStore(t *testing.T, ss store.Store) { h1.CommandId = model.NewId() h1.UserId = model.NewId() h1.ChannelId = model.NewId() - h1 = (<-cws.Save(h1)).Data.(*model.CommandWebhook) + h1, err := cws.Save(h1) + require.Nil(t, err) if r1 := <-cws.Get(h1.Id); r1.Err != nil { t.Fatal(r1.Err) @@ -33,7 +36,7 @@ func testCommandWebhookStore(t *testing.T, ss store.Store) { } } - if err := (<-cws.Get("123")).Err; err.StatusCode != http.StatusNotFound { + if err = (<-cws.Get("123")).Err; err.StatusCode != http.StatusNotFound { t.Fatal("Should have set the status as not found for missing id") } @@ -42,7 +45,8 @@ func testCommandWebhookStore(t *testing.T, ss store.Store) { h2.CommandId = model.NewId() h2.UserId = model.NewId() h2.ChannelId = model.NewId() - h2 = (<-cws.Save(h2)).Data.(*model.CommandWebhook) + h2, err = cws.Save(h2) + require.Nil(t, err) if err := (<-cws.Get(h2.Id)).Err; err == nil || err.StatusCode != http.StatusNotFound { t.Fatal("Should have set the status as not found for expired webhook") diff --git a/store/storetest/mocks/CommandWebhookStore.go b/store/storetest/mocks/CommandWebhookStore.go index 5129388aeb..d57e36f9b0 100644 --- a/store/storetest/mocks/CommandWebhookStore.go +++ b/store/storetest/mocks/CommandWebhookStore.go @@ -35,19 +35,28 @@ func (_m *CommandWebhookStore) Get(id string) store.StoreChannel { } // Save provides a mock function with given fields: webhook -func (_m *CommandWebhookStore) Save(webhook *model.CommandWebhook) store.StoreChannel { +func (_m *CommandWebhookStore) Save(webhook *model.CommandWebhook) (*model.CommandWebhook, *model.AppError) { ret := _m.Called(webhook) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(*model.CommandWebhook) store.StoreChannel); ok { + var r0 *model.CommandWebhook + if rf, ok := ret.Get(0).(func(*model.CommandWebhook) *model.CommandWebhook); ok { r0 = rf(webhook) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.CommandWebhook) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(*model.CommandWebhook) *model.AppError); ok { + r1 = rf(webhook) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // TryUse provides a mock function with given fields: id, limit