models: fix issue with transfer repository

README: fix typo

3 files changed, 70 insertions, 58 deletions

diff --git a/models/access.go b/models/access.go
index 29d1a730b7..683282f336 100644
--- a/models/access.go
+++ b/models/access.go
@@ -4,6 +4,10 @@
 
 package models
 
+import (
+	"fmt"
+)
+
 type AccessMode int
 
 const (
@@ -128,7 +132,7 @@ func (repo *Repository) recalculateAccesses(e Engine) error {
 			}
 
 			if err = team.getMembers(e); err != nil {
-				return err
+				return fmt.Errorf("getMembers '%d': %v", team.ID, err)
 			}
 			for _, u := range team.Members {
 				accessMap[u.Id] = maxAccessMode(accessMap[u.Id], team.Authorize)
@@ -157,9 +161,9 @@ func (repo *Repository) recalculateAccesses(e Engine) error {
 
 	// Delete old accesses and insert new ones for repository.
 	if _, err = e.Delete(&Access{RepoID: repo.Id}); err != nil {
-		return err
+		return fmt.Errorf("delete old accesses: %v", err)
 	} else if _, err = e.Insert(newAccesses); err != nil {
-		return err
+		return fmt.Errorf("insert new accesses: %v", err)
 	}
 
 	return nil
diff --git a/models/org.go b/models/org.go
index 629e9345c3..b47fbced5f 100644
--- a/models/org.go
+++ b/models/org.go
@@ -28,7 +28,7 @@ func (org *User) IsOwnedBy(uid int64) bool {
 
 // IsOrgMember returns true if given user is member of organization.
 func (org *User) IsOrgMember(uid int64) bool {
-	return IsOrganizationMember(org.Id, uid)
+	return org.IsOrganization() && IsOrganizationMember(org.Id, uid)
 }
 
 func (org *User) getTeam(e Engine, name string) (*Team, error) {
@@ -493,19 +493,19 @@ func (t *Team) addRepository(e Engine, repo *Repository) (err error) {
 
 	t.NumRepos++
 	if _, err = e.Id(t.ID).AllCols().Update(t); err != nil {
-		return err
+		return fmt.Errorf("update team: %v", err)
 	}
 
 	if err = repo.recalculateAccesses(e); err != nil {
-		return err
+		return fmt.Errorf("recalculateAccesses: %v", err)
 	}
 
 	if err = t.getMembers(e); err != nil {
-		return fmt.Errorf("get team members: %v", err)
+		return fmt.Errorf("getMembers: %v", err)
 	}
 	for _, u := range t.Members {
 		if err = watchRepo(e, u.Id, repo.Id, true); err != nil {
-			return err
+			return fmt.Errorf("watchRepo: %v", err)
 		}
 	}
 	return nil
@@ -772,10 +772,20 @@ func IsTeamMember(orgID, teamID, uid int64) bool {
 	return isTeamMember(x, orgID, teamID, uid)
 }
 
-func getTeamMembers(e Engine, teamID int64) ([]*User, error) {
-	us := make([]*User, 0, 10)
-	err := e.Sql("SELECT * FROM `user` JOIN `team_user` ON `team_user`.`team_id` = ? AND `team_user`.`uid` = `user`.`id`", teamID).Find(&us)
-	return us, err
+func getTeamMembers(e Engine, teamID int64) (_ []*User, err error) {
+	teamUsers := make([]*TeamUser, 0, 10)
+	if err = e.Where("team_id=?", teamID).Find(&teamUsers); err != nil {
+		return nil, fmt.Errorf("get team-users: %v", err)
+	}
+	members := make([]*User, 0, len(teamUsers))
+	for i := range teamUsers {
+		member := new(User)
+		if _, err = e.Id(teamUsers[i].Uid).Get(member); err != nil {
+			return nil, fmt.Errorf("get user '%d': %v", teamUsers[i].Uid, err)
+		}
+		members = append(members, member)
+	}
+	return members, nil
 }
 
 // GetTeamMembers returns all members in given team of organization.
diff --git a/models/repo.go b/models/repo.go
index 7f87bbb2a7..0f75029096 100644
--- a/models/repo.go
+++ b/models/repo.go
@@ -223,16 +223,12 @@ func (repo *Repository) DescriptionHtml() template.HTML {
 }
 
 // IsRepositoryExist returns true if the repository with given name under user has already existed.
-func IsRepositoryExist(u *User, repoName string) (bool, error) {
-	repo := Repository{OwnerId: u.Id}
-	has, err := x.Where("lower_name = ?", strings.ToLower(repoName)).Get(&repo)
-	if err != nil {
-		return has, err
-	} else if !has {
-		return false, nil
-	}
-
-	return com.IsDir(RepoPath(u.Name, repoName)), nil
+func IsRepositoryExist(u *User, repoName string) bool {
+	has, _ := x.Get(&Repository{
+		OwnerId:   u.Id,
+		LowerName: strings.ToLower(repoName),
+	})
+	return has && com.IsDir(RepoPath(u.Name, repoName))
 }
 
 // CloneLink represents different types of clone URLs of repository.
@@ -514,15 +510,12 @@ func initRepository(e Engine, f string, u *User, repo *Repository, initReadme bo
 }
 
 // CreateRepository creates a repository for given user or organization.
-func CreateRepository(u *User, name, desc, lang, license string, isPrivate, isMirror, initReadme bool) (*Repository, error) {
+func CreateRepository(u *User, name, desc, lang, license string, isPrivate, isMirror, initReadme bool) (_ *Repository, err error) {
 	if !IsLegalName(name) {
 		return nil, ErrRepoNameIllegal
 	}
 
-	isExist, err := IsRepositoryExist(u, name)
-	if err != nil {
-		return nil, err
-	} else if isExist {
+	if IsRepositoryExist(u, name) {
 		return nil, ErrRepoAlreadyExist
 	}
 
@@ -631,71 +624,79 @@ func RepoPath(userName, repoName string) string {
 func TransferOwnership(u *User, newOwnerName string, repo *Repository) error {
 	newOwner, err := GetUserByName(newOwnerName)
 	if err != nil {
-		return fmt.Errorf("get new owner(%s): %v", newOwnerName, err)
+		return fmt.Errorf("get new owner '%s': %v", newOwnerName, err)
 	}
 
 	// Check if new owner has repository with same name.
-	has, err := IsRepositoryExist(newOwner, repo.Name)
-	if err != nil {
-		return err
-	} else if has {
+	if IsRepositoryExist(newOwner, repo.Name) {
 		return ErrRepoAlreadyExist
 	}
 
 	sess := x.NewSession()
 	defer sessionRelease(sess)
 	if err = sess.Begin(); err != nil {
-		return err
+		return fmt.Errorf("sess.Begin: %v", err)
 	}
 
 	owner := repo.Owner
 
-	// Update repository.
+	// Note: we have to set value here to make sure recalculate accesses is based on
+	//	new owner.
 	repo.OwnerId = newOwner.Id
 	repo.Owner = newOwner
+
+	// Update repository.
 	if _, err := sess.Id(repo.Id).Update(repo); err != nil {
-		return err
+		return fmt.Errorf("update owner: %v", err)
 	}
 
-	// Remove redundant collaborators
+	// Remove redundant collaborators.
 	collaborators, err := repo.GetCollaborators()
 	if err != nil {
-		return err
+		return fmt.Errorf("GetCollaborators: %v", err)
 	}
+
+	// Dummy object.
+	collaboration := &Collaboration{RepoID: repo.Id}
 	for _, c := range collaborators {
+		collaboration.UserID = c.Id
 		if c.Id == newOwner.Id || newOwner.IsOrgMember(c.Id) {
-			if _, err = sess.Delete(&Collaboration{RepoID: repo.Id, UserID: c.Id}); err != nil {
-				return err
+			if _, err = sess.Delete(collaboration); err != nil {
+				return fmt.Errorf("remove collaborator '%d': %v", c.Id, err)
 			}
 		}
 	}
 
 	if newOwner.IsOrganization() {
-		// Update owner team info and count.
 		t, err := newOwner.GetOwnerTeam()
 		if err != nil {
-			return err
+			return fmt.Errorf("GetOwnerTeam: %v", err)
 		} else if err = t.addRepository(sess, repo); err != nil {
-			return err
+			return fmt.Errorf("add to owner team: %v", err)
+		}
+	} else {
+		// Organization called this in addRepository method.
+		if err = repo.recalculateAccesses(sess); err != nil {
+			return fmt.Errorf("recalculateAccesses: %v", err)
 		}
 	}
 
-	// Update user repository number.
-	if _, err = sess.Exec("UPDATE `user` SET num_repos = num_repos + 1 WHERE id = ?", newOwner.Id); err != nil {
-		return err
-	} else if _, err = sess.Exec("UPDATE `user` SET num_repos = num_repos - 1 WHERE id = ?", owner.Id); err != nil {
-		return err
-	} else if err = repo.recalculateAccesses(sess); err != nil {
-		return err
-	} else if err = watchRepo(sess, newOwner.Id, repo.Id, true); err != nil {
-		return err
+	// Update repository count.
+	if _, err = sess.Exec("UPDATE `user` SET num_repos=num_repos+1 WHERE id=?", newOwner.Id); err != nil {
+		return fmt.Errorf("increase new owner repository count: %v", err)
+	} else if _, err = sess.Exec("UPDATE `user` SET num_repos=num_repos-1 WHERE id=?", owner.Id); err != nil {
+		return fmt.Errorf("decrease old owner repository count: %v", err)
+	}
+
+	if err = watchRepo(sess, newOwner.Id, repo.Id, true); err != nil {
+		return fmt.Errorf("watchRepo: %v", err)
 	} else if err = transferRepoAction(sess, u, owner, newOwner, repo); err != nil {
-		return err
+		return fmt.Errorf("transferRepoAction: %v", err)
 	}
 
 	// Change repository directory name.
 	if err = os.Rename(RepoPath(owner.Name, repo.Name), RepoPath(newOwner.Name, repo.Name)); err != nil {
-		return err
+		return fmt.Errorf("rename directory: %v", err)
 	}
 
 	return sess.Commit()
@@ -1275,11 +1276,8 @@ func IsStaring(uid, repoId int64) bool {
 //  \___  / \____/|__|  |__|_ \
 //      \/                   \/
 
-func ForkRepository(u *User, oldRepo *Repository, name, desc string) (*Repository, error) {
-	isExist, err := IsRepositoryExist(u, name)
-	if err != nil {
-		return nil, err
-	} else if isExist {
+func ForkRepository(u *User, oldRepo *Repository, name, desc string) (_ *Repository, err error) {
+	if IsRepositoryExist(u, name) {
 		return nil, ErrRepoAlreadyExist
 	}