Handle refactor (#3339)

* Replace all ctx.Handle with ctx.ServerError or ctx.NotFound

* Change Handle(403) to NotFound, avoid using macaron's NotFound

Author: thehowl

modules/context/context.go

@@ -5,7 +5,6 @@
 package context
 
 import (
-	"fmt"
 	"html"
 	"html/template"
 	"io"
@@ -92,34 +91,43 @@ func (ctx *Context) RenderWithErr(msg string, tpl base.TplName, form interface{}
 	ctx.HTML(200, tpl)
 }
 
-// Handle handles and logs error by given status.
-func (ctx *Context) Handle(status int, title string, err error) {
+// NotFound displays a 404 (Not Found) page and prints the given error, if any.
+func (ctx *Context) NotFound(title string, err error) {
 	if err != nil {
 		log.Error(4, "%s: %v", title, err)
 		if macaron.Env != macaron.PROD {
 			ctx.Data["ErrorMsg"] = err
 		}
 	}
 
-	switch status {
-	case 404:
-		ctx.Data["Title"] = "Page Not Found"
-	case 500:
-		ctx.Data["Title"] = "Internal Server Error"
+	ctx.Data["Title"] = "Page Not Found"
+	ctx.HTML(http.StatusNotFound, base.TplName("status/404"))
+}
+
+// ServerError displays a 500 (Internal Server Error) page and prints the given
+// error, if any.
+func (ctx *Context) ServerError(title string, err error) {
+	if err != nil {
+		log.Error(4, "%s: %v", title, err)
+		if macaron.Env != macaron.PROD {
+			ctx.Data["ErrorMsg"] = err
+		}
 	}
-	ctx.HTML(status, base.TplName(fmt.Sprintf("status/%d", status)))
+
+	ctx.Data["Title"] = "Internal Server Error"
+	ctx.HTML(404, base.TplName("status/500"))
 }
 
 // NotFoundOrServerError use error check function to determine if the error
 // is about not found. It responses with 404 status code for not found error,
 // or error context description for logging purpose of 500 server error.
 func (ctx *Context) NotFoundOrServerError(title string, errck func(error) bool, err error) {
 	if errck(err) {
-		ctx.Handle(404, title, err)
+		ctx.NotFound(title, err)
 		return
 	}
 
-	ctx.Handle(500, title, err)
+	ctx.ServerError(title, err)
 }
 
 // HandleText handles HTTP status code
@@ -218,7 +226,7 @@ func Contexter() macaron.Handler {
 		// If request sends files, parse them here otherwise the Query() can't be parsed and the CsrfToken will be invalid.
 		if ctx.Req.Method == "POST" && strings.Contains(ctx.Req.Header.Get("Content-Type"), "multipart/form-data") {
 			if err := ctx.Req.ParseMultipartForm(setting.AttachmentMaxSize << 20); err != nil && !strings.Contains(err.Error(), "EOF") { // 32MB max size
-				ctx.Handle(500, "ParseMultipartForm", err)
+				ctx.ServerError("ParseMultipartForm", err)
 				return
 			}
 		}

modules/context/org.go

@@ -51,9 +51,9 @@ func HandleOrgAssignment(ctx *Context, args ...bool) {
 	ctx.Org.Organization, err = models.GetUserByName(orgName)
 	if err != nil {
 		if models.IsErrUserNotExist(err) {
-			ctx.Handle(404, "GetUserByName", err)
+			ctx.NotFound("GetUserByName", err)
 		} else {
-			ctx.Handle(500, "GetUserByName", err)
+			ctx.ServerError("GetUserByName", err)
 		}
 		return
 	}
@@ -75,7 +75,7 @@ func HandleOrgAssignment(ctx *Context, args ...bool) {
 	} else if ctx.IsSigned {
 		ctx.Org.IsOwner, err = org.IsOwnedBy(ctx.User.ID)
 		if err != nil {
-			ctx.Handle(500, "IsOwnedBy", err)
+			ctx.ServerError("IsOwnedBy", err)
 			return
 		}
 
@@ -86,7 +86,7 @@ func HandleOrgAssignment(ctx *Context, args ...bool) {
 		} else {
 			ctx.Org.IsMember, err = org.IsOrgMember(ctx.User.ID)
 			if err != nil {
-				ctx.Handle(500, "IsOrgMember", err)
+				ctx.ServerError("IsOrgMember", err)
 				return
 			}
 		}
@@ -96,7 +96,7 @@ func HandleOrgAssignment(ctx *Context, args ...bool) {
 	}
 	if (requireMember && !ctx.Org.IsMember) ||
 		(requireOwner && !ctx.Org.IsOwner) {
-		ctx.Handle(404, "OrgAssignment", err)
+		ctx.NotFound("OrgAssignment", err)
 		return
 	}
 	ctx.Data["IsOrganizationOwner"] = ctx.Org.IsOwner
@@ -109,13 +109,13 @@ func HandleOrgAssignment(ctx *Context, args ...bool) {
 	if ctx.Org.IsMember {
 		if ctx.Org.IsOwner {
 			if err := org.GetTeams(); err != nil {
-				ctx.Handle(500, "GetTeams", err)
+				ctx.ServerError("GetTeams", err)
 				return
 			}
 		} else {
 			org.Teams, err = org.GetUserTeams(ctx.User.ID)
 			if err != nil {
-				ctx.Handle(500, "GetUserTeams", err)
+				ctx.ServerError("GetUserTeams", err)
 				return
 			}
 		}
@@ -135,20 +135,20 @@ func HandleOrgAssignment(ctx *Context, args ...bool) {
 		}
 
 		if !teamExists {
-			ctx.Handle(404, "OrgAssignment", err)
+			ctx.NotFound("OrgAssignment", err)
 			return
 		}
 
 		ctx.Data["IsTeamMember"] = ctx.Org.IsTeamMember
 		if requireTeamMember && !ctx.Org.IsTeamMember {
-			ctx.Handle(404, "OrgAssignment", err)
+			ctx.NotFound("OrgAssignment", err)
 			return
 		}
 
 		ctx.Org.IsTeamAdmin = ctx.Org.Team.IsOwnerTeam() || ctx.Org.Team.Authorize >= models.AccessModeAdmin
 		ctx.Data["IsTeamAdmin"] = ctx.Org.IsTeamAdmin
 		if requireTeamAdmin && !ctx.Org.IsTeamAdmin {
-			ctx.Handle(404, "OrgAssignment", err)
+			ctx.NotFound("OrgAssignment", err)
 			return
 		}
 	}