Migration of UserTermsOfServiceStore to return plain errors (#14788)

* Migration of UserTermsOfService Store

* Ordering translations file

* Fix imports

* Fix translations]

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
Этот коммит содержится в:
Rodrigo Villablanca
2020-06-19 08:19:30 -04:00
коммит произвёл GitHub
родитель d21f1183ab
Коммит 2760497660
10 изменённых файлов: 80 добавлений и 59 удалений

Просмотреть файл

@@ -4543,7 +4543,7 @@ func TestGetUserTermsOfService(t *testing.T) {
defer th.TearDown() defer th.TearDown()
_, resp := th.Client.GetUserTermsOfService(th.BasicUser.Id, "") _, resp := th.Client.GetUserTermsOfService(th.BasicUser.Id, "")
CheckErrorMessage(t, resp, "store.sql_user_terms_of_service.get_by_user.no_rows.app_error") CheckErrorMessage(t, resp, "app.user_terms_of_service.get_by_user.no_rows.app_error")
termsOfService, err := th.App.CreateTermsOfService("terms of service", th.BasicUser.Id) termsOfService, err := th.App.CreateTermsOfService("terms of service", th.BasicUser.Id)
require.Nil(t, err) require.Nil(t, err)

Просмотреть файл

@@ -3,10 +3,27 @@
package app package app
import "github.com/mattermost/mattermost-server/v5/model" import (
"errors"
"net/http"
"github.com/mattermost/mattermost-server/v5/model"
"github.com/mattermost/mattermost-server/v5/store"
)
func (a *App) GetUserTermsOfService(userId string) (*model.UserTermsOfService, *model.AppError) { func (a *App) GetUserTermsOfService(userId string) (*model.UserTermsOfService, *model.AppError) {
return a.Srv().Store.UserTermsOfService().GetByUser(userId) u, err := a.Srv().Store.UserTermsOfService().GetByUser(userId)
if err != nil {
var nfErr *store.ErrNotFound
switch {
case errors.As(err, &nfErr):
return nil, model.NewAppError("GetUserTermsOfService", "app.user_terms_of_service.get_by_user.no_rows.app_error", nil, nfErr.Error(), http.StatusNotFound)
default:
return nil, model.NewAppError("GetUserTermsOfService", "app.user_terms_of_service.get_by_user.app_error", nil, err.Error(), http.StatusInternalServerError)
}
}
return u, nil
} }
func (a *App) SaveUserTermsOfService(userId, termsOfServiceId string, accepted bool) *model.AppError { func (a *App) SaveUserTermsOfService(userId, termsOfServiceId string, accepted bool) *model.AppError {
@@ -17,11 +34,17 @@ func (a *App) SaveUserTermsOfService(userId, termsOfServiceId string, accepted b
} }
if _, err := a.Srv().Store.UserTermsOfService().Save(userTermsOfService); err != nil { if _, err := a.Srv().Store.UserTermsOfService().Save(userTermsOfService); err != nil {
return err var appErr *model.AppError
switch {
case errors.As(err, &appErr):
return appErr
default:
return model.NewAppError("SaveUserTermsOfService", "app.user_terms_of_service.save.app_error", nil, err.Error(), http.StatusInternalServerError)
}
} }
} else { } else {
if err := a.Srv().Store.UserTermsOfService().Delete(userId, termsOfServiceId); err != nil { if err := a.Srv().Store.UserTermsOfService().Delete(userId, termsOfServiceId); err != nil {
return err return model.NewAppError("SaveUserTermsOfService", "app.user_terms_of_service.delete.app_error", nil, err.Error(), http.StatusInternalServerError)
} }
} }

Просмотреть файл

