vole
Clone
Read only, no credentials:
https://rodent.software/git/vole.git
Write over SSH with a registered key:
git clone ssh://git@0.0.0.0:2222/vole.git
A free and open-source forge that lives in your Git repo.
Reorganise the web package around one implementation per rule
c717d4eb92b4257b05fa065918b0071f6302f53e · Keenan Nemetz · 13d ago · parent 4506a4d3
Reorganise the web package around one implementation per rule
server/web had grown by feature, so several rules and widgets existed twice: the
comment stack for issues and change requests, the pager, the file tree, the write
preamble, and the ACL predicates. Each pair agreed today and would drift
tomorrow. No route, stored format, or permission rule changes; the few behaviour
differences found along the way are listed at the end.
Files now say what they answer:
web.go the Web type, render, open, the error pages
page.go Page, pageSection, who, nav
templatefuncs.go the template function map, with the escaping rules noted
ui.go relative time, avatars, breadcrumbs, the pager
markdown.go the goldmark and bluemonday boundary
codebrowser.go the code browser, out of web.go
tokens.go the label/assignee token input
rolepicker.go the role control, out of admin.go
comments.go the comment view, thread builder, action menus, snippet
comment_writes.go add, edit and delete a comment, for both surfaces
write_context.go beginWrite/beginPost/beginAdminPost, fail, refuse
capabilities.go the ACL rules a page and a handler share
audit.go the audit log
request_view.go the change conversation page, out of requests.go
admin_access.go the instance-admin access rules, out of settings.go
A comment is one thing, so it has one view and one thread builder: metadata.Comment
is the same type on both surfaces and only a change request can anchor one to a
diff line. Adding, editing and deleting are one implementation each, with a
commentTopic naming what differs per surface. The pager is one control for issues,
changes and the instance feed; the file tree is one widget for the code browser
and a change's Files tab.
Every write starts at beginWrite (session, body bound, CSRF, repository, rules,
form) and asks capabilities.go for the one rule it needs, so the control a page
draws and the check a handler makes cannot disagree. A rejected write answers
through refuse; a write that fails for another reason answers through fail, which
re-renders the section for HTMX and returns text otherwise.
Templates follow the same split: chrome.html, components.html, diff.html and
role-picker.html hold what more than one surface renders, and each page owns its
fragments. The route table moved to routes.go, one function per surface.
Behaviour changes found while unifying:
- A reply cycle (two comments answering each other, possible through a pushed
metadata branch) used to drop both comments from the page; anything the walk
does not reach is now emitted as a root.
- A rejected anchored comment keeps its "Commenting on path:line" note and
hidden anchor fields, so the reviewer does not have to pick the line again.
- A repository whose access rules cannot be read answers 500 rather than 403;
the account was being blamed for a broken instance.
- Pager wording is now "Showing a-b of n / page x of y" with Previous/Next
everywhere, and the instance feed's page links keep their kind filter.
- The activity feed, issue list and change list share one page control.
Docs: docs/ui-plan.md gains a "Where the code lives" map, the write gate, the
shared comment model, the route table, and rules for rejected writes, shared
template fragments, and the stylesheet's section order. The passkey script is now
an ordinary asset (static/auth.js) under the same CSP as the rest.
server/web/issues.go +24−288
| @@ -1,70 +1,20 @@ | |||
| 1 | 1 | package web | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | "bytes" | ||
| 5 | 4 | "html/template" | |
| 6 | 5 | "net/http" | |
| 7 | 6 | "net/url" | |
| 8 | "regexp" | ||
| 9 | 7 | "sort" | |
| 10 | 8 | "strconv" | |
| 11 | 9 | "strings" | |
| 12 | 10 | ||
| 13 | "github.com/microcosm-cc/bluemonday" | ||
| 14 | "github.com/yuin/goldmark" | ||
| 15 | "github.com/yuin/goldmark/extension" | ||
| 16 | |||
| 17 | 11 | "github.com/rodent-software/vole/core/git" | |
| 18 | 12 | voleid "github.com/rodent-software/vole/core/id" | |
| 19 | "github.com/rodent-software/vole/forge/access" | ||
| 20 | 13 | "github.com/rodent-software/vole/forge/index" | |
| 21 | 14 | "github.com/rodent-software/vole/forge/repo" | |
| 22 | 15 | "github.com/rodent-software/vole/metadata" | |
| 23 | 16 | ) | |
| 24 | 17 | ||
| 25 | var ( | ||
| 26 | markdown = goldmark.New(goldmark.WithExtensions(extension.GFM, extension.Typographer)) | ||
| 27 | sanitizer = bluemonday.UGCPolicy() | ||
| 28 | xrefRE = regexp.MustCompile(`#([0-9A-HJKMNP-TV-Z]{26})\b`) | ||
| 29 | ) | ||
| 30 | |||
| 31 | // renderMarkdown converts untrusted commit content into HTML that is safe to put | ||
| 32 | // in a page. Issues are authored by pushes, so this is the boundary where a | ||
| 33 | // <script> tag stops being text. | ||
| 34 | // | ||
| 35 | // `#<ulid>` becomes a link to the issue. That is done on the markdown source | ||
| 36 | // rather than on the HTML, and fenced code blocks are skipped so a reference | ||
| 37 | // inside a code sample stays literal. | ||
| 38 | func (w *Web) renderMarkdown(repoName, src string) template.HTML { | ||
| 39 | src = linkCrossReferences(repoName, src) | ||
| 40 | var buf bytes.Buffer | ||
| 41 | if err := markdown.Convert([]byte(src), &buf); err != nil { | ||
| 42 | return template.HTML(template.HTMLEscapeString(src)) | ||
| 43 | } | ||
| 44 | return template.HTML(sanitizer.SanitizeBytes(buf.Bytes())) | ||
| 45 | } | ||
| 46 | |||
| 47 | func linkCrossReferences(repoName, src string) string { | ||
| 48 | var out strings.Builder | ||
| 49 | inFence := false | ||
| 50 | for _, line := range strings.SplitAfter(src, "\n") { | ||
| 51 | if strings.HasPrefix(strings.TrimSpace(line), "```") { | ||
| 52 | inFence = !inFence | ||
| 53 | out.WriteString(line) | ||
| 54 | continue | ||
| 55 | } | ||
| 56 | if inFence { | ||
| 57 | out.WriteString(line) | ||
| 58 | continue | ||
| 59 | } | ||
| 60 | out.WriteString(xrefRE.ReplaceAllStringFunc(line, func(m string) string { | ||
| 61 | id := strings.TrimPrefix(m, "#") | ||
| 62 | return "[#" + id + "](/r/" + repoName + "/issues/" + id + ")" | ||
| 63 | })) | ||
| 64 | } | ||
| 65 | return out.String() | ||
| 66 | } | ||
| 67 | |||
| 68 | 18 | // Issues lists a repo's issues from the index, refreshing it if the issues branch | |
| 69 | 19 | // moved since it was last read. | |
| 70 | 20 | func (w *Web) Issues(rw http.ResponseWriter, r *http.Request) { | |
| @@ -115,12 +65,11 @@ func (w *Web) Issues(rw http.ResponseWriter, r *http.Request) { | |||
| 115 | 65 | return rows[i].ID > rows[j].ID | |
| 116 | 66 | }) | |
| 117 | 67 | page, _ := strconv.Atoi(r.URL.Query().Get("page")) | |
| 118 | pg := newPager(len(rows), 25, page, func(n int) string { | ||
| 68 | pg := newPager(len(rows), listPageSize, page, func(n int) string { | ||
| 119 | 69 | return issueListURL(m.Name, state, filter.Query, filter.Label, filter.Assignee, n) | |
| 120 | }) | ||
| 121 | if pg.Total > 0 { | ||
| 122 | rows = rows[(pg.Page-1)*25 : pg.To] | ||
| 123 | } | ||
| 70 | }).forSwap("Issue pages", "#issue-list") | ||
| 71 | from, to := pg.rows() | ||
| 72 | rows = rows[from:to] | ||
| 124 | 73 | labels, err := w.Index.Labels(m.ID) | |
| 125 | 74 | if err != nil { | |
| 126 | 75 | labels = nil | |
| @@ -156,7 +105,7 @@ func (w *Web) Issues(rw http.ResponseWriter, r *http.Request) { | |||
| 156 | 105 | Open: open, | |
| 157 | 106 | Closed: len(all) - open, | |
| 158 | 107 | Filtered: filter.Label != "" || filter.Assignee != "" || filter.Query != "" || (state != "open" && len(all) > 0), | |
| 159 | CanDiscuss: acl != nil && w.canDiscuss(r, acl), | ||
| 108 | CanDiscuss: w.canDiscuss(r, acl), | ||
| 160 | 109 | Pager: pg, | |
| 161 | 110 | } | |
| 162 | 111 | if htmxRequest(r) { | |
| @@ -186,25 +135,6 @@ func issueListURL(name, state, query, label, assignee string, page int) string { | |||
| 186 | 135 | return "/r/" + name + "/issues?" + v.Encode() | |
| 187 | 136 | } | |
| 188 | 137 | ||
| 189 | // issueCommentView is a comment with its body already rendered through the | ||
| 190 | // markdown boundary. | ||
| 191 | type issueCommentView struct { | ||
| 192 | metadata.Comment | ||
| 193 | Body template.HTML | ||
| 194 | Replies []issueCommentView | ||
| 195 | // ReplyToName names the account this comment answers. ReplyToDeleted marks a | ||
| 196 | // parent that is a tombstone, so the UI can say so instead of showing no one. | ||
| 197 | ReplyToName string | ||
| 198 | ReplyToDeleted bool | ||
| 199 | CanReply bool | ||
| 200 | CanEdit bool | ||
| 201 | CanDelete bool | ||
| 202 | Source string | ||
| 203 | CSRF string | ||
| 204 | EditURL string | ||
| 205 | DeleteURL string | ||
| 206 | } | ||
| 207 | |||
| 208 | 138 | // issueSidebarView is the data every sidebar fragment needs. Keeping it one | |
| 209 | 139 | // type is what lets a fragment replace one section after an HTMX write without | |
| 210 | 140 | // rebuilding the whole page. | |
| @@ -225,15 +155,13 @@ type issueSidebarView struct { | |||
| 225 | 155 | type issuePageView struct { | |
| 226 | 156 | issueSidebarView | |
| 227 | 157 | Description template.HTML | |
| 228 | Comments []issueCommentView | ||
| 158 | Thread threadView | ||
| 229 | 159 | Backlinks []git.Commit | |
| 230 | ThreadError string | ||
| 231 | 160 | } | |
| 232 | 161 | ||
| 233 | 162 | func (w *Web) issueSidebar(r *http.Request, m *repo.Meta, iss *metadata.Issue, title, errMsg string) issueSidebarView { | |
| 234 | 163 | acl, _ := m.ACL() | |
| 235 | pr := w.principal(r) | ||
| 236 | canManage := pr != nil && acl != nil && (acl.CanTriage(pr.account.Name) || iss.Author == pr.account.Name) | ||
| 164 | canManage := canTriage(acl, w.accountName(r), iss.Author) | ||
| 237 | 165 | _ = w.Index.EnsureIssues(m) | |
| 238 | 166 | labels, _ := w.Index.Labels(m.ID) | |
| 239 | 167 | assignable := mergeNames(w.assignableAccounts(m), iss.Assignees) | |
| @@ -242,7 +170,7 @@ func (w *Web) issueSidebar(r *http.Request, m *repo.Meta, iss *metadata.Issue, t | |||
| 242 | 170 | Name: m.Name, | |
| 243 | 171 | Issue: iss, | |
| 244 | 172 | CanClose: canManage, | |
| 245 | CanDiscuss: acl != nil && w.canDiscuss(r, acl), | ||
| 173 | CanDiscuss: w.canDiscuss(r, acl), | ||
| 246 | 174 | CanEditLabels: canManage, | |
| 247 | 175 | CanEditAssignees: canManage, | |
| 248 | 176 | AllLabels: labels, | |
| @@ -251,120 +179,21 @@ func (w *Web) issueSidebar(r *http.Request, m *repo.Meta, iss *metadata.Issue, t | |||
| 251 | 179 | } | |
| 252 | 180 | } | |
| 253 | 181 | ||
| 254 | type issueThreadView struct { | ||
| 255 | Page | ||
| 256 | Name string | ||
| 257 | Issue *metadata.Issue | ||
| 258 | Comments []issueCommentView | ||
| 259 | CanDiscuss bool | ||
| 260 | ThreadError string | ||
| 261 | } | ||
| 262 | |||
| 263 | func (w *Web) issueComments(m *repo.Meta, iss *metadata.Issue) []issueCommentView { | ||
| 264 | comments := make([]issueCommentView, 0, len(iss.Comments)) | ||
| 265 | for _, c := range iss.Comments { | ||
| 266 | comments = append(comments, issueCommentView{Comment: c, Body: w.renderMarkdown(m.Name, c.Body), Source: c.Body}) | ||
| 267 | } | ||
| 268 | return threadIssueComments(comments) | ||
| 269 | } | ||
| 270 | |||
| 271 | // threadIssueComments puts every descendant under its top-level root. The | ||
| 272 | // metadata allows a reply to any comment; the page still shows one level. Missing | ||
| 273 | // parents become tombstones, so replies to a deleted comment keep their thread. | ||
| 274 | func threadIssueComments(comments []issueCommentView) []issueCommentView { | ||
| 275 | nodes := make(map[string]issueCommentView, len(comments)) | ||
| 276 | for _, c := range comments { | ||
| 277 | nodes[c.ID] = c | ||
| 278 | } | ||
| 279 | for _, c := range comments { | ||
| 280 | if c.ReplyTo != "" { | ||
| 281 | if _, ok := nodes[c.ReplyTo]; !ok { | ||
| 282 | nodes[c.ReplyTo] = issueCommentView{Comment: metadata.Comment{ID: c.ReplyTo, Deleted: true}} | ||
| 283 | } | ||
| 284 | } | ||
| 285 | } | ||
| 286 | ids := make([]string, 0, len(nodes)) | ||
| 287 | for id := range nodes { | ||
| 288 | ids = append(ids, id) | ||
| 289 | } | ||
| 290 | sort.Strings(ids) | ||
| 291 | children := make(map[string][]string, len(nodes)) | ||
| 292 | var roots []string | ||
| 293 | for _, id := range ids { | ||
| 294 | c := nodes[id] | ||
| 295 | if c.ReplyTo == "" || c.ReplyTo == id { | ||
| 296 | roots = append(roots, id) | ||
| 297 | continue | ||
| 298 | } | ||
| 299 | if _, ok := nodes[c.ReplyTo]; ok { | ||
| 300 | children[c.ReplyTo] = append(children[c.ReplyTo], id) | ||
| 301 | } else { | ||
| 302 | roots = append(roots, id) | ||
| 303 | } | ||
| 304 | } | ||
| 305 | out := make([]issueCommentView, 0, len(roots)) | ||
| 306 | for _, id := range roots { | ||
| 307 | root := nodes[id] | ||
| 308 | seen := map[string]bool{id: true} | ||
| 309 | var replies []string | ||
| 310 | var walk func(string) | ||
| 311 | walk = func(parent string) { | ||
| 312 | for _, child := range children[parent] { | ||
| 313 | if seen[child] { | ||
| 314 | continue | ||
| 315 | } | ||
| 316 | seen[child] = true | ||
| 317 | replies = append(replies, child) | ||
| 318 | walk(child) | ||
| 319 | } | ||
| 320 | } | ||
| 321 | walk(id) | ||
| 322 | sort.Strings(replies) | ||
| 323 | for _, rid := range replies { | ||
| 324 | reply := nodes[rid] | ||
| 325 | if parent, ok := nodes[reply.ReplyTo]; ok { | ||
| 326 | reply.ReplyToName = parent.Author | ||
| 327 | reply.ReplyToDeleted = parent.Deleted | ||
| 328 | } | ||
| 329 | root.Replies = append(root.Replies, reply) | ||
| 330 | } | ||
| 331 | out = append(out, root) | ||
| 332 | } | ||
| 333 | return out | ||
| 334 | } | ||
| 335 | |||
| 336 | func (w *Web) issueThread(r *http.Request, m *repo.Meta, iss *metadata.Issue, errMsg string) issueThreadView { | ||
| 337 | acl, _ := m.ACL() | ||
| 338 | canDiscuss := acl != nil && w.canDiscuss(r, acl) | ||
| 339 | comments := w.issueComments(m, iss) | ||
| 340 | w.markIssueCommentActions(r, m, iss.ID, comments, canDiscuss) | ||
| 341 | return issueThreadView{ | ||
| 342 | Page: w.page(r, ""), Name: m.Name, Issue: iss, | ||
| 343 | Comments: comments, CanDiscuss: canDiscuss, ThreadError: errMsg, | ||
| 344 | } | ||
| 345 | } | ||
| 346 | |||
| 347 | func (w *Web) markIssueCommentActions(r *http.Request, m *repo.Meta, issueID string, comments []issueCommentView, canDiscuss bool) { | ||
| 348 | pr := w.principal(r) | ||
| 349 | acl, _ := m.ACL() | ||
| 350 | admin := pr != nil && acl != nil && acl.RoleFor(pr.account.Name).AtLeast(access.Admin) | ||
| 351 | csrf := "" | ||
| 352 | if pr != nil { | ||
| 353 | csrf = pr.session.CSRF | ||
| 354 | } | ||
| 355 | for i := range comments { | ||
| 356 | c := &comments[i] | ||
| 357 | c.CSRF = csrf | ||
| 358 | c.CanReply = canDiscuss && !c.Deleted | ||
| 359 | c.CanEdit = !c.Deleted && pr != nil && (c.Author == pr.account.Name || admin) | ||
| 360 | c.CanDelete = c.CanEdit | ||
| 361 | if c.CanEdit { | ||
| 362 | base := "/r/" + m.Name + "/issues/" + issueID + "/comments/" + c.ID | ||
| 363 | c.EditURL = base + "/edit" | ||
| 364 | c.DeleteURL = base + "/delete" | ||
| 365 | } | ||
| 366 | w.markIssueCommentActions(r, m, issueID, c.Replies, canDiscuss) | ||
| 367 | } | ||
| 182 | // issueThread is the discussion section of an issue: the shared thread fragment | ||
| 183 | // with this surface's heading, composer and empty state. | ||
| 184 | func (w *Web) issueThread(r *http.Request, m *repo.Meta, iss *metadata.Issue, errMsg string) threadView { | ||
| 185 | return w.thread(r, m, threadView{ | ||
| 186 | Name: m.Name, | ||
| 187 | Section: "issue-thread", | ||
| 188 | Composer: "issue-comment-form", | ||
| 189 | PostURL: "/r/" + m.Name + "/issues/" + iss.ID + "/comments", | ||
| 190 | BackURL: "/r/" + m.Name + "/issues/" + iss.ID, | ||
| 191 | Title: "Comments", | ||
| 192 | EmptyText: "No comments yet.", | ||
| 193 | Hint: `vole issue comment ` + iss.ID + ` -m "..."`, | ||
| 194 | Topic: iss.ID, | ||
| 195 | ThreadError: errMsg, | ||
| 196 | }, w.commentViews(m, iss.Comments)) | ||
| 368 | 197 | } | |
| 369 | 198 | ||
| 370 | 199 | // IssueView renders one issue and its thread. The issue is read from git rather | |
| @@ -384,13 +213,11 @@ func (w *Web) IssueView(rw http.ResponseWriter, r *http.Request) { | |||
| 384 | 213 | w.notFound(rw, r) | |
| 385 | 214 | return | |
| 386 | 215 | } | |
| 387 | comments := w.issueComments(m, iss) | ||
| 388 | 216 | sidebar := w.issueSidebar(r, m, iss, m.Name+" · "+iss.Title, "") | |
| 389 | w.markIssueCommentActions(r, m, iss.ID, comments, sidebar.CanDiscuss) | ||
| 390 | 217 | w.render(rw, "issue", issuePageView{ | |
| 391 | 218 | issueSidebarView: sidebar, | |
| 392 | 219 | Description: w.renderMarkdown(m.Name, iss.Description), | |
| 393 | Comments: comments, | ||
| 220 | Thread: w.issueThread(r, m, iss, ""), | ||
| 394 | 221 | Backlinks: w.backlinks(m, iss.ID), | |
| 395 | 222 | }, 0) | |
| 396 | 223 | } | |
| @@ -433,94 +260,3 @@ func (w *Web) NewIssuePage(rw http.ResponseWriter, r *http.Request) { | |||
| 433 | 260 | Assignees []string | |
| 434 | 261 | }{w.page(r, m.Name+" · new issue"), m.Name, labels, w.assignableAccounts(m)}, 0) | |
| 435 | 262 | } | |
| 436 | |||
| 437 | // tokenPickerData is the small view model behind the token input. It is used for | ||
| 438 | // labels and assignees: Selected is the current set, All is what to suggest, and | ||
| 439 | // Field/Item name the form values the browser and the no-JS fallback submit. | ||
| 440 | type tokenPickerData struct { | ||
| 441 | Selected []string | ||
| 442 | All []string | ||
| 443 | Field string | ||
| 444 | Item string | ||
| 445 | Placeholder string | ||
| 446 | } | ||
| 447 | |||
| 448 | func tokenPicker(selected, all []string, field, item, placeholder string) tokenPickerData { | ||
| 449 | return tokenPickerData{Selected: selected, All: all, Field: field, Item: item, Placeholder: placeholder} | ||
| 450 | } | ||
| 451 | |||
| 452 | // assignableAccounts lists active accounts that can see this repository. A | ||
| 453 | // read-only account is still a valid assignee: the point is that they can read | ||
| 454 | // the issue they were given, even if they cannot comment on it. | ||
| 455 | func (w *Web) assignableAccounts(m *repo.Meta) []string { | ||
| 456 | if w.Ident == nil { | ||
| 457 | return nil | ||
| 458 | } | ||
| 459 | acl, err := m.ACL() | ||
| 460 | if err != nil { | ||
| 461 | return nil | ||
| 462 | } | ||
| 463 | snap := w.Ident.Snapshot() | ||
| 464 | if snap == nil { | ||
| 465 | return nil | ||
| 466 | } | ||
| 467 | var names []string | ||
| 468 | for _, acct := range snap.ByName { | ||
| 469 | if acct.Active() && acl.Can(acct.Name, access.Read) { | ||
| 470 | names = append(names, acct.Name) | ||
| 471 | } | ||
| 472 | } | ||
| 473 | sort.Strings(names) | ||
| 474 | return names | ||
| 475 | } | ||
| 476 | |||
| 477 | // unknownNames reports names that are not in the allowed set. It keeps the | ||
| 478 | // browser from storing an assignee nobody can resolve. | ||
| 479 | func unknownNames(want, allowed []string) []string { | ||
| 480 | set := map[string]bool{} | ||
| 481 | for _, n := range allowed { | ||
| 482 | set[n] = true | ||
| 483 | } | ||
| 484 | var bad []string | ||
| 485 | for _, n := range want { | ||
| 486 | if !set[n] { | ||
| 487 | bad = append(bad, n) | ||
| 488 | } | ||
| 489 | } | ||
| 490 | return bad | ||
| 491 | } | ||
| 492 | |||
| 493 | // mergeNames returns the union of two name lists, sorted, without duplicates. | ||
| 494 | func mergeNames(a, b []string) []string { | ||
| 495 | set := map[string]bool{} | ||
| 496 | for _, n := range a { | ||
| 497 | set[n] = true | ||
| 498 | } | ||
| 499 | for _, n := range b { | ||
| 500 | set[n] = true | ||
| 501 | } | ||
| 502 | return metadata.SortedNames(set) | ||
| 503 | } | ||
| 504 | |||
| 505 | // normalizeLabelSet is what the browser writes: NormalizeLabels plus dedupe, so | ||
| 506 | // checking two spellings of the same label cannot store it twice. | ||
| 507 | func normalizeLabelSet(in []string) []string { | ||
| 508 | set := map[string]bool{} | ||
| 509 | for _, v := range metadata.NormalizeLabels(in) { | ||
| 510 | set[v] = true | ||
| 511 | } | ||
| 512 | return metadata.SortedNames(set) | ||
| 513 | } | ||
| 514 | |||
| 515 | // sameStrings compares the normalised label lists a writer would store. | ||
| 516 | func sameStrings(a, b []string) bool { | ||
| 517 | if len(a) != len(b) { | ||
| 518 | return false | ||
| 519 | } | ||
| 520 | for i := range a { | ||
| 521 | if a[i] != b[i] { | ||
| 522 | return false | ||
| 523 | } | ||
| 524 | } | ||
| 525 | return true | ||
| 526 | } | ||