Skip to content

Fix 404 when click compare on non-forked repository #948

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Fix 404 when click compare on non-forked repository #948

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c295ff2
Fix changed branch is not reflected when creating PR (#3604)
unknwon Jan 27, 2017
1dd2c4c
remove the wrong username
lunny Feb 16, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 31 additions & 31 deletions modules/context/repo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,37 +323,6 @@ func RepoAssignment(args ...bool) macaron.Handler {
ctx.Data["BranchName"] = ctx.Repo.BranchName
ctx.Data["CommitID"] = ctx.Repo.CommitID

if repo.IsFork {
RetrieveBaseRepo(ctx, repo)
if ctx.Written() {
return
}
}

// People who have push access or have fored repository can propose a new pull request.
if ctx.Repo.IsWriter() || (ctx.IsSigned && ctx.User.HasForkedRepo(ctx.Repo.Repository.ID)) {
// Pull request is allowed if this is a fork repository
// and base repository accepts pull requests.
if repo.BaseRepo != nil {
if repo.BaseRepo.AllowsPulls() {
ctx.Data["BaseRepo"] = repo.BaseRepo
ctx.Repo.PullRequest.BaseRepo = repo.BaseRepo
ctx.Repo.PullRequest.Allowed = true
ctx.Repo.PullRequest.HeadInfo = ctx.Repo.Owner.Name + ":" + ctx.Repo.BranchName
}
} else {
// Or, this is repository accepts pull requests between branches.
if repo.AllowsPulls() {
ctx.Data["BaseRepo"] = repo
ctx.Repo.PullRequest.BaseRepo = repo
ctx.Repo.PullRequest.Allowed = true
ctx.Repo.PullRequest.SameRepo = true
ctx.Repo.PullRequest.HeadInfo = ctx.Repo.BranchName
}
}
}
ctx.Data["PullRequestCtx"] = ctx.Repo.PullRequest

if ctx.Query("go-get") == "1" {
ctx.Data["GoGetImport"] = composeGoGetImport(owner.Name, repo.Name)
prefix := setting.AppURL + path.Join(owner.Name, repo.Name, "src", ctx.Repo.BranchName)
Expand Down Expand Up @@ -466,6 +435,37 @@ func RepoRef() macaron.Handler {
ctx.Data["IsViewTag"] = ctx.Repo.IsViewTag
ctx.Data["IsViewCommit"] = ctx.Repo.IsViewCommit

if ctx.Repo.Repository.IsFork {
RetrieveBaseRepo(ctx, ctx.Repo.Repository)
if ctx.Written() {
return
}
}

// People who have push access or have fored repository can propose a new pull request.
if ctx.Repo.IsWriter() || (ctx.IsSigned && ctx.User.HasForkedRepo(ctx.Repo.Repository.ID)) {
// Pull request is allowed if this is a fork repository
// and base repository accepts pull requests.
if ctx.Repo.Repository.BaseRepo != nil {
if ctx.Repo.Repository.BaseRepo.AllowsPulls() {
ctx.Data["BaseRepo"] = ctx.Repo.Repository.BaseRepo
ctx.Repo.PullRequest.BaseRepo = ctx.Repo.Repository.BaseRepo
ctx.Repo.PullRequest.Allowed = true
ctx.Repo.PullRequest.HeadInfo = ctx.Repo.Owner.Name + ":" + ctx.Repo.BranchName
}
} else {
// Or, this is repository accepts pull requests between branches.
if ctx.Repo.Repository.AllowsPulls() {
ctx.Data["BaseRepo"] = ctx.Repo.Repository
ctx.Repo.PullRequest.BaseRepo = ctx.Repo.Repository
ctx.Repo.PullRequest.Allowed = true
ctx.Repo.PullRequest.SameRepo = true
ctx.Repo.PullRequest.HeadInfo = ctx.Repo.BranchName
}
}
}
ctx.Data["PullRequestCtx"] = ctx.Repo.PullRequest

ctx.Repo.CommitsCount, err = ctx.Repo.Commit.CommitsCount()
if err != nil {
ctx.Handle(500, "CommitsCount", err)
Expand Down
2 changes: 1 addition & 1 deletion templates/repo/home.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<div class="ui secondary menu">
{{if .PullRequestCtx.Allowed}}
<div class="fitted item">
<a href="{{.BaseRepo.Link}}/compare/{{.BaseRepo.DefaultBranch}}...{{.Username}}:{{.BranchName}}">
<a href="{{.BaseRepo.Link}}/compare/{{.BaseRepo.DefaultBranch}}...{{.BranchName}}">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing {{.Username}} fixes two-branches-inside-the-same-repo comparisons, but break comparisons across forks.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But on compare UI, it's not ready for compare between forks. I think this need another PR to do that. For this one, we should let the button works at first.
Gitea's vs Github's
untitled

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about you just make sure that .Username expands to what you expect it to expand, and later work on allowing different names ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the later work should be similar with github. Add two dropdown menus and list the forked repos.

<button class="ui green small button"><i class="octicon octicon-git-compare"></i></button>
</a>
</div>
Expand Down