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.
Make the change request sidebar compact and live
d3dcf54d8b7eb81fcc2b944296329098db7f14f4 · Keenan Nemetz · 11d ago · parent 7137f372
Make the change request sidebar compact and live Move the review and approval explanations into tooltips, show only the state in each section, and use the overview's run rows for checks. The aside now polls a new fragment endpoint so approvals, checks and landing facts appear without a reload.
server/web/request_view.go +76−37
| @@ -11,6 +11,7 @@ import ( | |||
| 11 | 11 | "strings" | |
| 12 | 12 | "time" | |
| 13 | 13 | ||
| 14 | "github.com/rodent-software/vole/ci/runs" | ||
| 14 | 15 | "github.com/rodent-software/vole/core/git" | |
| 15 | 16 | "github.com/rodent-software/vole/forge/access" | |
| 16 | 17 | "github.com/rodent-software/vole/forge/landing" | |
| @@ -48,11 +49,6 @@ func currentApprovals(acl *access.ACL, req *metadata.Request) int { | |||
| 48 | 49 | return n | |
| 49 | 50 | } | |
| 50 | 51 | ||
| 51 | type requestCheck struct { | ||
| 52 | ID, Workflow, Job, Status string | ||
| 53 | Exit int | ||
| 54 | } | ||
| 55 | |||
| 56 | 52 | // branchOption is one choice in the request target picker. The selected flag is | |
| 57 | 53 | // built here so the template does not have to compare strings, and so a branch that | |
| 58 | 54 | // has gone missing can stay visible as the current value instead of silently | |
| @@ -98,7 +94,7 @@ type requestSidebarView struct { | |||
| 98 | 94 | // name issues they were not given directly. | |
| 99 | 95 | Numbers map[string]int | |
| 100 | 96 | Approvals []requestApproval | |
| 101 | Checks []requestCheck | ||
| 97 | Checks []*runs.Run | ||
| 102 | 98 | Required int | |
| 103 | 99 | Current int | |
| 104 | 100 | CanApprove bool | |
| @@ -128,19 +124,35 @@ type requestSidebarView struct { | |||
| 128 | 124 | Error string | |
| 129 | 125 | } | |
| 130 | 126 | ||
| 127 | // requestLandingView is the "What would land" block: whether the branch still | ||
| 128 | // merges, which issues it closes, and how much it changes. It is split from the | ||
| 129 | // sidebar so the asides of the conversation and Runs tabs can refresh it from | ||
| 130 | // the same computation the full page uses. | ||
| 131 | type requestLandingView struct { | ||
| 132 | FileCount int | ||
| 133 | Added int | ||
| 134 | Removed int | ||
| 135 | AlreadyMerged bool | ||
| 136 | Conflicts []string | ||
| 137 | MergeChecked bool | ||
| 138 | } | ||
| 139 | |||
| 131 | 140 | type requestPageView struct { | |
| 132 | 141 | requestSidebarView | |
| 142 | requestLandingView | ||
| 133 | 143 | Tab string | |
| 134 | 144 | Description template.HTML | |
| 135 | 145 | CommentTarget *commentTarget | |
| 136 | 146 | LineCommentURL string | |
| 137 | FileCount int | ||
| 138 | Added int | ||
| 139 | Removed int | ||
| 140 | AlreadyMerged bool | ||
| 141 | 147 | Thread threadView | |
| 142 | Conflicts []string | ||
| 143 | MergeChecked bool | ||
| 148 | } | ||
| 149 | |||
| 150 | // requestAsideView is the polling fragment: the landing block and the sidebar | ||
| 151 | // together, with none of the page shell the handler that serves a full request | ||
| 152 | // needs. | ||
| 153 | type requestAsideView struct { | ||
| 154 | requestSidebarView | ||
| 155 | requestLandingView | ||
| 144 | 156 | } | |
| 145 | 157 | ||
| 146 | 158 | func (w *Web) requestSidebar(r *http.Request, m *repo.Meta, req *metadata.Request, title, errMsg string) requestSidebarView { | |
| @@ -161,14 +173,11 @@ func (w *Web) requestSidebar(r *http.Request, m *repo.Meta, req *metadata.Reques | |||
| 161 | 173 | if acl != nil { | |
| 162 | 174 | required = acl.RequiredApprovals() | |
| 163 | 175 | } | |
| 164 | var checks []requestCheck | ||
| 176 | var checks []*runs.Run | ||
| 165 | 177 | if w.Runs != nil && req.Head != "" { | |
| 166 | 178 | if res, err := w.Runs.ForSHA(m.Name, req.Head); err == nil { | |
| 167 | 179 | for _, run := range res { | |
| 168 | checks = append(checks, requestCheck{ | ||
| 169 | ID: run.ID, Workflow: run.Workflow, Job: run.Job, | ||
| 170 | Status: run.Status, Exit: run.ExitCode, | ||
| 171 | }) | ||
| 180 | checks = append(checks, run) | ||
| 172 | 181 | } | |
| 173 | 182 | sort.Slice(checks, func(i, j int) bool { | |
| 174 | 183 | if checks[i].Workflow != checks[j].Workflow { | |
| @@ -304,11 +313,11 @@ func (w *Web) RequestView(rw http.ResponseWriter, r *http.Request) { | |||
| 304 | 313 | w.renderRequestPage(rw, r, "", http.StatusOK) | |
| 305 | 314 | } | |
| 306 | 315 | ||
| 307 | // renderRequestPage is the request page, optionally with a message at the top and a | ||
| 308 | // status of its own. A write that is refused for a reason the page can explain | ||
| 309 | // re-renders here rather than answering with bare text: the reader keeps the review, | ||
| 310 | // the conflict block and the controls, and the message sits beside them. | ||
| 311 | func (w *Web) renderRequestPage(rw http.ResponseWriter, r *http.Request, errMsg string, status int) { | ||
| 316 | // RequestAside serves just the change sidebar for the page's poll. It is the | ||
| 317 | // same fragment the page renders, so an approval, a finished run, or a push from | ||
| 318 | // somewhere else appears without a reload and without the reader losing the | ||
| 319 | // discussion or diff they were reading. | ||
| 320 | func (w *Web) RequestAside(rw http.ResponseWriter, r *http.Request) { | ||
| 312 | 321 | m, ok := w.open(rw, r) | |
| 313 | 322 | if !ok { | |
| 314 | 323 | return | |
| @@ -318,18 +327,29 @@ func (w *Web) renderRequestPage(rw http.ResponseWriter, r *http.Request, errMsg | |||
| 318 | 327 | w.notFound(rw, r) | |
| 319 | 328 | return | |
| 320 | 329 | } | |
| 330 | w.render(rw, "request_aside", requestAsideView{ | ||
| 331 | requestSidebarView: w.requestSidebar(r, m, req, "", ""), | ||
| 332 | requestLandingView: w.requestLanding(m, req), | ||
| 333 | }, 0) | ||
| 334 | } | ||
| 321 | 335 | ||
| 336 | // requestLanding asks the two questions a reviewer has before approving: does the | ||
| 337 | // branch still merge, and how much does it change? A diff says what changed; it | ||
| 338 | // cannot say whether the branch still merges. merge-tree answers that in memory, | ||
| 339 | // so the reviewer sees "this needs a rebase" before anybody approves — and the | ||
| 340 | // answer is only claimed when the question could actually be asked (base branch | ||
| 341 | // present, head reachable), because a page that says "clean" when it did not | ||
| 342 | // check is worse than one that says nothing. | ||
| 343 | func (w *Web) requestLanding(m *repo.Meta, req *metadata.Request) requestLandingView { | ||
| 322 | 344 | files, _, alreadyMerged := w.requestDiff(m, req) | |
| 323 | comments := w.requestComments(m, req) | ||
| 324 | attachRequestComments(files, comments) | ||
| 325 | added, removed := diffSummary(files) | ||
| 345 | return w.landingFrom(m, req, files, alreadyMerged) | ||
| 346 | } | ||
| 326 | 347 | ||
| 327 | // Would it land? A diff says what changed; it cannot say whether the branch | ||
| 328 | // still merges. merge-tree answers that in memory, so the reviewer sees "this | ||
| 329 | // needs a rebase" before anybody approves — and the answer is only claimed | ||
| 330 | // when the question could actually be asked (base branch present, head | ||
| 331 | // reachable), because a page that says "clean" when it did not check is worse | ||
| 332 | // than one that says nothing. | ||
| 348 | // landingFrom is requestLanding for a caller that already has the diff: the full | ||
| 349 | // page needs the same files to attach inline comments, so it must not run the | ||
| 350 | // diff twice. | ||
| 351 | func (w *Web) landingFrom(m *repo.Meta, req *metadata.Request, files []diffFile, alreadyMerged bool) requestLandingView { | ||
| 352 | added, removed := diffSummary(files) | ||
| 333 | 353 | var conflicts []string | |
| 334 | 354 | mergeChecked := false | |
| 335 | 355 | if req.Open() && req.Head != "" { | |
| @@ -339,21 +359,40 @@ func (w *Web) renderRequestPage(rw http.ResponseWriter, r *http.Request, errMsg | |||
| 339 | 359 | } | |
| 340 | 360 | } | |
| 341 | 361 | } | |
| 362 | return requestLandingView{ | ||
| 363 | FileCount: len(files), Added: added, Removed: removed, | ||
| 364 | AlreadyMerged: alreadyMerged, Conflicts: conflicts, MergeChecked: mergeChecked, | ||
| 365 | } | ||
| 366 | } | ||
| 367 | |||
| 368 | // renderRequestPage is the request page, optionally with a message at the top and a | ||
| 369 | // status of its own. A write that is refused for a reason the page can explain | ||
| 370 | // re-renders here rather than answering with bare text: the reader keeps the review, | ||
| 371 | // the conflict block and the controls, and the message sits beside them. | ||
| 372 | func (w *Web) renderRequestPage(rw http.ResponseWriter, r *http.Request, errMsg string, status int) { | ||
| 373 | m, ok := w.open(rw, r) | ||
| 374 | if !ok { | ||
| 375 | return | ||
| 376 | } | ||
| 377 | req, err := metadata.GetRequest(m.Git(), r.PathValue("id")) | ||
| 378 | if err != nil { | ||
| 379 | w.notFound(rw, r) | ||
| 380 | return | ||
| 381 | } | ||
| 382 | |||
| 383 | files, _, alreadyMerged := w.requestDiff(m, req) | ||
| 384 | comments := w.requestComments(m, req) | ||
| 385 | attachRequestComments(files, comments) | ||
| 342 | 386 | ||
| 343 | 387 | sidebar := w.requestSidebar(r, m, req, m.Name+" · "+req.Title, errMsg) | |
| 344 | 388 | w.render(rw, "request", requestPageView{ | |
| 345 | 389 | requestSidebarView: sidebar, | |
| 390 | requestLandingView: w.landingFrom(m, req, files, alreadyMerged), | ||
| 346 | 391 | Tab: "conversation", | |
| 347 | 392 | Description: w.renderMarkdown(m.Name, req.Description), | |
| 348 | 393 | CommentTarget: commentTargetFromQuery(r), | |
| 349 | 394 | LineCommentURL: "/r/" + m.Name + "/requests/" + req.ID, | |
| 350 | FileCount: len(files), | ||
| 351 | Added: added, | ||
| 352 | Removed: removed, | ||
| 353 | AlreadyMerged: alreadyMerged, | ||
| 354 | 395 | Thread: w.requestThreadFrom(r, m, req, commentTargetFromQuery(r), "", comments), | |
| 355 | Conflicts: conflicts, | ||
| 356 | MergeChecked: mergeChecked, | ||
| 357 | 396 | }, status) | |
| 358 | 397 | } | |
| 359 | 398 | ||