Merge branch 'master' into improve-strings

Author: techknowlogick

models/user.go

@@ -14,7 +14,6 @@ import (
 	"errors"
 	"fmt"
 	_ "image/jpeg" // Needed for jpeg support
-	"net/mail"
 	"os"
 	"path/filepath"
 	"regexp"
@@ -809,9 +808,8 @@ func CreateUser(u *User) (err error) {
 		return ErrEmailAlreadyUsed{u.Email}
 	}
 
-	_, err = mail.ParseAddress(u.Email)
-	if err != nil {
-		return ErrEmailInvalid{u.Email}
+	if err = ValidateEmail(u.Email); err != nil {
+		return err
 	}
 
 	isExist, err = isEmailUsed(sess, u.Email)
@@ -956,11 +954,10 @@ func checkDupEmail(e Engine, u *User) error {
 	return nil
 }
 
-func updateUser(e Engine, u *User) error {
+func updateUser(e Engine, u *User) (err error) {
 	u.Email = strings.ToLower(u.Email)
-	_, err := mail.ParseAddress(u.Email)
-	if err != nil {
-		return ErrEmailInvalid{u.Email}
+	if err = ValidateEmail(u.Email); err != nil {
+		return err
 	}
 	_, err = e.ID(u.ID).AllCols().Update(u)
 	return err
@@ -982,13 +979,21 @@ func updateUserCols(e Engine, u *User, cols ...string) error {
 }
 
 // UpdateUserSetting updates user's settings.
-func UpdateUserSetting(u *User) error {
+func UpdateUserSetting(u *User) (err error) {
+	sess := x.NewSession()
+	defer sess.Close()
+	if err = sess.Begin(); err != nil {
+		return err
+	}
 	if !u.IsOrganization() {
-		if err := checkDupEmail(x, u); err != nil {
+		if err = checkDupEmail(sess, u); err != nil {
 			return err
 		}
 	}
-	return updateUser(x, u)
+	if err = updateUser(sess, u); err != nil {
+		return err
+	}
+	return sess.Commit()
 }
 
 // deleteBeans deletes all given beans, beans should contain delete conditions.

models/user_mail.go

@@ -33,6 +33,21 @@ type EmailAddress struct {
 	IsPrimary   bool `xorm:"-"`
 }
 
+// ValidateEmail check if email is a allowed address
+func ValidateEmail(email string) error {
+	if len(email) == 0 {
+		return nil
+	}
+
+	if _, err := mail.ParseAddress(email); err != nil {
+		return ErrEmailInvalid{email}
+	}
+
+	// TODO: add an email allow/block list
+
+	return nil
+}
+
 // GetEmailAddresses returns all email addresses belongs to given user.
 func GetEmailAddresses(uid int64) ([]*EmailAddress, error) {
 	emails := make([]*EmailAddress, 0, 5)
@@ -144,9 +159,8 @@ func addEmailAddress(e Engine, email *EmailAddress) error {
 		return ErrEmailAlreadyUsed{email.Email}
 	}
 
-	_, err = mail.ParseAddress(email.Email)
-	if err != nil {
-		return ErrEmailInvalid{email.Email}
+	if err = ValidateEmail(email.Email); err != nil {
+		return err
 	}
 
 	_, err = e.Insert(email)
@@ -173,9 +187,8 @@ func AddEmailAddresses(emails []*EmailAddress) error {
 		} else if used {
 			return ErrEmailAlreadyUsed{emails[i].Email}
 		}
-		_, err = mail.ParseAddress(emails[i].Email)
-		if err != nil {
-			return ErrEmailInvalid{emails[i].Email}
+		if err = ValidateEmail(emails[i].Email); err != nil {
+			return err
 		}
 	}
 

routers/repo/issue.go

@@ -2392,67 +2392,34 @@ func attachmentsHTML(ctx *context.Context, attachments []*models.Attachment) str
 }
 
 func combineLabelComments(issue *models.Issue) {
-	for i := 0; i < len(issue.Comments); {
-		c := issue.Comments[i]
-		var shouldMerge bool
-		var removingCur bool
-		var prev *models.Comment
-
-		if i == 0 {
-			shouldMerge = false
-		} else {
+	for i := 0; i < len(issue.Comments); i++ {
+		var (
+			prev *models.Comment
+			cur  = issue.Comments[i]
+		)
+		if i > 0 {
 			prev = issue.Comments[i-1]
-			removingCur = c.Content != "1"
-
-			shouldMerge = prev.PosterID == c.PosterID && c.CreatedUnix-prev.CreatedUnix < 60 &&
-				c.Type == prev.Type
 		}
-
-		if c.Type == models.CommentTypeLabel {
-			if !shouldMerge {
-				if removingCur {
-					c.RemovedLabels = make([]*models.Label, 1)
-					c.AddedLabels = make([]*models.Label, 0)
-					c.RemovedLabels[0] = c.Label
+		if i == 0 || cur.Type != models.CommentTypeLabel ||
+			(prev != nil && prev.PosterID != cur.PosterID) ||
+			(prev != nil && cur.CreatedUnix-prev.CreatedUnix >= 60) {
+			if cur.Type == models.CommentTypeLabel {
+				if cur.Content != "1" {
+					cur.RemovedLabels = append(cur.RemovedLabels, cur.Label)
 				} else {
-					c.RemovedLabels = make([]*models.Label, 0)
-					c.AddedLabels = make([]*models.Label, 1)
-					c.AddedLabels[0] = c.Label
+					cur.AddedLabels = append(cur.AddedLabels, cur.Label)
 				}
-			} else {
-				// Remove duplicated "added" and "removed" labels
-				// This way, adding and immediately removing a label won't generate a comment.
-				var appendingTo *[]*models.Label
-				var other *[]*models.Label
-
-				if removingCur {
-					appendingTo = &prev.RemovedLabels
-					other = &prev.AddedLabels
-				} else {
-					appendingTo = &prev.AddedLabels
-					other = &prev.RemovedLabels
-				}
-
-				appending := true
-
-				for i := 0; i < len(*other); i++ {
-					l := (*other)[i]
-					if l.ID == c.Label.ID {
-						*other = append((*other)[:i], (*other)[i+1:]...)
-						appending = false
-						break
-					}
-				}
-
-				if appending {
-					*appendingTo = append(*appendingTo, c.Label)
-				}
-
-				prev.CreatedUnix = c.CreatedUnix
-				issue.Comments = append(issue.Comments[:i], issue.Comments[i+1:]...)
-				continue
 			}
+			continue
 		}
-		i++
+
+		if cur.Content != "1" {
+			prev.RemovedLabels = append(prev.RemovedLabels, cur.Label)
+		} else {
+			prev.AddedLabels = append(prev.AddedLabels, cur.Label)
+		}
+		prev.CreatedUnix = cur.CreatedUnix
+		issue.Comments = append(issue.Comments[:i], issue.Comments[i+1:]...)
+		i--
 	}
 }