diff --git a/api4/user_test.go b/api4/user_test.go index 2814680c7e..4c65ae8ce0 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -4543,7 +4543,7 @@ func TestGetUserTermsOfService(t *testing.T) { defer th.TearDown() _, 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) require.Nil(t, err) diff --git a/app/user_terms_of_service.go b/app/user_terms_of_service.go index 7873ee3abc..80c1bbe413 100644 --- a/app/user_terms_of_service.go +++ b/app/user_terms_of_service.go @@ -3,10 +3,27 @@ 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) { - 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 { @@ -17,11 +34,17 @@ func (a *App) SaveUserTermsOfService(userId, termsOfServiceId string, accepted b } 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 { 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) } } diff --git a/app/user_terms_of_service_test.go b/app/user_terms_of_service_test.go index d836d3e405..928b62d5ea 100644 --- a/app/user_terms_of_service_test.go +++ b/app/user_terms_of_service_test.go @@ -16,7 +16,7 @@ func TestUserTermsOfService(t *testing.T) { userTermsOfService, err := th.App.GetUserTermsOfService(th.BasicUser.Id) checkError(t, err) 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) checkNoError(t, err) diff --git a/i18n/en.json b/i18n/en.json index 32982228ea..5fb4195e25 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -3990,6 +3990,22 @@ "id": "app.user_access_token.invalid_or_missing", "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", "translation": "Bleve is already started." @@ -7626,22 +7642,6 @@ "id": "store.sql_user_access_token.update_token_enable.app_error", "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", "translation": "Unable to count the incoming webhooks." diff --git a/store/opentracing_layer.go b/store/opentracing_layer.go index d45996febe..16c30fadba 100644 --- a/store/opentracing_layer.go +++ b/store/opentracing_layer.go @@ -8639,7 +8639,7 @@ func (s *OpenTracingLayerUserAccessTokenStore) UpdateTokenEnable(tokenId string) 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() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserTermsOfServiceStore.Delete") s.Root.Store.SetContext(newCtx) @@ -8657,7 +8657,7 @@ func (s *OpenTracingLayerUserTermsOfServiceStore) Delete(userId string, termsOfS 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() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserTermsOfServiceStore.GetByUser") s.Root.Store.SetContext(newCtx) @@ -8675,7 +8675,7 @@ func (s *OpenTracingLayerUserTermsOfServiceStore) GetByUser(userId string) (*mod 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() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserTermsOfServiceStore.Save") s.Root.Store.SetContext(newCtx) diff --git a/store/sqlstore/user_terms_of_service.go b/store/sqlstore/user_terms_of_service.go index 9c473be104..0ad64ee4f6 100644 --- a/store/sqlstore/user_terms_of_service.go +++ b/store/sqlstore/user_terms_of_service.go @@ -5,10 +5,11 @@ package sqlstore import ( "database/sql" - "net/http" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/store" + + "github.com/pkg/errors" ) type SqlUserTermsOfServiceStore struct { @@ -31,20 +32,20 @@ func (s SqlUserTermsOfServiceStore) createIndexesIfNotExists() { 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 err := s.GetReplica().SelectOne(&userTermsOfService, "SELECT * FROM UserTermsOfService WHERE UserId = :userId", map[string]interface{}{"userId": userId}) if err != nil { 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 } -func (s SqlUserTermsOfServiceStore) Save(userTermsOfService *model.UserTermsOfService) (*model.UserTermsOfService, *model.AppError) { +func (s SqlUserTermsOfServiceStore) Save(userTermsOfService *model.UserTermsOfService) (*model.UserTermsOfService, error) { userTermsOfService.PreSave() if err := userTermsOfService.IsValid(); err != nil { @@ -53,21 +54,21 @@ func (s SqlUserTermsOfServiceStore) Save(userTermsOfService *model.UserTermsOfSe c, err := s.GetMaster().Update(userTermsOfService) 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 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 } -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 { - 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 } diff --git a/store/store.go b/store/store.go index 070367560b..18ee413497 100644 --- a/store/store.go +++ b/store/store.go @@ -624,9 +624,9 @@ type TermsOfServiceStore interface { } type UserTermsOfServiceStore interface { - GetByUser(userId string) (*model.UserTermsOfService, *model.AppError) - Save(userTermsOfService *model.UserTermsOfService) (*model.UserTermsOfService, *model.AppError) - Delete(userId, termsOfServiceId string) *model.AppError + GetByUser(userId string) (*model.UserTermsOfService, error) + Save(userTermsOfService *model.UserTermsOfService) (*model.UserTermsOfService, error) + Delete(userId, termsOfServiceId string) error } type GroupStore interface { diff --git a/store/storetest/mocks/UserTermsOfServiceStore.go b/store/storetest/mocks/UserTermsOfServiceStore.go index 6efe3cf353..2fce026a93 100644 --- a/store/storetest/mocks/UserTermsOfServiceStore.go +++ b/store/storetest/mocks/UserTermsOfServiceStore.go @@ -15,23 +15,21 @@ type UserTermsOfServiceStore struct { } // 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) - var r0 *model.AppError - if rf, ok := ret.Get(0).(func(string, string) *model.AppError); ok { + var r0 error + if rf, ok := ret.Get(0).(func(string, string) error); ok { r0 = rf(userId, termsOfServiceId) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.AppError) - } + r0 = ret.Error(0) } return r0 } // 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) var r0 *model.UserTermsOfService @@ -43,20 +41,18 @@ func (_m *UserTermsOfServiceStore) GetByUser(userId string) (*model.UserTermsOfS } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { r1 = rf(userId) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // 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) var r0 *model.UserTermsOfService @@ -68,13 +64,11 @@ func (_m *UserTermsOfServiceStore) Save(userTermsOfService *model.UserTermsOfSer } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(*model.UserTermsOfService) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(*model.UserTermsOfService) error); ok { r1 = rf(userTermsOfService) } 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_terms_of_service.go b/store/storetest/user_terms_of_service.go index c566096e2f..19470aa6d6 100644 --- a/store/storetest/user_terms_of_service.go +++ b/store/storetest/user_terms_of_service.go @@ -4,6 +4,7 @@ package storetest import ( + "errors" "testing" "github.com/mattermost/mattermost-server/v5/model" @@ -63,5 +64,7 @@ func testDeleteUserTermsOfService(t *testing.T, ss store.Store) { require.Nil(t, err) _, 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)) } diff --git a/store/timer_layer.go b/store/timer_layer.go index 94c9890cfb..2bdb26e37a 100644 --- a/store/timer_layer.go +++ b/store/timer_layer.go @@ -7812,7 +7812,7 @@ func (s *TimerLayerUserAccessTokenStore) UpdateTokenEnable(tokenId string) *mode return resultVar0 } -func (s *TimerLayerUserTermsOfServiceStore) Delete(userId string, termsOfServiceId string) *model.AppError { +func (s *TimerLayerUserTermsOfServiceStore) Delete(userId string, termsOfServiceId string) error { start := timemodule.Now() resultVar0 := s.UserTermsOfServiceStore.Delete(userId, termsOfServiceId) @@ -7828,7 +7828,7 @@ func (s *TimerLayerUserTermsOfServiceStore) Delete(userId string, termsOfService return resultVar0 } -func (s *TimerLayerUserTermsOfServiceStore) GetByUser(userId string) (*model.UserTermsOfService, *model.AppError) { +func (s *TimerLayerUserTermsOfServiceStore) GetByUser(userId string) (*model.UserTermsOfService, error) { start := timemodule.Now() resultVar0, resultVar1 := s.UserTermsOfServiceStore.GetByUser(userId) @@ -7844,7 +7844,7 @@ func (s *TimerLayerUserTermsOfServiceStore) GetByUser(userId string) (*model.Use 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() resultVar0, resultVar1 := s.UserTermsOfServiceStore.Save(userTermsOfService)