diff --git a/models/list.go b/models/list.go index 5b39a1210a9..d79cd02c029 100644 --- a/models/list.go +++ b/models/list.go @@ -22,7 +22,7 @@ type List struct { func (l *List) AfterLoad() { // Get the owner - l.Owner, _, _ = GetUserByID(l.OwnerID) + l.Owner, _= GetUserByID(l.OwnerID) // Get the list tasks l.Tasks, _ = GetTasksByListID(l.ID) @@ -51,7 +51,7 @@ func GetListsByNamespaceID(nID int64) (lists []*List, err error) { // ReadAll gets all lists a user has access to func (l *List) ReadAll(user *User) (interface{}, error) { lists := []List{} - fullUser, _, err := GetUserByID(user.ID) + fullUser, err := GetUserByID(user.ID) if err != nil { return lists, err } diff --git a/models/list_create_test.go b/models/list_create_test.go index 5894ce981ea..8c4ebfd0fc5 100644 --- a/models/list_create_test.go +++ b/models/list_create_test.go @@ -10,7 +10,7 @@ func TestList_Create(t *testing.T) { //assert.NoError(t, PrepareTestDatabase()) // Get our doer - doer, _, err := GetUserByID(1) + doer, err := GetUserByID(1) assert.NoError(t, err) // Dummy list for testing diff --git a/models/list_create_update.go b/models/list_create_update.go index 19e10cdf40d..eee1d7dbefe 100644 --- a/models/list_create_update.go +++ b/models/list_create_update.go @@ -44,7 +44,7 @@ func (l *List) Update() (err error) { // Create implements the create method of CRUDable func (l *List) Create(doer *User) (err error) { // Check rights - user, _, err := GetUserByID(doer.ID) + user, err := GetUserByID(doer.ID) if err != nil { return } diff --git a/models/list_items.go b/models/list_items.go index 3f82337e3a1..82ce86adaa2 100644 --- a/models/list_items.go +++ b/models/list_items.go @@ -84,7 +84,7 @@ func GetListTaskByID(listTaskID int64) (listTask ListTask, err error) { return ListTask{}, ErrListTaskDoesNotExist{listTaskID} } - user, _, err := GetUserByID(listTask.CreatedByID) + user, err := GetUserByID(listTask.CreatedByID) if err != nil { return } diff --git a/models/list_items_create_update.go b/models/list_items_create_update.go index 1e146a4b7b9..7392075aee6 100644 --- a/models/list_items_create_update.go +++ b/models/list_items_create_update.go @@ -37,7 +37,7 @@ func createOrUpdateListTask(i *ListTask, doer *User) (err error) { if i.ID != 0 { _, err = x.ID(i.ID).Update(i) } else { - user, _, err := GetUserByID(doer.ID) + user, err := GetUserByID(doer.ID) if err != nil { return err } diff --git a/models/list_items_test.go b/models/list_items_test.go index 6c1c29411c3..5ed88d671e0 100644 --- a/models/list_items_test.go +++ b/models/list_items_test.go @@ -16,7 +16,7 @@ func TestListTask_Create(t *testing.T) { } // Add one point to a list - doer, _, err := GetUserByID(1) + doer, err := GetUserByID(1) assert.NoError(t, err) assert.True(t, listtask.CanCreate(&doer)) diff --git a/models/list_users_create.go b/models/list_users_create.go index fe7c6d5091c..a7f9fd38baf 100644 --- a/models/list_users_create.go +++ b/models/list_users_create.go @@ -15,7 +15,7 @@ func (ul *ListUser) Create(user *User) (err error) { } // Check if the user exists - if _, _, err = GetUserByID(ul.UserID); err != nil { + if _, err = GetUserByID(ul.UserID); err != nil { return err } diff --git a/models/list_users_delete.go b/models/list_users_delete.go index e510b7bc24d..64004001666 100644 --- a/models/list_users_delete.go +++ b/models/list_users_delete.go @@ -4,7 +4,7 @@ package models func (lu *ListUser) Delete() (err error) { // Check if the user exists - _, _, err = GetUserByID(lu.UserID) + _, err = GetUserByID(lu.UserID) if err != nil { return } diff --git a/models/lists_get_test.go b/models/lists_get_test.go index ad43fb6ea09..9593f41b723 100644 --- a/models/lists_get_test.go +++ b/models/lists_get_test.go @@ -16,7 +16,7 @@ func TestList_ReadAll(t *testing.T) { assert.Equal(t, len(lists), 2) // Get all lists our user has access to - user, _, err := GetUserByID(1) + user, err := GetUserByID(1) assert.NoError(t, err) lists2 := List{} diff --git a/models/namespace_create.go b/models/namespace_create.go index 8aa919cb404..9245170e7cb 100644 --- a/models/namespace_create.go +++ b/models/namespace_create.go @@ -9,7 +9,7 @@ func (n *Namespace) Create(doer *User) (err error) { n.ID = 0 // This would otherwise prevent the creation of new lists after one was created // Check if the User exists - n.Owner, _, err = GetUserByID(doer.ID) + n.Owner, err = GetUserByID(doer.ID) if err != nil { return } diff --git a/models/namespace_test.go b/models/namespace_test.go index 69c0a3ea501..757028450d9 100644 --- a/models/namespace_test.go +++ b/models/namespace_test.go @@ -17,7 +17,7 @@ func TestNamespace_Create(t *testing.T) { } // Doer - doer, _, err := GetUserByID(1) + doer, err := GetUserByID(1) assert.NoError(t, err) // Try creating it diff --git a/models/namespace_update.go b/models/namespace_update.go index 2216bc41190..b46cea33b3f 100644 --- a/models/namespace_update.go +++ b/models/namespace_update.go @@ -15,7 +15,7 @@ func (n *Namespace) Update() (err error) { // Check if the (new) owner exists if currentNamespace.OwnerID != n.OwnerID { - n.Owner, _, err = GetUserByID(n.OwnerID) + n.Owner, err = GetUserByID(n.OwnerID) if err != nil { return } diff --git a/models/namespaces.go b/models/namespaces.go index 6d905162cea..03863b37bbd 100644 --- a/models/namespaces.go +++ b/models/namespaces.go @@ -23,7 +23,7 @@ func (Namespace) TableName() string { // AfterLoad gets the owner func (n *Namespace) AfterLoad() { - n.Owner, _, _ = GetUserByID(n.OwnerID) + n.Owner, _ = GetUserByID(n.OwnerID) } // GetNamespaceByID returns a namespace object by its ID @@ -39,7 +39,7 @@ func GetNamespaceByID(id int64) (namespace Namespace, err error) { } // Get the namespace Owner - namespace.Owner, _, err = GetUserByID(namespace.OwnerID) + namespace.Owner, err = GetUserByID(namespace.OwnerID) if err != nil { return namespace, err } diff --git a/models/team_list_test.go b/models/team_list_test.go index b639401ae72..84aeec739ab 100644 --- a/models/team_list_test.go +++ b/models/team_list_test.go @@ -15,7 +15,7 @@ func TestTeamList(t *testing.T) { } // Dummyuser - user, _, err := GetUserByID(1) + user, err := GetUserByID(1) assert.NoError(t, err) // Check normal creation diff --git a/models/team_members_create.go b/models/team_members_create.go index 3613642fad8..b84afe104f0 100644 --- a/models/team_members_create.go +++ b/models/team_members_create.go @@ -9,7 +9,7 @@ func (tm *TeamMember) Create(doer *User) (err error) { } // Check if the user exists - _, _, err = GetUserByID(tm.UserID) + _, err = GetUserByID(tm.UserID) if err != nil { return } diff --git a/models/team_members_test.go b/models/team_members_test.go index d2184ba9e97..73b11f1d7f2 100644 --- a/models/team_members_test.go +++ b/models/team_members_test.go @@ -14,7 +14,7 @@ func TestTeamMember_Create(t *testing.T) { } // Doer - doer, _, err := GetUserByID(1) + doer, err := GetUserByID(1) assert.NoError(t, err) // Insert a new team member diff --git a/models/team_namespace_test.go b/models/team_namespace_test.go index 1f0f39a7e67..f3502d4c56d 100644 --- a/models/team_namespace_test.go +++ b/models/team_namespace_test.go @@ -14,7 +14,7 @@ func TestTeamNamespace(t *testing.T) { Right: TeamRightAdmin, } - dummyuser, _, err := GetUserByID(1) + dummyuser, err := GetUserByID(1) assert.NoError(t, err) // Test normal creation diff --git a/models/teams.go b/models/teams.go index 10cba168c7b..ad4b0f52154 100644 --- a/models/teams.go +++ b/models/teams.go @@ -25,7 +25,7 @@ func (Team) TableName() string { // AfterLoad gets the created by user object func (t *Team) AfterLoad() { // Get the owner - t.CreatedBy, _, _ = GetUserByID(t.CreatedByID) + t.CreatedBy, _ = GetUserByID(t.CreatedByID) // Get all members x.Select("*"). diff --git a/models/teams_test.go b/models/teams_test.go index 696a39ca134..bd311850bf3 100644 --- a/models/teams_test.go +++ b/models/teams_test.go @@ -14,7 +14,7 @@ func TestTeam_Create(t *testing.T) { } // Doer - doer, _, err := GetUserByID(1) + doer, err := GetUserByID(1) assert.NoError(t, err) // Insert it diff --git a/models/user.go b/models/user.go index 5d5a8aa04d1..a38ab8ede52 100644 --- a/models/user.go +++ b/models/user.go @@ -46,40 +46,36 @@ func (apiUser *APIUserPassword) APIFormat() User { } // GetUserByID gets informations about a user by its ID -func GetUserByID(id int64) (user User, exists bool, err error) { +func GetUserByID(id int64) (user User, err error) { // Apparently xorm does otherwise look for all users but return only one, which leads to returing one even if the ID is 0 if id == 0 { - return User{}, false, nil + return User{}, nil } return GetUser(User{ID: id}) } // GetUser gets a user object -func GetUser(user User) (userOut User, exists bool, err error) { +func GetUser(user User) (userOut User, err error) { userOut = user - exists, err = x.Get(&userOut) + exists, err := x.Get(&userOut) if !exists { - return User{}, false, ErrUserDoesNotExist{} + return User{}, ErrUserDoesNotExist{} } - return userOut, exists, err + return userOut, err } // CheckUserCredentials checks user credentials func CheckUserCredentials(u *UserLogin) (User, error) { // Check if the user exists - user, exists, err := GetUser(User{Username: u.Username}) + user, err := GetUser(User{Username: u.Username}) if err != nil { return User{}, err } - if !exists { - return User{}, ErrUserDoesNotExist{} - } - // Check the users password err = bcrypt.CompareHashAndPassword([]byte(user.Password), []byte(u.Password)) diff --git a/models/user_add_update.go b/models/user_add_update.go index ece61461dda..faaafbb520d 100644 --- a/models/user_add_update.go +++ b/models/user_add_update.go @@ -15,18 +15,27 @@ func CreateUser(user User) (newUser User, err error) { } // Check if the user already existst with that username - existingUser, exists, err := GetUser(User{Username: newUser.Username}) - if err != nil && !IsErrUserDoesNotExist(err) { - return User{}, err + var exists bool + existingUser, err := GetUser(User{Username: newUser.Username}) + if err != nil { + if IsErrUserDoesNotExist(err) { + exists = true + } else { + return User{}, err + } } if exists { return User{}, ErrUsernameExists{existingUser.ID, existingUser.Username} } // Check if the user already existst with that email - existingUser, exists, err = GetUser(User{Email: newUser.Email}) + existingUser, err = GetUser(User{Email: newUser.Email}) if err != nil && !IsErrUserDoesNotExist(err) { - return User{}, err + if IsErrUserDoesNotExist(err) { + exists = true + } else { + return User{}, err + } } if exists { return User{}, ErrUserEmailExists{existingUser.ID, existingUser.Email} @@ -45,7 +54,7 @@ func CreateUser(user User) (newUser User, err error) { } // Get the full new User - newUserOut, _, err := GetUser(newUser) + newUserOut, err := GetUser(newUser) if err != nil { return User{}, err } @@ -70,12 +79,11 @@ func hashPassword(password string) (string, error) { func UpdateUser(user User) (updatedUser User, err error) { // Check if it exists - theUser, exists, err := GetUserByID(user.ID) + theUser, err := GetUserByID(user.ID) if err != nil { return User{}, err } - if exists { // Check if we have at least a username if user.Username == "" { //return User{}, ErrNoUsername{user.ID} @@ -91,30 +99,23 @@ func UpdateUser(user User) (updatedUser User, err error) { } // Get the newly updated user - updatedUser, _, err = GetUserByID(user.ID) + updatedUser, err = GetUserByID(user.ID) if err != nil { return User{}, err } return updatedUser, err - } - - return User{}, ErrUserDoesNotExist{user.ID} } // UpdateUserPassword updates the password of a user func UpdateUserPassword(userID int64, newPassword string, doer *User) (err error) { // Get all user details - user, exists, err := GetUserByID(userID) + user, err := GetUserByID(userID) if err != nil { return err } - if !exists { - return ErrUserDoesNotExist{userID} - } - // Hash the new password and set it hashed, err := hashPassword(newPassword) if err != nil { diff --git a/models/user_test.go b/models/user_test.go index 3524531e8c9..253801fb46a 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -10,7 +10,7 @@ func TestCreateUser(t *testing.T) { //assert.NoError(t, PrepareTestDatabase()) // Get our doer - doer, _, err := GetUserByID(1) + doer, err := GetUserByID(1) assert.NoError(t, err) // Our dummy user for testing @@ -60,19 +60,16 @@ func TestCreateUser(t *testing.T) { assert.True(t, IsErrNoUsernamePassword(err)) // Check if he exists - theuser, exists, err := GetUser(createdUser) + theuser, err := GetUser(createdUser) assert.NoError(t, err) - assert.True(t, exists) // Get by his ID - _, exists, err = GetUserByID(theuser.ID) + _, err = GetUserByID(theuser.ID) assert.NoError(t, err) - assert.True(t, exists) // Passing 0 as ID should return an empty user - _, exists, err = GetUserByID(0) + _, err = GetUserByID(0) assert.NoError(t, err) - assert.False(t, exists) // Check the user credentials user, err := CheckUserCredentials(&UserLogin{"testuu", "1234"}) diff --git a/routes/api/v1/user_add_update.go b/routes/api/v1/user_add_update.go index 3cbd0ad82a8..a96841258eb 100644 --- a/routes/api/v1/user_add_update.go +++ b/routes/api/v1/user_add_update.go @@ -58,9 +58,14 @@ func userAddOrUpdate(c echo.Context) error { } // Check if the user exists - _, exists, err := models.GetUserByID(datUser.ID) + var exists bool + _, err := models.GetUserByID(datUser.ID) if err != nil { - return c.JSON(http.StatusInternalServerError, models.Message{"Could not check if the user exists."}) + if models.IsErrUserDoesNotExist(err) { + exists = true + } else { + return c.JSON(http.StatusInternalServerError, models.Message{"Could not check if the user exists."}) + } } // Insert or update the user diff --git a/routes/api/v1/user_delete.go b/routes/api/v1/user_delete.go index 228c77218d2..b62c9f72d14 100644 --- a/routes/api/v1/user_delete.go +++ b/routes/api/v1/user_delete.go @@ -22,16 +22,15 @@ func UserDelete(c echo.Context) error { } // Check if the user exists - _, exists, err := models.GetUserByID(userID) + _, err = models.GetUserByID(userID) if err != nil { + if models.IsErrUserDoesNotExist(err) { + return c.JSON(http.StatusNotFound, models.Message{"The user does not exist."}) + } return c.JSON(http.StatusInternalServerError, models.Message{"Could not get user."}) } - if !exists { - return c.JSON(http.StatusNotFound, models.Message{"The user does not exist."}) - } - // Get the doer options doer, err := models.GetCurrentUser(c) if err != nil { diff --git a/routes/api/v1/user_show.go b/routes/api/v1/user_show.go index 11c537a99c9..d4a89807517 100644 --- a/routes/api/v1/user_show.go +++ b/routes/api/v1/user_show.go @@ -25,17 +25,15 @@ func UserShow(c echo.Context) error { } // Get User Infos - userInfos, exists, err := models.GetUserByID(userID) + userInfos, err := models.GetUserByID(userID) if err != nil { + if models.IsErrUserDoesNotExist(err) { + return c.JSON(http.StatusNotFound, models.Message{"The user does not exist."}) + } return c.JSON(http.StatusInternalServerError, models.Message{"Error getting user infos."}) } - // Check if it exists - if !exists { - return c.JSON(http.StatusNotFound, models.Message{"User not found."}) - } - // Obfucate his password userInfos.Password = "" diff --git a/routes/api/v1/user_update_password.go b/routes/api/v1/user_update_password.go index 6d57bc369d7..ffffc1d4849 100644 --- a/routes/api/v1/user_update_password.go +++ b/routes/api/v1/user_update_password.go @@ -49,17 +49,15 @@ func UserChangePassword(c echo.Context) error { } // Get User Infos - _, exists, err := models.GetUserByID(userID) + _, err = models.GetUserByID(userID) if err != nil { + if models.IsErrUserDoesNotExist(err) { + return c.JSON(http.StatusNotFound, models.Message{"The user does not exist."}) + } return c.JSON(http.StatusInternalServerError, models.Message{"Error getting user infos."}) } - // Check if it exists - if !exists { - return c.JSON(http.StatusNotFound, models.Message{"User not found."}) - } - // Get the doer options doer, err := models.GetCurrentUser(c) if err != nil {