From 6d2c29ae85ba0d99a68fd6f801acc4ac56e604d3 Mon Sep 17 00:00:00 2001 From: TimedIn Date: Tue, 8 Oct 2024 10:39:52 +0200 Subject: [PATCH 1/2] feat: "assign to me" button on PRs and issues includes: Tests for assignees on issues Move assignees selector of new Issue to assignees.tmpl (cherry picked from commit 2feb3d03d73a89c0d52a174c92915ca3930f08a7) --- options/locale/locale_en-US.ini | 1 + services/forms/repo_form.go | 2 +- templates/repo/issue/new_form.tmpl | 37 +------- .../repo/issue/view_content/sidebar.tmpl | 2 +- .../issue/view_content/sidebar/assignees.tmpl | 37 +++++--- tests/e2e/issue-sidebar.test.e2e.js | 85 +++++++++++++++++++ web_src/js/features/repo-issue.js | 54 ++++++++++++ web_src/js/features/repo-legacy.js | 22 +---- 8 files changed, 172 insertions(+), 68 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index d6b0fb3066..6e4b81e004 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1507,6 +1507,7 @@ issues.new.closed_milestone = Closed milestones issues.new.assignees = Assignees issues.new.clear_assignees = Clear assignees issues.new.no_assignees = No assignees +issues.new.assign_to_me = Assign to me issues.new.no_reviewers = No reviewers issues.edit.already_changed = Unable to save changes to the issue. It appears the content has already been changed by another user. Please refresh the page and try editing again to avoid overwriting their changes issues.choose.get_started = Get started diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index c3d9c3edc9..654106c47a 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -317,7 +317,7 @@ type WebhookForm struct { type CreateIssueForm struct { Title string `binding:"Required;MaxSize(255)"` LabelIDs string `form:"label_ids"` - AssigneeIDs string `form:"assignee_ids"` + AssigneeIDs string `form:"assignee_id"` Ref string `form:"ref"` MilestoneID int64 ProjectID int64 diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index fe10c1f9b9..c2cf4ee7a7 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -140,42 +140,7 @@ {{end}}
- -
- - {{ctx.Locale.Tr "repo.issues.new.assignees"}} - {{if .HasIssuesOrPullsWritePermission}} - {{svg "octicon-gear" 16 "tw-ml-1"}} - {{end}} - -
-
- {{svg "octicon-search" 16}} - -
-
{{ctx.Locale.Tr "repo.issues.new.clear_assignees"}}
- {{range .Assignees}} - - {{svg "octicon-check"}} - - {{ctx.AvatarUtils.Avatar . 28 "tw-mr-2"}}{{template "repo/search_name" .}} - - - {{end}} -
-
-
- - {{ctx.Locale.Tr "repo.issues.new.no_assignees"}} - -
- {{range .Assignees}} - - {{ctx.AvatarUtils.Avatar . 28 "tw-mr-2 tw-align-middle"}}{{.GetDisplayName}} - - {{end}} -
-
+ {{template "repo/issue/view_content/sidebar/assignees" dict "isExistingIssue" false "." .}} {{if and .PageIsComparePull (not (eq .HeadRepo.FullName .BaseCompareRepo.FullName)) .CanWriteToHeadRepo}}
diff --git a/templates/repo/issue/view_content/sidebar.tmpl b/templates/repo/issue/view_content/sidebar.tmpl index ba15539841..623023bd51 100644 --- a/templates/repo/issue/view_content/sidebar.tmpl +++ b/templates/repo/issue/view_content/sidebar.tmpl @@ -17,7 +17,7 @@ {{template "repo/issue/view_content/sidebar/projects" .}}
- {{template "repo/issue/view_content/sidebar/assignees" .}} + {{template "repo/issue/view_content/sidebar/assignees" dict "isExistingIssue" true "." .}}
{{if .Participants}} diff --git a/templates/repo/issue/view_content/sidebar/assignees.tmpl b/templates/repo/issue/view_content/sidebar/assignees.tmpl index e51bda95de..8e5043c076 100644 --- a/templates/repo/issue/view_content/sidebar/assignees.tmpl +++ b/templates/repo/issue/view_content/sidebar/assignees.tmpl @@ -1,12 +1,12 @@ -
+
{{ctx.Locale.Tr "repo.issues.new.assignees"}} {{if and .HasIssuesOrPullsWritePermission (not .Repository.IsArchived)}} {{svg "octicon-gear" 16 "tw-ml-1"}} {{end}} -
+
{{svg "octicon-search" 16}} @@ -31,15 +31,32 @@
- {{ctx.Locale.Tr "repo.issues.new.no_assignees"}} -
- {{range .Issue.Assignees}} -
- - {{ctx.AvatarUtils.Avatar . 28 "tw-mr-2"}} - {{.GetDisplayName}} + + {{ctx.Locale.Tr "repo.issues.new.no_assignees"}} + {{if and .HasIssuesOrPullsWritePermission (not .Repository.IsArchived)}} + {{with index .Assignees 0}} + – + + {{ctx.Locale.Tr "repo.issues.new.assign_to_me"}} -
+ {{end}} + {{end}} + +
+ {{if .isExistingIssue}} + {{range .Issue.Assignees}} +
+ + {{ctx.AvatarUtils.Avatar . 28 "tw-mr-2"}}{{.GetDisplayName}} + +
+ {{end}} + {{else}} + {{range .Assignees}} + + {{ctx.AvatarUtils.Avatar . 28 "tw-mr-2"}}{{.GetDisplayName}} + + {{end}} {{end}}
diff --git a/tests/e2e/issue-sidebar.test.e2e.js b/tests/e2e/issue-sidebar.test.e2e.js index 0311910a34..15567c5500 100644 --- a/tests/e2e/issue-sidebar.test.e2e.js +++ b/tests/e2e/issue-sidebar.test.e2e.js @@ -96,6 +96,91 @@ test('Issue: Labels', async ({browser}, workerInfo) => { await expect(labelList.filter({hasText: 'label1'})).toBeVisible(); }); +test('Issue: Assignees', async ({browser}, workerInfo) => { + test.skip(workerInfo.project.name === 'Mobile Safari', 'Unable to get tests working on Safari Mobile, see https://codeberg.org/forgejo/forgejo/pulls/3445#issuecomment-1789636'); + const page = await login({browser}, workerInfo); + // select label list in sidebar only + const assigneesList = page.locator('.issue-content-right .assignees.list .selected .item a'); + + const response = await page.goto('/org3/repo3/issues/1'); + await expect(response?.status()).toBe(200); + // preconditions + await expect(assigneesList.filter({hasText: 'user2'})).toBeVisible(); + await expect(assigneesList.filter({hasText: 'user4'})).toBeHidden(); + await expect(page.locator('.ui.assignees.list .item.no-select')).toBeHidden(); + + // Clear all assignees + await page.locator('.select-assignees-modify.dropdown').click(); + await page.locator('.select-assignees-modify.dropdown .no-select.item').click(); + await page.waitForLoadState('networkidle'); + await expect(assigneesList.filter({hasText: 'user2'})).toBeHidden(); + await expect(assigneesList.filter({hasText: 'user4'})).toBeHidden(); + await expect(page.locator('.ui.assignees.list .item.no-select')).toBeVisible(); + + // Assign other user (with searchbox) + await page.locator('.select-assignees-modify.dropdown').click(); + await page.type('.select-assignees-modify .menu .search input', 'user4'); + await expect(page.locator('.select-assignees-modify .menu .item').filter({hasText: 'user2'})).toBeHidden(); + await expect(page.locator('.select-assignees-modify .menu .item').filter({hasText: 'user4'})).toBeVisible(); + await page.locator('.select-assignees-modify .menu .item').filter({hasText: 'user4'}).click(); + await page.locator('.select-assignees-modify.dropdown').click(); + await page.waitForLoadState('networkidle'); + await expect(assigneesList.filter({hasText: 'user4'})).toBeVisible(); + + // remove user4 + await page.locator('.select-assignees-modify.dropdown').click(); + await page.locator('.select-assignees-modify .menu .item').filter({hasText: 'user4'}).click(); + await page.locator('.select-assignees-modify.dropdown').click(); + await page.waitForLoadState('networkidle'); + await expect(page.locator('.ui.assignees.list .item.no-select')).toBeVisible(); + await expect(assigneesList.filter({hasText: 'user4'})).toBeHidden(); + + // Test assign me + await page.locator('.ui.assignees .select-assign-me').click(); + await page.waitForLoadState('networkidle'); + await expect(assigneesList.filter({hasText: 'user2'})).toBeVisible(); + await expect(page.locator('.ui.assignees.list .item.no-select')).toBeHidden(); +}); + +test('New Issue: Assignees', async ({browser}, workerInfo) => { + test.skip(workerInfo.project.name === 'Mobile Safari', 'Unable to get tests working on Safari Mobile, see https://codeberg.org/forgejo/forgejo/pulls/3445#issuecomment-1789636'); + const page = await login({browser}, workerInfo); + // select label list in sidebar only + const assigneesList = page.locator('.issue-content-right .assignees.list .selected .item'); + + const response = await page.goto('/org3/repo3/issues/new'); + await expect(response?.status()).toBe(200); + // preconditions + await expect(page.locator('.ui.assignees.list .item.no-select')).toBeVisible(); + await expect(assigneesList.filter({hasText: 'user2'})).toBeHidden(); + await expect(assigneesList.filter({hasText: 'user4'})).toBeHidden(); + + // Assign other user (with searchbox) + await page.locator('.select-assignees.dropdown').click(); + await page.type('.select-assignees .menu .search input', 'user4'); + await expect(page.locator('.select-assignees .menu .item').filter({hasText: 'user2'})).toBeHidden(); + await expect(page.locator('.select-assignees .menu .item').filter({hasText: 'user4'})).toBeVisible(); + await page.locator('.select-assignees .menu .item').filter({hasText: 'user4'}).click(); + await page.locator('.select-assignees.dropdown').click(); + await expect(assigneesList.filter({hasText: 'user4'})).toBeVisible(); + + // remove user4 + await page.locator('.select-assignees.dropdown').click(); + await page.locator('.select-assignees .menu .item').filter({hasText: 'user4'}).click(); + await page.locator('.select-assignees.dropdown').click(); + await expect(page.locator('.ui.assignees.list .item.no-select')).toBeVisible(); + await expect(assigneesList.filter({hasText: 'user4'})).toBeHidden(); + + // Test assign me + await page.locator('.ui.assignees .select-assign-me').click(); + await expect(assigneesList.filter({hasText: 'user2'})).toBeVisible(); + await expect(page.locator('.ui.assignees.list .item.no-select')).toBeHidden(); + + await page.locator('.select-assignees.dropdown').click(); + await page.fill('.select-assignees .menu .search input', ''); + await page.locator('.select-assignees.dropdown .no-select.item').click(); +}); + test('Issue: Milestone', async ({browser}, workerInfo) => { test.skip(workerInfo.project.name === 'Mobile Safari', 'Unable to get tests working on Safari Mobile, see https://codeberg.org/forgejo/forgejo/pulls/3445#issuecomment-1789636'); const page = await login({browser}, workerInfo); diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index feba449c16..c4bd70b71d 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -12,6 +12,26 @@ import {emojiHTML} from './emoji.js'; const {appSubUrl} = window.config; +// if there are draft comments, confirm before reloading, to avoid losing comments +export function reloadConfirmDraftComment() { + const commentTextareas = [ + document.querySelector('.edit-content-zone:not(.tw-hidden) textarea'), + document.querySelector('#comment-form textarea'), + ]; + for (const textarea of commentTextareas) { + // Most users won't feel too sad if they lose a comment with 10 chars, they can re-type these in seconds. + // But if they have typed more (like 50) chars and the comment is lost, they will be very unhappy. + if (textarea && textarea.value.trim().length > 10) { + textarea.parentElement.scrollIntoView(); + if (!window.confirm('Page will be reloaded, but there are draft comments. Continuing to reload will discard the comments. Continue?')) { + return; + } + break; + } + } + window.location.reload(); +} + export function initRepoIssueTimeTracking() { $(document).on('click', '.issue-add-time', () => { $('.issue-start-time-modal').modal({ @@ -668,6 +688,40 @@ export function initRepoIssueBranchSelect() { }); } +export function initRepoIssueAssignMe() { + // Assign to me button + document.querySelector('.ui.assignees.list .item.no-select .select-assign-me') + ?.addEventListener('click', (e) => { + e.preventDefault(); + const selectMe = e.target; + const noSelect = selectMe.parentElement; + const selectorList = document.querySelector('.ui.select-assignees .menu'); + + if (selectMe.getAttribute('data-action') === 'update') { + (async () => { + await updateIssuesMeta( + selectMe.getAttribute('data-update-url'), + selectMe.getAttribute('data-action'), + selectMe.getAttribute('data-issue-id'), + selectMe.getAttribute('data-id'), + ); + reloadConfirmDraftComment(); + })(); + } else { + for (const item of selectorList.querySelectorAll('.item')) { + if (item.getAttribute('data-id') === selectMe.getAttribute('data-id')) { + item.classList.add('checked'); + item.querySelector('.octicon-check').classList.remove('tw-invisible'); + } + } + document.querySelector(selectMe.getAttribute('data-id-selector')).classList.remove('tw-hidden'); + noSelect.classList.add('tw-hidden'); + document.querySelector(selectorList.getAttribute('data-id')).value = selectMe.getAttribute('data-id'); + return false; + } + }); +} + export function initSingleCommentEditor($commentForm) { // pages: // * normal new issue/pr page, no status-button diff --git a/web_src/js/features/repo-legacy.js b/web_src/js/features/repo-legacy.js index b25fc28dea..cb92fcedb5 100644 --- a/web_src/js/features/repo-legacy.js +++ b/web_src/js/features/repo-legacy.js @@ -4,6 +4,7 @@ import { initRepoIssueComments, initRepoIssueDependencyDelete, initRepoIssueReferenceIssue, initRepoIssueTitleEdit, initRepoIssueWipToggle, initRepoPullRequestUpdate, updateIssuesMeta, handleReply, initIssueTemplateCommentEditors, initSingleCommentEditor, + initRepoIssueAssignMe, reloadConfirmDraftComment, } from './repo-issue.js'; import {initUnicodeEscapeButton} from './repo-unicode-escape.js'; import {svg} from '../svg.js'; @@ -29,26 +30,6 @@ import {POST, GET} from '../modules/fetch.js'; const {csrfToken} = window.config; -// if there are draft comments, confirm before reloading, to avoid losing comments -function reloadConfirmDraftComment() { - const commentTextareas = [ - document.querySelector('.edit-content-zone:not(.tw-hidden) textarea'), - document.querySelector('#comment-form textarea'), - ]; - for (const textarea of commentTextareas) { - // Most users won't feel too sad if they lose a comment with 10 chars, they can re-type these in seconds. - // But if they have typed more (like 50) chars and the comment is lost, they will be very unhappy. - if (textarea && textarea.value.trim().length > 10) { - textarea.parentElement.scrollIntoView(); - if (!window.confirm('Page will be reloaded, but there are draft comments. Continuing to reload will discard the comments. Continue?')) { - return; - } - break; - } - } - window.location.reload(); -} - export function initRepoCommentForm() { const $commentForm = $('.comment.form'); if (!$commentForm.length) return; @@ -243,6 +224,7 @@ export function initRepoCommentForm() { // Init labels and assignees initListSubmits('select-label', 'labels'); initListSubmits('select-assignees', 'assignees'); + initRepoIssueAssignMe(); initListSubmits('select-assignees-modify', 'assignees'); initListSubmits('select-reviewers-modify', 'assignees'); From 5ae3b81f3cadd1e672c28fd2650b46df75c15b3d Mon Sep 17 00:00:00 2001 From: TimedIn Date: Wed, 9 Oct 2024 14:12:46 +0200 Subject: [PATCH 2/2] Small fixes and rename for #5482 - New Issue Fixed assign me being hidden after assignees were cleared https://codeberg.org/forgejo/forgejo/pulls/5482/files#issuecomment-2365431 - Test for verifying the above - Removed wait for network idle from e2e test - Renamed templ key assigneeId to assigneeIds --- services/forms/repo_form.go | 2 +- templates/repo/issue/view_content/sidebar/assignees.tmpl | 6 +++--- tests/e2e/issue-sidebar.test.e2e.js | 6 ++---- web_src/js/features/repo-legacy.js | 4 ++-- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 654106c47a..c3d9c3edc9 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -317,7 +317,7 @@ type WebhookForm struct { type CreateIssueForm struct { Title string `binding:"Required;MaxSize(255)"` LabelIDs string `form:"label_ids"` - AssigneeIDs string `form:"assignee_id"` + AssigneeIDs string `form:"assignee_ids"` Ref string `form:"ref"` MilestoneID int64 ProjectID int64 diff --git a/templates/repo/issue/view_content/sidebar/assignees.tmpl b/templates/repo/issue/view_content/sidebar/assignees.tmpl index 8e5043c076..7a928fae46 100644 --- a/templates/repo/issue/view_content/sidebar/assignees.tmpl +++ b/templates/repo/issue/view_content/sidebar/assignees.tmpl @@ -1,4 +1,4 @@ - +
{{ctx.Locale.Tr "repo.issues.new.assignees"}} @@ -6,7 +6,7 @@ {{svg "octicon-gear" 16 "tw-ml-1"}} {{end}} -
+
{{svg "octicon-search" 16}} @@ -36,7 +36,7 @@ {{if and .HasIssuesOrPullsWritePermission (not .Repository.IsArchived)}} {{with index .Assignees 0}} – - + {{ctx.Locale.Tr "repo.issues.new.assign_to_me"}} {{end}} diff --git a/tests/e2e/issue-sidebar.test.e2e.js b/tests/e2e/issue-sidebar.test.e2e.js index 15567c5500..61d3281809 100644 --- a/tests/e2e/issue-sidebar.test.e2e.js +++ b/tests/e2e/issue-sidebar.test.e2e.js @@ -112,10 +112,10 @@ test('Issue: Assignees', async ({browser}, workerInfo) => { // Clear all assignees await page.locator('.select-assignees-modify.dropdown').click(); await page.locator('.select-assignees-modify.dropdown .no-select.item').click(); - await page.waitForLoadState('networkidle'); await expect(assigneesList.filter({hasText: 'user2'})).toBeHidden(); await expect(assigneesList.filter({hasText: 'user4'})).toBeHidden(); await expect(page.locator('.ui.assignees.list .item.no-select')).toBeVisible(); + await expect(page.locator('.select-assign-me')).toBeVisible(); // Assign other user (with searchbox) await page.locator('.select-assignees-modify.dropdown').click(); @@ -124,20 +124,17 @@ test('Issue: Assignees', async ({browser}, workerInfo) => { await expect(page.locator('.select-assignees-modify .menu .item').filter({hasText: 'user4'})).toBeVisible(); await page.locator('.select-assignees-modify .menu .item').filter({hasText: 'user4'}).click(); await page.locator('.select-assignees-modify.dropdown').click(); - await page.waitForLoadState('networkidle'); await expect(assigneesList.filter({hasText: 'user4'})).toBeVisible(); // remove user4 await page.locator('.select-assignees-modify.dropdown').click(); await page.locator('.select-assignees-modify .menu .item').filter({hasText: 'user4'}).click(); await page.locator('.select-assignees-modify.dropdown').click(); - await page.waitForLoadState('networkidle'); await expect(page.locator('.ui.assignees.list .item.no-select')).toBeVisible(); await expect(assigneesList.filter({hasText: 'user4'})).toBeHidden(); // Test assign me await page.locator('.ui.assignees .select-assign-me').click(); - await page.waitForLoadState('networkidle'); await expect(assigneesList.filter({hasText: 'user2'})).toBeVisible(); await expect(page.locator('.ui.assignees.list .item.no-select')).toBeHidden(); }); @@ -179,6 +176,7 @@ test('New Issue: Assignees', async ({browser}, workerInfo) => { await page.locator('.select-assignees.dropdown').click(); await page.fill('.select-assignees .menu .search input', ''); await page.locator('.select-assignees.dropdown .no-select.item').click(); + await expect(page.locator('.select-assign-me')).toBeVisible(); }); test('Issue: Milestone', async ({browser}, workerInfo) => { diff --git a/web_src/js/features/repo-legacy.js b/web_src/js/features/repo-legacy.js index cb92fcedb5..73aaa457f2 100644 --- a/web_src/js/features/repo-legacy.js +++ b/web_src/js/features/repo-legacy.js @@ -256,7 +256,7 @@ export function initRepoCommentForm() { icon = svg('octicon-milestone', 18, 'tw-mr-2'); } else if (input_id === '#project_id') { icon = svg('octicon-project', 18, 'tw-mr-2'); - } else if (input_id === '#assignee_id') { + } else if (input_id === '#assignee_ids') { icon = `avatar`; } @@ -296,7 +296,7 @@ export function initRepoCommentForm() { // Milestone, Assignee, Project selectItem('.select-project', '#project_id'); selectItem('.select-milestone', '#milestone_id'); - selectItem('.select-assignee', '#assignee_id'); + selectItem('.select-assignee', '#assignee_ids'); } async function onEditContent(event) {