!419 · krz/gitbay

.gitbay/wiki/Users.org +4

		@@ -420,6 +420,10 @@ its change forward: =mr close 4 --by 7= records it and both pages show
420	420	it, and =mr edit 4 --superseded-by 7\|none= sets or clears it
421	421	afterwards, refused on anything but a closed merge request.
422	422
	423	The web list shows each request's combined check state and comment
	424	count alongside its title, so open work needing attention stands out
	425	without opening it.
	426
423	427	Semantics worth knowing:
424	428
425	429	- the MR head lives in the target repository as

e2e/mrweb_test.go +49

		@@ -238,6 +238,55 @@ func TestMRWebCreate(t *testing.T) {
238	238	}
239	239	}
240	240
	241	// TestMRListRows checks that the merge request list shows each row's
	242	// combined check state and comment count (#230).
	243	func TestMRListRows(t *testing.T) {
	244	inst := startInstanceWith(t, "[web]\nmode = \"accounts\"\n")
	245	aliceKey := inst.newKey(t, "alice")
	246	inst.admin(t, "admin", "user", "create", "alice",
	247	"--key", aliceKey+".pub", "--email", "alice@example.test", "--verified")
	248	if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/lib"); code != 0 {
	249	t.Fatalf("repo create: %s", errOut)
	250	}
	251	env := inst.gitEnv(aliceKey)
	252	work := t.TempDir()
	253	mustGit(t, work, env, "clone", inst.sshURL("alice/lib"), "w")
	254	dir := filepath.Join(work, "w")
	255	os.WriteFile(filepath.Join(dir, "a.txt"), []byte("a\n"), 0o644)
	256	mustGit(t, dir, env, "checkout", "-q", "-b", "main")
	257	mustGit(t, dir, env, "add", ".")
	258	mustGit(t, dir, env, "commit", "-q", "-m", "base")
	259	mustGit(t, dir, env, "push", "-q", "origin", "main")
	260	mustGit(t, dir, env, "checkout", "-q", "-b", "topic")
	261	os.WriteFile(filepath.Join(dir, "b.txt"), []byte("b\n"), 0o644)
	262	mustGit(t, dir, env, "add", ".")
	263	mustGit(t, dir, env, "commit", "-q", "-m", "topic work")
	264	mustGit(t, dir, env, "push", "-q", "origin", "topic")
	265	sha := strings.TrimSpace(mustGit(t, dir, env, "rev-parse", "topic"))
	266
	267	if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "create", "alice/lib",
	268	"--source", "topic", "--target", "main", "--title", "feature"); code != 0 {
	269	t.Fatalf("mr create: %s", errOut)
	270	}
	271	if _, errOut, code := inst.ssh(t, aliceKey, "", "status", "set", "alice/lib", sha,
	272	"--context", "ci/test", "--state", "success"); code != 0 {
	273	t.Fatalf("status set: %s", errOut)
	274	}
	275	if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "comment", "alice/lib", "1",
	276	"--message", "hi"); code != 0 {
	277	t.Fatalf("mr comment: %s", errOut)
	278	}
	279
	280	alice := inst.login(t, aliceKey)
	281	_, body := browserGet(t, alice, inst.base()+"/alice/lib/mrs")
	282	if !strings.Contains(body, `class="chip check-success"`) {
	283	t.Errorf("no check chip on the list:\n%s", body)
	284	}
	285	if !strings.Contains(body, `>1 <span class="vh">comments</span>`) {
	286	t.Errorf("no comment count on the list:\n%s", body)
	287	}
	288	}
	289
241	290	// TestMRWebDiffThreads opens a review thread on a diff line and replies to
242	291	// it from the browser. The CLI's view of the threads afterwards is what
243	292	// proves the page dispatched mr diff-comment rather than writing its own

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/store/mrs.go +49

		@@ -487,6 +487,55 @@ func (s *Store) ListMRComments(mrID int64) ([]IssueComment, error) {
487	487	return out, rows.Err()
488	488	}
489	489
	490	// MRCommentCounts totals, per MR, conversation comments plus diff-thread
	491	// roots — what the list page shows as one comment count. System comments,
	492	// diff-thread replies, and pending (unpublished) diff comments do not
	493	// count. The list handler asks for every row on a page in one call rather
	494	// than one query per MR.
	495	func (s *Store) MRCommentCounts(repoID int64, mrIDs []int64) (map[int64]int, error) {
	496	out := map[int64]int{}
	497	if len(mrIDs) == 0 {
	498	return out, nil
	499	}
	500	ph := "?" + strings.Repeat(",?", len(mrIDs)-1)
	501	args := make([]any, 0, len(mrIDs)+1)
	502	args = append(args, repoID)
	503	for _, id := range mrIDs {
	504	args = append(args, id)
	505	}
	506	add := func(query string) error {
	507	rows, err := s.DB.Query(query, args...)
	508	if err != nil {
	509	return err
	510	}
	511	defer rows.Close()
	512	for rows.Next() {
	513	var mrID int64
	514	var n int
	515	if err := rows.Scan(&mrID, &n); err != nil {
	516	return err
	517	}
	518	out[mrID] += n
	519	}
	520	return rows.Err()
	521	}
	522	if err := add(`
	523	SELECT c.mr_id, COUNT(*) FROM mr_comments c
	524	JOIN merge_requests m ON m.id = c.mr_id
	525	WHERE m.repo_id = ? AND c.kind <> 'system' AND c.mr_id IN (` + ph + `)
	526	GROUP BY c.mr_id`); err != nil {
	527	return nil, err
	528	}
	529	if err := add(`
	530	SELECT c.mr_id, COUNT(*) FROM mr_diff_comments c
	531	JOIN merge_requests m ON m.id = c.mr_id
	532	WHERE m.repo_id = ? AND c.reply_to IS NULL AND c.pending = 0 AND c.mr_id IN (` + ph + `)
	533	GROUP BY c.mr_id`); err != nil {
	534	return nil, err
	535	}
	536	return out, nil
	537	}
	538
490	539	func (s *Store) AddMRReview(mrID, reviewerID int64, verdict, headSHA string) error {
491	540	_, err := s.DB.Exec(
492	541	"INSERT INTO mr_reviews (mr_id, reviewer_id, verdict, head_sha) VALUES (?, ?, ?, ?)",

internal/store/mrs_test.go +50

		@@ -184,3 +184,53 @@ func TestPreferredVerifiedEmailDeterministicTiebreak(t *testing.T) {
184	184	t.Fatalf("tiebreak should be alphabetical: %q, %v", addr, err)
185	185	}
186	186	}
	187
	188	// MRCommentCounts folds conversation comments and diff-thread roots into
	189	// one count per MR, for the list page. System comments, diff-thread
	190	// replies, and pending (unpublished) diff comments do not count.
	191	func TestMRCommentCounts(t *testing.T) {
	192	s, repoID, uid := mrFixture(t)
	193	mr1, err := s.MRByNumber(repoID, 1)
	194	if err != nil {
	195	t.Fatal(err)
	196	}
	197	if _, err := s.CreateMR(repoID, uid, repoID, "feature2", "main", "t2", "", "def456", "md", false); err != nil {
	198	t.Fatal(err)
	199	}
	200	mr2, err := s.MRByNumber(repoID, 2)
	201	if err != nil {
	202	t.Fatal(err)
	203	}
	204
	205	if err := s.AddMRComment(mr1.ID, uid, "hi", "md"); err != nil {
	206	t.Fatal(err)
	207	}
	208	if err := s.AddMRSystemComment(mr1.ID, uid, "merged"); err != nil {
	209	t.Fatal(err)
	210	}
	211	rootID, err := s.AddDiffComment(mr1.ID, uid, "abc123", "file.txt", "new", 1, "root", 0, false)
	212	if err != nil {
	213	t.Fatal(err)
	214	}
	215	if _, err := s.AddDiffComment(mr1.ID, uid, "abc123", "file.txt", "new", 1, "reply", rootID, false); err != nil {
	216	t.Fatal(err)
	217	}
	218	if _, err := s.AddDiffComment(mr1.ID, uid, "abc123", "file.txt", "new", 2, "pending root", 0, true); err != nil {
	219	t.Fatal(err)
	220	}
	221
	222	counts, err := s.MRCommentCounts(repoID, []int64{mr1.ID, mr2.ID})
	223	if err != nil {
	224	t.Fatal(err)
	225	}
	226	if counts[mr1.ID] != 2 {
	227	t.Fatalf("mr1 count = %d, want 2 (1 comment + 1 diff root)", counts[mr1.ID])
	228	}
	229	if counts[mr2.ID] != 0 {
	230	t.Fatalf("mr2 count = %d, want 0", counts[mr2.ID])
	231	}
	232
	233	if empty, err := s.MRCommentCounts(repoID, nil); err != nil \|\| len(empty) != 0 {
	234	t.Fatalf("MRCommentCounts(nil) = %v, %v", empty, err)
	235	}
	236	}

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}}

web: merge request list rows show check state and comment count !419