Commit 82b9f3aba2
Verified · cmc
Layout: unified · split
internal/httpd/compare.go +4 −2
| @@ -73,6 +73,8 @@ func (s *Server) compare(w http.ResponseWriter, r *http.Request) { | |||
| 73 | } | 73 | } |
| 74 | commits = append(commits, cr) | 74 | commits = append(commits, cr) |
| 75 | } | 75 | } |
| 76 | canWrite := s.canWriteRepo(r, p.Repo) | ||
| 77 | canOpenMR := canWrite || len(s.writableForks(s.viewer(r), p.Repo)) > 0 | ||
| 76 | s.render(w, "compare.html", struct { | 78 | s.render(w, "compare.html", struct { |
| 77 | repoPage | 79 | repoPage |
| 78 | Base, Head, BaseSHA, HeadSHA, MergeBase string | 80 | Base, Head, BaseSHA, HeadSHA, MergeBase string |
| @@ -81,6 +83,6 @@ func (s *Server) compare(w http.ResponseWriter, r *http.Request) { | |||
| 81 | DiffFiles []diffFile | 83 | DiffFiles []diffFile |
| 82 | DiffTruncated bool | 84 | DiffTruncated bool |
| 83 | Stat diffStat | 85 | Stat diffStat |
| 84 | CanWrite bool | 86 | CanOpenMR bool |
| 85 | }{p, base, head, baseSHA, headSHA, mergeBase, commits, total, files, truncated, statOf(files), s.canWriteRepo(r, p.Repo)}) | 87 | }{p, base, head, baseSHA, headSHA, mergeBase, commits, total, files, truncated, statOf(files), canOpenMR}) |
| 86 | } | 88 | } |
internal/httpd/mractions.go +22 −12
| @@ -187,26 +187,36 @@ type mrNewPage struct { | |||
| 187 | Draft *draft | 187 | Draft *draft |
| 188 | } | 188 | } |
| 189 | 189 | ||
| 190 | // writableForks lists the forks of repo that u can push to — the source | ||
| 191 | // half of what a merge request may be opened from, alongside the | ||
| 192 | // repository's own branches. Write is the filter because a contributor | ||
| 193 | // proposes from a fork they own; the command still checks the source for | ||
| 194 | // itself (#168). | ||
| 195 | func (s *Server) writableForks(u store.User, repo store.Repo) []store.Repo { | ||
| 196 | if u.ID == 0 { | ||
| 197 | return nil | ||
| 198 | } | ||
| 199 | forks, _ := s.st.ListForks(repo.ID) | ||
| 200 | var out []store.Repo | ||
| 201 | for _, f := range forks { | ||
| 202 | grant, _ := s.st.AccessRole(f.ID, u.ID) | ||
| 203 | if policy.CanWrite(u, f, grant) { | ||
| 204 | out = append(out, f) | ||
| 205 | } | ||
| 206 | } | ||
| 207 | return out | ||
| 208 | } | ||
| 209 | |||
| 190 | // mrSources lists the branches a merge request may be opened from, in the | 210 | // mrSources lists the branches a merge request may be opened from, in the |
| 191 | // form the command takes: this repository's branches by name, and those of | 211 | // form the command takes: this repository's branches by name, and those of |
| 192 | // any fork of it the viewer can push to as "owner/name:branch". | 212 | // any writable fork as "owner/name:branch". |
| 193 | // Write is the filter because a contributor proposes from a fork they | ||
| 194 | // own; the command still checks the source for itself (#168). | ||
| 195 | func (s *Server) mrSources(u store.User, p repoPage) []string { | 213 | func (s *Server) mrSources(u store.User, p repoPage) []string { |
| 196 | var out []string | 214 | var out []string |
| 197 | branches, _ := gitutil.Refs(p.Dir, "heads") | 215 | branches, _ := gitutil.Refs(p.Dir, "heads") |
| 198 | for _, b := range branches { | 216 | for _, b := range branches { |
| 199 | out = append(out, b.Name) | 217 | out = append(out, b.Name) |
| 200 | } | 218 | } |
| 201 | if u.ID == 0 { | 219 | for _, f := range s.writableForks(u, p.Repo) { |
| 202 | return out | ||
| 203 | } | ||
| 204 | forks, _ := s.st.ListForks(p.Repo.ID) | ||
| 205 | for _, f := range forks { | ||
| 206 | grant, _ := s.st.AccessRole(f.ID, u.ID) | ||
| 207 | if !policy.CanWrite(u, f, grant) { | ||
| 208 | continue | ||
| 209 | } | ||
| 210 | dir := control.RepoDir(s.cfg.Server.Root, f.OwnerName, f.Name) | 220 | dir := control.RepoDir(s.cfg.Server.Root, f.OwnerName, f.Name) |
| 211 | refs, _ := gitutil.Refs(dir, "heads") | 221 | refs, _ := gitutil.Refs(dir, "heads") |
| 212 | for _, b := range refs { | 222 | for _, b := range refs { |
internal/httpd/mrslist_test.go +17 −1
| @@ -31,7 +31,15 @@ func TestMRsListContributionHintByAccess(t *testing.T) { | |||
| 31 | if err != nil { | 31 | if err != nil { |
| 32 | t.Fatal(err) | 32 | t.Fatal(err) |
| 33 | } | 33 | } |
| 34 | if _, err := st.CreateRepo("user", owner, "app", "public"); err != nil { | 34 | forker, err := st.CreateUser("carol", false) |
| 35 | if err != nil { | ||
| 36 | t.Fatal(err) | ||
| 37 | } | ||
| 38 | repoID, err := st.CreateRepo("user", owner, "app", "public") | ||
| 39 | if err != nil { | ||
| 40 | t.Fatal(err) | ||
| 41 | } | ||
| 42 | if _, err := st.CreateFork("user", forker, "app", "public", repoID); err != nil { | ||
| 35 | t.Fatal(err) | 43 | t.Fatal(err) |
| 36 | } | 44 | } |
| 37 | 45 | ||
| @@ -83,4 +91,12 @@ func TestMRsListContributionHintByAccess(t *testing.T) { | |||
| 83 | if !strings.Contains(ownerOut, "New merge request") { | 91 | if !strings.Contains(ownerOut, "New merge request") { |
| 84 | t.Errorf("owner: missing New merge request link:\n%s", ownerOut) | 92 | t.Errorf("owner: missing New merge request link:\n%s", ownerOut) |
| 85 | } | 93 | } |
| 94 | |||
| 95 | forkerOut := get(forker) | ||
| 96 | if !strings.Contains(forkerOut, "New merge request") { | ||
| 97 | t.Errorf("reader with a writable fork: missing New merge request link:\n%s", forkerOut) | ||
| 98 | } | ||
| 99 | if strings.Contains(forkerOut, "Fork this repository to propose a change") { | ||
| 100 | t.Error("reader with a writable fork should not see the fork hint") | ||
| 101 | } | ||
| 86 | } | 102 | } |
internal/httpd/mrsrow_test.go +7 −7
| @@ -10,13 +10,13 @@ import ( | |||
| 10 | // mrsPageData mirrors the anonymous struct the mrs handler renders with. | 10 | // mrsPageData mirrors the anonymous struct the mrs handler renders with. |
| 11 | type mrsPageData struct { | 11 | type mrsPageData struct { |
| 12 | repoPage | 12 | repoPage |
| 13 | State string | 13 | State string |
| 14 | Query string | 14 | Query string |
| 15 | Filters []listFilter | 15 | Filters []listFilter |
| 16 | Facets []facetGroup | 16 | Facets []facetGroup |
| 17 | MRs []mrRow | 17 | MRs []mrRow |
| 18 | Older string | 18 | Older string |
| 19 | CanWrite bool | 19 | CanOpenMR bool |
| 20 | } | 20 | } |
| 21 | 21 | ||
| 22 | func renderMRs(t *testing.T, rows []mrRow, state string) string { | 22 | func renderMRs(t *testing.T, rows []mrRow, state string) string { |
internal/httpd/web.go +3 −2
| @@ -2095,6 +2095,7 @@ func (s *Server) mrs(w http.ResponseWriter, r *http.Request) { | |||
| 2095 | allLabels, _ := s.st.ListLabels(p.Repo, readable) | 2095 | allLabels, _ := s.st.ListLabels(p.Repo, readable) |
| 2096 | openMS, _ := s.st.ListMilestones(p.Repo, "open", readable) | 2096 | openMS, _ := s.st.ListMilestones(p.Repo, "open", readable) |
| 2097 | facets := listFacets(base, []string{"open", "merged", "closed", "all"}, state, allLabels, openMS, true) | 2097 | facets := listFacets(base, []string{"open", "merged", "closed", "all"}, state, allLabels, openMS, true) |
| 2098 | canOpenMR := canWrite || len(s.writableForks(s.viewer(r), p.Repo)) > 0 | ||
| 2098 | s.render(w, "mrs.html", struct { | 2099 | s.render(w, "mrs.html", struct { |
| 2099 | repoPage | 2100 | repoPage |
| 2100 | State string | 2101 | State string |
| @@ -2104,10 +2105,10 @@ func (s *Server) mrs(w http.ResponseWriter, r *http.Request) { | |||
| 2104 | MRs []mrRow | 2105 | MRs []mrRow |
| 2105 | LabelColors map[string]template.CSS | 2106 | LabelColors map[string]template.CSS |
| 2106 | Older string | 2107 | Older string |
| 2107 | CanWrite bool | 2108 | CanOpenMR bool |
| 2108 | }{p, state, mf.Search, | 2109 | }{p, state, mf.Search, |
| 2109 | activeFilters(state, [][2]string{{"label", mf.Label}, {"author", mf.Author}, {"milestone", mf.Milestone}}), | 2110 | activeFilters(state, [][2]string{{"label", mf.Label}, {"author", mf.Author}, {"milestone", mf.Milestone}}), |
| 2110 | facets, rows, s.labelColors(p.Repo), older, canWrite}) | 2111 | facets, rows, s.labelColors(p.Repo), older, canOpenMR}) |
| 2111 | } | 2112 | } |
| 2112 | 2113 | ||
| 2113 | func (s *Server) mr(w http.ResponseWriter, r *http.Request) { | 2114 | func (s *Server) mr(w http.ResponseWriter, r *http.Request) { |
internal/web/templates/compare.html +1 −1
| @@ -2,7 +2,7 @@ | |||
| 2 | {{define "title"}}compare {{.Base}}...{{.Head}} · {{.Repo.OwnerName}}/{{.Repo.Name}}{{end}} | 2 | {{define "title"}}compare {{.Base}}...{{.Head}} · {{.Repo.OwnerName}}/{{.Repo.Name}}{{end}} |
| 3 | {{define "content"}} | 3 | {{define "content"}} |
| 4 | <h1>Compare <code>{{.Base}}</code> … <code>{{.Head}}</code></h1> | 4 | <h1>Compare <code>{{.Base}}</code> … <code>{{.Head}}</code></h1> |
| 5 | <p class="meta">{{len .Commits}}{{if gt .CommitsTotal (len .Commits)}} of {{.CommitsTotal}}{{end}} commit{{if ne .CommitsTotal 1}}s{{end}} on <code>{{.Head}}</code> since the merge base <a href="/{{.Repo.OwnerName}}/{{.Repo.Name}}/commit/{{.MergeBase}}"><code>{{short .MergeBase}}</code></a>{{if .CanWrite}} · <a href="/{{.Repo.OwnerName}}/{{.Repo.Name}}/mrs/new?source={{.Head}}&target={{.Base}}">open a merge request</a>{{end}}</p> | 5 | <p class="meta">{{len .Commits}}{{if gt .CommitsTotal (len .Commits)}} of {{.CommitsTotal}}{{end}} commit{{if ne .CommitsTotal 1}}s{{end}} on <code>{{.Head}}</code> since the merge base <a href="/{{.Repo.OwnerName}}/{{.Repo.Name}}/commit/{{.MergeBase}}"><code>{{short .MergeBase}}</code></a>{{if .CanOpenMR}} · <a href="/{{.Repo.OwnerName}}/{{.Repo.Name}}/mrs/new?source={{.Head}}&target={{.Base}}">open a merge request</a>{{end}}</p> |
| 6 | {{if .Commits}}<ul class="loglist"> | 6 | {{if .Commits}}<ul class="loglist"> |
| 7 | {{range .Commits}}<li> | 7 | {{range .Commits}}<li> |
| 8 | <div class="commitmain"> | 8 | <div class="commitmain"> |
internal/web/templates/mrs.html +1 −1
| @@ -13,7 +13,7 @@ | |||
| 13 | </form> | 13 | </form> |
| 14 | {{range .Filters}}<p class="meta">{{.Key}}: {{if eq .Key "label"}}<span class="chip label" style="{{index $.LabelColors .Value}}">{{.Value}}</span>{{else}}<b>{{.Value}}</b>{{end}} <a href="{{.Clear}}">clear</a></p>{{end}} | 14 | {{range .Filters}}<p class="meta">{{.Key}}: {{if eq .Key "label"}}<span class="chip label" style="{{index $.LabelColors .Value}}">{{.Value}}</span>{{else}}<b>{{.Value}}</b>{{end}} <a href="{{.Clear}}">clear</a></p>{{end}} |
| 15 | </div> | 15 | </div> |
| 16 | {{if .CanWrite}}<p class="meta"><a href="/{{.Repo.OwnerName}}/{{.Repo.Name}}/mrs/new">New merge request</a></p> | 16 | {{if .CanOpenMR}}<p class="meta"><a href="/{{.Repo.OwnerName}}/{{.Repo.Name}}/mrs/new">New merge request</a></p> |
| 17 | {{else if .Viewer}}<p class="meta"><a href="/{{.Repo.OwnerName}}/{{.Repo.Name}}/fork">Fork this repository to propose a change</a></p> | 17 | {{else if .Viewer}}<p class="meta"><a href="/{{.Repo.OwnerName}}/{{.Repo.Name}}/fork">Fork this repository to propose a change</a></p> |
| 18 | {{else}}<p class="meta"><a href="/login">Sign in to propose a change</a></p>{{end}} | 18 | {{else}}<p class="meta"><a href="/login">Sign in to propose a change</a></p>{{end}} |
| 19 | <ul class="issuelist rows"> | 19 | <ul class="issuelist rows"> |