Skip to content
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.

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

Fix to the new pull request button #6248

Open
wants to merge 9 commits into
base: master
from

Conversation

7 participants
@User00015
Copy link

User00015 commented Mar 5, 2019

Fixing an issue where the new pull request button was pointing to an incorrect repository.

To reproduce:

  1. Create a fresh new gitea
  2. Register as a new user, admin or otherwise.
  3. Create a new organization.
  4. Go to the organizations dashboard,
  5. Create a repository under that organization and initilize it
  6. Return to the organization dashboard and select the newly created repository
  7. Click the green new pull request icon

Expected Result:
You are taken to a branch-to-branch merge with the repository

Actual result:
You are taken to a non-existent fork that assumes to exist that leads to a 404

wb129 and others added some commits Mar 5, 2019

wb129
.gitignore Outdated
@@ -73,3 +73,4 @@ prime/
*.snap
*.snap-build
*_source.tar.bz2
/.vs

This comment has been minimized.

@techknowlogick

techknowlogick Mar 5, 2019

Member

Please revert the change to this file.

@GiteaBot GiteaBot added the lgtm/need 2 label Mar 5, 2019

wb129 and others added some commits Mar 5, 2019

@User00015
Copy link
Author

User00015 left a comment

Removed.

@lunny lunny added the kind/bug label Mar 5, 2019

@silverwind

This comment has been minimized.

Copy link
Contributor

silverwind commented Mar 6, 2019

Removed.

Diff still shows a newline added, you might want to remove it :)

@User00015
Copy link
Author

User00015 left a comment

Removed for real this time.

@Piraty

This comment has been minimized.

Copy link

Piraty commented Mar 18, 2019

please squash the commits and give it an appropriate comment

@zeripath
Copy link
Contributor

zeripath left a comment

There's no need to squash commits - we squash them on merging.

@@ -64,7 +64,7 @@
{{if eq $n 0}}
{{if and .PullRequestCtx.Allowed .IsViewBranch (not .Repository.IsArchived)}}
<div class="fitted item">
<a href="{{.BaseRepo.Link}}/compare/{{.BaseRepo.DefaultBranch | EscapePound}}...{{if ne .Repository.Owner.Name .BaseRepo.Owner.Name}}{{.Repository.Owner.Name}}:{{end}}{{.BranchName | EscapePound}}">
<a href="{{.BaseRepo.Link}}/compare/{{.BaseRepo.DefaultBranch | EscapePound}}...{{ if or .HasForkedRepo (.Repository.IsFork) }}{{ .SignedUserName }}:{{ end }}{{.BranchName | EscapePound}}">

This comment has been minimized.

@zeripath

zeripath Mar 18, 2019

Contributor

Additional indent here isn't necessary, put it back to the level it was.

@@ -378,6 +378,7 @@ func RepoAssignment() macaron.Handler {
ctx.Data["CanWriteCode"] = ctx.Repo.CanWrite(models.UnitTypeCode)
ctx.Data["CanWriteIssues"] = ctx.Repo.CanWrite(models.UnitTypeIssues)
ctx.Data["CanWritePulls"] = ctx.Repo.CanWrite(models.UnitTypePullRequests)
ctx.Data["HasForkedRepo"] = ctx.User.HasForkedRepo(ctx.Repo.Repository.ID)

This comment has been minimized.

@zeripath

zeripath Mar 18, 2019

Contributor

This is failing on the tests because ctx.User may be nil if the user is not logged in. You must check if it's not nil before using it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.