Skip to content

Commit

Permalink
Use link in UI which returned a relative url but not html_url which c…
Browse files Browse the repository at this point in the history
…ontains an absolute url (#21986)

partially fix #19345

This PR add some `Link` methods for different objects. The `Link`
methods are not different from `HTMLURL`, they are lack of the absolute
URL. And most of UI `HTMLURL` have been replaced to `Link` so that users
can visit them from a different domain or IP.

This PR also introduces a new javascript configuration
`window.config.reqAppUrl` which is different from `appUrl` which is
still an absolute url but the domain has been replaced to the current
requested domain.
  • Loading branch information
lunny authored Feb 6, 2023
1 parent 189d5b7 commit 769be87
Show file tree
Hide file tree
Showing 43 changed files with 189 additions and 81 deletions.
51 changes: 46 additions & 5 deletions models/activities/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,49 @@ func (a *Action) GetRepoAbsoluteLink() string {
return setting.AppURL + url.PathEscape(a.GetRepoUserName()) + "/" + url.PathEscape(a.GetRepoName())
}

// GetCommentHTMLURL returns link to action comment.
func (a *Action) GetCommentHTMLURL() string {
return a.getCommentHTMLURL(db.DefaultContext)
}

func (a *Action) loadComment(ctx context.Context) (err error) {
if a.CommentID == 0 || a.Comment != nil {
return nil
}
a.Comment, err = issues_model.GetCommentByID(ctx, a.CommentID)
return err
}

func (a *Action) getCommentHTMLURL(ctx context.Context) string {
if a == nil {
return "#"
}
_ = a.loadComment(ctx)
if a.Comment != nil {
return a.Comment.HTMLURL()
}
if len(a.GetIssueInfos()) == 0 {
return "#"
}
// Return link to issue
issueIDString := a.GetIssueInfos()[0]
issueID, err := strconv.ParseInt(issueIDString, 10, 64)
if err != nil {
return "#"
}

issue, err := issues_model.GetIssueByID(ctx, issueID)
if err != nil {
return "#"
}

if err = issue.LoadRepo(ctx); err != nil {
return "#"
}

return issue.HTMLURL()
}

// GetCommentLink returns link to action comment.
func (a *Action) GetCommentLink() string {
return a.getCommentLink(db.DefaultContext)
Expand All @@ -232,11 +275,9 @@ func (a *Action) getCommentLink(ctx context.Context) string {
if a == nil {
return "#"
}
if a.Comment == nil && a.CommentID != 0 {
a.Comment, _ = issues_model.GetCommentByID(ctx, a.CommentID)
}
_ = a.loadComment(ctx)
if a.Comment != nil {
return a.Comment.HTMLURL()
return a.Comment.Link()
}
if len(a.GetIssueInfos()) == 0 {
return "#"
Expand All @@ -257,7 +298,7 @@ func (a *Action) getCommentLink(ctx context.Context) string {
return "#"
}

return issue.HTMLURL()
return issue.Link()
}

// GetBranch returns the action's repository branch.
Expand Down
2 changes: 1 addition & 1 deletion models/activities/action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestAction_GetRepoLink(t *testing.T) {
expected := path.Join(setting.AppSubURL, owner.Name, repo.Name)
assert.Equal(t, expected, action.GetRepoLink())
assert.Equal(t, repo.HTMLURL(), action.GetRepoAbsoluteLink())
assert.Equal(t, comment.HTMLURL(), action.GetCommentLink())
assert.Equal(t, comment.HTMLURL(), action.GetCommentHTMLURL())
}

func TestGetFeeds(t *testing.T) {
Expand Down
16 changes: 16 additions & 0 deletions models/activities/notification.go
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,22 @@ func (n *Notification) HTMLURL() string {
return ""
}

// Link formats a relative URL-string to the notification
func (n *Notification) Link() string {
switch n.Source {
case NotificationSourceIssue, NotificationSourcePullRequest:
if n.Comment != nil {
return n.Comment.Link()
}
return n.Issue.Link()
case NotificationSourceCommit:
return n.Repository.Link() + "/commit/" + url.PathEscape(n.CommitID)
case NotificationSourceRepository:
return n.Repository.Link()
}
return ""
}

// APIURL formats a URL-string to the notification
func (n *Notification) APIURL() string {
return setting.AppURL + "api/v1/notifications/threads/" + strconv.FormatInt(n.ID, 10)
Expand Down
33 changes: 26 additions & 7 deletions models/issues/comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,21 +391,40 @@ func (c *Comment) HTMLURL() string {
log.Error("loadRepo(%d): %v", c.Issue.RepoID, err)
return ""
}
return c.Issue.HTMLURL() + c.hashLink()
}

// Link formats a relative URL-string to the issue-comment
func (c *Comment) Link() string {
err := c.LoadIssue(db.DefaultContext)
if err != nil { // Silently dropping errors :unamused:
log.Error("LoadIssue(%d): %v", c.IssueID, err)
return ""
}
err = c.Issue.LoadRepo(db.DefaultContext)
if err != nil { // Silently dropping errors :unamused:
log.Error("loadRepo(%d): %v", c.Issue.RepoID, err)
return ""
}
return c.Issue.Link() + c.hashLink()
}

func (c *Comment) hashLink() string {
if c.Type == CommentTypeCode {
if c.ReviewID == 0 {
return fmt.Sprintf("%s/files#%s", c.Issue.HTMLURL(), c.HashTag())
return "/files#" + c.HashTag()
}
if c.Review == nil {
if err := c.LoadReview(); err != nil {
log.Warn("LoadReview(%d): %v", c.ReviewID, err)
return fmt.Sprintf("%s/files#%s", c.Issue.HTMLURL(), c.HashTag())
return "/files#" + c.HashTag()
}
}
if c.Review.Type <= ReviewTypePending {
return fmt.Sprintf("%s/files#%s", c.Issue.HTMLURL(), c.HashTag())
return "/files#" + c.HashTag()
}
}
return fmt.Sprintf("%s#%s", c.Issue.HTMLURL(), c.HashTag())
return "#" + c.HashTag()
}

// APIURL formats a API-string to the issue-comment
Expand Down Expand Up @@ -708,8 +727,8 @@ func (c *Comment) UnsignedLine() uint64 {
return uint64(c.Line)
}

// CodeCommentURL returns the url to a comment in code
func (c *Comment) CodeCommentURL() string {
// CodeCommentLink returns the url to a comment in code
func (c *Comment) CodeCommentLink() string {
err := c.LoadIssue(db.DefaultContext)
if err != nil { // Silently dropping errors :unamused:
log.Error("LoadIssue(%d): %v", c.IssueID, err)
Expand All @@ -720,7 +739,7 @@ func (c *Comment) CodeCommentURL() string {
log.Error("loadRepo(%d): %v", c.Issue.RepoID, err)
return ""
}
return fmt.Sprintf("%s/files#%s", c.Issue.HTMLURL(), c.HashTag())
return fmt.Sprintf("%s/files#%s", c.Issue.Link(), c.HashTag())
}

// LoadPushCommits Load push commits
Expand Down
2 changes: 1 addition & 1 deletion models/issues/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ func (issue *Issue) HTMLURL() string {
return fmt.Sprintf("%s/%s/%d", issue.Repo.HTMLURL(), path, issue.Index)
}

// Link returns the Link URL to this issue.
// Link returns the issue's relative URL.
func (issue *Issue) Link() string {
var path string
if issue.IsPull {
Expand Down
12 changes: 6 additions & 6 deletions models/issues/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -759,20 +759,20 @@ func GetPullRequestsByHeadBranch(ctx context.Context, headBranch string, headRep
return prs, nil
}

// GetBaseBranchHTMLURL returns the HTML URL of the base branch
func (pr *PullRequest) GetBaseBranchHTMLURL() string {
// GetBaseBranchLink returns the relative URL of the base branch
func (pr *PullRequest) GetBaseBranchLink() string {
if err := pr.LoadBaseRepo(db.DefaultContext); err != nil {
log.Error("LoadBaseRepo: %v", err)
return ""
}
if pr.BaseRepo == nil {
return ""
}
return pr.BaseRepo.HTMLURL() + "/src/branch/" + util.PathEscapeSegments(pr.BaseBranch)
return pr.BaseRepo.Link() + "/src/branch/" + util.PathEscapeSegments(pr.BaseBranch)
}

// GetHeadBranchHTMLURL returns the HTML URL of the head branch
func (pr *PullRequest) GetHeadBranchHTMLURL() string {
// GetHeadBranchLink returns the relative URL of the head branch
func (pr *PullRequest) GetHeadBranchLink() string {
if pr.Flow == PullRequestFlowAGit {
return ""
}
Expand All @@ -784,7 +784,7 @@ func (pr *PullRequest) GetHeadBranchHTMLURL() string {
if pr.HeadRepo == nil {
return ""
}
return pr.HeadRepo.HTMLURL() + "/src/branch/" + util.PathEscapeSegments(pr.HeadBranch)
return pr.HeadRepo.Link() + "/src/branch/" + util.PathEscapeSegments(pr.HeadBranch)
}

// UpdateAllowEdits update if PR can be edited from maintainers
Expand Down
2 changes: 1 addition & 1 deletion models/packages/descriptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ type PackageFileDescriptor struct {

// PackageWebLink returns the package web link
func (pd *PackageDescriptor) PackageWebLink() string {
return fmt.Sprintf("%s/-/packages/%s/%s", pd.Owner.HTMLURL(), string(pd.Package.Type), url.PathEscape(pd.Package.LowerName))
return fmt.Sprintf("%s/-/packages/%s/%s", pd.Owner.HomeLink(), string(pd.Package.Type), url.PathEscape(pd.Package.LowerName))
}

// FullWebLink returns the package version web link
Expand Down
1 change: 1 addition & 0 deletions models/project/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ func (p *Project) LoadRepo(ctx context.Context) (err error) {
return err
}

// Link returns the project's relative URL.
func (p *Project) Link() string {
if p.OwnerID > 0 {
err := p.LoadOwner(db.DefaultContext)
Expand Down
5 changes: 5 additions & 0 deletions models/repo/release.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,11 @@ func (r *Release) HTMLURL() string {
return r.Repo.HTMLURL() + "/releases/tag/" + util.PathEscapeSegments(r.TagName)
}

// Link the relative url for a release on the web UI. release must have attributes loaded
func (r *Release) Link() string {
return r.Repo.Link() + "/releases/tag/" + util.PathEscapeSegments(r.TagName)
}

// IsReleaseExist returns true if release with given tag name already exists.
func IsReleaseExist(ctx context.Context, repoID int64, tagName string) (bool, error) {
if len(tagName) == 0 {
Expand Down
2 changes: 1 addition & 1 deletion models/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ func (repo *Repository) RepoPath() string {
return RepoPath(repo.OwnerName, repo.Name)
}

// Link returns the repository link
// Link returns the repository relative url
func (repo *Repository) Link() string {
return setting.AppSubURL + "/" + url.PathEscape(repo.OwnerName) + "/" + url.PathEscape(repo.Name)
}
Expand Down
1 change: 1 addition & 0 deletions modules/structs/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ type Repository struct {
Language string `json:"language"`
LanguagesURL string `json:"languages_url"`
HTMLURL string `json:"html_url"`
Link string `json:"link"`
SSHURL string `json:"ssh_url"`
CloneURL string `json:"clone_url"`
OriginalURL string `json:"original_url"`
Expand Down
2 changes: 1 addition & 1 deletion routers/web/feed/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func feedActionsToFeedItems(ctx *context.Context, actions activities_model.Actio

var content, desc, title string

link := &feeds.Link{Href: act.GetCommentLink()}
link := &feeds.Link{Href: act.GetCommentHTMLURL()}

// title
title = act.ActUser.DisplayName() + " "
Expand Down
4 changes: 2 additions & 2 deletions routers/web/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,8 +339,8 @@ func setMergeTarget(ctx *context.Context, pull *issues_model.PullRequest) {
ctx.Data["HeadTarget"] = pull.MustHeadUserName(ctx) + "/" + pull.HeadRepo.Name + ":" + pull.HeadBranch
}
ctx.Data["BaseTarget"] = pull.BaseBranch
ctx.Data["HeadBranchHTMLURL"] = pull.GetHeadBranchHTMLURL()
ctx.Data["BaseBranchHTMLURL"] = pull.GetBaseBranchHTMLURL()
ctx.Data["HeadBranchLink"] = pull.GetHeadBranchLink()
ctx.Data["BaseBranchLink"] = pull.GetBaseBranchLink()
}

// PrepareMergedViewPullInfo show meta information for a merged pull request view page
Expand Down
1 change: 1 addition & 0 deletions routers/web/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,7 @@ func SearchRepo(ctx *context.Context) {
Mirror: repo.IsMirror,
Stars: repo.NumStars,
HTMLURL: repo.HTMLURL(),
Link: repo.Link(),
Internal: !repo.IsPrivate && repo.Owner.Visibility == api.VisibleTypePrivate,
}
}
Expand Down
6 changes: 3 additions & 3 deletions templates/code/searchresults.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@
<div class="diff-file-box diff-box file-content non-diff-file-content repo-search-result">
<h4 class="ui top attached normal header">
<span class="file">
<a rel="nofollow" href="{{$repo.HTMLURL}}">{{$repo.FullName}}</a>
<a rel="nofollow" href="{{$repo.Link}}">{{$repo.FullName}}</a>
{{if $repo.IsArchived}}
<span class="ui basic label">{{$.locale.Tr "repo.desc.archived"}}</span>
{{end}}
- {{.Filename}}
</span>
<a class="ui basic tiny button" rel="nofollow" href="{{$repo.HTMLURL}}/src/commit/{{$result.CommitID | PathEscape}}/{{.Filename | PathEscapeSegments}}">{{$.locale.Tr "repo.diff.view_file"}}</a>
<a class="ui basic tiny button" rel="nofollow" href="{{$repo.Link}}/src/commit/{{$result.CommitID | PathEscape}}/{{.Filename | PathEscapeSegments}}">{{$.locale.Tr "repo.diff.view_file"}}</a>
</h4>
<div class="ui attached table segment">
<div class="file-body file-code code-view">
Expand All @@ -28,7 +28,7 @@
<tr>
<td class="lines-num">
{{range .LineNumbers}}
<a href="{{$repo.HTMLURL}}/src/commit/{{$result.CommitID | PathEscape}}/{{$result.Filename | PathEscapeSegments}}#L{{.}}"><span>{{.}}</span></a>
<a href="{{$repo.Link}}/src/commit/{{$result.CommitID | PathEscape}}/{{$result.Filename | PathEscapeSegments}}#L{{.}}"><span>{{.}}</span></a>
{{end}}
</td>
<td class="lines-code chroma"><code class="code-inner">{{.FormattedLines | Safe}}</code></td>
Expand Down
2 changes: 1 addition & 1 deletion templates/mail/issue/assigned.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
<title>{{.Subject}}</title>
</head>

{{$repo_url := printf "<a href='%s'>%s</a>" (Escape .Issue.Repo.HTMLURL) (Escape .Issue.Repo.FullName)}}
{{$repo_url := printf "<a href='%s'>%s</a>" (Escape .Issue.Repo.Link) (Escape .Issue.Repo.FullName)}}
{{$link := printf "<a href='%s'>#%d</a>" (Escape .Link) .Issue.Index}}
<body>
<p>
Expand Down
6 changes: 3 additions & 3 deletions templates/mail/issue/default.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@
{{if eq .ActionName "push"}}
<p>
{{if .Comment.IsForcePush}}
{{$oldCommitUrl := printf "%s/commit/%s" .Comment.Issue.PullRequest.BaseRepo.HTMLURL .Comment.OldCommit}}
{{$oldCommitUrl := printf "%s/commit/%s" .Comment.Issue.PullRequest.BaseRepo.Link .Comment.OldCommit}}
{{$oldShortSha := ShortSha .Comment.OldCommit}}
{{$oldCommitLink := printf "<a href='%[1]s'><b>%[2]s</b></a>" (Escape $oldCommitUrl) (Escape $oldShortSha)}}

{{$newCommitUrl := printf "%s/commit/%s" .Comment.Issue.PullRequest.BaseRepo.HTMLURL .Comment.NewCommit}}
{{$newCommitUrl := printf "%s/commit/%s" .Comment.Issue.PullRequest.BaseRepo.Link .Comment.NewCommit}}
{{$newShortSha := ShortSha .Comment.NewCommit}}
{{$newCommitLink := printf "<a href='%[1]s'><b>%[2]s</b></a>" (Escape $newCommitUrl) (Escape $newShortSha)}}

Expand Down Expand Up @@ -72,7 +72,7 @@
<ul>
{{range .Comment.Commits}}
<li>
<a href="{{$.Comment.Issue.PullRequest.BaseRepo.HTMLURL}}/commit/{{.ID}}">
<a href="{{$.Comment.Issue.PullRequest.BaseRepo.Link}}/commit/{{.ID}}">
{{ShortSha .ID.String}}
</a> - {{.Summary}}
</li>
Expand Down
4 changes: 2 additions & 2 deletions templates/mail/release.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@

</head>

{{$release_url := printf "<a href='%s'>%s</a>" (.Release.HTMLURL | Escape) (.Release.TagName | Escape)}}
{{$repo_url := printf "<a href='%s'>%s</a>" (.Release.Repo.HTMLURL | Escape) (.Release.Repo.FullName | Escape)}}
{{$release_url := printf "<a href='%s'>%s</a>" (.Release.Link | Escape) (.Release.TagName | Escape)}}
{{$repo_url := printf "<a href='%s'>%s</a>" (.Release.Repo.Link | Escape) (.Release.Repo.FullName | Escape)}}
<body>
<p>
{{.locale.Tr "mail.release.new.text" .Release.Publisher.Name $release_url $repo_url | Str2html}}
Expand Down
4 changes: 2 additions & 2 deletions templates/package/shared/list.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
{{$hasRepositoryAccess = index $.RepositoryAccessMap .Repository.ID}}
{{end}}
{{if $hasRepositoryAccess}}
{{$.locale.Tr "packages.published_by_in" $timeStr .Creator.HomeLink (.Creator.GetDisplayName | Escape) .Repository.HTMLURL (.Repository.FullName | Escape) | Safe}}
{{$.locale.Tr "packages.published_by_in" $timeStr .Creator.HomeLink (.Creator.GetDisplayName | Escape) .Repository.Link (.Repository.FullName | Escape) | Safe}}
{{else}}
{{$.locale.Tr "packages.published_by" $timeStr .Creator.HomeLink (.Creator.GetDisplayName | Escape) | Safe}}
{{end}}
Expand All @@ -41,7 +41,7 @@
{{svg "octicon-package" 32}}
<h2>{{.locale.Tr "packages.empty"}}</h2>
{{if and .Repository .CanWritePackages}}
{{$packagesUrl := URLJoin .Owner.HTMLURL "-" "packages"}}
{{$packagesUrl := URLJoin .Owner.HomeLink "-" "packages"}}
<p>{{.locale.Tr "packages.empty.repo" $packagesUrl | Safe}}</p>
{{end}}
<p>{{.locale.Tr "packages.empty.documentation" | Safe}}</p>
Expand Down
Loading

0 comments on commit 769be87

Please sign in to comment.