vole
Clone
Read only, no credentials:
https://rodent.software/vole.git
Write over SSH with a registered key:
git clone ssh://git@rodent.software:2222/vole.git
A free and open-source forge that lives in your Git repo.
Edit issue and change details in place
802e0e52533746522b3e65b953e5cc0141aa70b9 · Keenan Nemetz · 5d ago · parent 90f1ab59
Edit issue and change details in place Put the title pencil beside the heading and the description menu on its card, matching the comment editor. Each control saves only its own field: the title response is the heading and the description response is the card, so an open draft in the other field is not replaced. The standalone /edit page stays as the fallback. Prefilled edit fields size themselves to their content and remain vertically resizable. Prose copied from a menu is query-escaped in the attribute, so untrusted markdown in an issue or comment cannot appear verbatim in the page.
server/web/details_writes.go +206−35
| @@ -2,10 +2,11 @@ package web | |||
| 2 | 2 | ||
| 3 | 3 | // Editing what an issue or a change says. | |
| 4 | 4 | // | |
| 5 | // The operation is the same on both surfaces: replace the title and the | ||
| 5 | // The operation is the same on both surfaces: replace the title or the | ||
| 6 | 6 | // description through the metadata writer, leaving the discussion, labels, | |
| 7 | // assignees, and review state alone. The two surfaces differ only in which writer | ||
| 8 | // runs and where the page lives, so they share one form and one refusal shape. | ||
| 7 | // assignees, and review state alone. The two surfaces share the in-place forms | ||
| 8 | // and their refusal shapes; the standalone /edit page stays as the no-script | ||
| 9 | // fallback. | ||
| 9 | 10 | ||
| 10 | 11 | import ( | |
| 11 | 12 | "errors" | |
| @@ -18,6 +19,49 @@ import ( | |||
| 18 | 19 | "github.com/rodent-software/vole/metadata" | |
| 19 | 20 | ) | |
| 20 | 21 | ||
| 22 | // fieldEdit is one in-place field: whether it is open, what the reader typed, and | ||
| 23 | // why a write was refused. The title and description carry one each because they | ||
| 24 | // are separate controls now; saving one must not replace or reopen the other. | ||
| 25 | type fieldEdit struct { | ||
| 26 | Active bool | ||
| 27 | Value string | ||
| 28 | Error string | ||
| 29 | } | ||
| 30 | |||
| 31 | // detailsEdit is the in-place state of the heading and description card on an | ||
| 32 | // issue or change page. A full page leaves it zero. | ||
| 33 | type detailsEdit struct { | ||
| 34 | Title fieldEdit | ||
| 35 | Description fieldEdit | ||
| 36 | // Inline says this page carries the title and description forms. The change | ||
| 37 | // head is also used on the Files and Runs tabs, which do not, so their title | ||
| 38 | // pencil stays an ordinary link to the standalone page. | ||
| 39 | Inline bool | ||
| 40 | } | ||
| 41 | |||
| 42 | // issueDetailsView is the data for an issue page fragment: the heading or the | ||
| 43 | // description card, depending on which template renders it. | ||
| 44 | func (w *Web) issueDetailsView(r *http.Request, m *repo.Meta, iss *metadata.Issue, edit detailsEdit) issuePageView { | ||
| 45 | sidebar := w.issueSidebar(r, m, iss, "") | ||
| 46 | sidebar.Details = edit | ||
| 47 | return issuePageView{ | ||
| 48 | issueSidebarView: sidebar, | ||
| 49 | Description: w.renderMarkdown(m.Name, iss.Description), | ||
| 50 | } | ||
| 51 | } | ||
| 52 | |||
| 53 | // requestDetailsView is the change page's counterpart to issueDetailsView. | ||
| 54 | func (w *Web) requestDetailsView(r *http.Request, m *repo.Meta, req *metadata.Request, edit detailsEdit) requestPageView { | ||
| 55 | edit.Inline = true | ||
| 56 | sidebar := w.requestSidebar(r, m, req, "", "") | ||
| 57 | sidebar.Details = edit | ||
| 58 | return requestPageView{ | ||
| 59 | requestSidebarView: sidebar, | ||
| 60 | Tab: "conversation", | ||
| 61 | Description: w.renderMarkdown(m.Name, req.Description), | ||
| 62 | } | ||
| 63 | } | ||
| 64 | |||
| 21 | 65 | // detailsForm is the data the edit page and a refused write share: the prose to | |
| 22 | 66 | // put back in the fields, the record it belongs to, and the reason it did not | |
| 23 | 67 | // save. Keeping the typed values here is what lets a correction keep its work. | |
| @@ -78,9 +122,10 @@ func (w *Web) IssueEditPage(rw http.ResponseWriter, r *http.Request) { | |||
| 78 | 122 | w.render(rw, "issue_edit", w.detailsData(r, m, id, iss.Title, iss.Description, ""), 0) | |
| 79 | 123 | } | |
| 80 | 124 | ||
| 81 | // UpdateIssueDetails saves the issue edit page. A blank title is refused before | ||
| 82 | // the writer runs, so the reader gets the form's own words rather than a | ||
| 83 | // validation error from the file format. | ||
| 125 | // UpdateIssueDetails walks the two in-place forms on the issue page and the | ||
| 126 | // combined form on the standalone page. A title-only or description-only write | ||
| 127 | // leaves the other field as the file has it now, so a draft in the other editor | ||
| 128 | // is not overwritten by a save the reader did not make. | ||
| 84 | 129 | func (w *Web) UpdateIssueDetails(rw http.ResponseWriter, r *http.Request) { | |
| 85 | 130 | ctx, ok := w.beginWrite(rw, r, maxFormBody) | |
| 86 | 131 | if !ok { | |
| @@ -97,28 +142,92 @@ func (w *Web) UpdateIssueDetails(rw http.ResponseWriter, r *http.Request) { | |||
| 97 | 142 | w.refuse(rw, r, "you do not have permission to edit this issue") | |
| 98 | 143 | return | |
| 99 | 144 | } | |
| 145 | field := strings.TrimSpace(r.PostFormValue("field")) | ||
| 100 | 146 | title := strings.TrimSpace(r.PostFormValue("title")) | |
| 101 | 147 | description := strings.TrimSpace(r.PostFormValue("body")) | |
| 102 | form := func(msg string) (string, any) { | ||
| 103 | return "issue_edit", w.detailsData(r, m, id, title, description, msg) | ||
| 148 | titleFragment := func(msg string) (string, any) { | ||
| 149 | return "issue_head", w.issueDetailsView(r, m, iss, detailsEdit{ | ||
| 150 | Title: fieldEdit{Active: true, Value: title, Error: msg}, | ||
| 151 | }) | ||
| 104 | 152 | } | |
| 105 | if title == "" { | ||
| 106 | w.fail(rw, r, http.StatusBadRequest, "an issue needs a title", form) | ||
| 107 | return | ||
| 153 | bodyFragment := func(msg string) (string, any) { | ||
| 154 | return "issue_description", w.issueDetailsView(r, m, iss, detailsEdit{ | ||
| 155 | Description: fieldEdit{Active: true, Value: description, Error: msg}, | ||
| 156 | }) | ||
| 157 | } | ||
| 158 | fullFragment := func(msg string) (string, any) { | ||
| 159 | return "issue_edit", w.detailsData(r, m, id, title, description, msg) | ||
| 108 | 160 | } | |
| 109 | _, err = metadata.UpdateIssue(m.Git(), id, func(cur *metadata.Issue) error { | ||
| 110 | if cur.Title == title && cur.Description == description { | ||
| 111 | return metadata.ErrNoChange | ||
| 161 | switch field { | ||
| 162 | case "title": | ||
| 163 | if title == "" { | ||
| 164 | w.fail(rw, r, http.StatusBadRequest, "an issue needs a title", titleFragment) | ||
| 165 | return | ||
| 112 | 166 | } | |
| 113 | cur.Title, cur.Description = title, description | ||
| 114 | return nil | ||
| 115 | }, ctx.Name) | ||
| 116 | if err != nil && !errors.Is(err, metadata.ErrNoChange) { | ||
| 117 | w.fail(rw, r, http.StatusBadRequest, "cannot update the issue: "+err.Error(), form) | ||
| 118 | return | ||
| 167 | updated, err := metadata.UpdateIssue(m.Git(), id, func(cur *metadata.Issue) error { | ||
| 168 | if cur.Title == title { | ||
| 169 | return metadata.ErrNoChange | ||
| 170 | } | ||
| 171 | cur.Title = title | ||
| 172 | return nil | ||
| 173 | }, ctx.Name) | ||
| 174 | if err != nil && !errors.Is(err, metadata.ErrNoChange) { | ||
| 175 | w.fail(rw, r, http.StatusBadRequest, "cannot update the issue: "+err.Error(), titleFragment) | ||
| 176 | return | ||
| 177 | } | ||
| 178 | w.audit("web.issue.edit", ctx.Name, m.Name, id) | ||
| 179 | if !htmxRequest(r) { | ||
| 180 | http.Redirect(rw, r, "/r/"+m.Name+"/issues/"+id, http.StatusSeeOther) | ||
| 181 | return | ||
| 182 | } | ||
| 183 | if updated == nil { | ||
| 184 | updated = iss | ||
| 185 | } | ||
| 186 | w.render(rw, "issue_head", w.issueDetailsView(r, m, updated, detailsEdit{}), 0) | ||
| 187 | case "body": | ||
| 188 | updated, err := metadata.UpdateIssue(m.Git(), id, func(cur *metadata.Issue) error { | ||
| 189 | if cur.Description == description { | ||
| 190 | return metadata.ErrNoChange | ||
| 191 | } | ||
| 192 | cur.Description = description | ||
| 193 | return nil | ||
| 194 | }, ctx.Name) | ||
| 195 | if err != nil && !errors.Is(err, metadata.ErrNoChange) { | ||
| 196 | w.fail(rw, r, http.StatusBadRequest, "cannot update the issue: "+err.Error(), bodyFragment) | ||
| 197 | return | ||
| 198 | } | ||
| 199 | w.audit("web.issue.edit", ctx.Name, m.Name, id) | ||
| 200 | if !htmxRequest(r) { | ||
| 201 | http.Redirect(rw, r, "/r/"+m.Name+"/issues/"+id, http.StatusSeeOther) | ||
| 202 | return | ||
| 203 | } | ||
| 204 | if updated == nil { | ||
| 205 | updated = iss | ||
| 206 | } | ||
| 207 | w.render(rw, "issue_description", w.issueDetailsView(r, m, updated, detailsEdit{}), 0) | ||
| 208 | default: | ||
| 209 | if field != "" { | ||
| 210 | w.fail(rw, r, http.StatusBadRequest, "unknown part of the issue to edit", nil) | ||
| 211 | return | ||
| 212 | } | ||
| 213 | if title == "" { | ||
| 214 | w.fail(rw, r, http.StatusBadRequest, "an issue needs a title", fullFragment) | ||
| 215 | return | ||
| 216 | } | ||
| 217 | _, err := metadata.UpdateIssue(m.Git(), id, func(cur *metadata.Issue) error { | ||
| 218 | if cur.Title == title && cur.Description == description { | ||
| 219 | return metadata.ErrNoChange | ||
| 220 | } | ||
| 221 | cur.Title, cur.Description = title, description | ||
| 222 | return nil | ||
| 223 | }, ctx.Name) | ||
| 224 | if err != nil && !errors.Is(err, metadata.ErrNoChange) { | ||
| 225 | w.fail(rw, r, http.StatusBadRequest, "cannot update the issue: "+err.Error(), fullFragment) | ||
| 226 | return | ||
| 227 | } | ||
| 228 | w.audit("web.issue.edit", ctx.Name, m.Name, id) | ||
| 229 | http.Redirect(rw, r, "/r/"+m.Name+"/issues/"+id, http.StatusSeeOther) | ||
| 119 | 230 | } | |
| 120 | w.audit("web.issue.edit", ctx.Name, m.Name, id) | ||
| 121 | http.Redirect(rw, r, "/r/"+m.Name+"/issues/"+id, http.StatusSeeOther) | ||
| 122 | 231 | } | |
| 123 | 232 | ||
| 124 | 233 | // RequestEditPage serves the form that rewrites a change's title and description. | |
| @@ -150,9 +259,10 @@ func (w *Web) RequestEditPage(rw http.ResponseWriter, r *http.Request) { | |||
| 150 | 259 | w.render(rw, "request_edit", w.detailsData(r, m, id, req.Title, req.Description, ""), 0) | |
| 151 | 260 | } | |
| 152 | 261 | ||
| 153 | // UpdateRequestDetails saves the change edit page. The writer validates the whole | ||
| 154 | // record, and the page checks the metadata tree afterwards like every other write | ||
| 155 | // that touches a change. | ||
| 262 | // UpdateRequestDetails walks the two in-place forms on the change page and the | ||
| 263 | // combined form on the standalone page. The writer validates the whole record, | ||
| 264 | // and the tree is checked afterwards like every other write that touches a | ||
| 265 | // change. | ||
| 156 | 266 | func (w *Web) UpdateRequestDetails(rw http.ResponseWriter, r *http.Request) { | |
| 157 | 267 | ctx, ok := w.beginWrite(rw, r, maxFormBody) | |
| 158 | 268 | if !ok { | |
| @@ -169,22 +279,83 @@ func (w *Web) UpdateRequestDetails(rw http.ResponseWriter, r *http.Request) { | |||
| 169 | 279 | w.refuse(rw, r, "you do not have permission to edit this change") | |
| 170 | 280 | return | |
| 171 | 281 | } | |
| 282 | field := strings.TrimSpace(r.PostFormValue("field")) | ||
| 172 | 283 | title := strings.TrimSpace(r.PostFormValue("title")) | |
| 173 | 284 | description := strings.TrimSpace(r.PostFormValue("body")) | |
| 174 | form := func(msg string) (string, any) { | ||
| 175 | return "request_edit", w.detailsData(r, m, id, title, description, msg) | ||
| 285 | titleFragment := func(msg string) (string, any) { | ||
| 286 | return "request_title_head", w.requestDetailsView(r, m, req, detailsEdit{ | ||
| 287 | Title: fieldEdit{Active: true, Value: title, Error: msg}, | ||
| 288 | }) | ||
| 176 | 289 | } | |
| 177 | if title == "" { | ||
| 178 | w.fail(rw, r, http.StatusBadRequest, "a change needs a title", form) | ||
| 179 | return | ||
| 290 | bodyFragment := func(msg string) (string, any) { | ||
| 291 | return "request_description", w.requestDetailsView(r, m, req, detailsEdit{ | ||
| 292 | Description: fieldEdit{Active: true, Value: description, Error: msg}, | ||
| 293 | }) | ||
| 180 | 294 | } | |
| 181 | if _, err := metadata.SetRequestDetails(m.Git(), id, title, description, ctx.Name); err != nil { | ||
| 182 | w.fail(rw, r, http.StatusBadRequest, "cannot update the change: "+err.Error(), form) | ||
| 183 | return | ||
| 295 | fullFragment := func(msg string) (string, any) { | ||
| 296 | return "request_edit", w.detailsData(r, m, id, title, description, msg) | ||
| 297 | } | ||
| 298 | switch field { | ||
| 299 | case "title": | ||
| 300 | if title == "" { | ||
| 301 | w.fail(rw, r, http.StatusBadRequest, "a change needs a title", titleFragment) | ||
| 302 | return | ||
| 303 | } | ||
| 304 | updated, err := metadata.SetRequestDetails(m.Git(), id, title, req.Description, ctx.Name) | ||
| 305 | if err != nil { | ||
| 306 | w.fail(rw, r, http.StatusBadRequest, "cannot update the change: "+err.Error(), titleFragment) | ||
| 307 | return | ||
| 308 | } | ||
| 309 | w.audit("web.request.edit", ctx.Name, m.Name, id) | ||
| 310 | w.checkRequestsAfterEdit(m, id) | ||
| 311 | if !htmxRequest(r) { | ||
| 312 | http.Redirect(rw, r, "/r/"+m.Name+"/requests/"+id, http.StatusSeeOther) | ||
| 313 | return | ||
| 314 | } | ||
| 315 | if updated == nil { | ||
| 316 | updated = req | ||
| 317 | } | ||
| 318 | w.render(rw, "request_title_head", w.requestDetailsView(r, m, updated, detailsEdit{}), 0) | ||
| 319 | case "body": | ||
| 320 | updated, err := metadata.SetRequestDetails(m.Git(), id, req.Title, description, ctx.Name) | ||
| 321 | if err != nil { | ||
| 322 | w.fail(rw, r, http.StatusBadRequest, "cannot update the change: "+err.Error(), bodyFragment) | ||
| 323 | return | ||
| 324 | } | ||
| 325 | w.audit("web.request.edit", ctx.Name, m.Name, id) | ||
| 326 | w.checkRequestsAfterEdit(m, id) | ||
| 327 | if !htmxRequest(r) { | ||
| 328 | http.Redirect(rw, r, "/r/"+m.Name+"/requests/"+id, http.StatusSeeOther) | ||
| 329 | return | ||
| 330 | } | ||
| 331 | if updated == nil { | ||
| 332 | updated = req | ||
| 333 | } | ||
| 334 | w.render(rw, "request_description", w.requestDetailsView(r, m, updated, detailsEdit{}), 0) | ||
| 335 | default: | ||
| 336 | if field != "" { | ||
| 337 | w.fail(rw, r, http.StatusBadRequest, "unknown part of the change to edit", nil) | ||
| 338 | return | ||
| 339 | } | ||
| 340 | if title == "" { | ||
| 341 | w.fail(rw, r, http.StatusBadRequest, "a change needs a title", fullFragment) | ||
| 342 | return | ||
| 343 | } | ||
| 344 | if _, err := metadata.SetRequestDetails(m.Git(), id, title, description, ctx.Name); err != nil { | ||
| 345 | w.fail(rw, r, http.StatusBadRequest, "cannot update the change: "+err.Error(), fullFragment) | ||
| 346 | return | ||
| 347 | } | ||
| 348 | w.audit("web.request.edit", ctx.Name, m.Name, id) | ||
| 349 | w.checkRequestsAfterEdit(m, id) | ||
| 350 | http.Redirect(rw, r, "/r/"+m.Name+"/requests/"+id, http.StatusSeeOther) | ||
| 184 | 351 | } | |
| 185 | w.audit("web.request.edit", ctx.Name, m.Name, id) | ||
| 352 | } | ||
| 353 | |||
| 354 | // checkRequestsAfterEdit logs a request tree the writer left invalid. It is a | ||
| 355 | // report, not a refusal: the edit committed, and the next write will say what is | ||
| 356 | // wrong. | ||
| 357 | func (w *Web) checkRequestsAfterEdit(m *repo.Meta, id string) { | ||
| 186 | 358 | if problems := metadata.ValidateRequests(m.Git(), metadata.Ref); len(problems) > 0 { | |
| 187 | 359 | w.Logger("web: request edit %s left the request tree invalid: %v", id, problems) | |
| 188 | 360 | } | |
| 189 | http.Redirect(rw, r, "/r/"+m.Name+"/requests/"+id, http.StatusSeeOther) | ||
| 190 | 361 | } | |