From 210fa4ee2de4647eaafc43ff2e13cfe3ab6312e9 Mon Sep 17 00:00:00 2001 From: Matthew Knight Date: Sat, 14 Feb 2026 14:11:05 -0800 Subject: [PATCH] attachment fixes --- internal/forgejo/client.go | 2 + internal/handlers/public/routes.go | 3 +- internal/handlers/public/tickets.go | 174 +++++++++++------------- web/static/css/input.css | 11 ++ web/templates/pages/tickets/detail.html | 5 +- 5 files changed, 99 insertions(+), 96 deletions(-) diff --git a/internal/forgejo/client.go b/internal/forgejo/client.go index 8ebd565..d17e5af 100644 --- a/internal/forgejo/client.go +++ b/internal/forgejo/client.go @@ -141,6 +141,7 @@ type TimelineView struct { EventText string CreatedAt time.Time Attachments []Attachment + CommentID int64 // needed so templates can generate comment-asset download URLs } // RelatedIssue represents a cross-referenced issue with visibility info. @@ -256,6 +257,7 @@ func BuildTimelineViews(events []TimelineEvent, botLogin string, isAdmin bool) [ IsTeam: !isCustomer, CreatedAt: e.CreatedAt, Attachments: e.Assets, + CommentID: e.ID, }) case "close": views = append(views, TimelineView{ diff --git a/internal/handlers/public/routes.go b/internal/handlers/public/routes.go index f560013..85a90d0 100644 --- a/internal/handlers/public/routes.go +++ b/internal/handlers/public/routes.go @@ -77,7 +77,8 @@ func NewRouter(deps Dependencies) *gin.Engine { authenticated.POST("/tickets", ticketHandler.Create) authenticated.GET("/tickets/:id", ticketHandler.Detail) authenticated.POST("/tickets/:id/comments", ticketHandler.AddComment) - authenticated.GET("/tickets/:id/attachments/:attachmentId/*filename", ticketHandler.DownloadAttachment) + authenticated.GET("/tickets/:id/assets/:attachmentId/*filename", ticketHandler.DownloadIssueAttachment) + authenticated.GET("/tickets/:id/comments/:commentId/assets/:attachmentId/*filename", ticketHandler.DownloadCommentAttachment) } } diff --git a/internal/handlers/public/tickets.go b/internal/handlers/public/tickets.go index 82f52d3..7e109b0 100644 --- a/internal/handlers/public/tickets.go +++ b/internal/handlers/public/tickets.go @@ -137,7 +137,20 @@ func (h *TicketHandler) NewForm(c *gin.Context) { func (h *TicketHandler) Create(c *gin.Context) { user := auth.CurrentUser(c) - repoID, err := uuid.Parse(c.PostForm("repo_id")) + // Parse multipart form first (ensures files are available) + form, err := c.MultipartForm() + if err != nil { + log.Error().Err(err).Msg("parse multipart form error") + } + + getField := func(name string) string { + if form != nil && form.Value[name] != nil { + return form.Value[name][0] + } + return "" + } + + repoID, err := uuid.Parse(getField("repo_id")) if err != nil { h.deps.Renderer.RenderError(c.Writer, c.Request, http.StatusBadRequest, "Invalid product selection") return @@ -150,8 +163,8 @@ func (h *TicketHandler) Create(c *gin.Context) { return } - title := c.PostForm("title") - description := c.PostForm("description") + title := getField("title") + description := getField("description") if title == "" || description == "" { var repos []models.Repo @@ -198,7 +211,6 @@ func (h *TicketHandler) Create(c *gin.Context) { } // Upload attachments if any - form, _ := c.MultipartForm() if form != nil && form.File["attachments"] != nil { for _, fh := range form.File["attachments"] { f, err := fh.Open() @@ -398,10 +410,17 @@ func (h *TicketHandler) AddComment(c *gin.Context) { return } - body := c.PostForm("body") + // Parse multipart form first (ensures files are available) + form, err := c.MultipartForm() + if err != nil { + log.Error().Err(err).Msg("parse multipart form error") + } + + body := "" + if form != nil && form.Value["body"] != nil { + body = form.Value["body"][0] + } - // Check if there are attachments - form, _ := c.MultipartForm() hasAttachments := form != nil && len(form.File["attachments"]) > 0 if body == "" && !hasAttachments { @@ -450,119 +469,88 @@ func (h *TicketHandler) AddComment(c *gin.Context) { c.Redirect(http.StatusSeeOther, "/tickets/"+ticket.ID.String()) } -// DownloadAttachment proxies an attachment download from Forgejo. -func (h *TicketHandler) DownloadAttachment(c *gin.Context) { +// verifyTicketOwnership validates ticket access and returns the ticket and repo. +func (h *TicketHandler) verifyTicketOwnership(c *gin.Context) (*models.Ticket, *models.Repo, bool) { user := auth.CurrentUser(c) ticketID, err := uuid.Parse(c.Param("id")) if err != nil { h.deps.Renderer.RenderError(c.Writer, c.Request, http.StatusBadRequest, "Invalid ticket ID") - return + return nil, nil, false } var ticket models.Ticket if err := h.deps.DB.First(&ticket, "id = ?", ticketID).Error; err != nil { h.deps.Renderer.RenderError(c.Writer, c.Request, http.StatusNotFound, "Ticket not found") - return + return nil, nil, false } if ticket.UserID != user.ID { h.deps.Renderer.RenderError(c.Writer, c.Request, http.StatusForbidden, "Access denied") + return nil, nil, false + } + + var repo models.Repo + h.deps.DB.First(&repo, "id = ?", ticket.RepoID) + + return &ticket, &repo, true +} + +// proxyAssetDownload fetches an asset from Forgejo API and streams it to the client. +func (h *TicketHandler) proxyAssetDownload(c *gin.Context, assetURL, filename string) { + resp, err := h.deps.ForgejoClient.ProxyDownload(assetURL) + if err != nil { + log.Error().Err(err).Str("url", assetURL).Msg("proxy attachment download error") + h.deps.Renderer.RenderError(c.Writer, c.Request, http.StatusBadGateway, "Failed to download file") + return + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + h.deps.Renderer.RenderError(c.Writer, c.Request, resp.StatusCode, "Failed to download file") + return + } + + contentType := resp.Header.Get("Content-Type") + if contentType == "" { + contentType = "application/octet-stream" + } + c.Header("Content-Type", contentType) + c.Header("Content-Disposition", "attachment; filename=\""+filename+"\"") + if cl := resp.Header.Get("Content-Length"); cl != "" { + c.Header("Content-Length", cl) + } + c.Status(http.StatusOK) + io.Copy(c.Writer, resp.Body) +} + +// DownloadIssueAttachment proxies an issue-level attachment download via Forgejo API. +func (h *TicketHandler) DownloadIssueAttachment(c *gin.Context) { + ticket, repo, ok := h.verifyTicketOwnership(c) + if !ok { return } attachmentID := c.Param("attachmentId") filename := c.Param("filename") - var repo models.Repo - h.deps.DB.First(&repo, "id = ?", ticket.RepoID) + assetURL := h.deps.ForgejoClient.BaseURL() + "/api/v1/repos/" + repo.ForgejoOwner + "/" + repo.ForgejoRepo + "/issues/" + strconv.FormatInt(ticket.ForgejoIssueNumber, 10) + "/assets/" + attachmentID - // Build the Forgejo download URL - downloadURL := h.deps.ForgejoClient.BaseURL() + "/attachments/" + attachmentID - - resp, err := h.deps.ForgejoClient.ProxyDownload(downloadURL) - if err != nil { - log.Error().Err(err).Msg("proxy download error") - h.deps.Renderer.RenderError(c.Writer, c.Request, http.StatusBadGateway, "Failed to download file") - return - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - h.deps.Renderer.RenderError(c.Writer, c.Request, resp.StatusCode, "Failed to download file") - return - } - - // Forward content type and set download headers - contentType := resp.Header.Get("Content-Type") - if contentType == "" { - contentType = "application/octet-stream" - } - c.Header("Content-Type", contentType) - c.Header("Content-Disposition", "attachment; filename=\""+filename+"\"") - if cl := resp.Header.Get("Content-Length"); cl != "" { - c.Header("Content-Length", cl) - } - c.Status(http.StatusOK) - io.Copy(c.Writer, resp.Body) + h.proxyAssetDownload(c, assetURL, filename) } -// GetIssueAttachment proxies an issue-level attachment download using the Forgejo asset API. -func (h *TicketHandler) GetIssueAttachment(c *gin.Context) { - user := auth.CurrentUser(c) - - ticketID, err := uuid.Parse(c.Param("id")) - if err != nil { - h.deps.Renderer.RenderError(c.Writer, c.Request, http.StatusBadRequest, "Invalid ticket ID") +// DownloadCommentAttachment proxies a comment-level attachment download via Forgejo API. +func (h *TicketHandler) DownloadCommentAttachment(c *gin.Context) { + _, repo, ok := h.verifyTicketOwnership(c) + if !ok { return } - var ticket models.Ticket - if err := h.deps.DB.First(&ticket, "id = ?", ticketID).Error; err != nil { - h.deps.Renderer.RenderError(c.Writer, c.Request, http.StatusNotFound, "Ticket not found") - return - } - - if ticket.UserID != user.ID { - h.deps.Renderer.RenderError(c.Writer, c.Request, http.StatusForbidden, "Access denied") - return - } - - attachmentID, err := strconv.ParseInt(c.Param("attachmentId"), 10, 64) - if err != nil { - h.deps.Renderer.RenderError(c.Writer, c.Request, http.StatusBadRequest, "Invalid attachment ID") - return - } + commentID := c.Param("commentId") + attachmentID := c.Param("attachmentId") filename := c.Param("filename") - var repo models.Repo - h.deps.DB.First(&repo, "id = ?", ticket.RepoID) + assetURL := h.deps.ForgejoClient.BaseURL() + "/api/v1/repos/" + repo.ForgejoOwner + "/" + repo.ForgejoRepo + "/issues/comments/" + commentID + "/assets/" + attachmentID - // Use the Forgejo API to get the asset - assetURL := h.deps.ForgejoClient.BaseURL() + "/api/v1/repos/" + repo.ForgejoOwner + "/" + repo.ForgejoRepo + "/issues/" + strconv.FormatInt(ticket.ForgejoIssueNumber, 10) + "/assets/" + strconv.FormatInt(attachmentID, 10) - - resp, err := h.deps.ForgejoClient.ProxyDownload(assetURL) - if err != nil { - log.Error().Err(err).Msg("proxy attachment download error") - h.deps.Renderer.RenderError(c.Writer, c.Request, http.StatusBadGateway, "Failed to download file") - return - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - h.deps.Renderer.RenderError(c.Writer, c.Request, resp.StatusCode, "Failed to download file") - return - } - - contentType := resp.Header.Get("Content-Type") - if contentType == "" { - contentType = "application/octet-stream" - } - c.Header("Content-Type", contentType) - c.Header("Content-Disposition", "attachment; filename=\""+filename+"\"") - if cl := resp.Header.Get("Content-Length"); cl != "" { - c.Header("Content-Length", cl) - } - c.Status(http.StatusOK) - io.Copy(c.Writer, resp.Body) + h.proxyAssetDownload(c, assetURL, filename) } diff --git a/web/static/css/input.css b/web/static/css/input.css index ccc0526..a222e0a 100644 --- a/web/static/css/input.css +++ b/web/static/css/input.css @@ -15,6 +15,17 @@ background-color: var(--color-blue-100); } +/* Make inline code visually distinct */ +.prose :where(code):not(:where([class~="not-prose"], [class~="not-prose"] *)) { + background-color: var(--color-gray-100); + padding: 0.125rem 0.375rem; + border-radius: 0.25rem; +} +.prose :where(code):not(:where([class~="not-prose"], [class~="not-prose"] *))::before, +.prose :where(code):not(:where([class~="not-prose"], [class~="not-prose"] *))::after { + content: none; +} + /* Task list checkbox styling */ .prose input[type="checkbox"] { margin-right: 0.375rem; diff --git a/web/templates/pages/tickets/detail.html b/web/templates/pages/tickets/detail.html index e81cb03..7d86616 100644 --- a/web/templates/pages/tickets/detail.html +++ b/web/templates/pages/tickets/detail.html @@ -47,7 +47,7 @@

Attachments

{{range .Ticket.Attachments}} - + {{.Name}} ({{.Size}} bytes) {{end}} @@ -92,9 +92,10 @@
{{renderMarkdown .Body $.Data.Mentions}}
{{if .Attachments}} + {{$commentID := .CommentID}}
{{range .Attachments}} - + {{.Name}} {{end}}