From f52fe82add25cba5532b6f650cb702f6b31f4aa9 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Wed, 21 Sep 2022 22:51:42 +0200 Subject: [PATCH] Use absolute links in feeds (#21229) fixes #20864 Co-authored-by: wxiaoguang Co-authored-by: techknowlogick --- models/activities/action.go | 5 +++ models/activities/action_test.go | 10 ++++-- routers/web/feed/convert.go | 62 ++++++++++++++++---------------- 3 files changed, 43 insertions(+), 34 deletions(-) diff --git a/models/activities/action.go b/models/activities/action.go index 31ecdedd5b..3c8acb5ba8 100644 --- a/models/activities/action.go +++ b/models/activities/action.go @@ -218,6 +218,11 @@ func (a *Action) GetRepoLink() string { return path.Join(setting.AppSubURL, "/", url.PathEscape(a.GetRepoUserName()), url.PathEscape(a.GetRepoName())) } +// GetRepoAbsoluteLink returns the absolute link to action repository. +func (a *Action) GetRepoAbsoluteLink() string { + return setting.AppURL + url.PathEscape(a.GetRepoUserName()) + "/" + url.PathEscape(a.GetRepoName()) +} + // GetCommentLink returns link to action comment. func (a *Action) GetCommentLink() string { return a.getCommentLink(db.DefaultContext) diff --git a/models/activities/action_test.go b/models/activities/action_test.go index 83fd9ee38d..ac2a3043a6 100644 --- a/models/activities/action_test.go +++ b/models/activities/action_test.go @@ -10,6 +10,7 @@ import ( activities_model "code.gitea.io/gitea/models/activities" "code.gitea.io/gitea/models/db" + issue_model "code.gitea.io/gitea/models/issues" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" @@ -20,7 +21,7 @@ import ( func TestAction_GetRepoPath(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) action := &activities_model.Action{RepoID: repo.ID} assert.Equal(t, path.Join(owner.Name, repo.Name), action.GetRepoPath()) @@ -28,12 +29,15 @@ func TestAction_GetRepoPath(t *testing.T) { func TestAction_GetRepoLink(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) - action := &activities_model.Action{RepoID: repo.ID} + comment := unittest.AssertExistsAndLoadBean(t, &issue_model.Comment{ID: 2}) + action := &activities_model.Action{RepoID: repo.ID, CommentID: comment.ID} setting.AppSubURL = "/suburl" 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()) } func TestGetFeeds(t *testing.T) { diff --git a/routers/web/feed/convert.go b/routers/web/feed/convert.go index e16c54b8d8..645d9370d5 100644 --- a/routers/web/feed/convert.go +++ b/routers/web/feed/convert.go @@ -24,27 +24,27 @@ import ( ) func toBranchLink(act *activities_model.Action) string { - return act.GetRepoLink() + "/src/branch/" + util.PathEscapeSegments(act.GetBranch()) + return act.GetRepoAbsoluteLink() + "/src/branch/" + util.PathEscapeSegments(act.GetBranch()) } func toTagLink(act *activities_model.Action) string { - return act.GetRepoLink() + "/src/tag/" + util.PathEscapeSegments(act.GetTag()) + return act.GetRepoAbsoluteLink() + "/src/tag/" + util.PathEscapeSegments(act.GetTag()) } func toIssueLink(act *activities_model.Action) string { - return act.GetRepoLink() + "/issues/" + url.PathEscape(act.GetIssueInfos()[0]) + return act.GetRepoAbsoluteLink() + "/issues/" + url.PathEscape(act.GetIssueInfos()[0]) } func toPullLink(act *activities_model.Action) string { - return act.GetRepoLink() + "/pulls/" + url.PathEscape(act.GetIssueInfos()[0]) + return act.GetRepoAbsoluteLink() + "/pulls/" + url.PathEscape(act.GetIssueInfos()[0]) } func toSrcLink(act *activities_model.Action) string { - return act.GetRepoLink() + "/src/" + util.PathEscapeSegments(act.GetBranch()) + return act.GetRepoAbsoluteLink() + "/src/" + util.PathEscapeSegments(act.GetBranch()) } func toReleaseLink(act *activities_model.Action) string { - return act.GetRepoLink() + "/releases/tag/" + util.PathEscapeSegments(act.GetBranch()) + return act.GetRepoAbsoluteLink() + "/releases/tag/" + util.PathEscapeSegments(act.GetBranch()) } // renderMarkdown creates a minimal markdown render context from an action. @@ -79,17 +79,17 @@ func feedActionsToFeedItems(ctx *context.Context, actions activities_model.Actio title = act.ActUser.DisplayName() + " " switch act.OpType { case activities_model.ActionCreateRepo: - title += ctx.TrHTMLEscapeArgs("action.create_repo", act.GetRepoLink(), act.ShortRepoPath()) - link.Href = act.GetRepoLink() + title += ctx.TrHTMLEscapeArgs("action.create_repo", act.GetRepoAbsoluteLink(), act.ShortRepoPath()) + link.Href = act.GetRepoAbsoluteLink() case activities_model.ActionRenameRepo: - title += ctx.TrHTMLEscapeArgs("action.rename_repo", act.GetContent(), act.GetRepoLink(), act.ShortRepoPath()) - link.Href = act.GetRepoLink() + title += ctx.TrHTMLEscapeArgs("action.rename_repo", act.GetContent(), act.GetRepoAbsoluteLink(), act.ShortRepoPath()) + link.Href = act.GetRepoAbsoluteLink() case activities_model.ActionCommitRepo: link.Href = toBranchLink(act) if len(act.Content) != 0 { - title += ctx.TrHTMLEscapeArgs("action.commit_repo", act.GetRepoLink(), link.Href, act.GetBranch(), act.ShortRepoPath()) + title += ctx.TrHTMLEscapeArgs("action.commit_repo", act.GetRepoAbsoluteLink(), link.Href, act.GetBranch(), act.ShortRepoPath()) } else { - title += ctx.TrHTMLEscapeArgs("action.create_branch", act.GetRepoLink(), link.Href, act.GetBranch(), act.ShortRepoPath()) + title += ctx.TrHTMLEscapeArgs("action.create_branch", act.GetRepoAbsoluteLink(), link.Href, act.GetBranch(), act.ShortRepoPath()) } case activities_model.ActionCreateIssue: link.Href = toIssueLink(act) @@ -98,11 +98,11 @@ func feedActionsToFeedItems(ctx *context.Context, actions activities_model.Actio link.Href = toPullLink(act) title += ctx.TrHTMLEscapeArgs("action.create_pull_request", link.Href, act.GetIssueInfos()[0], act.ShortRepoPath()) case activities_model.ActionTransferRepo: - link.Href = act.GetRepoLink() - title += ctx.TrHTMLEscapeArgs("action.transfer_repo", act.GetContent(), act.GetRepoLink(), act.ShortRepoPath()) + link.Href = act.GetRepoAbsoluteLink() + title += ctx.TrHTMLEscapeArgs("action.transfer_repo", act.GetContent(), act.GetRepoAbsoluteLink(), act.ShortRepoPath()) case activities_model.ActionPushTag: link.Href = toTagLink(act) - title += ctx.TrHTMLEscapeArgs("action.push_tag", act.GetRepoLink(), link.Href, act.GetTag(), act.ShortRepoPath()) + title += ctx.TrHTMLEscapeArgs("action.push_tag", act.GetRepoAbsoluteLink(), link.Href, act.GetTag(), act.ShortRepoPath()) case activities_model.ActionCommentIssue: issueLink := toIssueLink(act) if link.Href == "#" { @@ -140,26 +140,26 @@ func feedActionsToFeedItems(ctx *context.Context, actions activities_model.Actio } title += ctx.TrHTMLEscapeArgs("action.reopen_pull_request", pullLink, act.GetIssueInfos()[0], act.ShortRepoPath()) case activities_model.ActionDeleteTag: - link.Href = act.GetRepoLink() - title += ctx.TrHTMLEscapeArgs("action.delete_tag", act.GetRepoLink(), act.GetTag(), act.ShortRepoPath()) + link.Href = act.GetRepoAbsoluteLink() + title += ctx.TrHTMLEscapeArgs("action.delete_tag", act.GetRepoAbsoluteLink(), act.GetTag(), act.ShortRepoPath()) case activities_model.ActionDeleteBranch: - link.Href = act.GetRepoLink() - title += ctx.TrHTMLEscapeArgs("action.delete_branch", act.GetRepoLink(), html.EscapeString(act.GetBranch()), act.ShortRepoPath()) + link.Href = act.GetRepoAbsoluteLink() + title += ctx.TrHTMLEscapeArgs("action.delete_branch", act.GetRepoAbsoluteLink(), html.EscapeString(act.GetBranch()), act.ShortRepoPath()) case activities_model.ActionMirrorSyncPush: srcLink := toSrcLink(act) if link.Href == "#" { link.Href = srcLink } - title += ctx.TrHTMLEscapeArgs("action.mirror_sync_push", act.GetRepoLink(), srcLink, act.GetBranch(), act.ShortRepoPath()) + title += ctx.TrHTMLEscapeArgs("action.mirror_sync_push", act.GetRepoAbsoluteLink(), srcLink, act.GetBranch(), act.ShortRepoPath()) case activities_model.ActionMirrorSyncCreate: srcLink := toSrcLink(act) if link.Href == "#" { link.Href = srcLink } - title += ctx.TrHTMLEscapeArgs("action.mirror_sync_create", act.GetRepoLink(), srcLink, act.GetBranch(), act.ShortRepoPath()) + title += ctx.TrHTMLEscapeArgs("action.mirror_sync_create", act.GetRepoAbsoluteLink(), srcLink, act.GetBranch(), act.ShortRepoPath()) case activities_model.ActionMirrorSyncDelete: - link.Href = act.GetRepoLink() - title += ctx.TrHTMLEscapeArgs("action.mirror_sync_delete", act.GetRepoLink(), act.GetBranch(), act.ShortRepoPath()) + link.Href = act.GetRepoAbsoluteLink() + title += ctx.TrHTMLEscapeArgs("action.mirror_sync_delete", act.GetRepoAbsoluteLink(), act.GetBranch(), act.ShortRepoPath()) case activities_model.ActionApprovePullRequest: pullLink := toPullLink(act) title += ctx.TrHTMLEscapeArgs("action.approve_pull_request", pullLink, act.GetIssueInfos()[0], act.ShortRepoPath()) @@ -174,16 +174,16 @@ func feedActionsToFeedItems(ctx *context.Context, actions activities_model.Actio if link.Href == "#" { link.Href = releaseLink } - title += ctx.TrHTMLEscapeArgs("action.publish_release", act.GetRepoLink(), releaseLink, act.ShortRepoPath(), act.Content) + title += ctx.TrHTMLEscapeArgs("action.publish_release", act.GetRepoAbsoluteLink(), releaseLink, act.ShortRepoPath(), act.Content) case activities_model.ActionPullReviewDismissed: pullLink := toPullLink(act) title += ctx.TrHTMLEscapeArgs("action.review_dismissed", pullLink, act.GetIssueInfos()[0], act.ShortRepoPath(), act.GetIssueInfos()[1]) case activities_model.ActionStarRepo: - link.Href = act.GetRepoLink() - title += ctx.TrHTMLEscapeArgs("action.starred_repo", act.GetRepoLink(), act.GetRepoPath()) + link.Href = act.GetRepoAbsoluteLink() + title += ctx.TrHTMLEscapeArgs("action.starred_repo", act.GetRepoAbsoluteLink(), act.GetRepoPath()) case activities_model.ActionWatchRepo: - link.Href = act.GetRepoLink() - title += ctx.TrHTMLEscapeArgs("action.watched_repo", act.GetRepoLink(), act.GetRepoPath()) + link.Href = act.GetRepoAbsoluteLink() + title += ctx.TrHTMLEscapeArgs("action.watched_repo", act.GetRepoAbsoluteLink(), act.GetRepoPath()) default: return nil, fmt.Errorf("unknown action type: %v", act.OpType) } @@ -193,14 +193,14 @@ func feedActionsToFeedItems(ctx *context.Context, actions activities_model.Actio switch act.OpType { case activities_model.ActionCommitRepo, activities_model.ActionMirrorSyncPush: push := templates.ActionContent2Commits(act) - repoLink := act.GetRepoLink() + repoLink := act.GetRepoAbsoluteLink() for _, commit := range push.Commits { if len(desc) != 0 { desc += "\n\n" } desc += fmt.Sprintf("%s\n%s", - html.EscapeString(fmt.Sprintf("%s/commit/%s", act.GetRepoLink(), commit.Sha1)), + html.EscapeString(fmt.Sprintf("%s/commit/%s", act.GetRepoAbsoluteLink(), commit.Sha1)), commit.Sha1, templates.RenderCommitMessage(ctx, commit.Message, repoLink, nil), ) @@ -209,7 +209,7 @@ func feedActionsToFeedItems(ctx *context.Context, actions activities_model.Actio if push.Len > 1 { link = &feeds.Link{Href: fmt.Sprintf("%s/%s", setting.AppSubURL, push.CompareURL)} } else if push.Len == 1 { - link = &feeds.Link{Href: fmt.Sprintf("%s/commit/%s", act.GetRepoLink(), push.Commits[0].Sha1)} + link = &feeds.Link{Href: fmt.Sprintf("%s/commit/%s", act.GetRepoAbsoluteLink(), push.Commits[0].Sha1)} } case activities_model.ActionCreateIssue, activities_model.ActionCreatePullRequest: