General code quality improvement

1 files changed, 29 insertions, 34 deletions

diff --git a/models/update.go b/models/update.go
index 952794538a..79d62c8f6a 100644
--- a/models/update.go
+++ b/models/update.go
@@ -74,22 +74,22 @@ func ListToPushCommits(l *list.List) *PushCommits {
 }
 
 type PushUpdateOptions struct {
-	RefName      string
-	OldCommitID  string
-	NewCommitID  string
 	PusherID     int64
 	PusherName   string
 	RepoUserName string
 	RepoName     string
+	RefFullName  string
+	OldCommitID  string
+	NewCommitID  string
 }
 
 // PushUpdate must be called for any push actions in order to
 // generates necessary push action history feeds.
 func PushUpdate(opts PushUpdateOptions) (err error) {
-	isNewRef := strings.HasPrefix(opts.OldCommitID, "0000000")
-	isDelRef := strings.HasPrefix(opts.NewCommitID, "0000000")
+	isNewRef := opts.OldCommitID == git.EMPTY_SHA
+	isDelRef := opts.NewCommitID == git.EMPTY_SHA
 	if isNewRef && isDelRef {
-		return fmt.Errorf("Old and new revisions both start with 000000")
+		return fmt.Errorf("Old and new revisions are both %s", git.EMPTY_SHA)
 	}
 
 	repoPath := RepoPath(opts.RepoUserName, opts.RepoName)
@@ -102,7 +102,7 @@ func PushUpdate(opts PushUpdateOptions) (err error) {
 
 	if isDelRef {
 		log.GitLogger.Info("Reference '%s' has been deleted from '%s/%s' by %d",
-			opts.RefName, opts.RepoUserName, opts.RepoName, opts.PusherName)
+			opts.RefFullName, opts.RepoUserName, opts.RepoName, opts.PusherName)
 		return nil
 	}
 
@@ -111,41 +111,30 @@ func PushUpdate(opts PushUpdateOptions) (err error) {
 		return fmt.Errorf("OpenRepository: %v", err)
 	}
 
-	repoUser, err := GetUserByName(opts.RepoUserName)
+	owner, err := GetUserByName(opts.RepoUserName)
 	if err != nil {
 		return fmt.Errorf("GetUserByName: %v", err)
 	}
 
-	repo, err := GetRepositoryByName(repoUser.ID, opts.RepoName)
+	repo, err := GetRepositoryByName(owner.ID, opts.RepoName)
 	if err != nil {
 		return fmt.Errorf("GetRepositoryByName: %v", err)
 	}
 
 	// Push tags.
-	if strings.HasPrefix(opts.RefName, "refs/tags/") {
-		tag, err := gitRepo.GetTag(git.RefEndName(opts.RefName))
-		if err != nil {
-			return fmt.Errorf("gitRepo.GetTag: %v", err)
-		}
-
-		// When tagger isn't available, fall back to get committer email.
-		var actEmail string
-		if tag.Tagger != nil {
-			actEmail = tag.Tagger.Email
-		} else {
-			cmt, err := tag.Commit()
-			if err != nil {
-				return fmt.Errorf("tag.Commit: %v", err)
-			}
-			actEmail = cmt.Committer.Email
-		}
-
-		commit := &PushCommits{}
-		if err = CommitRepoAction(opts.PusherID, repoUser.ID, opts.PusherName, actEmail,
-			repo.ID, opts.RepoUserName, opts.RepoName, opts.RefName, commit, opts.OldCommitID, opts.NewCommitID); err != nil {
+	if strings.HasPrefix(opts.RefFullName, git.TAG_PREFIX) {
+		if err := CommitRepoAction(CommitRepoActionOptions{
+			PusherName:  opts.PusherName,
+			RepoOwnerID: owner.ID,
+			RepoName:    repo.Name,
+			RefFullName: opts.RefFullName,
+			OldCommitID: opts.OldCommitID,
+			NewCommitID: opts.NewCommitID,
+			Commits:     &PushCommits{},
+		}); err != nil {
 			return fmt.Errorf("CommitRepoAction (tag): %v", err)
 		}
-		return err
+		return nil
 	}
 
 	newCommit, err := gitRepo.GetCommit(opts.NewCommitID)
@@ -167,9 +156,15 @@ func PushUpdate(opts PushUpdateOptions) (err error) {
 		}
 	}
 
-	if err = CommitRepoAction(opts.PusherID, repoUser.ID, opts.PusherName, repoUser.Email,
-		repo.ID, opts.RepoUserName, opts.RepoName, opts.RefName, ListToPushCommits(l),
-		opts.OldCommitID, opts.NewCommitID); err != nil {
+	if err := CommitRepoAction(CommitRepoActionOptions{
+		PusherName:  opts.PusherName,
+		RepoOwnerID: owner.ID,
+		RepoName:    repo.Name,
+		RefFullName: opts.RefFullName,
+		OldCommitID: opts.OldCommitID,
+		NewCommitID: opts.NewCommitID,
+		Commits:     ListToPushCommits(l),
+	}); err != nil {
 		return fmt.Errorf("CommitRepoAction (branch): %v", err)
 	}
 	return nil