From c349da723c896d3e259aeb063a4c8f42341b3172 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Wed, 17 Jul 2024 16:53:43 +0200 Subject: [PATCH] Revert "ui: update pull request icons (#4455)" (partial) This reverts commit 8e56f61d0f022c58eaf21b410fa9013ae9a5e8c0. Refs: https://codeberg.org/forgejo/discussions/issues/192 --- tests/integration/pull_icon_test.go | 256 --------------------- web_src/js/components/ContextPopup.test.js | 177 ++------------ 2 files changed, 25 insertions(+), 408 deletions(-) delete mode 100644 tests/integration/pull_icon_test.go diff --git a/tests/integration/pull_icon_test.go b/tests/integration/pull_icon_test.go deleted file mode 100644 index 58dab92c39..0000000000 --- a/tests/integration/pull_icon_test.go +++ /dev/null @@ -1,256 +0,0 @@ -// 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 3884408a7d..1db6c38301 100644 --- a/web_src/js/components/ContextPopup.test.js +++ b/web_src/js/components/ContextPopup.test.js @@ -1,166 +1,39 @@ -// Copyright 2024 The Forgejo Authors. All rights reserved. -// SPDX-License-Identifier: AGPL-3.0-only - -import {flushPromises, mount} from '@vue/test-utils'; +import {mount, flushPromises} from '@vue/test-utils'; import ContextPopup from './ContextPopup.vue'; -async function assertPopup(popupData, expectedIconColor, expectedIcon) { - const date = new Date('2024-07-13T22:00:00Z'); - +test('renders a issue info popup', async () => { + const owner = 'user2'; + const repo = 'repo1'; + const index = 1; vi.spyOn(global, 'fetch').mockResolvedValue({ json: vi.fn().mockResolvedValue({ ok: true, - created_at: date.toISOString(), - repository: {full_name: 'user2/repo1'}, - ...popupData, - }), - ok: true, - }); - - const popup = mount(ContextPopup); - popup.vm.$el.dispatchEvent(new CustomEvent('ce-load-context-popup', { - detail: {owner: 'user2', repo: 'repo1', index: popupData.number}, - })); - await flushPromises(); - - 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', + created_at: '2023-09-30T19:00:00Z', + repository: {full_name: owner}, 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, + title: 'Normal issue', + body: 'Lorem ipsum...', + number: index, + labels: [{color: 'ee0701', name: "Bug :+1: "}], }), ok: true, }); - const popup = mount(ContextPopup); - popup.vm.$el.dispatchEvent(new CustomEvent('ce-load-context-popup', { - detail: {owner: 'user2', repo: 'repo1', index: 1}, - })); + const wrapper = mount(ContextPopup); + wrapper.vm.$el.dispatchEvent(new CustomEvent('ce-load-context-popup', {detail: {owner, repo, index}})); await flushPromises(); - expect(popup.get('.ui.label').text()).toEqual('Tag 👍'); + // 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 👍 "); });