Commit f5294b9a1e
Verified · cmc
internal/httpd/mrsrow_test.go added +72
| @@ -0,0 +1,72 @@ | ||
| 1 | package httpd | |
| 2 | ||
| 3 | import ( | |
| 4 | "strings" | |
| 5 | "testing" | |
| 6 | ||
| 7 | "gitbay.org/gitbay/internal/web" | |
| 8 | ) | |
| 9 | ||
| 10 | // mrsPageData mirrors the anonymous struct the mrs handler renders with. | |
| 11 | type mrsPageData struct { | |
| 12 | repoPage | |
| 13 | State string | |
| 14 | Query string | |
| 15 | Filters []listFilter | |
| 16 | MRs []mrRow | |
| 17 | Older string | |
| 18 | } | |
| 19 | ||
| 20 | func renderMRs(t *testing.T, rows []mrRow, state string) string { | |
| 21 | t.Helper() | |
| 22 | var sb strings.Builder | |
| 23 | if err := web.Render(&sb, "mrs.html", mrsPageData{ | |
| 24 | repoPage: testRepoPage(), State: state, MRs: rows, | |
| 25 | }); err != nil { | |
| 26 | t.Fatalf("render: %v", err) | |
| 27 | } | |
| 28 | return sb.String() | |
| 29 | } | |
| 30 | ||
| 31 | // Each row carries its head's combined check state as a chip linking to | |
| 32 | // the diff, so a reviewer can tell what needs attention without opening | |
| 33 | // every request (#230). | |
| 34 | func TestMRListRowShowsCheck(t *testing.T) { | |
| 35 | out := renderMRs(t, []mrRow{{MR: testMR("open"), Check: "success"}}, "open") | |
| 36 | if !strings.Contains(out, `class="chip check-success"`) { | |
| 37 | t.Errorf("no check chip:\n%s", out) | |
| 38 | } | |
| 39 | if !strings.Contains(out, `/krz/gitbay/mrs/42?view=diff`) { | |
| 40 | t.Errorf("check chip does not link to the diff:\n%s", out) | |
| 41 | } | |
| 42 | } | |
| 43 | ||
| 44 | // No checks have reported yet: no chip, not an empty one. | |
| 45 | func TestMRListRowHidesEmptyCheck(t *testing.T) { | |
| 46 | out := renderMRs(t, []mrRow{{MR: testMR("open")}}, "open") | |
| 47 | if strings.Contains(out, "check-") { | |
| 48 | t.Errorf("empty check rendered a chip:\n%s", out) | |
| 49 | } | |
| 50 | } | |
| 51 | ||
| 52 | // The comment count folds conversation and diff-thread comments (see | |
| 53 | // store.MRCommentCounts) and is hidden, not zero, when there are none. | |
| 54 | func TestMRListRowShowsCommentCount(t *testing.T) { | |
| 55 | out := renderMRs(t, []mrRow{{MR: testMR("open"), Comments: 3}}, "open") | |
| 56 | if !strings.Contains(out, `title="3 comments"`) { | |
| 57 | t.Errorf("no plural comment count:\n%s", out) | |
| 58 | } | |
| 59 | if !strings.Contains(out, `>3 <span class="vh">comments</span>`) { | |
| 60 | t.Errorf("no comment count text:\n%s", out) | |
| 61 | } | |
| 62 | ||
| 63 | one := renderMRs(t, []mrRow{{MR: testMR("open"), Comments: 1}}, "open") | |
| 64 | if !strings.Contains(one, `title="1 comment"`) { | |
| 65 | t.Errorf("comment count not singular for 1:\n%s", one) | |
| 66 | } | |
| 67 | ||
| 68 | none := renderMRs(t, []mrRow{{MR: testMR("open")}}, "open") | |
| 69 | if strings.Contains(none, "vh\">comments</span>") { | |
| 70 | t.Errorf("zero comments rendered a count:\n%s", none) | |
| 71 | } | |
| 72 | } | |
internal/httpd/web.go +29 −2
| @@ -1790,6 +1790,15 @@ func (s *Server) canEditItem(r *http.Request, repo store.Repo, author string) bo | ||
| 1790 | 1790 | return policy.CanWrite(u, repo, grant) |
| 1791 | 1791 | } |
| 1792 | 1792 | |
| 1793 | // mrRow is one row of the merge request list: the MR plus its head's | |
| 1794 | // combined check state and its comment count. Errors gathering either | |
| 1795 | // fall back to zero values (#230) — the list must still render. | |
| 1796 | type mrRow struct { | |
| 1797 | store.MR | |
| 1798 | Check string | |
| 1799 | Comments int | |
| 1800 | } | |
| 1801 | ||
| 1793 | 1802 | func (s *Server) mrs(w http.ResponseWriter, r *http.Request) { |
| 1794 | 1803 | p, ok := s.repoFor(w, r, "") |
| 1795 | 1804 | if !ok { |
| @@ -1818,15 +1827,33 @@ func (s *Server) mrs(w http.ResponseWriter, r *http.Request) { | ||
| 1818 | 1827 | mrs = mrs[:listPage] |
| 1819 | 1828 | older = olderLink(r, mrs[len(mrs)-1].Number) |
| 1820 | 1829 | } |
| 1830 | shas := make([]string, len(mrs)) | |
| 1831 | ids := make([]int64, len(mrs)) | |
| 1832 | for i, m := range mrs { | |
| 1833 | shas[i] = m.HeadSHA | |
| 1834 | ids[i] = m.ID | |
| 1835 | } | |
| 1836 | checks, err := s.st.CombinedStatusFor(p.Repo.ID, shas) | |
| 1837 | if err != nil { | |
| 1838 | checks = map[string]string{} | |
| 1839 | } | |
| 1840 | comments, err := s.st.MRCommentCounts(p.Repo.ID, ids) | |
| 1841 | if err != nil { | |
| 1842 | comments = map[int64]int{} | |
| 1843 | } | |
| 1844 | rows := make([]mrRow, len(mrs)) | |
| 1845 | for i, m := range mrs { | |
| 1846 | rows[i] = mrRow{MR: m, Check: checks[m.HeadSHA], Comments: comments[m.ID]} | |
| 1847 | } | |
| 1821 | 1848 | s.render(w, "mrs.html", struct { |
| 1822 | 1849 | repoPage |
| 1823 | 1850 | State string |
| 1824 | 1851 | Query string |
| 1825 | 1852 | Filters []listFilter |
| 1826 | MRs []store.MR | |
| 1853 | MRs []mrRow | |
| 1827 | 1854 | Older string |
| 1828 | 1855 | }{p, state, mf.Search, |
| 1829 | activeFilters(state, [][2]string{{"author", mf.Author}, {"milestone", mf.Milestone}}), mrs, older}) | |
| 1856 | activeFilters(state, [][2]string{{"author", mf.Author}, {"milestone", mf.Milestone}}), rows, older}) | |
| 1830 | 1857 | } |
| 1831 | 1858 | |
| 1832 | 1859 | func (s *Server) mr(w http.ResponseWriter, r *http.Request) { |
internal/web/static/style.css +1
| @@ -906,6 +906,7 @@ ul.issuelist li:hover { background: var(--hover); } | ||
| 906 | 906 | ul.issuelist li.empty { display: block; } |
| 907 | 907 | ul.issuelist p { margin: 0; } |
| 908 | 908 | ul.issuelist .issuemain { flex: 1 1 auto; min-width: 0; } |
| 909 | ul.issuelist .issueside { display: flex; gap: var(--sp-2); align-items: center; flex: none; } | |
| 909 | 910 | ul.issuelist .title a { color: var(--fg); font-weight: 500; } |
| 910 | 911 | ul.issuelist .title a:hover { color: var(--link); } |
| 911 | 912 | |
internal/web/templates/mrs.html +4 −2
| @@ -17,12 +17,14 @@ | ||
| 17 | 17 | </div> |
| 18 | 18 | {{if .Viewer}}<p class="meta"><a href="/{{.Repo.OwnerName}}/{{.Repo.Name}}/mrs/new">New merge request</a></p>{{end}} |
| 19 | 19 | <ul class="issuelist"> |
| 20 | {{range .MRs}}<li> | |
| 20 | {{range .MRs}}{{$n := .Number}}<li> | |
| 21 | 21 | <div class="issuemain"> |
| 22 | 22 | <p class="title"><a href="/{{$.Repo.OwnerName}}/{{$.Repo.Name}}/mrs/{{.Number}}">{{.Title}}</a></p> |
| 23 | 23 | <p class="meta">!{{.Number}} by <a href="/{{.Author}}">{{.Author}}</a> · {{if .SourcePath}}{{.SourcePath}}:{{end}}{{.SourceRef}} → {{.TargetRef}}</p> |
| 24 | 24 | </div> |
| 25 | {{if .Draft}}<span class="chip chip-neutral">draft</span> {{end}}{{if eq $.State "all"}}<span class="chip chip-{{.State}}">{{.State}}</span>{{end}} | |
| 25 | <div class="issueside"> | |
| 26 | {{if .Draft}}<span class="chip chip-neutral">draft</span> {{end}}{{if eq $.State "all"}}<span class="chip chip-{{.State}}">{{.State}}</span> {{end}}{{with .Check}}<a class="chip check-{{.}}" href="/{{$.Repo.OwnerName}}/{{$.Repo.Name}}/mrs/{{$n}}?view=diff" title="checks: {{.}}">{{.}}</a> {{end}}{{if .Comments}}<span class="chip chip-neutral" title="{{.Comments}} comment{{if ne .Comments 1}}s{{end}}">{{.Comments}} <span class="vh">comments</span></span>{{end}} | |
| 27 | </div> | |
| 26 | 28 | </li> |
| 27 | 29 | {{else}}{{if .Query}}<li class="empty">no {{if ne .State "all"}}{{.State}} {{end}}merge requests matching “{{.Query}}”</li> |
| 28 | 30 | {{else}}<li class="empty">no {{if ne .State "all"}}{{.State}} {{end}}merge requests — open one with <code>gitbay mr create {{.Repo.OwnerName}}/{{.Repo.Name}} --source ... --target {{.Repo.DefaultBranch}}</code></li>{{end}}{{end}} |