Commit f1cc68cb41
Verified · cmc ci/build: success ci/sonar: success ci/test: success ci/vuln: success
Layout: unified · split
.gitbay/wiki/Parity.org +4 −2
| @@ -44,7 +44,7 @@ browser-only and the iOS build screen unable to say more than the log. | |||
| 44 | | create | yes | yes | yes | | 44 | | create | yes | yes | yes | |
| 45 | | draft, ready | yes | yes | no | | 45 | | draft, ready | yes | yes | no | |
| 46 | | search title and body | yes | yes | no | | 46 | | search title and body | yes | yes | no | |
| 47 | | create from a fork | yes | no | yes | | 47 | | create from a fork | yes | yes | yes | |
| 48 | | retarget | yes | yes | no | | 48 | | retarget | yes | yes | no | |
| 49 | | milestone | yes | yes | yes | | 49 | | milestone | yes | yes | yes | |
| 50 | | choose body markup | yes | no | no | | 50 | | choose body markup | yes | no | no | |
| @@ -78,7 +78,9 @@ an org or team — so a collaborator with write access who has not touched | |||
| 78 | a thread hears nothing until they do (krz/gitbay#145). | 78 | a thread hears nothing until they do (krz/gitbay#145). |
| 79 | 79 | ||
| 80 | Creating from a fork works anywhere the source can be typed as | 80 | Creating from a fork works anywhere the source can be typed as |
| 81 | =owner/name:branch=; only the web lacks it. Retargeting moves an open | 81 | =owner/name:branch=. The web's source picker offers the branches of |
| 82 | every fork the viewer can push to in that form; forking itself is still | ||
| 83 | a CLI operation. Retargeting moves an open | ||
| 82 | merge request onto another branch of the same repository and stales the | 84 | merge request onto another branch of the same repository and stales the |
| 83 | reviews, since an approval was of the diff against the old branch. | 85 | reviews, since an approval was of the diff against the old branch. |
| 84 | 86 | ||
e2e/mrforkweb_test.go added +83
| @@ -0,0 +1,83 @@ | |||
| 1 | package e2e | ||
| 2 | |||
| 3 | import ( | ||
| 4 | "net/url" | ||
| 5 | "os" | ||
| 6 | "path/filepath" | ||
| 7 | "strings" | ||
| 8 | "testing" | ||
| 9 | ) | ||
| 10 | |||
| 11 | // A contributor without write access proposes a change from the browser: | ||
| 12 | // the source picker offers the branches of a fork they can push to, and | ||
| 13 | // the merge request opens against the parent (#168). | ||
| 14 | func TestMRFromForkWeb(t *testing.T) { | ||
| 15 | inst := startInstanceWith(t, "[web]\nmode = \"accounts\"\n") | ||
| 16 | aliceKey := inst.newKey(t, "alice") | ||
| 17 | bobKey := inst.newKey(t, "bob") | ||
| 18 | eveKey := inst.newKey(t, "eve") | ||
| 19 | inst.admin(t, "admin", "user", "create", "alice", "--key", aliceKey+".pub") | ||
| 20 | inst.admin(t, "admin", "user", "create", "bob", "--key", bobKey+".pub") | ||
| 21 | inst.admin(t, "admin", "user", "create", "eve", "--key", eveKey+".pub") | ||
| 22 | |||
| 23 | if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/app"); code != 0 { | ||
| 24 | t.Fatalf("repo create: %s", errOut) | ||
| 25 | } | ||
| 26 | env := inst.gitEnv(aliceKey) | ||
| 27 | work := t.TempDir() | ||
| 28 | mustGit(t, work, env, "clone", inst.sshURL("alice/app"), "w") | ||
| 29 | dir := filepath.Join(work, "w") | ||
| 30 | os.WriteFile(filepath.Join(dir, "f.txt"), []byte("x\n"), 0o644) | ||
| 31 | mustGit(t, dir, env, "checkout", "-q", "-b", "main") | ||
| 32 | mustGit(t, dir, env, "add", ".") | ||
| 33 | mustGit(t, dir, env, "commit", "-q", "-m", "base") | ||
| 34 | mustGit(t, dir, env, "push", "-q", "origin", "main") | ||
| 35 | |||
| 36 | // bob forks and pushes a branch to the fork. | ||
| 37 | if _, errOut, code := inst.ssh(t, bobKey, "", "repo", "fork", "alice/app"); code != 0 { | ||
| 38 | t.Fatalf("fork: %s", errOut) | ||
| 39 | } | ||
| 40 | benv := inst.gitEnv(bobKey) | ||
| 41 | bwork := t.TempDir() | ||
| 42 | mustGit(t, bwork, benv, "clone", inst.sshURL("bob/app"), "w") | ||
| 43 | bdir := filepath.Join(bwork, "w") | ||
| 44 | mustGit(t, bdir, benv, "checkout", "-q", "-b", "feat") | ||
| 45 | os.WriteFile(filepath.Join(bdir, "f.txt"), []byte("y\n"), 0o644) | ||
| 46 | mustGit(t, bdir, benv, "add", ".") | ||
| 47 | mustGit(t, bdir, benv, "commit", "-q", "-m", "change") | ||
| 48 | mustGit(t, bdir, benv, "push", "-q", "origin", "feat") | ||
| 49 | |||
| 50 | // bob's fork branch is offered on alice's repo; eve, who can push to | ||
| 51 | // neither, sees only the target's own branches. | ||
| 52 | bob := inst.login(t, bobKey) | ||
| 53 | eve := inst.login(t, eveKey) | ||
| 54 | base := inst.base() + "/alice/app" | ||
| 55 | _, page := browserGet(t, bob, base+"/mrs/new") | ||
| 56 | if !strings.Contains(page, `value="bob/app:feat"`) { | ||
| 57 | t.Fatalf("fork branch not offered:\n%s", page) | ||
| 58 | } | ||
| 59 | if _, p := browserGet(t, eve, base+"/mrs/new"); strings.Contains(p, "bob/app:feat") { | ||
| 60 | t.Fatal("a fork branch is offered to someone who cannot push to it") | ||
| 61 | } | ||
| 62 | |||
| 63 | // Opening it lands on the merge request, with the fork as its source. | ||
| 64 | if status, _ := browserPost(t, bob, base+"/mrs/new", url.Values{ | ||
| 65 | "source": {"bob/app:feat"}, "target": {"main"}, "title": {"change"}}); status != 200 { | ||
| 66 | t.Fatal("mr create from a fork failed") | ||
| 67 | } | ||
| 68 | out, _, _ := inst.ssh(t, bobKey, "", "mr", "show", "alice/app", "1", "--json") | ||
| 69 | if !strings.Contains(out, `"source":"bob/app:feat"`) { | ||
| 70 | t.Fatalf("merge request source is not the fork:\n%s", out) | ||
| 71 | } | ||
| 72 | |||
| 73 | // A branch of a repository that is not a fork of the target is | ||
| 74 | // refused by the command, whatever the form posts. | ||
| 75 | if _, errOut, code := inst.ssh(t, eveKey, "", "repo", "create", "eve/other"); code != 0 { | ||
| 76 | t.Fatalf("repo create: %s", errOut) | ||
| 77 | } | ||
| 78 | _, body := browserPost(t, eve, base+"/mrs/new", url.Values{ | ||
| 79 | "source": {"eve/other:main"}, "target": {"main"}, "title": {"sneak"}}) | ||
| 80 | if !strings.Contains(body, `class="error"`) { | ||
| 81 | t.Errorf("a non-fork source was accepted:\n%s", body) | ||
| 82 | } | ||
| 83 | } | ||
internal/httpd/mractions.go +32 −1
| @@ -9,6 +9,7 @@ import ( | |||
| 9 | 9 | ||
| 10 | "gitbay.org/gitbay/internal/control" | 10 | "gitbay.org/gitbay/internal/control" |
| 11 | "gitbay.org/gitbay/internal/gitutil" | 11 | "gitbay.org/gitbay/internal/gitutil" |
| 12 | "gitbay.org/gitbay/internal/policy" | ||
| 12 | "gitbay.org/gitbay/internal/store" | 13 | "gitbay.org/gitbay/internal/store" |
| 13 | ) | 14 | ) |
| 14 | 15 | ||
| @@ -158,6 +159,7 @@ func (s *Server) mrThreadSubmit(w http.ResponseWriter, r *http.Request, u store. | |||
| 158 | type mrNewPage struct { | 159 | type mrNewPage struct { |
| 159 | repoPage | 160 | repoPage |
| 160 | Branches []gitutil.Ref | 161 | Branches []gitutil.Ref |
| 162 | Sources []string | ||
| 161 | Source string | 163 | Source string |
| 162 | Target string | 164 | Target string |
| 163 | Title string | 165 | Title string |
| @@ -166,6 +168,35 @@ type mrNewPage struct { | |||
| 166 | Notice string | 168 | Notice string |
| 167 | } | 169 | } |
| 168 | 170 | ||
| 171 | // mrSources lists the branches a merge request may be opened from, in the | ||
| 172 | // form the command takes: this repository's branches by name, and those of | ||
| 173 | // any fork of it the viewer can push to as "owner/name:branch". | ||
| 174 | // Write is the filter because a contributor proposes from a fork they | ||
| 175 | // own; the command still checks the source for itself (#168). | ||
| 176 | func (s *Server) mrSources(u store.User, p repoPage) []string { | ||
| 177 | var out []string | ||
| 178 | branches, _ := gitutil.Refs(p.Dir, "heads") | ||
| 179 | for _, b := range branches { | ||
| 180 | out = append(out, b.Name) | ||
| 181 | } | ||
| 182 | if u.ID == 0 { | ||
| 183 | return out | ||
| 184 | } | ||
| 185 | forks, _ := s.st.ListForks(p.Repo.ID) | ||
| 186 | for _, f := range forks { | ||
| 187 | grant, _ := s.st.AccessRole(f.ID, u.ID) | ||
| 188 | if !policy.CanWrite(u, f, grant) { | ||
| 189 | continue | ||
| 190 | } | ||
| 191 | dir := control.RepoDir(s.cfg.Server.Root, f.OwnerName, f.Name) | ||
| 192 | refs, _ := gitutil.Refs(dir, "heads") | ||
| 193 | for _, b := range refs { | ||
| 194 | out = append(out, f.Path()+":"+b.Name) | ||
| 195 | } | ||
| 196 | } | ||
| 197 | return out | ||
| 198 | } | ||
| 199 | |||
| 169 | func (s *Server) mrCreateForm(w http.ResponseWriter, r *http.Request, u store.User) { | 200 | func (s *Server) mrCreateForm(w http.ResponseWriter, r *http.Request, u store.User) { |
| 170 | p, ok := s.repoFor(w, r, "") | 201 | p, ok := s.repoFor(w, r, "") |
| 171 | if !ok { | 202 | if !ok { |
| @@ -183,7 +214,7 @@ func (s *Server) mrCreateForm(w http.ResponseWriter, r *http.Request, u store.Us | |||
| 183 | format = "md" | 214 | format = "md" |
| 184 | } | 215 | } |
| 185 | s.render(w, "mrnew.html", mrNewPage{ | 216 | s.render(w, "mrnew.html", mrNewPage{ |
| 186 | repoPage: p, Branches: branches, | 217 | repoPage: p, Branches: branches, Sources: s.mrSources(u, p), |
| 187 | Source: q.Get("source"), Target: target, | 218 | Source: q.Get("source"), Target: target, |
| 188 | Title: q.Get("title"), Body: q.Get("body"), Format: format, Notice: s.takeFlash(w, r), | 219 | Title: q.Get("title"), Body: q.Get("body"), Format: format, Notice: s.takeFlash(w, r), |
| 189 | }) | 220 | }) |
internal/store/repos.go +19
| @@ -354,6 +354,25 @@ func (s *Store) ListPublicRepos() ([]Repo, error) { | |||
| 354 | return out, rows.Err() | 354 | return out, rows.Err() |
| 355 | } | 355 | } |
| 356 | 356 | ||
| 357 | // ListForks returns the repositories forked from one repo. The caller | ||
| 358 | // filters by what the viewer may see. | ||
| 359 | func (s *Store) ListForks(repoID int64) ([]Repo, error) { | ||
| 360 | rows, err := s.DB.Query(repoSelect+" WHERE r.fork_of = ? ORDER BY 4, r.name", repoID) | ||
| 361 | if err != nil { | ||
| 362 | return nil, err | ||
| 363 | } | ||
| 364 | defer rows.Close() | ||
| 365 | var out []Repo | ||
| 366 | for rows.Next() { | ||
| 367 | r, err := scanRepo(rows) | ||
| 368 | if err != nil { | ||
| 369 | return nil, err | ||
| 370 | } | ||
| 371 | out = append(out, r) | ||
| 372 | } | ||
| 373 | return out, rows.Err() | ||
| 374 | } | ||
| 375 | |||
| 357 | func (s *Store) UpdateDefaultBranch(repoID int64, branch string) error { | 376 | func (s *Store) UpdateDefaultBranch(repoID int64, branch string) error { |
| 358 | _, err := s.DB.Exec("UPDATE repos SET default_branch = ? WHERE id = ?", branch, repoID) | 377 | _, err := s.DB.Exec("UPDATE repos SET default_branch = ? WHERE id = ?", branch, repoID) |
| 359 | return err | 378 | return err |
internal/web/templates/mrnew.html +4 −3
| @@ -7,7 +7,7 @@ | |||
| 7 | <label for="source">Merge</label> | 7 | <label for="source">Merge</label> |
| 8 | <select id="source" name="source" required> | 8 | <select id="source" name="source" required> |
| 9 | <option value="">Choose a branch…</option> | 9 | <option value="">Choose a branch…</option> |
| 10 | {{range .Branches}}<option value="{{.Name}}"{{if eq .Name $.Source}} selected{{end}}>{{.Name}}</option>{{end}} | 10 | {{range .Sources}}<option value="{{.}}"{{if eq . $.Source}} selected{{end}}>{{.}}</option>{{end}} |
| 11 | </select> | 11 | </select> |
| 12 | <label for="target">into</label> | 12 | <label for="target">into</label> |
| 13 | <select id="target" name="target"> | 13 | <select id="target" name="target"> |
| @@ -19,6 +19,7 @@ | |||
| 19 | {{template "formatpicker" .Format}} | 19 | {{template "formatpicker" .Format}} |
| 20 | <p><button type="submit">Open merge request</button></p> | 20 | <p><button type="submit">Open merge request</button></p> |
| 21 | </form> | 21 | </form> |
| 22 | <p class="meta">A branch in a fork opens from the CLI: | 22 | <p class="meta">Branches of a fork you can push to are offered as |
| 23 | <code>gitbay mr create {{.Repo.OwnerName}}/{{.Repo.Name}} --source owner/fork:branch --target {{.Repo.DefaultBranch}} --title "…"</code></p> | 23 | <code>owner/name:branch</code>. Fork this repository to propose a change |
| 24 | without write access here: <code>gitbay repo fork {{.Repo.OwnerName}}/{{.Repo.Name}}</code></p> | ||
| 24 | {{end}} | 25 | {{end}} |