@@ -16,7 +16,7 @@ func TestUserTermsOfService(t *testing.T) {
userTermsOfService, err := th.App.GetUserTermsOfService(th.BasicUser.Id) userTermsOfService, err := th.App.GetUserTermsOfService(th.BasicUser.Id)
checkError(t, err) checkError(t, err)
assert.Nil(t, userTermsOfService) assert.Nil(t, userTermsOfService)
assert.Equal(t, "store.sql_user_terms_of_service.get_by_user.no_rows.app_error", err.Id) assert.Equal(t, "app.user_terms_of_service.get_by_user.no_rows.app_error", err.Id)
termsOfService, err := th.App.CreateTermsOfService("terms of service", th.BasicUser.Id) termsOfService, err := th.App.CreateTermsOfService("terms of service", th.BasicUser.Id)
checkNoError(t, err) checkNoError(t, err)

Просмотреть файл

@@ -3990,6 +3990,22 @@
"id": "app.user_access_token.invalid_or_missing", "id": "app.user_access_token.invalid_or_missing",
"translation": "Invalid or missing token." "translation": "Invalid or missing token."
}, },
{
"id": "app.user_terms_of_service.delete.app_error",
"translation": "Unable to delete terms of service."
},
{
"id": "app.user_terms_of_service.get_by_user.app_error",
"translation": "Unable to fetch terms of service."
},
{
"id": "app.user_terms_of_service.get_by_user.no_rows.app_error",
"translation": "No terms of service found."
},
{
"id": "app.user_terms_of_service.save.app_error",
"translation": "Unable to save terms of service."
},
{ {
"id": "bleveengine.already_started.error", "id": "bleveengine.already_started.error",
"translation": "Bleve is already started." "translation": "Bleve is already started."
@@ -7626,22 +7642,6 @@
"id": "store.sql_user_access_token.update_token_enable.app_error", "id": "store.sql_user_access_token.update_token_enable.app_error",
"translation": "Unable to enable the access token." "translation": "Unable to enable the access token."
}, },
{
"id": "store.sql_user_terms_of_service.delete.app_error",
"translation": "Unable to delete terms of service."
},
{
"id": "store.sql_user_terms_of_service.get_by_user.app_error",
"translation": "Unable to fetch terms of service."
},
{
"id": "store.sql_user_terms_of_service.get_by_user.no_rows.app_error",
"translation": "No terms of service found."
},
{
"id": "store.sql_user_terms_of_service.save.app_error",
"translation": "Unable to save terms of service."
},
{ {
"id": "store.sql_webhooks.analytics_incoming_count.app_error", "id": "store.sql_webhooks.analytics_incoming_count.app_error",
"translation": "Unable to count the incoming webhooks." "translation": "Unable to count the incoming webhooks."

Просмотреть файл

@@ -8639,7 +8639,7 @@ func (s *OpenTracingLayerUserAccessTokenStore) UpdateTokenEnable(tokenId string)
return resultVar0 return resultVar0
} }
func (s *OpenTracingLayerUserTermsOfServiceStore) Delete(userId string, termsOfServiceId string) *model.AppError { func (s *OpenTracingLayerUserTermsOfServiceStore) Delete(userId string, termsOfServiceId string) error {
origCtx := s.Root.Store.Context() origCtx := s.Root.Store.Context()
span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserTermsOfServiceStore.Delete") span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserTermsOfServiceStore.Delete")
s.Root.Store.SetContext(newCtx) s.Root.Store.SetContext(newCtx)
@@ -8657,7 +8657,7 @@ func (s *OpenTracingLayerUserTermsOfServiceStore) Delete(userId string, termsOfS
return resultVar0 return resultVar0
} }
func (s *OpenTracingLayerUserTermsOfServiceStore) GetByUser(userId string) (*model.UserTermsOfService, *model.AppError) { func (s *OpenTracingLayerUserTermsOfServiceStore) GetByUser(userId string) (*model.UserTermsOfService, error) {
origCtx := s.Root.Store.Context() origCtx := s.Root.Store.Context()
span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserTermsOfServiceStore.GetByUser") span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserTermsOfServiceStore.GetByUser")
s.Root.Store.SetContext(newCtx) s.Root.Store.SetContext(newCtx)
@@ -8675,7 +8675,7 @@ func (s *OpenTracingLayerUserTermsOfServiceStore) GetByUser(userId string) (*mod
return resultVar0, resultVar1 return resultVar0, resultVar1
} }
func (s *OpenTracingLayerUserTermsOfServiceStore) Save(userTermsOfService *model.UserTermsOfService) (*model.UserTermsOfService, *model.AppError) { func (s *OpenTracingLayerUserTermsOfServiceStore) Save(userTermsOfService *model.UserTermsOfService) (*model.UserTermsOfService, error) {
origCtx := s.Root.Store.Context() origCtx := s.Root.Store.Context()
span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserTermsOfServiceStore.Save") span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserTermsOfServiceStore.Save")
s.Root.Store.SetContext(newCtx) s.Root.Store.SetContext(newCtx)

Просмотреть файл

@@ -5,10 +5,11 @@ package sqlstore
import ( import (
"database/sql" "database/sql"
"net/http"
"github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/model"
"github.com/mattermost/mattermost-server/v5/store" "github.com/mattermost/mattermost-server/v5/store"
"github.com/pkg/errors"
) )
type SqlUserTermsOfServiceStore struct { type SqlUserTermsOfServiceStore struct {
@@ -31,20 +32,20 @@ func (s SqlUserTermsOfServiceStore) createIndexesIfNotExists() {
s.CreateIndexIfNotExists("idx_user_terms_of_service_user_id", "UserTermsOfService", "UserId") s.CreateIndexIfNotExists("idx_user_terms_of_service_user_id", "UserTermsOfService", "UserId")
} }
func (s SqlUserTermsOfServiceStore) GetByUser(userId string) (*model.UserTermsOfService, *model.AppError) { func (s SqlUserTermsOfServiceStore) GetByUser(userId string) (*model.UserTermsOfService, error) {
var userTermsOfService *model.UserTermsOfService var userTermsOfService *model.UserTermsOfService
err := s.GetReplica().SelectOne(&userTermsOfService, "SELECT * FROM UserTermsOfService WHERE UserId = :userId", map[string]interface{}{"userId": userId}) err := s.GetReplica().SelectOne(&userTermsOfService, "SELECT * FROM UserTermsOfService WHERE UserId = :userId", map[string]interface{}{"userId": userId})
if err != nil { if err != nil {
if err == sql.ErrNoRows { if err == sql.ErrNoRows {
return nil, model.NewAppError("NewSqlUserTermsOfServiceStore.GetByUser", "store.sql_user_terms_of_service.get_by_user.no_rows.app_error", nil, "", http.StatusNotFound) return nil, store.NewErrNotFound("UserTermsOfService", "userId="+userId)
} }
return nil, model.NewAppError("NewSqlUserTermsOfServiceStore.GetByUser", "store.sql_user_terms_of_service.get_by_user.app_error", nil, "", http.StatusInternalServerError) return nil, errors.Wrapf(err, "failed to get UserTermsOfService with userId=%s", userId)
} }
return userTermsOfService, nil return userTermsOfService, nil
} }
func (s SqlUserTermsOfServiceStore) Save(userTermsOfService *model.UserTermsOfService) (*model.UserTermsOfService, *model.AppError) { func (s SqlUserTermsOfServiceStore) Save(userTermsOfService *model.UserTermsOfService) (*model.UserTermsOfService, error) {
userTermsOfService.PreSave() userTermsOfService.PreSave()
if err := userTermsOfService.IsValid(); err != nil { if err := userTermsOfService.IsValid(); err != nil {
@@ -53,21 +54,21 @@ func (s SqlUserTermsOfServiceStore) Save(userTermsOfService *model.UserTermsOfSe
c, err := s.GetMaster().Update(userTermsOfService) c, err := s.GetMaster().Update(userTermsOfService)
if err != nil { if err != nil {
return nil, model.NewAppError("SqlUserTermsOfServiceStore.Save", "store.sql_user_terms_of_service.save.app_error", nil, "user_terms_of_service_user_id="+userTermsOfService.UserId+",user_terms_of_service_terms_of_service_id="+userTermsOfService.TermsOfServiceId+",err="+err.Error(), http.StatusInternalServerError) return nil, errors.Wrapf(err, "failed to update UserTermsOfService with userId=%s and termsOfServiceId=%s", userTermsOfService.UserId, userTermsOfService.TermsOfServiceId)
} }
if c == 0 { if c == 0 {
if err := s.GetMaster().Insert(userTermsOfService); err != nil { if err := s.GetMaster().Insert(userTermsOfService); err != nil {
return nil, model.NewAppError("SqlUserTermsOfServiceStore.Save", "store.sql_user_terms_of_service.save.app_error", nil, "user_terms_of_service_user_id="+userTermsOfService.UserId+",user_terms_of_service_terms_of_service_id="+userTermsOfService.TermsOfServiceId+",err="+err.Error(), http.StatusInternalServerError) return nil, errors.Wrapf(err, "failed to save UserTermsOfService with userId=%s and termsOfServiceId=%s", userTermsOfService.UserId, userTermsOfService.TermsOfServiceId)
} }
} }
return userTermsOfService, nil return userTermsOfService, nil
} }
func (s SqlUserTermsOfServiceStore) Delete(userId, termsOfServiceId string) *model.AppError { func (s SqlUserTermsOfServiceStore) Delete(userId, termsOfServiceId string) error {
if _, err := s.GetMaster().Exec("DELETE FROM UserTermsOfService WHERE UserId = :UserId AND TermsOfServiceId = :TermsOfServiceId", map[string]interface{}{"UserId": userId, "TermsOfServiceId": termsOfServiceId}); err != nil { if _, err := s.GetMaster().Exec("DELETE FROM UserTermsOfService WHERE UserId = :UserId AND TermsOfServiceId = :TermsOfServiceId", map[string]interface{}{"UserId": userId, "TermsOfServiceId": termsOfServiceId}); err != nil {
return model.NewAppError("SqlUserTermsOfServiceStore.Delete", "store.sql_user_terms_of_service.delete.app_error", nil, "userId="+userId+", termsOfServiceId="+termsOfServiceId, http.StatusInternalServerError) return errors.Wrapf(err, "failed to delete UserTermsOfService with userId=%s and termsOfServiceId=%s", userId, termsOfServiceId)
} }
return nil return nil
} }

Просмотреть файл

@@ -624,9 +624,9 @@ type TermsOfServiceStore interface {
} }
type UserTermsOfServiceStore interface { type UserTermsOfServiceStore interface {
GetByUser(userId string) (*model.UserTermsOfService, *model.AppError) GetByUser(userId string) (*model.UserTermsOfService, error)
Save(userTermsOfService *model.UserTermsOfService) (*model.UserTermsOfService, *model.AppError) Save(userTermsOfService *model.UserTermsOfService) (*model.UserTermsOfService, error)
Delete(userId, termsOfServiceId string) *model.AppError Delete(userId, termsOfServiceId string) error
} }
type GroupStore interface { type GroupStore interface {

Просмотреть файл

@@ -15,23 +15,21 @@ type UserTermsOfServiceStore struct {
} }
// Delete provides a mock function with given fields: userId, termsOfServiceId // Delete provides a mock function with given fields: userId, termsOfServiceId
func (_m *UserTermsOfServiceStore) Delete(userId string, termsOfServiceId string) *model.AppError { func (_m *UserTermsOfServiceStore) Delete(userId string, termsOfServiceId string) error {
ret := _m.Called(userId, termsOfServiceId) ret := _m.Called(userId, termsOfServiceId)
var r0 *model.AppError var r0 error
if rf, ok := ret.Get(0).(func(string, string) *model.AppError); ok { if rf, ok := ret.Get(0).(func(string, string) error); ok {
r0 = rf(userId, termsOfServiceId) r0 = rf(userId, termsOfServiceId)
} else { } else {
if ret.Get(0) != nil { r0 = ret.Error(0)
r0 = ret.Get(0).(*model.AppError)
}
} }
return r0 return r0
} }
// GetByUser provides a mock function with given fields: userId // GetByUser provides a mock function with given fields: userId
func (_m *UserTermsOfServiceStore) GetByUser(userId string) (*model.UserTermsOfService, *model.AppError) { func (_m *UserTermsOfServiceStore) GetByUser(userId string) (*model.UserTermsOfService, error) {
ret := _m.Called(userId) ret := _m.Called(userId)
var r0 *model.UserTermsOfService var r0 *model.UserTermsOfService
@@ -43,20 +41,18 @@ func (_m *UserTermsOfServiceStore) GetByUser(userId string) (*model.UserTermsOfS
} }
} }
var r1 *model.AppError var r1 error
if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { if rf, ok := ret.Get(1).(func(string) error); ok {
r1 = rf(userId) r1 = rf(userId)
} else { } else {
if ret.Get(1) != nil { r1 = ret.Error(1)
r1 = ret.Get(1).(*model.AppError)
}
} }
return r0, r1 return r0, r1
} }
// Save provides a mock function with given fields: userTermsOfService // Save provides a mock function with given fields: userTermsOfService
func (_m *UserTermsOfServiceStore) Save(userTermsOfService *model.UserTermsOfService) (*model.UserTermsOfService, *model.AppError) { func (_m *UserTermsOfServiceStore) Save(userTermsOfService *model.UserTermsOfService) (*model.UserTermsOfService, error) {
ret := _m.Called(userTermsOfService) ret := _m.Called(userTermsOfService)
var r0 *model.UserTermsOfService var r0 *model.UserTermsOfService
@@ -68,13 +64,11 @@ func (_m *UserTermsOfServiceStore) Save(userTermsOfService *model.UserTermsOfSer
} }
} }
var r1 *model.AppError var r1 error
if rf, ok := ret.Get(1).(func(*model.UserTermsOfService) *model.AppError); ok { if rf, ok := ret.Get(1).(func(*model.UserTermsOfService) error); ok {
r1 = rf(userTermsOfService) r1 = rf(userTermsOfService)
} else { } else {
if ret.Get(1) != nil { r1 = ret.Error(1)
r1 = ret.Get(1).(*model.AppError)
}
} }
return r0, r1 return r0, r1

Просмотреть файл

@@ -4,6 +4,7 @@
package storetest package storetest
import ( import (
"errors"
"testing" "testing"
"github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/model"
@@ -63,5 +64,7 @@ func testDeleteUserTermsOfService(t *testing.T, ss store.Store) {
require.Nil(t, err) require.Nil(t, err)
_, err = ss.UserTermsOfService().GetByUser(userTermsOfService.UserId) _, err = ss.UserTermsOfService().GetByUser(userTermsOfService.UserId)
assert.Equal(t, "store.sql_user_terms_of_service.get_by_user.no_rows.app_error", err.Id) var nfErr *store.ErrNotFound
assert.NotNil(t, err)
assert.True(t, errors.As(err, &nfErr))
} }

Просмотреть файл

@@ -7812,7 +7812,7 @@ func (s *TimerLayerUserAccessTokenStore) UpdateTokenEnable(tokenId string) *mode
return resultVar0 return resultVar0
} }
func (s *TimerLayerUserTermsOfServiceStore) Delete(userId string, termsOfServiceId string) *model.AppError { func (s *TimerLayerUserTermsOfServiceStore) Delete(userId string, termsOfServiceId string) error {
start := timemodule.Now() start := timemodule.Now()
resultVar0 := s.UserTermsOfServiceStore.Delete(userId, termsOfServiceId) resultVar0 := s.UserTermsOfServiceStore.Delete(userId, termsOfServiceId)
@@ -7828,7 +7828,7 @@ func (s *TimerLayerUserTermsOfServiceStore) Delete(userId string, termsOfService
return resultVar0 return resultVar0
} }
func (s *TimerLayerUserTermsOfServiceStore) GetByUser(userId string) (*model.UserTermsOfService, *model.AppError) { func (s *TimerLayerUserTermsOfServiceStore) GetByUser(userId string) (*model.UserTermsOfService, error) {
start := timemodule.Now() start := timemodule.Now()
resultVar0, resultVar1 := s.UserTermsOfServiceStore.GetByUser(userId) resultVar0, resultVar1 := s.UserTermsOfServiceStore.GetByUser(userId)
@@ -7844,7 +7844,7 @@ func (s *TimerLayerUserTermsOfServiceStore) GetByUser(userId string) (*model.Use
return resultVar0, resultVar1 return resultVar0, resultVar1
} }
func (s *TimerLayerUserTermsOfServiceStore) Save(userTermsOfService *model.UserTermsOfService) (*model.UserTermsOfService, *model.AppError) { func (s *TimerLayerUserTermsOfServiceStore) Save(userTermsOfService *model.UserTermsOfService) (*model.UserTermsOfService, error) {
start := timemodule.Now() start := timemodule.Now()
resultVar0, resultVar1 := s.UserTermsOfServiceStore.Save(userTermsOfService) resultVar0, resultVar1 := s.UserTermsOfServiceStore.Save(userTermsOfService)