[MM-15117] Migrate "WebHook.UpdateIncoming" to Sync by default (#10637)

* SyncStore: Migrate Webhooks.UpdateIncoming method to Sync

* Changes requested by @jespino

app/webhook.go

@@ -353,12 +353,12 @@ func (a *App) UpdateIncomingWebhook(oldHook, updatedHook *model.IncomingWebhook)
 	updatedHook.TeamId = oldHook.TeamId
 	updatedHook.DeleteAt = oldHook.DeleteAt
 
-	if result := <-a.Srv.Store.Webhook().UpdateIncoming(updatedHook); result.Err != nil {
-		return nil, result.Err
-	} else {
-		a.InvalidateCacheForWebhook(oldHook.Id)
-		return result.Data.(*model.IncomingWebhook), nil
+	newWebhook, err := a.Srv.Store.Webhook().UpdateIncoming(updatedHook)
+	if err != nil {
+		return nil, err
 	}
+	a.InvalidateCacheForWebhook(oldHook.Id)
+	return newWebhook, nil
 }
 
 func (a *App) DeleteIncomingWebhook(hookId string) *model.AppError {

cmd/mattermost/commands/channel.go

@@ -408,7 +408,7 @@ func moveChannel(a *app.App, team *model.Team, channel *model.Channel, user *mod
 		for _, webhook := range incomingWebhooks {
 			if webhook.ChannelId == channel.Id {
 				webhook.TeamId = team.Id
-				if result := <-a.Srv.Store.Webhook().UpdateIncoming(webhook); result.Err != nil {
+				if _, err := a.Srv.Store.Webhook().UpdateIncoming(webhook); err != nil {
 					CommandPrintErrorln("Failed to move incoming webhook '" + webhook.Id + "' to new team.")
 				}
 			}

store/sqlstore/webhook_store.go

@@ -109,16 +109,13 @@ func (s SqlWebhookStore) SaveIncoming(webhook *model.IncomingWebhook) store.Stor
 	})
 }
 
-func (s SqlWebhookStore) UpdateIncoming(hook *model.IncomingWebhook) store.StoreChannel {
-	return store.Do(func(result *store.StoreResult) {
-		hook.UpdateAt = model.GetMillis()
+func (s SqlWebhookStore) UpdateIncoming(hook *model.IncomingWebhook) (*model.IncomingWebhook, *model.AppError) {
+	hook.UpdateAt = model.GetMillis()
 
-		if _, err := s.GetMaster().Update(hook); err != nil {
-			result.Err = model.NewAppError("SqlWebhookStore.UpdateIncoming", "store.sql_webhooks.update_incoming.app_error", nil, "id="+hook.Id+", "+err.Error(), http.StatusInternalServerError)
-		} else {
-			result.Data = hook
-		}
-	})
+	if _, err := s.GetMaster().Update(hook); err != nil {
+		return nil, model.NewAppError("SqlWebhookStore.UpdateIncoming", "store.sql_webhooks.update_incoming.app_error", nil, "id="+hook.Id+", "+err.Error(), http.StatusInternalServerError)
+	}
+	return hook, nil
 }
 
 func (s SqlWebhookStore) GetIncoming(id string, allowFromCache bool) (*model.IncomingWebhook, *model.AppError) {

store/store.go

@@ -381,7 +381,7 @@ type WebhookStore interface {
 	GetIncoming(id string, allowFromCache bool) (*model.IncomingWebhook, *model.AppError)
 	GetIncomingList(offset, limit int) StoreChannel
 	GetIncomingByTeam(teamId string, offset, limit int) StoreChannel
-	UpdateIncoming(webhook *model.IncomingWebhook) StoreChannel
+	UpdateIncoming(webhook *model.IncomingWebhook) (*model.IncomingWebhook, *model.AppError)
 	GetIncomingByChannel(channelId string) StoreChannel
 	DeleteIncoming(webhookId string, time int64) StoreChannel
 	PermanentDeleteIncomingByChannel(channelId string) StoreChannel

store/storetest/mocks/WebhookStore.go

@@ -321,19 +321,28 @@ func (_m *WebhookStore) SaveOutgoing(webhook *model.OutgoingWebhook) store.Store
 }
 
 // UpdateIncoming provides a mock function with given fields: webhook
-func (_m *WebhookStore) UpdateIncoming(webhook *model.IncomingWebhook) store.StoreChannel {
+func (_m *WebhookStore) UpdateIncoming(webhook *model.IncomingWebhook) (*model.IncomingWebhook, *model.AppError) {
 	ret := _m.Called(webhook)
 
-	var r0 store.StoreChannel
-	if rf, ok := ret.Get(0).(func(*model.IncomingWebhook) store.StoreChannel); ok {
+	var r0 *model.IncomingWebhook
+	if rf, ok := ret.Get(0).(func(*model.IncomingWebhook) *model.IncomingWebhook); ok {
 		r0 = rf(webhook)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).(*model.IncomingWebhook)
 		}
 	}
 
-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func(*model.IncomingWebhook) *model.AppError); ok {
+		r1 = rf(webhook)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // UpdateOutgoing provides a mock function with given fields: hook

store/storetest/webhook_store.go

@@ -56,17 +56,17 @@ func testWebhookStoreUpdateIncoming(t *testing.T, ss store.Store) {
 	o1.DisplayName = "TestHook"
 	time.Sleep(10 * time.Millisecond)
 
-	if result := (<-ss.Webhook().UpdateIncoming(o1)); result.Err != nil {
-		t.Fatal("updation of incoming hook failed", result.Err)
-	} else {
-		if result.Data.(*model.IncomingWebhook).UpdateAt == previousUpdatedAt {
-			t.Fatal("should have updated the UpdatedAt of the hook")
-		}
+	webhook, err := ss.Webhook().UpdateIncoming(o1)
+	require.Nil(t, err)
 
-		if result.Data.(*model.IncomingWebhook).DisplayName != "TestHook" {
-			t.Fatal("display name is not updated")
-		}
+	if webhook.UpdateAt == previousUpdatedAt {
+		t.Fatal("should have updated the UpdatedAt of the hook")
 	}
+
+	if webhook.DisplayName != "TestHook" {
+		t.Fatal("display name is not updated")
+	}
+
 }
 
 func testWebhookStoreGetIncoming(t *testing.T, ss store.Store) {