From 389a6037d57ca6d145d1a5a2ee0a308078dd30d0 Mon Sep 17 00:00:00 2001
From: wemeya <wemeya@163.com>
Date: Thu, 2 Jun 2016 17:33:10 +0800
Subject: [PATCH] change code as the review reqired

---
 api/project.go    |  2 +-
 api/user.go       |  9 +++++----
 auth/ldap/ldap.go |  2 +-
 dao/accesslog.go  |  2 +-
 dao/user.go       | 11 ++++++-----
 ui/router.go      |  4 ++--
 6 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/api/project.go b/api/project.go
index 84cde81bc..5a4f9ba8b 100644
--- a/api/project.go
+++ b/api/project.go
@@ -199,7 +199,7 @@ func (p *ProjectAPI) List() {
 	p.ServeJSON()
 }
 
-// ToggleProjectPublic handles request POST /api/projects/:id/toggle_project_public
+// ToggleProjectPulic handles request PUT /api/projects/:id/publicity
 func (p *ProjectAPI) ToggleProjectPublic() {
 	p.userID = p.ValidateUser()
 	var req projectReq
diff --git a/api/user.go b/api/user.go
index 42445f866..93e398d28 100644
--- a/api/user.go
+++ b/api/user.go
@@ -280,7 +280,7 @@ func (ua *UserAPI) ChangePassword() {
 	}
 }
 
-// ToggleUserAdminRole handles Post api/users/{}/toggleadmin
+// ToggleUserAdminRole handles PUT api/users/{}/toggleadmin
 func (ua *UserAPI) ToggleUserAdminRole() {
 	if !ua.IsAdmin {
 		log.Warningf("current user, id: %d does not have admin role, can not update other user's role", ua.currentUserID)
@@ -288,12 +288,13 @@ func (ua *UserAPI) ToggleUserAdminRole() {
 		return
 	}
 	userQuery := models.User{UserID: ua.userID}
-	if err := dao.ToggleUserAdminRole(userQuery); err != nil {
+	ua.DecodeJSONReq(&userQuery)
+	if err := dao.ToggleUserAdminRole(userQuery.UserID,userQuery.HasAdminRole); err != nil {
 		log.Errorf("Error occurred in ToggleUserAdminRole: %v", err)
 		ua.CustomAbort(http.StatusInternalServerError, "Internal error.")
 	}
 }
-
+// validate only validate when user register
 func validate(user models.User) error {
 
 	if isIllegalLength(user.Username, 0, 20) {
@@ -311,7 +312,7 @@ func validate(user models.User) error {
 	return nil
 }
 
-//commonValidate validates information when user register or change their profile
+//commonValidate validates email, realname, comment information when user register or change their profile
 func commonValidate(user models.User) error {
 
 	if len(user.Email) > 0 {
diff --git a/auth/ldap/ldap.go b/auth/ldap/ldap.go
index d6710e97c..6929147d1 100644
--- a/auth/ldap/ldap.go
+++ b/auth/ldap/ldap.go
@@ -112,7 +112,7 @@ func (l *Auth) Authenticate(m models.AuthModel) (*models.User, error) {
 		u.Password = "12345678AbC"
 		u.Comment = "registered from LDAP."
 		if u.Email == "" {
-			u.Email = u.Username + "@vmware.com"
+			u.Email = u.Username + "@placeholder.com"
 		}
 		userID, err := dao.Register(u)
 		if err != nil {
diff --git a/dao/accesslog.go b/dao/accesslog.go
index 3c1883fcc..0e0174fb1 100644
--- a/dao/accesslog.go
+++ b/dao/accesslog.go
@@ -17,7 +17,7 @@ package dao
 
 import (
 	"strings"
-
+	"github.com/astaxie/beego/orm"
 	"github.com/vmware/harbor/models"
 	"github.com/vmware/harbor/utils/log"
 )
diff --git a/dao/user.go b/dao/user.go
index 9a4a8a0c1..673f684b0 100644
--- a/dao/user.go
+++ b/dao/user.go
@@ -109,12 +109,13 @@ func ListUsers(query models.User) ([]models.User, error) {
 }
 
 // ToggleUserAdminRole gives a user admin role.
-func ToggleUserAdminRole(u models.User) error {
+func ToggleUserAdminRole(userID, hasAdmin int) error {
 	o := GetOrmer()
-
-	sql := `update user set sysadmin_flag = not sysadmin_flag where user_id = ?`
-
-	r, err := o.Raw(sql, u.UserID).Exec()
+        queryParams := make([]interface{}, 1)
+	sql := `update user set sysadmin_flag = ? where user_id = ?`
+	queryParams = append(queryParams, hasAdmin)
+	queryParams = append(queryParams, userID)
+	r, err := o.Raw(sql, queryParams).Exec()
 	if err != nil {
 		return err
 	}
diff --git a/ui/router.go b/ui/router.go
index dabcfae3d..02ccfed9f 100644
--- a/ui/router.go
+++ b/ui/router.go
@@ -55,12 +55,12 @@ func initRouters() {
 	beego.Router("/api/projects/:pid/members/?:mid", &api.ProjectMemberAPI{})
 	beego.Router("/api/projects/", &api.ProjectAPI{}, "get:List")
 	beego.Router("/api/projects/?:id", &api.ProjectAPI{})
-	beego.Router("/api/projects/:id/publicity", &api.ProjectAPI{}, "post:ToggleProjectPublic")
+	beego.Router("/api/projects/:id/publicity", &api.ProjectAPI{}, "put:ToggleProjectPublic")
 	beego.Router("/api/statistics", &api.StatisticAPI{})
 	beego.Router("/api/projects/:id/logs/filter", &api.ProjectAPI{}, "post:FilterAccessLog")
 	beego.Router("/api/users/?:id", &api.UserAPI{})
 	beego.Router("/api/users/:id/password", &api.UserAPI{}, "put:ChangePassword")
-	beego.Router("/api/users/:id/sysadmin", &api.UserAPI{}, "post:ToggleUserAdminRole")
+	beego.Router("/api/users/:id/sysadmin", &api.UserAPI{}, "put:ToggleUserAdminRole")
 	beego.Router("/api/repositories", &api.RepositoryAPI{})
 	beego.Router("/api/repositories/tags", &api.RepositoryAPI{}, "get:GetTags")
 	beego.Router("/api/repositories/manifests", &api.RepositoryAPI{}, "get:GetManifests")