Refactor GetUser & GetUserByID

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
 	}

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

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
 	}

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
 	}

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
 		}

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))

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
 	}
 

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
 	}

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{}

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
 	}

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

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
 		}

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
 	}

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

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
 	}

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

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

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("*").

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

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))
 

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})
+	var exists bool
-	if err != nil && !IsErrUserDoesNotExist(err) {
+	existingUser, err := GetUser(User{Username: newUser.Username})
-		return User{}, err
+	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 {

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"})

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

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 {

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 = ""
 

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 {