From f7617997cebd0f347415da47545fbf16e5d0653e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Justin=20Nu=C3=9F?= Date: Fri, 25 Jul 2014 10:18:42 +0200 Subject: [PATCH 1/4] Ignore EOF error when parsing form. --- modules/middleware/context.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/middleware/context.go b/modules/middleware/context.go index cf849802d..90cfb1bb0 100644 --- a/modules/middleware/context.go +++ b/modules/middleware/context.go @@ -369,8 +369,8 @@ func InitContext() martini.Handler { } // If request sends files, parse them here otherwise the Query() can't be parsed and the CsrfToken will be invalid. - if strings.Contains(r.Header.Get("Content-Type"), "multipart/form-data") { - if err = ctx.Req.ParseMultipartForm(setting.AttachmentMaxSize << 20); err != nil { // 32MB max size + if r.Method == "POST" && strings.Contains(r.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, "issue.Comment(ctx.Req.ParseMultipartForm)", err) return } From 4e2477a1a5ae59ff4cb34d58eab74297b4ea2d24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Justin=20Nu=C3=9F?= Date: Fri, 25 Jul 2014 10:47:37 +0200 Subject: [PATCH 2/4] Fix #318. Switch to JS(ON) implementation for issue/comment creation. --- public/js/app.js | 79 ++++++++++++++++++++++++-- routers/repo/issue.go | 96 +++++++++++++++++++++----------- templates/repo/issue/create.tmpl | 5 +- templates/repo/issue/view.tmpl | 5 +- 4 files changed, 147 insertions(+), 38 deletions(-) diff --git a/public/js/app.js b/public/js/app.js index 62482965e..a88c8f6bc 100644 --- a/public/js/app.js +++ b/public/js/app.js @@ -579,26 +579,97 @@ function initIssue() { var $attachedList = $("#attached-list"); var $addButton = $("#attachments-button"); + var files = []; + var fileInput = document.getElementById("attachments-input"); if (fileInput === null) { return; } - fileInput.addEventListener("change", function(event) { - $attachedList.empty(); - $attachedList.append("Attachments: "); + $attachedList.on("click", "span.attachment-remove", function(event) { + var $parent = $(this).parent(); + + files.splice($parent.data("index"), 1); + $parent.remove(); + }); + + var clickedButton = undefined; + + $("button,input[type=\"submit\"]", fileInput.form).on("click", function() { + clickedButton = this; + }); + + fileInput.form.addEventListener("submit", function(event) { + event.stopImmediatePropagation(); + event.preventDefault(); + + //var data = new FormData(this); + + // Internet Explorer ... -_- + var data = new FormData(); + + $.each($("[name]", this), function(i, e) { + if (e.name == "attachments" || e.type == "submit") { + return; + } + + data.append(e.name, $(e).val()); + }); + + data.append(clickedButton.name, $(clickedButton).val()); + + files.forEach(function(file) { + data.append("attachments", file); + }); + + var xhr = new XMLHttpRequest(); + + xhr.addEventListener("error", function() { + debugger; + }); + + xhr.addEventListener("load", function() { + if (xhr.response.ok === false) { + $("#submit-error").text(xhr.response.error); + return; + } + + window.location.href = xhr.response.data; + }); + + xhr.responseType = "json"; + + xhr.open("POST", this.action, true); + xhr.send(data); + + return false; + }); + + fileInput.addEventListener("change", function(event) { for (var index = 0; index < fileInput.files.length; index++) { var file = fileInput.files[index]; + if (files.indexOf(file) > -1) { + continue; + } + var $span = $(""); $span.addClass("label"); $span.addClass("label-default"); - $span.append(file.name.toLowerCase()); + $span.data("index", files.length); + + $span.append(file.name); + $span.append(" "); + $attachedList.append($span); + + files.push(file); } + + this.value = ""; }); $addButton.on("click", function() { diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 0f03cea57..edfa39b02 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -188,33 +188,45 @@ func CreateIssue(ctx *middleware.Context, params martini.Params) { } func CreateIssuePost(ctx *middleware.Context, params martini.Params, form auth.CreateIssueForm) { - ctx.Data["Title"] = "Create issue" - ctx.Data["IsRepoToolbarIssues"] = true - ctx.Data["IsRepoToolbarIssuesList"] = false - ctx.Data["AttachmentsEnabled"] = setting.AttachmentEnabled + send := func(status int, data interface{}, err error) { + log.Error("issue.Comment(?): %s", err) + + if err != nil { + ctx.JSON(status, map[string]interface{}{ + "ok": false, + "status": status, + "error": err.Error(), + }) + } else { + ctx.JSON(status, map[string]interface{}{ + "ok": true, + "status": status, + "data": data, + }) + } + } var err error // Get all milestones. - ctx.Data["OpenMilestones"], err = models.GetMilestones(ctx.Repo.Repository.Id, false) + _, err = models.GetMilestones(ctx.Repo.Repository.Id, false) if err != nil { - ctx.Handle(500, "issue.ViewIssue(GetMilestones.1): %v", err) + send(500, nil, err) return } - ctx.Data["ClosedMilestones"], err = models.GetMilestones(ctx.Repo.Repository.Id, true) + _, err = models.GetMilestones(ctx.Repo.Repository.Id, true) if err != nil { - ctx.Handle(500, "issue.ViewIssue(GetMilestones.2): %v", err) + send(500, nil, err) return } - us, err := models.GetCollaborators(strings.TrimPrefix(ctx.Repo.RepoLink, "/")) + _, err = models.GetCollaborators(strings.TrimPrefix(ctx.Repo.RepoLink, "/")) if err != nil { - ctx.Handle(500, "issue.CreateIssue(GetCollaborators)", err) + send(500, nil, err) return } - ctx.Data["Collaborators"] = us if ctx.HasError() { - ctx.HTML(200, ISSUE_CREATE) + send(400, nil, errors.New(ctx.Flash.ErrorMsg)) return } @@ -233,11 +245,11 @@ func CreateIssuePost(ctx *middleware.Context, params martini.Params, form auth.C Content: form.Content, } if err := models.NewIssue(issue); err != nil { - ctx.Handle(500, "issue.CreateIssue(NewIssue)", err) + send(500, nil, err) return } else if err := models.NewIssueUserPairs(issue.RepoId, issue.Id, ctx.Repo.Owner.Id, ctx.User.Id, form.AssigneeId, ctx.Repo.Repository.Name); err != nil { - ctx.Handle(500, "issue.CreateIssue(NewIssueUserPairs)", err) + send(500, nil, err) return } @@ -253,7 +265,7 @@ func CreateIssuePost(ctx *middleware.Context, params martini.Params, form auth.C } if err := models.UpdateMentions(ms, issue.Id); err != nil { - ctx.Handle(500, "issue.CreateIssue(UpdateMentions)", err) + send(500, nil, err) return } } @@ -272,7 +284,7 @@ func CreateIssuePost(ctx *middleware.Context, params martini.Params, form auth.C } // Notify watchers. if err := models.NotifyWatchers(act); err != nil { - ctx.Handle(500, "issue.CreateIssue(NotifyWatchers)", err) + send(500, nil, err) return } @@ -280,7 +292,7 @@ func CreateIssuePost(ctx *middleware.Context, params martini.Params, form auth.C if setting.Service.EnableNotifyMail { tos, err := mailer.SendIssueNotifyMail(ctx.User, ctx.Repo.Owner, ctx.Repo.Repository, issue) if err != nil { - ctx.Handle(500, "issue.CreateIssue(SendIssueNotifyMail)", err) + send(500, nil, err) return } @@ -295,13 +307,13 @@ func CreateIssuePost(ctx *middleware.Context, params martini.Params, form auth.C } if err = mailer.SendIssueMentionMail(ctx.Render, ctx.User, ctx.Repo.Owner, ctx.Repo.Repository, issue, models.GetUserEmailsByNames(newTos)); err != nil { - ctx.Handle(500, "issue.CreateIssue(SendIssueMentionMail)", err) + send(500, nil, err) return } } log.Trace("%d Issue created: %d", ctx.Repo.Repository.Id, issue.Id) - ctx.Redirect(fmt.Sprintf("/%s/%s/issues/%d", params["username"], params["reponame"], issue.Index)) + send(200, fmt.Sprintf("/%s/%s/issues/%d", params["username"], params["reponame"], issue.Index), nil) } func checkLabels(labels, allLabels []*models.Label) { @@ -698,19 +710,38 @@ func uploadFiles(ctx *middleware.Context, issueId, commentId int64) { } func Comment(ctx *middleware.Context, params martini.Params) { + send := func(status int, data interface{}, err error) { + log.Error("issue.Comment(?): %s", err) + + if err != nil { + ctx.JSON(status, map[string]interface{}{ + "ok": false, + "status": status, + "error": err.Error(), + }) + } else { + ctx.JSON(status, map[string]interface{}{ + "ok": true, + "status": status, + "data": data, + }) + } + } + index, err := base.StrTo(ctx.Query("issueIndex")).Int64() if err != nil { - ctx.Handle(404, "issue.Comment(get index)", err) + send(404, nil, err) return } issue, err := models.GetIssueByIndex(ctx.Repo.Repository.Id, index) if err != nil { if err == models.ErrIssueNotExist { - ctx.Handle(404, "issue.Comment", err) + send(404, nil, err) } else { - ctx.Handle(200, "issue.Comment(get issue)", err) + send(200, nil, err) } + return } @@ -724,17 +755,17 @@ func Comment(ctx *middleware.Context, params martini.Params) { (strings.Contains(newStatus, "Close") && !issue.IsClosed) { issue.IsClosed = !issue.IsClosed if err = models.UpdateIssue(issue); err != nil { - ctx.Handle(500, "issue.Comment(UpdateIssue)", err) + send(500, nil, err) return } else if err = models.UpdateIssueUserPairsByStatus(issue.Id, issue.IsClosed); err != nil { - ctx.Handle(500, "issue.Comment(UpdateIssueUserPairsByStatus)", err) + send(500, nil, err) return } // Change open/closed issue counter for the associated milestone if issue.MilestoneId > 0 { if err = models.ChangeMilestoneIssueStats(issue); err != nil { - ctx.Handle(500, "issue.Comment(ChangeMilestoneIssueStats)", err) + send(500, nil, err) } } @@ -744,7 +775,7 @@ func Comment(ctx *middleware.Context, params martini.Params) { } if _, err = models.CreateComment(ctx.User.Id, ctx.Repo.Repository.Id, issue.Id, 0, 0, cmtType, "", nil); err != nil { - ctx.Handle(200, "issue.Comment(create status change comment)", err) + send(200, nil, err) return } log.Trace("%s Issue(%d) status changed: %v", ctx.Req.RequestURI, issue.Id, !issue.IsClosed) @@ -760,7 +791,7 @@ func Comment(ctx *middleware.Context, params martini.Params) { switch params["action"] { case "new": if comment, err = models.CreateComment(ctx.User.Id, ctx.Repo.Repository.Id, issue.Id, 0, 0, models.COMMENT, content, nil); err != nil { - ctx.Handle(500, "issue.Comment(create comment)", err) + send(500, nil, err) return } @@ -772,7 +803,7 @@ func Comment(ctx *middleware.Context, params martini.Params) { } if err := models.UpdateMentions(ms, issue.Id); err != nil { - ctx.Handle(500, "issue.CreateIssue(UpdateMentions)", err) + send(500, nil, err) return } } @@ -800,7 +831,7 @@ func Comment(ctx *middleware.Context, params martini.Params) { RepoName: ctx.Repo.Repository.LowerName, } if err = models.NotifyWatchers(act); err != nil { - ctx.Handle(500, "issue.CreateIssue(NotifyWatchers)", err) + send(500, nil, err) return } @@ -809,7 +840,7 @@ func Comment(ctx *middleware.Context, params martini.Params) { issue.Content = content tos, err := mailer.SendIssueNotifyMail(ctx.User, ctx.Repo.Owner, ctx.Repo.Repository, issue) if err != nil { - ctx.Handle(500, "issue.Comment(SendIssueNotifyMail)", err) + send(500, nil, err) return } @@ -824,12 +855,13 @@ func Comment(ctx *middleware.Context, params martini.Params) { } if err = mailer.SendIssueMentionMail(ctx.Render, ctx.User, ctx.Repo.Owner, ctx.Repo.Repository, issue, models.GetUserEmailsByNames(newTos)); err != nil { - ctx.Handle(500, "issue.Comment(SendIssueMentionMail)", err) + send(500, nil, err) return } } - ctx.Redirect(fmt.Sprintf("%s/issues/%d", ctx.Repo.RepoLink, index)) + log.Error("url: %#v", fmt.Sprintf("%s/issues/%d", ctx.Repo.RepoLink, index)) + send(200, fmt.Sprintf("%s/issues/%d", ctx.Repo.RepoLink, index), nil) } func NewLabel(ctx *middleware.Context, form auth.CreateLabelForm) { diff --git a/templates/repo/issue/create.tmpl b/templates/repo/issue/create.tmpl index 770584170..ad86d4eaf 100644 --- a/templates/repo/issue/create.tmpl +++ b/templates/repo/issue/create.tmpl @@ -95,6 +95,7 @@
+
@@ -103,7 +104,9 @@
{{if .AttachmentsEnabled}}
-
+
+ Attachments: +
{{end}}
diff --git a/templates/repo/issue/view.tmpl b/templates/repo/issue/view.tmpl index 247931c4a..b5d722819 100644 --- a/templates/repo/issue/view.tmpl +++ b/templates/repo/issue/view.tmpl @@ -134,6 +134,7 @@
+
@@ -143,7 +144,9 @@
{{if .AttachmentsEnabled}}
-
+
+ Attachments: +
{{end}}
From 12fb42de5a5b07ed8dffe91d9536615bbadeedea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Justin=20Nu=C3=9F?= Date: Fri, 25 Jul 2014 11:13:42 +0200 Subject: [PATCH 3/4] Fix IE bug and show errors. --- public/css/gogs.css | 7 +++++++ public/js/app.js | 36 ++++++++++++++++++++++++++------ routers/repo/issue.go | 9 ++++---- templates/repo/issue/create.tmpl | 2 +- templates/repo/issue/view.tmpl | 2 +- 5 files changed, 43 insertions(+), 13 deletions(-) diff --git a/public/css/gogs.css b/public/css/gogs.css index cc48f211f..361475bd4 100755 --- a/public/css/gogs.css +++ b/public/css/gogs.css @@ -1836,4 +1836,11 @@ body { #issue-create-form #attached { margin-bottom: 0; +} + +#submit-error { + display: none; + padding: 10px 15px 15px 15px; + font-weight: bold; + text-align: center; } \ No newline at end of file diff --git a/public/js/app.js b/public/js/app.js index a88c8f6bc..b0dff0efc 100644 --- a/public/js/app.js +++ b/public/js/app.js @@ -568,7 +568,7 @@ function initIssue() { }; var out = function() { - $hoverElement.hide(); + //$hoverElement.hide(); }; $(".issue-main .attachments .attachment").hover(over, out); @@ -598,6 +598,13 @@ function initIssue() { $("button,input[type=\"submit\"]", fileInput.form).on("click", function() { clickedButton = this; + + var $button = $(this); + + $button.removeClass("btn-success"); + $button.addClass("btn-warning"); + + $button.text("Submiting..."); }); fileInput.form.addEventListener("submit", function(event) { @@ -630,16 +637,33 @@ function initIssue() { }); xhr.addEventListener("load", function() { - if (xhr.response.ok === false) { - $("#submit-error").text(xhr.response.error); + var response = xhr.response; + + if (typeof response == "string") { + try { + response = JSON.parse(response); + } catch (err) { + response = { ok: false, error: "Could not parse JSON" }; + } + } + + if (response.ok === false) { + $("#submit-error").text(response.error); + $("#submit-error").show(); + + var $button = $(clickedButton); + + $button.removeClass("btn-warning"); + $button.addClass("btn-danger"); + + $button.text("An error encoured!") + return; } - window.location.href = xhr.response.data; + window.location.href = response.data; }); - xhr.responseType = "json"; - xhr.open("POST", this.action, true); xhr.send(data); diff --git a/routers/repo/issue.go b/routers/repo/issue.go index edfa39b02..4465a3995 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -189,9 +189,9 @@ func CreateIssue(ctx *middleware.Context, params martini.Params) { func CreateIssuePost(ctx *middleware.Context, params martini.Params, form auth.CreateIssueForm) { send := func(status int, data interface{}, err error) { - log.Error("issue.Comment(?): %s", err) - if err != nil { + log.Error("issue.CreateIssuePost(?): %s", err.Error()) + ctx.JSON(status, map[string]interface{}{ "ok": false, "status": status, @@ -711,9 +711,9 @@ func uploadFiles(ctx *middleware.Context, issueId, commentId int64) { func Comment(ctx *middleware.Context, params martini.Params) { send := func(status int, data interface{}, err error) { - log.Error("issue.Comment(?): %s", err) - if err != nil { + log.Error("issue.Comment(?): %s", err.Error()) + ctx.JSON(status, map[string]interface{}{ "ok": false, "status": status, @@ -860,7 +860,6 @@ func Comment(ctx *middleware.Context, params martini.Params) { } } - log.Error("url: %#v", fmt.Sprintf("%s/issues/%d", ctx.Repo.RepoLink, index)) send(200, fmt.Sprintf("%s/issues/%d", ctx.Repo.RepoLink, index), nil) } diff --git a/templates/repo/issue/create.tmpl b/templates/repo/issue/create.tmpl index ad86d4eaf..c01888427 100644 --- a/templates/repo/issue/create.tmpl +++ b/templates/repo/issue/create.tmpl @@ -95,7 +95,7 @@
-
+
diff --git a/templates/repo/issue/view.tmpl b/templates/repo/issue/view.tmpl index b5d722819..aec50ca62 100644 --- a/templates/repo/issue/view.tmpl +++ b/templates/repo/issue/view.tmpl @@ -134,7 +134,7 @@
-
+
From f0da8a68c2c409572902275888fec26689c2b0dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Justin=20Nu=C3=9F?= Date: Fri, 25 Jul 2014 15:47:42 +0200 Subject: [PATCH 4/4] Remove debug comment. --- public/js/app.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/js/app.js b/public/js/app.js index b0dff0efc..d7208119b 100644 --- a/public/js/app.js +++ b/public/js/app.js @@ -568,7 +568,7 @@ function initIssue() { }; var out = function() { - //$hoverElement.hide(); + $hoverElement.hide(); }; $(".issue-main .attachments .attachment").hover(over, out);