From 498d2e629abb83bd0d4d949cf35031d3cba3e3fd Mon Sep 17 00:00:00 2001 From: wemeya Date: Wed, 1 Jun 2016 12:27:26 +0800 Subject: [PATCH] change code according to review --- api/log.go | 25 ++++++++++--------------- api/user.go | 21 ++++++++++----------- dao/accesslog.go | 2 +- dao/register.go | 2 +- dao/user.go | 2 +- 5 files changed, 23 insertions(+), 29 deletions(-) diff --git a/api/log.go b/api/log.go index 3ea85cd8e..264ad8bd7 100644 --- a/api/log.go +++ b/api/log.go @@ -40,35 +40,30 @@ func (l *LogAPI) Prepare() { func (l *LogAPI) Get() { var err error startTime := l.GetString("start_time") - if len(startTime) == 0 { - startTime = "" - } else { + if len(startTime) != 0 { i, err := strconv.ParseInt(startTime, 10, 64) if err != nil { - l.CustomAbort(http.StatusInternalServerError, "Internal error") + log.Errorf("Parse startTime to int error, err: %v", err) + l.CustomAbort(http.StatusBadRequest, "startTime is not a valid integer") } startTime = time.Unix(i, 0).String() } endTime := l.GetString("end_time") - if len(endTime) == 0 { - endTime = "" - } else { + if len(endTime) != 0 { j, err := strconv.ParseInt(endTime, 10, 64) if err != nil { - l.CustomAbort(http.StatusInternalServerError, "Internal error") + log.Errorf("Parse endTime to int error, err: %v", err) + l.CustomAbort(http.StatusBadRequest, "endTime is not a valid integer") } endTime = time.Unix(j, 0).String() } var linesNum int lines := l.GetString("lines") - if len(lines) == 0 { - linesNum = 0 - if startTime == "" || endTime == "" { - linesNum = 10 - } - } else { + if len(lines) == 0 && len(startTime) == 0 && len(endTime) == 0 { + linesNum = 10 + } else if len(lines) != 0 { linesNum, err = strconv.Atoi(lines) if err != nil { log.Errorf("Get parameters error--lines, err: %v", err) @@ -79,8 +74,8 @@ func (l *LogAPI) Get() { var logList []models.AccessLog logList, err = dao.GetRecentLogs(l.userID, linesNum, startTime, endTime) if err != nil { + log.Errorf("Get recent logs error, err: %v", err) l.CustomAbort(http.StatusInternalServerError, "Internal error") - return } l.Data["json"] = logList l.ServeJSON() diff --git a/api/user.go b/api/user.go index 5649a290f..74daab862 100644 --- a/api/user.go +++ b/api/user.go @@ -143,7 +143,7 @@ func (ua *UserAPI) Put() { } if !ua.IsAdmin { if ua.userID != ua.currentUserID { - log.Error("Guests can only change their own account.") + log.Warning("Guests can only change their own account.") ua.CustomAbort(http.StatusForbidden, "Guests can only change their own account.") } } @@ -151,19 +151,19 @@ func (ua *UserAPI) Put() { ua.DecodeJSONReq(&user) err := commonValidate(user) if err != nil { - log.Errorf("Bad request in change user profile: %v", err) + log.Warning("Bad request in change user profile: %v", err) ua.RenderError(http.StatusBadRequest, "change user profile error:"+err.Error()) return } emailExist, err := dao.UserExists(user, "email") if err != nil { log.Errorf("Error occurred in change user profile: %v", err) - ua.RenderError(http.StatusInternalServerError, "Internal error.") - return + ua.CustomAbort(http.StatusInternalServerError, "Internal error.") } if emailExist { log.Warning("email has already been used!") - ua.CustomAbort(http.StatusForbidden, "email has already been used!") + ua.RenderError(http.StatusConflict, "email has already been used!") + return } if err := dao.ChangeUserProfile(user); err != nil { log.Errorf("Failed to update user profile, error: %v", err) @@ -187,25 +187,24 @@ func (ua *UserAPI) Post() { ua.DecodeJSONReq(&user) err := validate(user) if err != nil { - log.Errorf("Bad request in Register: %v", err) + log.Warning("Bad request in Register: %v", err) ua.RenderError(http.StatusBadRequest, "register error:"+err.Error()) return } userExist, err := dao.UserExists(user, "username") if err != nil { log.Errorf("Error occurred in Register: %v", err) - ua.RenderError(http.StatusInternalServerError, "Internal error.") - return + ua.CustomAbort(http.StatusInternalServerError, "Internal error.") } if userExist { log.Warning("username has already been used!") - ua.CustomAbort(http.StatusForbidden, "username has already been used!") + ua.RenderError(http.StatusConflict, "username has already been used!") + return } userID, err := dao.Register(user) if err != nil { log.Errorf("Error occurred in Register: %v", err) - ua.RenderError(http.StatusInternalServerError, "Internal error.") - return + ua.CustomAbort(http.StatusInternalServerError, "Internal error.") } ua.Redirect(http.StatusCreated, strconv.FormatInt(userID, 10)) diff --git a/dao/accesslog.go b/dao/accesslog.go index 56f591eb8..f50ad1f40 100644 --- a/dao/accesslog.go +++ b/dao/accesslog.go @@ -121,7 +121,7 @@ func GetRecentLogs(userID, linesNum int, startTime, endTime string) ([]models.Ac var recentLogList []models.AccessLog queryParam := make([]interface{}, 1) - sql := "select log_id, access_log.user_id, project_id, repo_name, repo_tag, GUID, operation, op_time, username from access_log left join user on access_log.user_id=user.user_id where project_id in (select distinct project_id from access_log where user_id = ?)" + sql := "select log_id, access_log.user_id, project_id, repo_name, repo_tag, GUID, operation, op_time, username from access_log left join user on access_log.user_id=user.user_id where project_id in (select distinct project_id from project_member where user_id = ?)" queryParam = append(queryParam, userID) if startTime != "" { sql += " and op_time >= ?" diff --git a/dao/register.go b/dao/register.go index 5f1bfb04c..852a52f04 100644 --- a/dao/register.go +++ b/dao/register.go @@ -26,7 +26,7 @@ import ( // Register is used for user to register, the password is encrypted before the record is inserted into database. func Register(user models.User) (int64, error) { //when register from ldap, email may be empty - if user.Email == "" || len(user.Email) == 0 { + if user.Email == "" { user.Email = user.Username + "@vmware.com" } o := GetOrmer() diff --git a/dao/user.go b/dao/user.go index 2be121a96..d265065f6 100644 --- a/dao/user.go +++ b/dao/user.go @@ -233,7 +233,7 @@ func DeleteUser(userID int) error { // ChangeUserProfile ... func ChangeUserProfile(user models.User) error { //email is null is permitted - if user.Email == "" || len(user.Email) == 0 { + if user.Email == "" { user.Email = user.Username + "@vmware.com" } o := GetOrmer()