diff --git a/templates/repo/branch/list.tmpl b/templates/repo/branch/list.tmpl index 6a0b726b67..e2c7c6f186 100644 --- a/templates/repo/branch/list.tmpl +++ b/templates/repo/branch/list.tmpl @@ -138,7 +138,9 @@ {{if .LatestPullRequest.HasMerged}} {{svg "octicon-git-merge" 16 "tw-mr-1"}}{{ctx.Locale.Tr "repo.pulls.merged"}} {{else if .LatestPullRequest.Issue.IsClosed}} - {{svg "octicon-git-pull-request" 16 "tw-mr-1"}}{{ctx.Locale.Tr "repo.issues.closed_title"}} + {{svg "octicon-git-pull-request-closed" 16 "tw-mr-1"}}{{ctx.Locale.Tr "repo.issues.closed_title"}} + {{else if .LatestPullRequest.IsWorkInProgress ctx}} + {{svg "octicon-git-pull-request-draft" 16 "tw-mr-1"}}{{ctx.Locale.Tr "repo.issues.draft_title"}} {{else}} {{svg "octicon-git-pull-request" 16 "tw-mr-1"}}{{ctx.Locale.Tr "repo.issues.open_title"}} {{end}} diff --git a/templates/repo/issue/view_title.tmpl b/templates/repo/issue/view_title.tmpl index 546c765d14..7123f4c048 100644 --- a/templates/repo/issue/view_title.tmpl +++ b/templates/repo/issue/view_title.tmpl @@ -37,7 +37,7 @@ {{if .HasMerged}}
{{svg "octicon-git-merge" 16 "tw-mr-1"}} {{if eq .Issue.PullRequest.Status 3}}{{ctx.Locale.Tr "repo.pulls.manually_merged"}}{{else}}{{ctx.Locale.Tr "repo.pulls.merged"}}{{end}}
{{else if .Issue.IsClosed}} -
{{if .Issue.IsPull}}{{svg "octicon-git-pull-request"}}{{else}}{{svg "octicon-issue-closed"}}{{end}} {{ctx.Locale.Tr "repo.issues.closed_title"}}
+
{{if .Issue.IsPull}}{{svg "octicon-git-pull-request-closed"}}{{else}}{{svg "octicon-issue-closed"}}{{end}} {{ctx.Locale.Tr "repo.issues.closed_title"}}
{{else if .Issue.IsPull}} {{if .IsPullWorkInProgress}}
{{svg "octicon-git-pull-request-draft"}} {{ctx.Locale.Tr "repo.issues.draft_title"}}
diff --git a/templates/shared/issueicon.tmpl b/templates/shared/issueicon.tmpl index a62714e988..d7c4414336 100644 --- a/templates/shared/issueicon.tmpl +++ b/templates/shared/issueicon.tmpl @@ -6,7 +6,7 @@ {{if .PullRequest.HasMerged}} {{svg "octicon-git-merge" 16 "text purple"}} {{else}} - {{svg "octicon-git-pull-request" 16 "text red"}} + {{svg "octicon-git-pull-request-closed" 16 "text red"}} {{end}} {{else}} {{if .PullRequest.IsWorkInProgress ctx}} diff --git a/tests/integration/pull_icon_test.go b/tests/integration/pull_icon_test.go new file mode 100644 index 0000000000..58dab92c39 --- /dev/null +++ b/tests/integration/pull_icon_test.go @@ -0,0 +1,256 @@ +// Copyright 2024 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: AGPL-3.0-only + +package integration + +import ( + "context" + "fmt" + "net/http" + "net/url" + "strings" + "testing" + "time" + + "code.gitea.io/gitea/models/db" + issues_model "code.gitea.io/gitea/models/issues" + repo_model "code.gitea.io/gitea/models/repo" + unit_model "code.gitea.io/gitea/models/unit" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git" + issue_service "code.gitea.io/gitea/services/issue" + pull_service "code.gitea.io/gitea/services/pull" + files_service "code.gitea.io/gitea/services/repository/files" + "code.gitea.io/gitea/tests" + + "github.com/PuerkitoBio/goquery" + "github.com/stretchr/testify/assert" +) + +func TestPullRequestIcons(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + repo, _, f := CreateDeclarativeRepo(t, user, "pr-icons", []unit_model.Type{unit_model.TypeCode, unit_model.TypePullRequests}, nil, nil) + defer f() + + session := loginUser(t, user.LoginName) + + // Individual PRs + t.Run("Open", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + pull := createOpenPullRequest(db.DefaultContext, t, user, repo) + testPullRequestIcon(t, session, pull, "green", "octicon-git-pull-request") + }) + + t.Run("WIP (Open)", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + pull := createOpenWipPullRequest(db.DefaultContext, t, user, repo) + testPullRequestIcon(t, session, pull, "grey", "octicon-git-pull-request-draft") + }) + + t.Run("Closed", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + pull := createClosedPullRequest(db.DefaultContext, t, user, repo) + testPullRequestIcon(t, session, pull, "red", "octicon-git-pull-request-closed") + }) + + t.Run("WIP (Closed)", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + pull := createClosedWipPullRequest(db.DefaultContext, t, user, repo) + testPullRequestIcon(t, session, pull, "red", "octicon-git-pull-request-closed") + }) + + t.Run("Merged", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + pull := createMergedPullRequest(db.DefaultContext, t, user, repo) + testPullRequestIcon(t, session, pull, "purple", "octicon-git-merge") + }) + + // List + req := NewRequest(t, "GET", repo.HTMLURL()+"/pulls?state=all") + resp := session.MakeRequest(t, req, http.StatusOK) + doc := NewHTMLParser(t, resp.Body) + + t.Run("List Open", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + testPullRequestListIcon(t, doc, "open", "green", "octicon-git-pull-request") + }) + + t.Run("List WIP (Open)", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + testPullRequestListIcon(t, doc, "open-wip", "grey", "octicon-git-pull-request-draft") + }) + + t.Run("List Closed", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + testPullRequestListIcon(t, doc, "closed", "red", "octicon-git-pull-request-closed") + }) + + t.Run("List Closed (WIP)", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + testPullRequestListIcon(t, doc, "closed-wip", "red", "octicon-git-pull-request-closed") + }) + + t.Run("List Merged", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + testPullRequestListIcon(t, doc, "merged", "purple", "octicon-git-merge") + }) + }) +} + +func testPullRequestIcon(t *testing.T, session *TestSession, pr *issues_model.PullRequest, expectedColor, expectedIcon string) { + req := NewRequest(t, "GET", pr.Issue.HTMLURL()) + resp := session.MakeRequest(t, req, http.StatusOK) + doc := NewHTMLParser(t, resp.Body) + doc.AssertElement(t, fmt.Sprintf("div.issue-state-label.%s > svg.%s", expectedColor, expectedIcon), true) + + req = NewRequest(t, "GET", pr.BaseRepo.HTMLURL()+"/branches") + resp = session.MakeRequest(t, req, http.StatusOK) + doc = NewHTMLParser(t, resp.Body) + doc.AssertElement(t, fmt.Sprintf(`a[href="/%s/pulls/%d"].%s > svg.%s`, pr.BaseRepo.FullName(), pr.Issue.Index, expectedColor, expectedIcon), true) +} + +func testPullRequestListIcon(t *testing.T, doc *HTMLDoc, name, expectedColor, expectedIcon string) { + sel := doc.doc.Find("div#issue-list > div.flex-item"). + FilterFunction(func(_ int, selection *goquery.Selection) bool { + return selection.Find(fmt.Sprintf(`div.flex-item-icon > svg.%s.%s`, expectedColor, expectedIcon)).Length() == 1 && + strings.HasSuffix(selection.Find("a.issue-title").Text(), name) + }) + + assert.Equal(t, 1, sel.Length()) +} + +func createOpenPullRequest(ctx context.Context, t *testing.T, user *user_model.User, repo *repo_model.Repository) *issues_model.PullRequest { + pull := createPullRequest(t, user, repo, "open") + + assert.False(t, pull.Issue.IsClosed) + assert.False(t, pull.HasMerged) + assert.False(t, pull.IsWorkInProgress(ctx)) + + return pull +} + +func createOpenWipPullRequest(ctx context.Context, t *testing.T, user *user_model.User, repo *repo_model.Repository) *issues_model.PullRequest { + pull := createPullRequest(t, user, repo, "open-wip") + + err := issue_service.ChangeTitle(ctx, pull.Issue, user, "WIP: "+pull.Issue.Title) + assert.NoError(t, err) + + assert.False(t, pull.Issue.IsClosed) + assert.False(t, pull.HasMerged) + assert.True(t, pull.IsWorkInProgress(ctx)) + + return pull +} + +func createClosedPullRequest(ctx context.Context, t *testing.T, user *user_model.User, repo *repo_model.Repository) *issues_model.PullRequest { + pull := createPullRequest(t, user, repo, "closed") + + err := issue_service.ChangeStatus(ctx, pull.Issue, user, "", true) + assert.NoError(t, err) + + assert.True(t, pull.Issue.IsClosed) + assert.False(t, pull.HasMerged) + assert.False(t, pull.IsWorkInProgress(ctx)) + + return pull +} + +func createClosedWipPullRequest(ctx context.Context, t *testing.T, user *user_model.User, repo *repo_model.Repository) *issues_model.PullRequest { + pull := createPullRequest(t, user, repo, "closed-wip") + + err := issue_service.ChangeTitle(ctx, pull.Issue, user, "WIP: "+pull.Issue.Title) + assert.NoError(t, err) + + err = issue_service.ChangeStatus(ctx, pull.Issue, user, "", true) + assert.NoError(t, err) + + assert.True(t, pull.Issue.IsClosed) + assert.False(t, pull.HasMerged) + assert.True(t, pull.IsWorkInProgress(ctx)) + + return pull +} + +func createMergedPullRequest(ctx context.Context, t *testing.T, user *user_model.User, repo *repo_model.Repository) *issues_model.PullRequest { + pull := createPullRequest(t, user, repo, "merged") + + gitRepo, err := git.OpenRepository(ctx, repo.RepoPath()) + defer gitRepo.Close() + + assert.NoError(t, err) + + err = pull_service.Merge(ctx, pull, user, gitRepo, repo_model.MergeStyleMerge, pull.HeadCommitID, "merge", false) + assert.NoError(t, err) + + assert.False(t, pull.Issue.IsClosed) + assert.True(t, pull.CanAutoMerge()) + assert.False(t, pull.IsWorkInProgress(ctx)) + + return pull +} + +func createPullRequest(t *testing.T, user *user_model.User, repo *repo_model.Repository, name string) *issues_model.PullRequest { + branch := "branch-" + name + title := "Testing " + name + + _, err := files_service.ChangeRepoFiles(git.DefaultContext, repo, user, &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "update", + TreePath: "README.md", + ContentReader: strings.NewReader("Update README"), + }, + }, + Message: "Update README", + OldBranch: "main", + NewBranch: branch, + Author: &files_service.IdentityOptions{ + Name: user.Name, + Email: user.Email, + }, + Committer: &files_service.IdentityOptions{ + Name: user.Name, + Email: user.Email, + }, + Dates: &files_service.CommitDateOptions{ + Author: time.Now(), + Committer: time.Now(), + }, + }) + + assert.NoError(t, err) + + pullIssue := &issues_model.Issue{ + RepoID: repo.ID, + Title: title, + PosterID: user.ID, + Poster: user, + IsPull: true, + } + + pullRequest := &issues_model.PullRequest{ + HeadRepoID: repo.ID, + BaseRepoID: repo.ID, + HeadBranch: branch, + BaseBranch: "main", + HeadRepo: repo, + BaseRepo: repo, + Type: issues_model.PullRequestGitea, + } + err = pull_service.NewPullRequest(git.DefaultContext, repo, pullIssue, nil, nil, pullRequest, nil) + assert.NoError(t, err) + + return pullRequest +} diff --git a/web_src/js/components/ContextPopup.test.js b/web_src/js/components/ContextPopup.test.js index 1db6c38301..3884408a7d 100644 --- a/web_src/js/components/ContextPopup.test.js +++ b/web_src/js/components/ContextPopup.test.js @@ -1,39 +1,166 @@ -import {mount, flushPromises} from '@vue/test-utils'; +// Copyright 2024 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: AGPL-3.0-only + +import {flushPromises, mount} from '@vue/test-utils'; import ContextPopup from './ContextPopup.vue'; -test('renders a issue info popup', async () => { - const owner = 'user2'; - const repo = 'repo1'; - const index = 1; +async function assertPopup(popupData, expectedIconColor, expectedIcon) { + const date = new Date('2024-07-13T22:00:00Z'); + vi.spyOn(global, 'fetch').mockResolvedValue({ json: vi.fn().mockResolvedValue({ ok: true, - created_at: '2023-09-30T19:00:00Z', - repository: {full_name: owner}, - pull_request: null, - state: 'open', - title: 'Normal issue', - body: 'Lorem ipsum...', - number: index, - labels: [{color: 'ee0701', name: "Bug :+1: "}], + created_at: date.toISOString(), + repository: {full_name: 'user2/repo1'}, + ...popupData, }), ok: true, }); - const wrapper = mount(ContextPopup); - wrapper.vm.$el.dispatchEvent(new CustomEvent('ce-load-context-popup', {detail: {owner, repo, index}})); + const popup = mount(ContextPopup); + popup.vm.$el.dispatchEvent(new CustomEvent('ce-load-context-popup', { + detail: {owner: 'user2', repo: 'repo1', index: popupData.number}, + })); await flushPromises(); - // Header - expect(wrapper.get('p:nth-of-type(1)').text()).toEqual('user2 on Sep 30, 2023'); - // Title - expect(wrapper.get('p:nth-of-type(2)').text()).toEqual('Normal issue #1'); - // Body - expect(wrapper.get('p:nth-of-type(3)').text()).toEqual('Lorem ipsum...'); - // Check that the state is correct. - expect(wrapper.get('svg').classes()).toContain('octicon-issue-opened'); - // Ensure that script is not an element. - expect(() => wrapper.get('.evil')).toThrowError(); - // Check content of label - expect(wrapper.get('.ui.label').text()).toContain("Bug 👍 "); + expect(popup.get('p:nth-of-type(1)').text()).toEqual(`user2/repo1 on ${date.toLocaleDateString(undefined, {year: 'numeric', month: 'short', day: 'numeric'})}`); + expect(popup.get('p:nth-of-type(2)').text()).toEqual(`${popupData.title} #${popupData.number}`); + expect(popup.get('p:nth-of-type(3)').text()).toEqual(popupData.body); + + expect(popup.get('svg').classes()).toContain(expectedIcon); + expect(popup.get('svg').classes()).toContain(expectedIconColor); + + for (const l of popupData.labels) { + expect(popup.findAll('.ui.label').map((x) => x.text())).toContain(l.name); + } +} + +test('renders an open issue popup', async () => { + await assertPopup({ + title: 'Open Issue', + body: 'Open Issue Body', + number: 1, + labels: [{color: 'd21b1fff', name: 'Bug'}, {color: 'aaff00', name: 'Confirmed'}], + state: 'open', + pull_request: null, + }, 'green', 'octicon-issue-opened'); +}); + +test('renders a closed issue popup', async () => { + await assertPopup({ + title: 'Closed Issue', + body: 'Closed Issue Body', + number: 1, + labels: [{color: 'd21b1fff', name: 'Bug'}, {color: 'aaff00', name: 'Confirmed'}], + state: 'closed', + pull_request: null, + }, 'red', 'octicon-issue-closed'); +}); + +test('renders an open PR popup', async () => { + await assertPopup({ + title: 'Open PR', + body: 'Open PR Body', + number: 1, + labels: [{color: 'd21b1fff', name: 'Bug'}, {color: 'aaff00', name: 'Confirmed'}], + state: 'open', + pull_request: {merged: false, draft: false}, + }, 'green', 'octicon-git-pull-request'); +}); + +test('renders an open WIP PR popup', async () => { + await assertPopup({ + title: 'WIP: Open PR', + body: 'WIP Open PR Body', + number: 1, + labels: [{color: 'd21b1fff', name: 'Bug'}, {color: 'aaff00', name: 'Confirmed'}], + state: 'open', + pull_request: {merged: false, draft: true}, + }, 'grey', 'octicon-git-pull-request-draft'); +}); + +test('renders a closed PR popup', async () => { + await assertPopup({ + title: 'Closed PR', + body: 'Closed PR Body', + number: 1, + labels: [{color: 'd21b1fff', name: 'Bug'}, {color: 'aaff00', name: 'Confirmed'}], + state: 'closed', + pull_request: {merged: false, draft: false}, + }, 'red', 'octicon-git-pull-request-closed'); +}); + +test('renders a closed WIP PR popup', async () => { + await assertPopup({ + title: 'WIP: Closed PR', + body: 'WIP Closed PR Body', + number: 1, + labels: [{color: 'd21b1fff', name: 'Bug'}, {color: 'aaff00', name: 'Confirmed'}], + state: 'closed', + pull_request: {merged: false, draft: true}, + }, 'red', 'octicon-git-pull-request-closed'); +}); + +test('renders a merged PR popup', async () => { + await assertPopup({ + title: 'Merged PR', + body: 'Merged PR Body', + number: 1, + labels: [{color: 'd21b1fff', name: 'Bug'}, {color: 'aaff00', name: 'Confirmed'}], + state: 'closed', + pull_request: {merged: true, draft: false}, + }, 'purple', 'octicon-git-merge'); +}); + +test('renders an issue popup with escaped HTML', async () => { + const evil = 'evil link'; + + vi.spyOn(global, 'fetch').mockResolvedValue({ + json: vi.fn().mockResolvedValue({ + ok: true, + created_at: '2024-07-13T22:00:00Z', + repository: {full_name: evil}, + title: evil, + body: evil, + labels: [{color: '000666', name: evil}], + state: 'open', + pull_request: null, + }), + ok: true, + }); + + const popup = mount(ContextPopup); + popup.vm.$el.dispatchEvent(new CustomEvent('ce-load-context-popup', { + detail: {owner: evil, repo: evil, index: 1}, + })); + await flushPromises(); + + expect(() => popup.get('.evil')).toThrowError(); + expect(popup.get('p:nth-of-type(1)').text()).toContain(evil); + expect(popup.get('p:nth-of-type(2)').text()).toContain(evil); + expect(popup.get('p:nth-of-type(3)').text()).toContain(evil); +}); + +test('renders an issue popup with emojis', async () => { + vi.spyOn(global, 'fetch').mockResolvedValue({ + json: vi.fn().mockResolvedValue({ + ok: true, + created_at: '2024-07-13T22:00:00Z', + repository: {full_name: 'user2/repo1'}, + title: 'Title', + body: 'Body', + labels: [{color: '000666', name: 'Tag :+1:'}], + state: 'open', + pull_request: null, + }), + ok: true, + }); + + const popup = mount(ContextPopup); + popup.vm.$el.dispatchEvent(new CustomEvent('ce-load-context-popup', { + detail: {owner: 'user2', repo: 'repo1', index: 1}, + })); + await flushPromises(); + + expect(popup.get('.ui.label').text()).toEqual('Tag 👍'); }); diff --git a/web_src/js/components/ContextPopup.vue b/web_src/js/components/ContextPopup.vue index 70b12dcb28..752a9a1d6b 100644 --- a/web_src/js/components/ContextPopup.vue +++ b/web_src/js/components/ContextPopup.vue @@ -30,33 +30,44 @@ export default { icon() { if (this.issue.pull_request !== null) { - if (this.issue.state === 'open') { - if (this.issue.pull_request.draft === true) { - return 'octicon-git-pull-request-draft'; // WIP PR - } - return 'octicon-git-pull-request'; // Open PR - } else if (this.issue.pull_request.merged === true) { + if (this.issue.pull_request.merged === true) { return 'octicon-git-merge'; // Merged PR } - return 'octicon-git-pull-request'; // Closed PR - } else if (this.issue.state === 'open') { - return 'octicon-issue-opened'; // Open Issue + + if (this.issue.state === 'closed') { + return 'octicon-git-pull-request-closed'; // Closed PR + } + + if (this.issue.pull_request.draft === true) { + return 'octicon-git-pull-request-draft'; // WIP PR + } + + return 'octicon-git-pull-request'; // Open PR } - return 'octicon-issue-closed'; // Closed Issue + + if (this.issue.state === 'closed') { + return 'octicon-issue-closed'; // Closed issue + } + + return 'octicon-issue-opened'; // Open issue }, color() { if (this.issue.pull_request !== null) { - if (this.issue.pull_request.draft === true) { - return 'grey'; // WIP PR - } else if (this.issue.pull_request.merged === true) { + if (this.issue.pull_request.merged === true) { return 'purple'; // Merged PR } + + if (this.issue.pull_request.draft === true && this.issue.state === 'open') { + return 'grey'; // WIP PR + } } - if (this.issue.state === 'open') { - return 'green'; // Open Issue + + if (this.issue.state === 'closed') { + return 'red'; // Closed issue } - return 'red'; // Closed Issue + + return 'green'; // Open issue }, labels() { diff --git a/web_src/js/svg.js b/web_src/js/svg.js index 913d26779f..9ef5f28e2f 100644 --- a/web_src/js/svg.js +++ b/web_src/js/svg.js @@ -33,6 +33,7 @@ import octiconGitBranch from '../../public/assets/img/svg/octicon-git-branch.svg import octiconGitCommit from '../../public/assets/img/svg/octicon-git-commit.svg'; import octiconGitMerge from '../../public/assets/img/svg/octicon-git-merge.svg'; import octiconGitPullRequest from '../../public/assets/img/svg/octicon-git-pull-request.svg'; +import octiconGitPullRequestClosed from '../../public/assets/img/svg/octicon-git-pull-request-closed.svg'; import octiconGitPullRequestDraft from '../../public/assets/img/svg/octicon-git-pull-request-draft.svg'; import octiconHeading from '../../public/assets/img/svg/octicon-heading.svg'; import octiconHorizontalRule from '../../public/assets/img/svg/octicon-horizontal-rule.svg'; @@ -106,6 +107,7 @@ const svgs = { 'octicon-git-commit': octiconGitCommit, 'octicon-git-merge': octiconGitMerge, 'octicon-git-pull-request': octiconGitPullRequest, + 'octicon-git-pull-request-closed': octiconGitPullRequestClosed, 'octicon-git-pull-request-draft': octiconGitPullRequestDraft, 'octicon-heading': octiconHeading, 'octicon-horizontal-rule': octiconHorizontalRule,