Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.

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.

Show what a merged change request landed

2841d922c15ce32fb600d992f9f5dc2d22c48f90 · Keenan Nemetz · 13d ago · parent c765ae2e

Show what a merged change request landed

Landing moves the base to include the head, so the three-dot diff the
Files tab reviews an open request with becomes empty by definition: the
merge base of the base and the head is the head itself. A merged request
therefore looked like it had never changed anything.

The landing mark now records `merged_base`, the tip the base pointed at
before it moved, and the Files tab diffs that against what landed: the
head for a fast-forward, the merge commit otherwise, so conflict
resolution is part of the review too. The record is written in the same
commit as the mark, and a retry that finds the head already landed keeps
the base it has instead of naming the moved tip.

`merged_base` is optional on read because landings made before the field
existed have none. Such a record falls back to the merge commit's first
parent when the landing was a merge, and claims nothing when it was a
fast-forward and the starting point is gone.

The request page shows the file count and the review link for a merged
request as well, with the merge named beside it, instead of replacing
them with "Already merged".

server/web_test.go +69

@@ -107,6 +107,75 @@ func TestRequestPageShowsWhatWouldLand(t *testing.T) {
107 107 }
108 108 }
109 109
110 // TestMergedRequestStillShowsWhatLanded: landing moves the base to include the head,
111 // so the three-dot review of an open request becomes empty — the merge base of the
112 // base and the head is the head itself. The record of the base tip is what keeps the
113 // change readable afterwards, on both the request page and the Files tab.
114 func TestMergedRequestStillShowsWhatLanded(t *testing.T) {
115 t.Parallel()
116 f := newFixture(t)
117 f.publish(f.repo)
118
119 base := workRepo(t, map[string]string{"readme.md": "v1\n"})
120 if _, errs, err := f.git(base, "push", f.sshURL(f.repo), "main:refs/heads/main"); err != nil {
121 t.Fatalf("main push: %s", errs)
122 }
123 baseTip, _, err := runGit(t, base, "rev-parse", "refs/heads/main")
124 if err != nil {
125 t.Fatal(err)
126 }
127 feature, head := f.cloneWithChange(t, map[string]string{"readme.md": "v1\nfeature line\n"}, "feature")
128 if _, errs, err := f.git(feature, "push", f.sshURL(f.repo), "HEAD:refs/heads/feature"); err != nil {
129 t.Fatalf("feature push: %s", errs)
130 }
131
132 meta := f.newMetaRepo(t)
133 if err := meta.apply(map[string]string{
134 "requests/" + reqID + "/request.md": requestFile("main", head, "keenan", "why this change\n"),
135 }, "request"); err != nil {
136 t.Fatalf("request push: %v", err)
137 }
138 if _, errs, err := f.cli(feature, "change", "merge", reqID); err != nil {
139 t.Fatalf("merge: %v\n%s", err, errs)
140 }
141
142 // The landing names where the branch started from, in the same commit that says
143 // it landed.
144 record, _, err := runGit(t, filepath.Join(f.cfg.ReposDir(), f.repo+".git"),
145 "show", "refs/heads/vole:requests/"+reqID+"/request.md")
146 if err != nil {
147 t.Fatalf("read the record: %v", err)
148 }
149 for _, want := range []string{"state: merged", "merged_base: " + strings.TrimSpace(baseTip)} {
150 if !strings.Contains(record, want) {
151 t.Fatalf("the merged record is missing %q:\n%s", want, record)
152 }
153 }
154
155 code, body := f.get("/r/" + f.repo + "/requests/" + reqID)
156 if code != 200 {
157 t.Fatalf("merged request page = %d", code)
158 }
159 for _, want := range []string{"1 file(s) changed", "Review files", "merged into main"} {
160 if !strings.Contains(body, want) {
161 t.Fatalf("merged request page does not show %q:\n%s", want, body)
162 }
163 }
164 code, body = f.get("/r/" + f.repo + "/requests/" + reqID + "/files")
165 if code != 200 {
166 t.Fatalf("merged request files page = %d", code)
167 }
168 for _, want := range []string{"readme.md", "feature line", `class="add"`, `id="review-tree"`} {
169 if !strings.Contains(body, want) {
170 t.Fatalf("merged request files page does not show %q:\n%s", want, body)
171 }
172 }
173 if strings.Contains(body, "No changes, or the head is not reachable") ||
174 strings.Contains(body, "Already merged into main.") {
175 t.Fatalf("a merged request with a recorded base hid its change:\n%s", body)
176 }
177 }
178
110 179 // TestCommitPageEscapesFileContent matters because the web renders other people's
111 180 // commits: a diff that executes a tag is an XSS, not a syntax error.
112 181 func TestCommitPageEscapesFileContent(t *testing.T) {