From d37f246d09ca0ea94cceed25befd2c366b5eb2b5 Mon Sep 17 00:00:00 2001 From: splitt3r Date: Sat, 19 Aug 2023 10:25:06 +0000 Subject: [PATCH 01/26] Add reviewers selection to new pull request form --- routers/api/v1/repo/pull.go | 2 +- routers/web/repo/compare.go | 43 ++ routers/web/repo/issue.go | 53 +- routers/web/repo/pull.go | 4 +- services/agit/agit.go | 2 +- services/forms/repo_form.go | 1 + services/pull/pull.go | 25 +- templates/repo/issue/new_form.tmpl | 52 ++ tests/integration/actions_trigger_test.go | 4 +- tests/integration/pull_merge_test.go | 2 +- tests/integration/pull_update_test.go | 2 +- web_src/js/features/repo-legacy.js | 600 ++++++++++++++++++++++ 12 files changed, 768 insertions(+), 22 deletions(-) create mode 100644 web_src/js/features/repo-legacy.js diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 34ebcb42d5aef..0f98507a7eb53 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -554,7 +554,7 @@ func CreatePullRequest(ctx *context.APIContext) { } } - if err := pull_service.NewPullRequest(ctx, repo, prIssue, labelIDs, []string{}, pr, assigneeIDs); err != nil { + if err := pull_service.NewPullRequest(ctx, repo, prIssue, labelIDs, []string{}, pr, assigneeIDs, []int64{}); err != nil { if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) { ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err) } else if errors.Is(err, user_model.ErrBlockedUser) { diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index e6a04782e5bbf..70d7d37d6599f 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -19,6 +19,7 @@ import ( "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" + "code.gitea.io/gitea/models/organization" access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" @@ -39,6 +40,7 @@ import ( "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/context/upload" "code.gitea.io/gitea/services/gitdiff" + repo_service "code.gitea.io/gitea/services/repository" ) const ( @@ -789,6 +791,47 @@ func CompareDiff(ctx *context.Context) { if ctx.Written() { return } + + // Get reviewer info for pr + var ( + reviewers []*user_model.User + teamReviewers []*organization.Team + reviewersResult []*repoReviewerSelection + ) + reviewers, err = repo_model.GetReviewers(ctx, ctx.Repo.Repository, ctx.Doer.ID, ctx.Doer.ID) + if err != nil { + ctx.ServerError("GetReviewers", err) + return + } + + teamReviewers, err = repo_service.GetReviewerTeams(ctx, ctx.Repo.Repository) + if err != nil { + ctx.ServerError("GetReviewerTeams", err) + return + } + + for _, user := range reviewers { + reviewersResult = append(reviewersResult, &repoReviewerSelection{ + IsTeam: false, + CanChange: true, + User: user, + ItemID: user.ID, + }) + } + + // negative reviewIDs represent team requests + for _, team := range teamReviewers { + reviewersResult = append(reviewersResult, &repoReviewerSelection{ + IsTeam: true, + CanChange: true, + Team: team, + ItemID: -team.ID, + }) + } + ctx.Data["Reviewers"] = reviewersResult + if ctx.Written() { + return + } } } beforeCommitID := ctx.Data["BeforeCommitID"].(string) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 507b5af9d904a..136dd66e5ad4f 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1118,7 +1118,7 @@ func DeleteIssue(ctx *context.Context) { } // ValidateRepoMetas check and returns repository's meta information -func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull bool) ([]int64, []int64, int64, int64) { +func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull bool) ([]int64, []int64, []int64, int64, int64) { var ( repo = ctx.Repo.Repository err error @@ -1126,7 +1126,7 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull labels := RetrieveRepoMetas(ctx, ctx.Repo.Repository, isPull) if ctx.Written() { - return nil, nil, 0, 0 + return nil, nil, nil, 0, 0 } var labelIDs []int64 @@ -1135,7 +1135,7 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull if len(form.LabelIDs) > 0 { labelIDs, err = base.StringsToInt64s(strings.Split(form.LabelIDs, ",")) if err != nil { - return nil, nil, 0, 0 + return nil, nil, nil, 0, 0 } labelIDMark := make(container.Set[int64]) labelIDMark.AddMultiple(labelIDs...) @@ -1158,11 +1158,11 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull milestone, err := issues_model.GetMilestoneByRepoID(ctx, ctx.Repo.Repository.ID, milestoneID) if err != nil { ctx.ServerError("GetMilestoneByID", err) - return nil, nil, 0, 0 + return nil, nil, nil, 0, 0 } if milestone.RepoID != repo.ID { ctx.ServerError("GetMilestoneByID", err) - return nil, nil, 0, 0 + return nil, nil, nil, 0, 0 } ctx.Data["Milestone"] = milestone ctx.Data["milestone_id"] = milestoneID @@ -1172,11 +1172,11 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull p, err := project_model.GetProjectByID(ctx, form.ProjectID) if err != nil { ctx.ServerError("GetProjectByID", err) - return nil, nil, 0, 0 + return nil, nil, nil, 0, 0 } if p.RepoID != ctx.Repo.Repository.ID && p.OwnerID != ctx.Repo.Repository.OwnerID { ctx.NotFound("", nil) - return nil, nil, 0, 0 + return nil, nil, nil, 0, 0 } ctx.Data["Project"] = p @@ -1188,7 +1188,7 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull if len(form.AssigneeIDs) > 0 { assigneeIDs, err = base.StringsToInt64s(strings.Split(form.AssigneeIDs, ",")) if err != nil { - return nil, nil, 0, 0 + return nil, nil, nil, 0, 0 } // Check if the passed assignees actually exists and is assignable @@ -1196,18 +1196,18 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull assignee, err := user_model.GetUserByID(ctx, aID) if err != nil { ctx.ServerError("GetUserByID", err) - return nil, nil, 0, 0 + return nil, nil, nil, 0, 0 } valid, err := access_model.CanBeAssigned(ctx, assignee, repo, isPull) if err != nil { ctx.ServerError("CanBeAssigned", err) - return nil, nil, 0, 0 + return nil, nil, nil, 0, 0 } if !valid { ctx.ServerError("canBeAssigned", repo_model.ErrUserDoesNotHaveAccessToRepo{UserID: aID, RepoName: repo.Name}) - return nil, nil, 0, 0 + return nil, nil, nil, 0, 0 } } } @@ -1217,7 +1217,34 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull assigneeIDs = append(assigneeIDs, form.AssigneeID) } - return labelIDs, assigneeIDs, milestoneID, form.ProjectID + // Check reviewers + var reviewerIDs []int64 + if len(form.ReviewerIDs) > 0 { + reviewerIDs, err = base.StringsToInt64s(strings.Split(form.ReviewerIDs, ",")) + if err != nil { + return nil, nil, nil, 0, 0 + } + + // Check if the passed reviewers actually exist + for _, rID := range reviewerIDs { + if rID < 0 { + _, err := organization.GetTeamByID(ctx, -rID) + if err != nil { + ctx.ServerError("GetTeamByID", err) + return nil, nil, nil, 0, 0 + } + continue + } + + _, err := user_model.GetUserByID(ctx, rID) + if err != nil { + ctx.ServerError("GetUserByID", err) + return nil, nil, nil, 0, 0 + } + } + } + + return labelIDs, assigneeIDs, reviewerIDs, milestoneID, form.ProjectID } // NewIssuePost response for creating new issue @@ -1235,7 +1262,7 @@ func NewIssuePost(ctx *context.Context) { attachments []string ) - labelIDs, assigneeIDs, milestoneID, projectID := ValidateRepoMetas(ctx, *form, false) + labelIDs, assigneeIDs, _, milestoneID, projectID := ValidateRepoMetas(ctx, *form, false) if ctx.Written() { return } diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 0efe1be76a310..05b47faeff130 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1268,7 +1268,7 @@ func CompareAndPullRequestPost(ctx *context.Context) { return } - labelIDs, assigneeIDs, milestoneID, projectID := ValidateRepoMetas(ctx, *form, true) + labelIDs, assigneeIDs, reviewerIDs, milestoneID, _ := ValidateRepoMetas(ctx, *form, true) if ctx.Written() { return } @@ -1318,7 +1318,7 @@ func CompareAndPullRequestPost(ctx *context.Context) { // FIXME: check error in the case two people send pull request at almost same time, give nice error prompt // instead of 500. - if err := pull_service.NewPullRequest(ctx, repo, pullIssue, labelIDs, attachments, pullRequest, assigneeIDs); err != nil { + if err := pull_service.NewPullRequest(ctx, repo, pullIssue, labelIDs, attachments, pullRequest, assigneeIDs, reviewerIDs); err != nil { switch { case repo_model.IsErrUserDoesNotHaveAccessToRepo(err): ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error()) diff --git a/services/agit/agit.go b/services/agit/agit.go index 82aa2791aa76d..b4ebb2eb18296 100644 --- a/services/agit/agit.go +++ b/services/agit/agit.go @@ -138,7 +138,7 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git. Flow: issues_model.PullRequestFlowAGit, } - if err := pull_service.NewPullRequest(ctx, repo, prIssue, []int64{}, []string{}, pr, []int64{}); err != nil { + if err := pull_service.NewPullRequest(ctx, repo, prIssue, []int64{}, []string{}, pr, []int64{}, []int64{}); err != nil { return nil, err } diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index ddd07a64cbf75..83f2dd6caacfc 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -447,6 +447,7 @@ type CreateIssueForm struct { Title string `binding:"Required;MaxSize(255)"` LabelIDs string `form:"label_ids"` AssigneeIDs string `form:"assignee_ids"` + ReviewerIDs string `form:"reviewer_ids"` Ref string `form:"ref"` MilestoneID int64 ProjectID int64 diff --git a/services/pull/pull.go b/services/pull/pull.go index bab4e49998e15..68e81814deb56 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -17,6 +17,7 @@ import ( "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" + "code.gitea.io/gitea/models/organization" access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" @@ -42,7 +43,7 @@ func getPullWorkingLockKey(prID int64) string { } // NewPullRequest creates new pull request with labels for repository. -func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *issues_model.Issue, labelIDs []int64, uuids []string, pr *issues_model.PullRequest, assigneeIDs []int64) error { +func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *issues_model.Issue, labelIDs []int64, uuids []string, pr *issues_model.PullRequest, assigneeIDs []int64, reviewerIDs []int64) error { if err := issue.LoadPoster(ctx); err != nil { return err } @@ -116,6 +117,28 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *iss assigneeCommentMap[assigneeID] = comment } + for _, reviewerID := range reviewerIDs { + if reviewerID < 0 { + team, err := organization.GetTeamByID(ctx, -reviewerID) + if err != nil { + return err + } + _, err = issue_service.TeamReviewRequest(ctx, issue, issue.Poster, team, true) + if err != nil { + return err + } + } + + reviewer, err := user_model.GetUserByID(ctx, reviewerID) + if err != nil { + return err + } + _, err = issue_service.ReviewRequest(ctx, issue, issue.Poster, reviewer, true) + if err != nil { + return err + } + } + pr.Issue = issue issue.PullRequest = pr diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index e56d1b9ecc7bd..db7a44fec3c09 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -48,6 +48,58 @@
{{template "repo/issue/branch_selector_field" .}} + {{if .PageIsComparePull}} + +
+ + {{.locale.Tr "repo.issues.review.reviewers"}} + {{svg "octicon-gear" 16 "gt-ml-2"}} + +
+
+ {{svg "octicon-search" 16}} + +
+
{{.locale.Tr "repo.issues.new.clear_reviewers"}}
+ {{range .Reviewers}} + {{if .User}} + + + + {{ctx.AvatarUtils.Avatar .User 28 "gt-mr-3"}}{{template "repo/search_name" .User}} + + + {{else if .Team}} + + + + {{svg "octicon-people" 16 "gt-ml-4 gt-mr-2"}}{{$.Repository.OwnerName}}/{{.Team.Name}} + + + {{end}} + {{end}} +
+
+
+ + {{.locale.Tr "repo.issues.new.no_reviewers"}} + +
+ {{range .Reviewers}} + {{if .User}} + + {{ctx.AvatarUtils.Avatar .User 28 "gt-mr-3 gt-vm"}}{{.User.GetDisplayName}} + + {{else if .Team}} + + {{svg "octicon-people" 20 "gt-mr-3"}}{{$.Repository.OwnerName}}/{{.Team.Name}} + + {{end}} + {{end}} +
+
+
+ {{end}} {{template "repo/issue/labels/labels_selector_field" .}} diff --git a/tests/integration/actions_trigger_test.go b/tests/integration/actions_trigger_test.go index c254c90958314..4a542884fca2b 100644 --- a/tests/integration/actions_trigger_test.go +++ b/tests/integration/actions_trigger_test.go @@ -145,7 +145,7 @@ func TestPullRequestTargetEvent(t *testing.T) { BaseRepo: baseRepo, Type: issues_model.PullRequestGitea, } - err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil) + err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil, nil) assert.NoError(t, err) // load and compare ActionRun @@ -199,7 +199,7 @@ func TestPullRequestTargetEvent(t *testing.T) { BaseRepo: baseRepo, Type: issues_model.PullRequestGitea, } - err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil) + err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil, nil) assert.NoError(t, err) // the new pull request cannot trigger actions, so there is still only 1 record diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index 43210e852e53b..729d22facfb33 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -520,7 +520,7 @@ func TestConflictChecking(t *testing.T) { BaseRepo: baseRepo, Type: issues_model.PullRequestGitea, } - err = pull.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil) + err = pull.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil, nil) assert.NoError(t, err) issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "PR with conflict!"}) diff --git a/tests/integration/pull_update_test.go b/tests/integration/pull_update_test.go index 5ae241f3af720..c283488563a8a 100644 --- a/tests/integration/pull_update_test.go +++ b/tests/integration/pull_update_test.go @@ -173,7 +173,7 @@ func createOutdatedPR(t *testing.T, actor, forkOrg *user_model.User) *issues_mod BaseRepo: baseRepo, Type: issues_model.PullRequestGitea, } - err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil) + err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil, nil) assert.NoError(t, err) issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "Test Pull -to-update-"}) diff --git a/web_src/js/features/repo-legacy.js b/web_src/js/features/repo-legacy.js new file mode 100644 index 0000000000000..9b1307d7bcd63 --- /dev/null +++ b/web_src/js/features/repo-legacy.js @@ -0,0 +1,600 @@ +import $ from 'jquery'; +import { + initRepoIssueBranchSelect, initRepoIssueCodeCommentCancel, initRepoIssueCommentDelete, + initRepoIssueComments, initRepoIssueDependencyDelete, initRepoIssueReferenceIssue, + initRepoIssueTitleEdit, initRepoIssueWipToggle, + initRepoPullRequestUpdate, updateIssuesMeta, handleReply, initIssueTemplateCommentEditors, initSingleCommentEditor, +} from './repo-issue.js'; +import {initUnicodeEscapeButton} from './repo-unicode-escape.js'; +import {svg} from '../svg.js'; +import {htmlEscape} from 'escape-goat'; +import {initRepoBranchTagSelector} from '../components/RepoBranchTagSelector.vue'; +import { + initRepoCloneLink, initRepoCommonBranchOrTagDropdown, initRepoCommonFilterSearchDropdown, + initRepoCommonLanguageStats, +} from './repo-common.js'; +import {initCitationFileCopyContent} from './citation.js'; +import {initCompLabelEdit} from './comp/LabelEdit.js'; +import {initRepoDiffConversationNav} from './repo-diff.js'; +import {createDropzone} from './dropzone.js'; +import {initCommentContent, initMarkupContent} from '../markup/content.js'; +import {initCompReactionSelector} from './comp/ReactionSelector.js'; +import {initRepoSettingBranches} from './repo-settings.js'; +import {initRepoPullRequestMergeForm} from './repo-issue-pr-form.js'; +import {hideElem, showElem} from '../utils/dom.js'; +import {getComboMarkdownEditor, initComboMarkdownEditor} from './comp/ComboMarkdownEditor.js'; +import {attachRefIssueContextPopup} from './contextpopup.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(.gt-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 === 0) { + return; + } + + if ($commentForm.find('.field.combo-editor-dropzone').length) { + // at the moment, if a form has multiple combo-markdown-editors, it must be a issue template form + initIssueTemplateCommentEditors($commentForm); + } else { + initSingleCommentEditor($commentForm); + } + + function initBranchSelector() { + const $selectBranch = $('.ui.select-branch'); + const $branchMenu = $selectBranch.find('.reference-list-menu'); + const $isNewIssue = $branchMenu.hasClass('new-issue'); + $branchMenu.find('.item:not(.no-select)').on('click', function () { + const selectedValue = $(this).data('id'); + const editMode = $('#editing_mode').val(); + $($(this).data('id-selector')).val(selectedValue); + if ($isNewIssue) { + $selectBranch.find('.ui .branch-name').text($(this).data('name')); + return; + } + + if (editMode === 'true') { + const form = $('#update_issueref_form'); + $.post(form.attr('action'), {_csrf: csrfToken, ref: selectedValue}, () => window.location.reload()); + } else if (editMode === '') { + $selectBranch.find('.ui .branch-name').text(selectedValue); + } + }); + $selectBranch.find('.reference.column').on('click', function () { + hideElem($selectBranch.find('.scrolling.reference-list-menu')); + $selectBranch.find('.reference .text').removeClass('black'); + showElem($($(this).data('target'))); + $(this).find('.text').addClass('black'); + return false; + }); + } + + initBranchSelector(); + + // List submits + function initListSubmits(selector, outerSelector) { + const $list = $(`.ui.${outerSelector}.list`); + const $noSelect = $list.find('.no-select'); + const $listMenu = $(`.${selector} .menu`); + let hasUpdateAction = $listMenu.data('action') === 'update'; + const items = {}; + + $(`.${selector}`).dropdown({ + 'action': 'nothing', // do not hide the menu if user presses Enter + fullTextSearch: 'exact', + async onHide() { + hasUpdateAction = $listMenu.data('action') === 'update'; // Update the var + if (hasUpdateAction) { + // TODO: Add batch functionality and make this 1 network request. + const itemEntries = Object.entries(items); + for (const [elementId, item] of itemEntries) { + await updateIssuesMeta( + item['update-url'], + item.action, + item['issue-id'], + elementId, + ); + } + if (itemEntries.length) { + reloadConfirmDraftComment(); + } + } + }, + }); + + $listMenu.find('.item:not(.no-select)').on('click', function (e) { + e.preventDefault(); + if ($(this).hasClass('ban-change')) { + return false; + } + + hasUpdateAction = $listMenu.data('action') === 'update'; // Update the var + + const clickedItem = $(this); + const scope = $(this).attr('data-scope'); + + $(this).parent().find('.item').each(function () { + if (scope) { + // Enable only clicked item for scoped labels + if ($(this).attr('data-scope') !== scope) { + return true; + } + if (!$(this).is(clickedItem) && !$(this).hasClass('checked')) { + return true; + } + } else if (!$(this).is(clickedItem)) { + // Toggle for other labels + return true; + } + + if ($(this).hasClass('checked')) { + $(this).removeClass('checked'); + $(this).find('.octicon-check').addClass('invisible'); + if (hasUpdateAction) { + if (!($(this).data('id') in items)) { + items[$(this).data('id')] = { + 'update-url': $listMenu.data('update-url'), + action: 'detach', + 'issue-id': $listMenu.data('issue-id'), + }; + } else { + delete items[$(this).data('id')]; + } + } + } else { + $(this).addClass('checked'); + $(this).find('.octicon-check').removeClass('invisible'); + if (hasUpdateAction) { + if (!($(this).data('id') in items)) { + items[$(this).data('id')] = { + 'update-url': $listMenu.data('update-url'), + action: 'attach', + 'issue-id': $listMenu.data('issue-id'), + }; + } else { + delete items[$(this).data('id')]; + } + } + } + }); + + // TODO: Which thing should be done for choosing review requests + // to make chosen items be shown on time here? + if (selector === 'select-reviewers-modify' || selector === 'select-assignees-modify') { + return false; + } + + const listIds = []; + $(this).parent().find('.item').each(function () { + if ($(this).hasClass('checked')) { + listIds.push($(this).data('id')); + $($(this).data('id-selector')).removeClass('gt-hidden'); + } else { + $($(this).data('id-selector')).addClass('gt-hidden'); + } + }); + if (listIds.length === 0) { + $noSelect.removeClass('gt-hidden'); + } else { + $noSelect.addClass('gt-hidden'); + } + $($(this).parent().data('id')).val(listIds.join(',')); + return false; + }); + $listMenu.find('.no-select.item').on('click', function (e) { + e.preventDefault(); + if (hasUpdateAction) { + updateIssuesMeta( + $listMenu.data('update-url'), + 'clear', + $listMenu.data('issue-id'), + '', + ).then(reloadConfirmDraftComment); + } + + $(this).parent().find('.item').each(function () { + $(this).removeClass('checked'); + $(this).find('.octicon-check').addClass('invisible'); + }); + + if (selector === 'select-reviewers-modify' || selector === 'select-assignees-modify') { + return false; + } + + $list.find('.item').each(function () { + $(this).addClass('gt-hidden'); + }); + $noSelect.removeClass('gt-hidden'); + $($(this).parent().data('id')).val(''); + }); + } + + // Init labels and assignees + initListSubmits('select-label', 'labels'); + initListSubmits('select-assignees', 'assignees'); + initListSubmits('select-assignees-modify', 'assignees'); + initListSubmits('select-reviewers', 'reviewers'); + initListSubmits('select-reviewers-modify', 'reviewers'); + + function selectItem(select_id, input_id) { + const $menu = $(`${select_id} .menu`); + const $list = $(`.ui${select_id}.list`); + const hasUpdateAction = $menu.data('action') === 'update'; + + $menu.find('.item:not(.no-select)').on('click', function () { + $(this).parent().find('.item').each(function () { + $(this).removeClass('selected active'); + }); + + $(this).addClass('selected active'); + if (hasUpdateAction) { + updateIssuesMeta( + $menu.data('update-url'), + '', + $menu.data('issue-id'), + $(this).data('id'), + ).then(reloadConfirmDraftComment); + } + + let icon = ''; + if (input_id === '#milestone_id') { + icon = svg('octicon-milestone', 18, 'gt-mr-3'); + } else if (input_id === '#project_id') { + icon = svg('octicon-project', 18, 'gt-mr-3'); + } else if (input_id === '#assignee_id') { + icon = `avatar`; + } else if (input_id === '#reviewer_id') { + icon = `avatar`; + } + + $list.find('.selected').html(` + + ${icon} + ${htmlEscape($(this).text())} + + `); + + $(`.ui${select_id}.list .no-select`).addClass('gt-hidden'); + $(input_id).val($(this).data('id')); + }); + $menu.find('.no-select.item').on('click', function () { + $(this).parent().find('.item:not(.no-select)').each(function () { + $(this).removeClass('selected active'); + }); + + if (hasUpdateAction) { + updateIssuesMeta( + $menu.data('update-url'), + '', + $menu.data('issue-id'), + $(this).data('id'), + ).then(reloadConfirmDraftComment); + } + + $list.find('.selected').html(''); + $list.find('.no-select').removeClass('gt-hidden'); + $(input_id).val(''); + }); + } + + // Milestone, Assignee, Project + selectItem('.select-project', '#project_id'); + selectItem('.select-milestone', '#milestone_id'); + selectItem('.select-assignee', '#assignee_id'); + selectItem('.select-reviewer', '#reviewer_id'); +} + + +async function onEditContent(event) { + event.preventDefault(); + + const $segment = $(this).closest('.header').next(); + const $editContentZone = $segment.find('.edit-content-zone'); + const $renderContent = $segment.find('.render-content'); + const $rawContent = $segment.find('.raw-content'); + + let comboMarkdownEditor; + + const setupDropzone = async ($dropzone) => { + if ($dropzone.length === 0) return null; + + let disableRemovedfileEvent = false; // when resetting the dropzone (removeAllFiles), disable the "removedfile" event + let fileUuidDict = {}; // to record: if a comment has been saved, then the uploaded files won't be deleted from server when clicking the Remove in the dropzone + const dz = await createDropzone($dropzone[0], { + url: $dropzone.attr('data-upload-url'), + headers: {'X-Csrf-Token': csrfToken}, + maxFiles: $dropzone.attr('data-max-file'), + maxFilesize: $dropzone.attr('data-max-size'), + acceptedFiles: (['*/*', ''].includes($dropzone.attr('data-accepts'))) ? null : $dropzone.attr('data-accepts'), + addRemoveLinks: true, + dictDefaultMessage: $dropzone.attr('data-default-message'), + dictInvalidFileType: $dropzone.attr('data-invalid-input-type'), + dictFileTooBig: $dropzone.attr('data-file-too-big'), + dictRemoveFile: $dropzone.attr('data-remove-file'), + timeout: 0, + thumbnailMethod: 'contain', + thumbnailWidth: 480, + thumbnailHeight: 480, + init() { + this.on('success', (file, data) => { + file.uuid = data.uuid; + fileUuidDict[file.uuid] = {submitted: false}; + const input = $(``).val(data.uuid); + $dropzone.find('.files').append(input); + }); + this.on('removedfile', (file) => { + if (disableRemovedfileEvent) return; + $(`#${file.uuid}`).remove(); + if ($dropzone.attr('data-remove-url') && !fileUuidDict[file.uuid].submitted) { + $.post($dropzone.attr('data-remove-url'), { + file: file.uuid, + _csrf: csrfToken, + }); + } + }); + this.on('submit', () => { + $.each(fileUuidDict, (fileUuid) => { + fileUuidDict[fileUuid].submitted = true; + }); + }); + this.on('reload', () => { + $.getJSON($editContentZone.attr('data-attachment-url'), (data) => { + // do not trigger the "removedfile" event, otherwise the attachments would be deleted from server + disableRemovedfileEvent = true; + dz.removeAllFiles(true); + $dropzone.find('.files').empty(); + fileUuidDict = {}; + disableRemovedfileEvent = false; + + for (const attachment of data) { + const imgSrc = `${$dropzone.attr('data-link-url')}/${attachment.uuid}`; + dz.emit('addedfile', attachment); + dz.emit('thumbnail', attachment, imgSrc); + dz.emit('complete', attachment); + dz.files.push(attachment); + fileUuidDict[attachment.uuid] = {submitted: true}; + $dropzone.find(`img[src='${imgSrc}']`).css('max-width', '100%'); + const input = $(``).val(attachment.uuid); + $dropzone.find('.files').append(input); + } + }); + }); + }, + }); + dz.emit('reload'); + return dz; + }; + + const cancelAndReset = (dz) => { + showElem($renderContent); + hideElem($editContentZone); + if (dz) { + dz.emit('reload'); + } + }; + + const saveAndRefresh = (dz, $dropzone) => { + showElem($renderContent); + hideElem($editContentZone); + const $attachments = $dropzone.find('.files').find('[name=files]').map(function () { + return $(this).val(); + }).get(); + $.post($editContentZone.attr('data-update-url'), { + _csrf: csrfToken, + content: comboMarkdownEditor.value(), + context: $editContentZone.attr('data-context'), + files: $attachments, + }, (data) => { + if (!data.content) { + $renderContent.html($('#no-content').html()); + $rawContent.text(''); + } else { + $renderContent.html(data.content); + $rawContent.text(comboMarkdownEditor.value()); + + const refIssues = $renderContent.find('p .ref-issue'); + attachRefIssueContextPopup(refIssues); + } + const $content = $segment; + if (!$content.find('.dropzone-attachments').length) { + if (data.attachments !== '') { + $content.append(`
`); + $content.find('.dropzone-attachments').replaceWith(data.attachments); + } + } else if (data.attachments === '') { + $content.find('.dropzone-attachments').remove(); + } else { + $content.find('.dropzone-attachments').replaceWith(data.attachments); + } + if (dz) { + dz.emit('submit'); + dz.emit('reload'); + } + initMarkupContent(); + initCommentContent(); + }); + }; + + if (!$editContentZone.html()) { + $editContentZone.html($('#issue-comment-editor-template').html()); + comboMarkdownEditor = await initComboMarkdownEditor($editContentZone.find('.combo-markdown-editor')); + + const $dropzone = $editContentZone.find('.dropzone'); + const dz = await setupDropzone($dropzone); + $editContentZone.find('.cancel.button').on('click', (e) => { + e.preventDefault(); + cancelAndReset(dz); + }); + $editContentZone.find('.save.button').on('click', (e) => { + e.preventDefault(); + saveAndRefresh(dz, $dropzone); + }); + } else { + comboMarkdownEditor = getComboMarkdownEditor($editContentZone.find('.combo-markdown-editor')); + } + + // Show write/preview tab and copy raw content as needed + showElem($editContentZone); + hideElem($renderContent); + if (!comboMarkdownEditor.value()) { + comboMarkdownEditor.value($rawContent.text()); + } + comboMarkdownEditor.focus(); +} + +export function initRepository() { + if ($('.page-content.repository').length === 0) { + return; + } + + initRepoBranchTagSelector('.js-branch-tag-selector'); + + // Options + if ($('.repository.settings.options').length > 0) { + // Enable or select internal/external wiki system and issue tracker. + $('.enable-system').on('change', function () { + if (this.checked) { + $($(this).data('target')).removeClass('disabled'); + if (!$(this).data('context')) $($(this).data('context')).addClass('disabled'); + } else { + $($(this).data('target')).addClass('disabled'); + if (!$(this).data('context')) $($(this).data('context')).removeClass('disabled'); + } + }); + $('.enable-system-radio').on('change', function () { + if (this.value === 'false') { + $($(this).data('target')).addClass('disabled'); + if ($(this).data('context') !== undefined) $($(this).data('context')).removeClass('disabled'); + } else if (this.value === 'true') { + $($(this).data('target')).removeClass('disabled'); + if ($(this).data('context') !== undefined) $($(this).data('context')).addClass('disabled'); + } + }); + const $trackerIssueStyleRadios = $('.js-tracker-issue-style'); + $trackerIssueStyleRadios.on('change input', () => { + const checkedVal = $trackerIssueStyleRadios.filter(':checked').val(); + $('#tracker-issue-style-regex-box').toggleClass('disabled', checkedVal !== 'regexp'); + }); + } + + // Labels + initCompLabelEdit('.repository.labels'); + + // Milestones + if ($('.repository.new.milestone').length > 0) { + $('#clear-date').on('click', () => { + $('#deadline').val(''); + return false; + }); + } + + // Repo Creation + if ($('.repository.new.repo').length > 0) { + $('input[name="gitignores"], input[name="license"]').on('change', () => { + const gitignores = $('input[name="gitignores"]').val(); + const license = $('input[name="license"]').val(); + if (gitignores || license) { + $('input[name="auto_init"]').prop('checked', true); + } + }); + } + + // Compare or pull request + const $repoDiff = $('.repository.diff'); + if ($repoDiff.length) { + initRepoCommonBranchOrTagDropdown('.choose.branch .dropdown'); + initRepoCommonFilterSearchDropdown('.choose.branch .dropdown'); + } + + initRepoCloneLink(); + initCitationFileCopyContent(); + initRepoCommonLanguageStats(); + initRepoSettingBranches(); + + // Issues + if ($('.repository.view.issue').length > 0) { + initRepoIssueCommentEdit(); + + initRepoIssueBranchSelect(); + initRepoIssueTitleEdit(); + initRepoIssueWipToggle(); + initRepoIssueComments(); + + initRepoDiffConversationNav(); + initRepoIssueReferenceIssue(); + + + initRepoIssueCommentDelete(); + initRepoIssueDependencyDelete(); + initRepoIssueCodeCommentCancel(); + initRepoPullRequestUpdate(); + initCompReactionSelector($(document)); + + initRepoPullRequestMergeForm(); + } + + // Pull request + const $repoComparePull = $('.repository.compare.pull'); + if ($repoComparePull.length > 0) { + // show pull request form + $repoComparePull.find('button.show-form').on('click', function (e) { + e.preventDefault(); + hideElem($(this).parent()); + + const $form = $repoComparePull.find('.pullrequest-form'); + showElem($form); + }); + } + + initUnicodeEscapeButton(); +} + +function initRepoIssueCommentEdit() { + // Edit issue or comment content + $(document).on('click', '.edit-content', onEditContent); + + // Quote reply + $(document).on('click', '.quote-reply', async function (event) { + event.preventDefault(); + const target = $(this).data('target'); + const quote = $(`#${target}`).text().replace(/\n/g, '\n> '); + const content = `> ${quote}\n\n`; + let editor; + if ($(this).hasClass('quote-reply-diff')) { + const $replyBtn = $(this).closest('.comment-code-cloud').find('button.comment-form-reply'); + editor = await handleReply($replyBtn); + } else { + // for normal issue/comment page + editor = getComboMarkdownEditor($('#comment-form .combo-markdown-editor')); + } + if (editor) { + if (editor.value()) { + editor.value(`${editor.value()}\n\n${content}`); + } else { + editor.value(content); + } + editor.focus(); + editor.moveCursorToEnd(); + } + }); +} From 5929b87112406376038107c1fcb747d9f4d42c07 Mon Sep 17 00:00:00 2001 From: splitt3r Date: Sat, 19 Aug 2023 10:49:01 +0000 Subject: [PATCH 02/26] Add translatanle clear reviewers label --- options/locale/locale_en-US.ini | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 06bf57fc62cf2..7ad56112098de 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1458,10 +1458,11 @@ issues.new.closed_milestone = Closed Milestones issues.new.assignees = Assignees issues.new.clear_assignees = Clear assignees issues.new.no_assignees = No Assignees -issues.new.no_reviewers = No reviewers +issues.new.no_reviewers = No Reviewers issues.new.blocked_user = Cannot create issue because you are blocked by the repository owner. 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.edit.blocked_user = Cannot edit content because you are blocked by the poster or repository owner. +issues.new.clear_reviewers = Clear reviewers issues.choose.get_started = Get Started issues.choose.open_external_link = Open issues.choose.blank = Default From 452828abbdee13e2a068697eb5105931fa5554f6 Mon Sep 17 00:00:00 2001 From: splitt3r Date: Sat, 19 Aug 2023 12:29:09 +0000 Subject: [PATCH 03/26] Add missing continue statement --- services/pull/pull.go | 1 + 1 file changed, 1 insertion(+) diff --git a/services/pull/pull.go b/services/pull/pull.go index 68e81814deb56..5a308dea2bbb0 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -127,6 +127,7 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *iss if err != nil { return err } + continue } reviewer, err := user_model.GetUserByID(ctx, reviewerID) From 906514e1cb93e5895ba5dd15e9622e924bf84399 Mon Sep 17 00:00:00 2001 From: splitt3r Date: Fri, 1 Sep 2023 10:57:52 +0000 Subject: [PATCH 04/26] Check pr creator permissions --- routers/web/repo/issue.go | 3 ++- routers/web/repo/pull.go | 2 +- services/pull/pull.go | 15 ++++++++++++++- templates/repo/issue/new_form.tmpl | 6 +++++- 4 files changed, 22 insertions(+), 4 deletions(-) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 136dd66e5ad4f..02ee60d0fd2a1 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1225,8 +1225,9 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull return nil, nil, nil, 0, 0 } - // Check if the passed reviewers actually exist + // Check if the passed reviewers (user/team) actually exist for _, rID := range reviewerIDs { + // negative reviewIDs represent team requests if rID < 0 { _, err := organization.GetTeamByID(ctx, -rID) if err != nil { diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 05b47faeff130..a56014efb6a12 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1268,7 +1268,7 @@ func CompareAndPullRequestPost(ctx *context.Context) { return } - labelIDs, assigneeIDs, reviewerIDs, milestoneID, _ := ValidateRepoMetas(ctx, *form, true) + labelIDs, assigneeIDs, reviewerIDs, milestoneID, projectID := ValidateRepoMetas(ctx, *form, true) if ctx.Written() { return } diff --git a/services/pull/pull.go b/services/pull/pull.go index 5a308dea2bbb0..db2c23a2fe2b0 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -43,7 +43,7 @@ func getPullWorkingLockKey(prID int64) string { } // NewPullRequest creates new pull request with labels for repository. -func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *issues_model.Issue, labelIDs []int64, uuids []string, pr *issues_model.PullRequest, assigneeIDs []int64, reviewerIDs []int64) error { +func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *issues_model.Issue, labelIDs []int64, uuids []string, pr *issues_model.PullRequest, assigneeIDs, reviewerIDs []int64) error { if err := issue.LoadPoster(ctx); err != nil { return err } @@ -118,11 +118,16 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *iss } for _, reviewerID := range reviewerIDs { + // negative reviewIDs represent team requests if reviewerID < 0 { team, err := organization.GetTeamByID(ctx, -reviewerID) if err != nil { return err } + err = issue_service.IsValidTeamReviewRequest(ctx, team, issue.Poster, true, issue) + if err != nil { + return err + } _, err = issue_service.TeamReviewRequest(ctx, issue, issue.Poster, team, true) if err != nil { return err @@ -134,6 +139,14 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *iss if err != nil { return err } + permDoer, err := access_model.GetUserRepoPermission(ctx, issue.Repo, issue.Poster) + if err != nil { + return err + } + err = issue_service.IsValidReviewRequest(ctx, reviewer, issue.Poster, true, issue, &permDoer) + if err != nil { + return err + } _, err = issue_service.ReviewRequest(ctx, issue, issue.Poster, reviewer, true) if err != nil { return err diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index db7a44fec3c09..070e9e6761bfa 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -69,7 +69,11 @@ {{ctx.AvatarUtils.Avatar .User 28 "gt-mr-3"}}{{template "repo/search_name" .User}} - {{else if .Team}} + {{end}} + {{end}} +
+ {{range .Reviewers}} + {{if .Team}} From 45b8a965a2139ed79396f095191617cece39857f Mon Sep 17 00:00:00 2001 From: splitt3r Date: Fri, 1 Sep 2023 11:07:05 +0000 Subject: [PATCH 05/26] Remove unintended change --- services/pull/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index db2c23a2fe2b0..4a267d39acd12 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -43,7 +43,7 @@ func getPullWorkingLockKey(prID int64) string { } // NewPullRequest creates new pull request with labels for repository. -func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *issues_model.Issue, labelIDs []int64, uuids []string, pr *issues_model.PullRequest, assigneeIDs, reviewerIDs []int64) error { +func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *issues_model.Issue, labelIDs []int64, uuids []string, pr *issues_model.PullRequest, assigneeIDs []int64, reviewerIDs []int64) error { if err := issue.LoadPoster(ctx); err != nil { return err } From 3e0dceac77e4f3cee78c7596121a79bab0af49f9 Mon Sep 17 00:00:00 2001 From: splitt3r Date: Sat, 9 Sep 2023 10:28:05 +0000 Subject: [PATCH 06/26] Check if Pull + run linter --- routers/web/repo/issue.go | 38 ++++++++++++++++++++------------------ services/pull/pull.go | 2 +- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 02ee60d0fd2a1..49e93f7de3825 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1219,28 +1219,30 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull // Check reviewers var reviewerIDs []int64 - if len(form.ReviewerIDs) > 0 { - reviewerIDs, err = base.StringsToInt64s(strings.Split(form.ReviewerIDs, ",")) - if err != nil { - return nil, nil, nil, 0, 0 - } + if isPull { + if len(form.ReviewerIDs) > 0 { + reviewerIDs, err = base.StringsToInt64s(strings.Split(form.ReviewerIDs, ",")) + if err != nil { + return nil, nil, nil, 0, 0 + } - // Check if the passed reviewers (user/team) actually exist - for _, rID := range reviewerIDs { - // negative reviewIDs represent team requests - if rID < 0 { - _, err := organization.GetTeamByID(ctx, -rID) + // Check if the passed reviewers (user/team) actually exist + for _, rID := range reviewerIDs { + // negative reviewIDs represent team requests + if rID < 0 { + _, err := organization.GetTeamByID(ctx, -rID) + if err != nil { + ctx.ServerError("GetTeamByID", err) + return nil, nil, nil, 0, 0 + } + continue + } + + _, err := user_model.GetUserByID(ctx, rID) if err != nil { - ctx.ServerError("GetTeamByID", err) + ctx.ServerError("GetUserByID", err) return nil, nil, nil, 0, 0 } - continue - } - - _, err := user_model.GetUserByID(ctx, rID) - if err != nil { - ctx.ServerError("GetUserByID", err) - return nil, nil, nil, 0, 0 } } } diff --git a/services/pull/pull.go b/services/pull/pull.go index 4a267d39acd12..db2c23a2fe2b0 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -43,7 +43,7 @@ func getPullWorkingLockKey(prID int64) string { } // NewPullRequest creates new pull request with labels for repository. -func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *issues_model.Issue, labelIDs []int64, uuids []string, pr *issues_model.PullRequest, assigneeIDs []int64, reviewerIDs []int64) error { +func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *issues_model.Issue, labelIDs []int64, uuids []string, pr *issues_model.PullRequest, assigneeIDs, reviewerIDs []int64) error { if err := issue.LoadPoster(ctx); err != nil { return err } From 978c465dd2f03b42db4d2fd6393c643797ab8430 Mon Sep 17 00:00:00 2001 From: Sebastian Sauer Date: Fri, 20 Oct 2023 22:04:40 +0200 Subject: [PATCH 07/26] Fix invisible class name --- templates/repo/issue/new_form.tmpl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index 070e9e6761bfa..6ac7c16088704 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -64,7 +64,7 @@ {{range .Reviewers}} {{if .User}} - + {{svg "octicon-check"}} {{ctx.AvatarUtils.Avatar .User 28 "gt-mr-3"}}{{template "repo/search_name" .User}} @@ -75,7 +75,7 @@ {{range .Reviewers}} {{if .Team}} - + {{svg "octicon-check" 16}} {{svg "octicon-people" 16 "gt-ml-4 gt-mr-2"}}{{$.Repository.OwnerName}}/{{.Team.Name}} From c92202e36317d9f55618b5558414e1bdac8aa63b Mon Sep 17 00:00:00 2001 From: Sebastian Sauer Date: Fri, 20 Oct 2023 22:17:45 +0200 Subject: [PATCH 08/26] Add request review to API --- modules/structs/pull.go | 4 +++- routers/api/v1/repo/pull.go | 41 +++++++++++++++++++++++++++++++++- templates/swagger/v1_json.tmpl | 14 ++++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/modules/structs/pull.go b/modules/structs/pull.go index ab627666c94af..55831e642c1a0 100644 --- a/modules/structs/pull.go +++ b/modules/structs/pull.go @@ -86,7 +86,9 @@ type CreatePullRequestOption struct { Milestone int64 `json:"milestone"` Labels []int64 `json:"labels"` // swagger:strfmt date-time - Deadline *time.Time `json:"due_date"` + Deadline *time.Time `json:"due_date"` + Reviewers []string `json:"reviewers"` + TeamReviewers []string `json:"team_reviewers"` } // EditPullRequestOption options when modify pull request diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 0f98507a7eb53..0732703f12313 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -16,6 +16,7 @@ import ( activities_model "code.gitea.io/gitea/models/activities" git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" + "code.gitea.io/gitea/models/organization" access_model "code.gitea.io/gitea/models/perm/access" pull_model "code.gitea.io/gitea/models/pull" repo_model "code.gitea.io/gitea/models/repo" @@ -553,8 +554,46 @@ func CreatePullRequest(ctx *context.APIContext) { return } } + // handle reviewers + var reviewerIds []int64 - if err := pull_service.NewPullRequest(ctx, repo, prIssue, labelIDs, []string{}, pr, assigneeIDs, []int64{}); err != nil { + for _, r := range form.Reviewers { + var reviewer *user_model.User + if strings.Contains(r, "@") { + reviewer, err = user_model.GetUserByEmail(ctx, r) + } else { + reviewer, err = user_model.GetUserByName(ctx, r) + } + + if err != nil { + if user_model.IsErrUserNotExist(err) { + ctx.NotFound("UserNotExist", fmt.Sprintf("User with id '%s' not exist", r)) + return + } + ctx.Error(http.StatusInternalServerError, "GetUser", err) + return + } + reviewerIds = append(reviewerIds, reviewer.ID) + } + + // handle teams as reviewers + if ctx.Repo.Repository.Owner.IsOrganization() && len(form.TeamReviewers) > 0 { + for _, t := range form.TeamReviewers { + var teamReviewer *organization.Team + teamReviewer, err = organization.GetTeam(ctx, ctx.Repo.Owner.ID, t) + if err != nil { + if organization.IsErrTeamNotExist(err) { + ctx.NotFound("TeamNotExist", fmt.Sprintf("Team '%s' not exist", t)) + return + } + ctx.Error(http.StatusInternalServerError, "ReviewRequest", err) + return + } + reviewerIds = append(reviewerIds, teamReviewer.ID) + } + } + + if err := pull_service.NewPullRequest(ctx, repo, prIssue, labelIDs, []string{}, pr, assigneeIDs, reviewerIds); err != nil { if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) { ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err) } else if errors.Is(err, user_model.ErrBlockedUser) { diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 4cbf511aacbe8..0c898eb672d9c 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -20086,6 +20086,20 @@ "format": "int64", "x-go-name": "Milestone" }, + "reviewers": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "Reviewers" + }, + "team_reviewers": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "TeamReviewers" + }, "title": { "type": "string", "x-go-name": "Title" From df3f36394a5f2a6f86e447e3a8a7f6fc99845f47 Mon Sep 17 00:00:00 2001 From: Sebastian Sauer Date: Wed, 15 Nov 2023 21:32:15 +0100 Subject: [PATCH 09/26] Fix call to locale in tmpl --- templates/repo/issue/new_form.tmpl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index 6ac7c16088704..adbe9e1e9e8ed 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -52,15 +52,15 @@
- {{.locale.Tr "repo.issues.review.reviewers"}} + {{ctx.Locale.Tr "repo.issues.review.reviewers"}} {{svg "octicon-gear" 16 "gt-ml-2"}}
{{svg "octicon-search" 16}} - +
-
{{.locale.Tr "repo.issues.new.clear_reviewers"}}
+
{{ctx.Locale.Tr "repo.issues.new.clear_reviewers"}}
{{range .Reviewers}} {{if .User}} @@ -86,7 +86,7 @@
- {{.locale.Tr "repo.issues.new.no_reviewers"}} + {{ctx.Locale.Tr "repo.issues.new.no_reviewers"}}
{{range .Reviewers}} From e4858b29327e1782e2272d7ff1df0bebb0bbe731 Mon Sep 17 00:00:00 2001 From: Sebastian Sauer Date: Wed, 15 Nov 2023 22:07:19 +0100 Subject: [PATCH 10/26] Improve markup --- templates/repo/issue/new_form.tmpl | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index adbe9e1e9e8ed..80714e8db7dca 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -90,15 +90,11 @@
{{range .Reviewers}} - {{if .User}} - - {{ctx.AvatarUtils.Avatar .User 28 "gt-mr-3 gt-vm"}}{{.User.GetDisplayName}} - - {{else if .Team}} - - {{svg "octicon-people" 20 "gt-mr-3"}}{{$.Repository.OwnerName}}/{{.Team.Name}} - - {{end}} + {{if .User}} + {{ctx.AvatarUtils.Avatar .User 20 "gt-mr-3"}}{{.User.GetDisplayName}} + {{else if .Team}} + {{svg "octicon-people" 20 "gt-mr-3"}}{{$.Repository.OwnerName}}/{{.Team.Name}} + {{end}} {{end}}
From f604ab1614d792cd1011b5f46f88477e01937d95 Mon Sep 17 00:00:00 2001 From: Sebastian Sauer Date: Wed, 15 Nov 2023 22:16:12 +0100 Subject: [PATCH 11/26] Fix teamreviewer ids this needs to be a negative integer --- routers/api/v1/repo/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 0732703f12313..7b71ce3105850 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -589,7 +589,7 @@ func CreatePullRequest(ctx *context.APIContext) { ctx.Error(http.StatusInternalServerError, "ReviewRequest", err) return } - reviewerIds = append(reviewerIds, teamReviewer.ID) + reviewerIds = append(reviewerIds, -teamReviewer.ID) } } From 60d9d9637399474baac167c9c1a0cc1b0c20efb5 Mon Sep 17 00:00:00 2001 From: bb-ben <70356237+bboerben@users.noreply.github.com> Date: Thu, 31 Oct 2024 22:11:03 +0800 Subject: [PATCH 12/26] update template to merge main --- templates/repo/issue/new_form.tmpl | 14 +++++++++----- templates/repo/issue/view_content/sidebar.tmpl | 2 +- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index 80714e8db7dca..fe856807a5733 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -63,10 +63,10 @@
{{ctx.Locale.Tr "repo.issues.new.clear_reviewers"}}
{{range .Reviewers}} {{if .User}} - - {{svg "octicon-check"}} + + {{svg "octicon-check"}} - {{ctx.AvatarUtils.Avatar .User 28 "gt-mr-3"}}{{template "repo/search_name" .User}} + {{ctx.AvatarUtils.Avatar .User 28 "tw-mr-2"}}{{template "repo/search_name" .User}} {{end}} @@ -91,9 +91,13 @@
{{range .Reviewers}} {{if .User}} - {{ctx.AvatarUtils.Avatar .User 20 "gt-mr-3"}}{{.User.GetDisplayName}} + + {{ctx.AvatarUtils.Avatar .User 28 "tw-mr-2 tw-align-middle"}}{{.User.GetDisplayName}} + {{else if .Team}} - {{svg "octicon-people" 20 "gt-mr-3"}}{{$.Repository.OwnerName}}/{{.Team.Name}} + + {{ctx.AvatarUtils.Avatar .Team 28 "tw-mr-2 tw-align-middle"}}{{$.Repository.OwnerName}}/{{.Team.Name}} + {{end}} {{end}}
diff --git a/templates/repo/issue/view_content/sidebar.tmpl b/templates/repo/issue/view_content/sidebar.tmpl index e49e90df56c9f..8cb9c80e5a4b4 100644 --- a/templates/repo/issue/view_content/sidebar.tmpl +++ b/templates/repo/issue/view_content/sidebar.tmpl @@ -53,7 +53,7 @@
{{if .User}} - {{ctx.AvatarUtils.Avatar .User 20 "tw-mr-2"}}{{.User.GetDisplayName}} + {{ctx.AvatarUtils.Avatar .User 28 "tw-mr-2"}}{{.User.GetDisplayName}} {{else if .Team}} {{svg "octicon-people" 20 "tw-mr-2"}}{{$.Issue.Repo.OwnerName}}/{{.Team.Name}} {{end}} From f7c0ea459e515622810962aeb27b2654c57aa6f4 Mon Sep 17 00:00:00 2001 From: bb-ben <70356237+bboerben@users.noreply.github.com> Date: Sat, 2 Nov 2024 15:06:42 +0800 Subject: [PATCH 13/26] move logics to proper file logics for fetching reviewers in pull requests is now placed in the RetrieveRepoMetas --- routers/web/repo/compare.go | 43 ------------------------------- routers/web/repo/issue.go | 50 ++++++++++++++++++++++++++++++++++--- 2 files changed, 46 insertions(+), 47 deletions(-) diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 70d7d37d6599f..e6a04782e5bbf 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -19,7 +19,6 @@ import ( "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" - "code.gitea.io/gitea/models/organization" access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" @@ -40,7 +39,6 @@ import ( "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/context/upload" "code.gitea.io/gitea/services/gitdiff" - repo_service "code.gitea.io/gitea/services/repository" ) const ( @@ -791,47 +789,6 @@ func CompareDiff(ctx *context.Context) { if ctx.Written() { return } - - // Get reviewer info for pr - var ( - reviewers []*user_model.User - teamReviewers []*organization.Team - reviewersResult []*repoReviewerSelection - ) - reviewers, err = repo_model.GetReviewers(ctx, ctx.Repo.Repository, ctx.Doer.ID, ctx.Doer.ID) - if err != nil { - ctx.ServerError("GetReviewers", err) - return - } - - teamReviewers, err = repo_service.GetReviewerTeams(ctx, ctx.Repo.Repository) - if err != nil { - ctx.ServerError("GetReviewerTeams", err) - return - } - - for _, user := range reviewers { - reviewersResult = append(reviewersResult, &repoReviewerSelection{ - IsTeam: false, - CanChange: true, - User: user, - ItemID: user.ID, - }) - } - - // negative reviewIDs represent team requests - for _, team := range teamReviewers { - reviewersResult = append(reviewersResult, &repoReviewerSelection{ - IsTeam: true, - CanChange: true, - Team: team, - ItemID: -team.ID, - }) - } - ctx.Data["Reviewers"] = reviewersResult - if ctx.Written() { - return - } } } beforeCommitID := ctx.Data["BeforeCommitID"].(string) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 49e93f7de3825..9633ab9890086 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -557,8 +557,8 @@ func renderMilestones(ctx *context.Context) { ctx.Data["ClosedMilestones"] = closedMilestones } -// RetrieveRepoMilestonesAndAssignees find all the milestones and assignees of a repository -func RetrieveRepoMilestonesAndAssignees(ctx *context.Context, repo *repo_model.Repository) { +// RetrieveRepoMilestonesAndAssignees find all the milestones, assignees, and reviewers of a repository +func RetrieveRepoMilestonesAssigneesAndReviewers(ctx *context.Context, repo *repo_model.Repository) { var err error ctx.Data["OpenMilestones"], err = db.Find[issues_model.Milestone](ctx, issues_model.FindMilestoneOptions{ RepoID: repo.ID, @@ -585,6 +585,8 @@ func RetrieveRepoMilestonesAndAssignees(ctx *context.Context, repo *repo_model.R ctx.Data["Assignees"] = shared_user.MakeSelfOnTop(ctx.Doer, assigneeUsers) handleTeamMentions(ctx) + + retrieveReviewers(ctx) } func retrieveProjects(ctx *context.Context, repo *repo_model.Repository) { @@ -862,7 +864,7 @@ func RetrieveRepoMetas(ctx *context.Context, repo *repo_model.Repository, isPull labels = append(labels, orgLabels...) } - RetrieveRepoMilestonesAndAssignees(ctx, repo) + RetrieveRepoMilestonesAssigneesAndReviewers(ctx, repo) if ctx.Written() { return nil } @@ -883,6 +885,46 @@ func RetrieveRepoMetas(ctx *context.Context, repo *repo_model.Repository, isPull return labels } +func retrieveReviewers(ctx *context.Context) { + // Get reviewer info for pr + var ( + reviewers []*user_model.User + teamReviewers []*organization.Team + reviewersResult []*repoReviewerSelection + ) + reviewers, err := repo_model.GetReviewers(ctx, ctx.Repo.Repository, ctx.Doer.ID, ctx.Doer.ID) + if err != nil { + ctx.ServerError("GetReviewers", err) + return + } + + teamReviewers, err = repo_service.GetReviewerTeams(ctx, ctx.Repo.Repository) + if err != nil { + ctx.ServerError("GetReviewerTeams", err) + return + } + + for _, user := range reviewers { + reviewersResult = append(reviewersResult, &repoReviewerSelection{ + IsTeam: false, + CanChange: true, + User: user, + ItemID: user.ID, + }) + } + + // negative reviewIDs represent team requests + for _, team := range teamReviewers { + reviewersResult = append(reviewersResult, &repoReviewerSelection{ + IsTeam: true, + CanChange: true, + Team: team, + ItemID: -team.ID, + }) + } + ctx.Data["Reviewers"] = reviewersResult +} + // Tries to load and set an issue template. The first return value indicates if a template was loaded. func setTemplateIfExists(ctx *context.Context, ctxDataKey string, possibleFiles []string) (bool, map[string]error) { commit, err := ctx.Repo.GitRepo.GetBranchCommit(ctx.Repo.Repository.DefaultBranch) @@ -1573,7 +1615,7 @@ func ViewIssue(ctx *context.Context) { // Check milestone and assignee. if ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) { - RetrieveRepoMilestonesAndAssignees(ctx, repo) + RetrieveRepoMilestonesAssigneesAndReviewers(ctx, repo) retrieveProjects(ctx, repo) if ctx.Written() { From c34dd4a8e606369f12838117f7201fabe2ca8d8b Mon Sep 17 00:00:00 2001 From: bb-ben <70356237+bboerben@users.noreply.github.com> Date: Sat, 2 Nov 2024 15:08:35 +0800 Subject: [PATCH 14/26] postpone the review request notification the review requet notifaction is now sent after the new pull request notification. --- services/pull/pull.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index db2c23a2fe2b0..7e7006d3055b3 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -147,10 +147,6 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *iss if err != nil { return err } - _, err = issue_service.ReviewRequest(ctx, issue, issue.Poster, reviewer, true) - if err != nil { - return err - } } pr.Issue = issue @@ -235,6 +231,17 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *iss notify_service.IssueChangeAssignee(ctx, issue.Poster, issue, assignee, false, assigneeCommentMap[assigneeID]) } + for _, reviewerID := range reviewerIDs { + reviewer, err := user_model.GetUserByID(ctx, reviewerID) + if err != nil { + return ErrDependenciesLeft + } + _, err = issue_service.ReviewRequest(ctx, issue, issue.Poster, reviewer, true) + if err != nil { + return err + } + } + return nil } From 03043248142d58448a9bc47d2e770beb724b040b Mon Sep 17 00:00:00 2001 From: bb-ben <70356237+bboerben@users.noreply.github.com> Date: Sat, 2 Nov 2024 16:24:35 +0800 Subject: [PATCH 15/26] fix linting issues --- routers/api/v1/repo/pull.go | 8 +- web_src/js/features/repo-legacy.js | 600 ----------------------------- web_src/js/features/repo-legacy.ts | 6 +- 3 files changed, 9 insertions(+), 605 deletions(-) delete mode 100644 web_src/js/features/repo-legacy.js diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 7b71ce3105850..f81e9cff24a14 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -555,7 +555,7 @@ func CreatePullRequest(ctx *context.APIContext) { } } // handle reviewers - var reviewerIds []int64 + var reviewerIDs []int64 for _, r := range form.Reviewers { var reviewer *user_model.User @@ -573,7 +573,7 @@ func CreatePullRequest(ctx *context.APIContext) { ctx.Error(http.StatusInternalServerError, "GetUser", err) return } - reviewerIds = append(reviewerIds, reviewer.ID) + reviewerIDs = append(reviewerIDs, reviewer.ID) } // handle teams as reviewers @@ -589,11 +589,11 @@ func CreatePullRequest(ctx *context.APIContext) { ctx.Error(http.StatusInternalServerError, "ReviewRequest", err) return } - reviewerIds = append(reviewerIds, -teamReviewer.ID) + reviewerIDs = append(reviewerIDs, -teamReviewer.ID) } } - if err := pull_service.NewPullRequest(ctx, repo, prIssue, labelIDs, []string{}, pr, assigneeIDs, reviewerIds); err != nil { + if err := pull_service.NewPullRequest(ctx, repo, prIssue, labelIDs, []string{}, pr, assigneeIDs, reviewerIDs); err != nil { if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) { ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err) } else if errors.Is(err, user_model.ErrBlockedUser) { diff --git a/web_src/js/features/repo-legacy.js b/web_src/js/features/repo-legacy.js deleted file mode 100644 index 9b1307d7bcd63..0000000000000 --- a/web_src/js/features/repo-legacy.js +++ /dev/null @@ -1,600 +0,0 @@ -import $ from 'jquery'; -import { - initRepoIssueBranchSelect, initRepoIssueCodeCommentCancel, initRepoIssueCommentDelete, - initRepoIssueComments, initRepoIssueDependencyDelete, initRepoIssueReferenceIssue, - initRepoIssueTitleEdit, initRepoIssueWipToggle, - initRepoPullRequestUpdate, updateIssuesMeta, handleReply, initIssueTemplateCommentEditors, initSingleCommentEditor, -} from './repo-issue.js'; -import {initUnicodeEscapeButton} from './repo-unicode-escape.js'; -import {svg} from '../svg.js'; -import {htmlEscape} from 'escape-goat'; -import {initRepoBranchTagSelector} from '../components/RepoBranchTagSelector.vue'; -import { - initRepoCloneLink, initRepoCommonBranchOrTagDropdown, initRepoCommonFilterSearchDropdown, - initRepoCommonLanguageStats, -} from './repo-common.js'; -import {initCitationFileCopyContent} from './citation.js'; -import {initCompLabelEdit} from './comp/LabelEdit.js'; -import {initRepoDiffConversationNav} from './repo-diff.js'; -import {createDropzone} from './dropzone.js'; -import {initCommentContent, initMarkupContent} from '../markup/content.js'; -import {initCompReactionSelector} from './comp/ReactionSelector.js'; -import {initRepoSettingBranches} from './repo-settings.js'; -import {initRepoPullRequestMergeForm} from './repo-issue-pr-form.js'; -import {hideElem, showElem} from '../utils/dom.js'; -import {getComboMarkdownEditor, initComboMarkdownEditor} from './comp/ComboMarkdownEditor.js'; -import {attachRefIssueContextPopup} from './contextpopup.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(.gt-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 === 0) { - return; - } - - if ($commentForm.find('.field.combo-editor-dropzone').length) { - // at the moment, if a form has multiple combo-markdown-editors, it must be a issue template form - initIssueTemplateCommentEditors($commentForm); - } else { - initSingleCommentEditor($commentForm); - } - - function initBranchSelector() { - const $selectBranch = $('.ui.select-branch'); - const $branchMenu = $selectBranch.find('.reference-list-menu'); - const $isNewIssue = $branchMenu.hasClass('new-issue'); - $branchMenu.find('.item:not(.no-select)').on('click', function () { - const selectedValue = $(this).data('id'); - const editMode = $('#editing_mode').val(); - $($(this).data('id-selector')).val(selectedValue); - if ($isNewIssue) { - $selectBranch.find('.ui .branch-name').text($(this).data('name')); - return; - } - - if (editMode === 'true') { - const form = $('#update_issueref_form'); - $.post(form.attr('action'), {_csrf: csrfToken, ref: selectedValue}, () => window.location.reload()); - } else if (editMode === '') { - $selectBranch.find('.ui .branch-name').text(selectedValue); - } - }); - $selectBranch.find('.reference.column').on('click', function () { - hideElem($selectBranch.find('.scrolling.reference-list-menu')); - $selectBranch.find('.reference .text').removeClass('black'); - showElem($($(this).data('target'))); - $(this).find('.text').addClass('black'); - return false; - }); - } - - initBranchSelector(); - - // List submits - function initListSubmits(selector, outerSelector) { - const $list = $(`.ui.${outerSelector}.list`); - const $noSelect = $list.find('.no-select'); - const $listMenu = $(`.${selector} .menu`); - let hasUpdateAction = $listMenu.data('action') === 'update'; - const items = {}; - - $(`.${selector}`).dropdown({ - 'action': 'nothing', // do not hide the menu if user presses Enter - fullTextSearch: 'exact', - async onHide() { - hasUpdateAction = $listMenu.data('action') === 'update'; // Update the var - if (hasUpdateAction) { - // TODO: Add batch functionality and make this 1 network request. - const itemEntries = Object.entries(items); - for (const [elementId, item] of itemEntries) { - await updateIssuesMeta( - item['update-url'], - item.action, - item['issue-id'], - elementId, - ); - } - if (itemEntries.length) { - reloadConfirmDraftComment(); - } - } - }, - }); - - $listMenu.find('.item:not(.no-select)').on('click', function (e) { - e.preventDefault(); - if ($(this).hasClass('ban-change')) { - return false; - } - - hasUpdateAction = $listMenu.data('action') === 'update'; // Update the var - - const clickedItem = $(this); - const scope = $(this).attr('data-scope'); - - $(this).parent().find('.item').each(function () { - if (scope) { - // Enable only clicked item for scoped labels - if ($(this).attr('data-scope') !== scope) { - return true; - } - if (!$(this).is(clickedItem) && !$(this).hasClass('checked')) { - return true; - } - } else if (!$(this).is(clickedItem)) { - // Toggle for other labels - return true; - } - - if ($(this).hasClass('checked')) { - $(this).removeClass('checked'); - $(this).find('.octicon-check').addClass('invisible'); - if (hasUpdateAction) { - if (!($(this).data('id') in items)) { - items[$(this).data('id')] = { - 'update-url': $listMenu.data('update-url'), - action: 'detach', - 'issue-id': $listMenu.data('issue-id'), - }; - } else { - delete items[$(this).data('id')]; - } - } - } else { - $(this).addClass('checked'); - $(this).find('.octicon-check').removeClass('invisible'); - if (hasUpdateAction) { - if (!($(this).data('id') in items)) { - items[$(this).data('id')] = { - 'update-url': $listMenu.data('update-url'), - action: 'attach', - 'issue-id': $listMenu.data('issue-id'), - }; - } else { - delete items[$(this).data('id')]; - } - } - } - }); - - // TODO: Which thing should be done for choosing review requests - // to make chosen items be shown on time here? - if (selector === 'select-reviewers-modify' || selector === 'select-assignees-modify') { - return false; - } - - const listIds = []; - $(this).parent().find('.item').each(function () { - if ($(this).hasClass('checked')) { - listIds.push($(this).data('id')); - $($(this).data('id-selector')).removeClass('gt-hidden'); - } else { - $($(this).data('id-selector')).addClass('gt-hidden'); - } - }); - if (listIds.length === 0) { - $noSelect.removeClass('gt-hidden'); - } else { - $noSelect.addClass('gt-hidden'); - } - $($(this).parent().data('id')).val(listIds.join(',')); - return false; - }); - $listMenu.find('.no-select.item').on('click', function (e) { - e.preventDefault(); - if (hasUpdateAction) { - updateIssuesMeta( - $listMenu.data('update-url'), - 'clear', - $listMenu.data('issue-id'), - '', - ).then(reloadConfirmDraftComment); - } - - $(this).parent().find('.item').each(function () { - $(this).removeClass('checked'); - $(this).find('.octicon-check').addClass('invisible'); - }); - - if (selector === 'select-reviewers-modify' || selector === 'select-assignees-modify') { - return false; - } - - $list.find('.item').each(function () { - $(this).addClass('gt-hidden'); - }); - $noSelect.removeClass('gt-hidden'); - $($(this).parent().data('id')).val(''); - }); - } - - // Init labels and assignees - initListSubmits('select-label', 'labels'); - initListSubmits('select-assignees', 'assignees'); - initListSubmits('select-assignees-modify', 'assignees'); - initListSubmits('select-reviewers', 'reviewers'); - initListSubmits('select-reviewers-modify', 'reviewers'); - - function selectItem(select_id, input_id) { - const $menu = $(`${select_id} .menu`); - const $list = $(`.ui${select_id}.list`); - const hasUpdateAction = $menu.data('action') === 'update'; - - $menu.find('.item:not(.no-select)').on('click', function () { - $(this).parent().find('.item').each(function () { - $(this).removeClass('selected active'); - }); - - $(this).addClass('selected active'); - if (hasUpdateAction) { - updateIssuesMeta( - $menu.data('update-url'), - '', - $menu.data('issue-id'), - $(this).data('id'), - ).then(reloadConfirmDraftComment); - } - - let icon = ''; - if (input_id === '#milestone_id') { - icon = svg('octicon-milestone', 18, 'gt-mr-3'); - } else if (input_id === '#project_id') { - icon = svg('octicon-project', 18, 'gt-mr-3'); - } else if (input_id === '#assignee_id') { - icon = `avatar`; - } else if (input_id === '#reviewer_id') { - icon = `avatar`; - } - - $list.find('.selected').html(` - - ${icon} - ${htmlEscape($(this).text())} - - `); - - $(`.ui${select_id}.list .no-select`).addClass('gt-hidden'); - $(input_id).val($(this).data('id')); - }); - $menu.find('.no-select.item').on('click', function () { - $(this).parent().find('.item:not(.no-select)').each(function () { - $(this).removeClass('selected active'); - }); - - if (hasUpdateAction) { - updateIssuesMeta( - $menu.data('update-url'), - '', - $menu.data('issue-id'), - $(this).data('id'), - ).then(reloadConfirmDraftComment); - } - - $list.find('.selected').html(''); - $list.find('.no-select').removeClass('gt-hidden'); - $(input_id).val(''); - }); - } - - // Milestone, Assignee, Project - selectItem('.select-project', '#project_id'); - selectItem('.select-milestone', '#milestone_id'); - selectItem('.select-assignee', '#assignee_id'); - selectItem('.select-reviewer', '#reviewer_id'); -} - - -async function onEditContent(event) { - event.preventDefault(); - - const $segment = $(this).closest('.header').next(); - const $editContentZone = $segment.find('.edit-content-zone'); - const $renderContent = $segment.find('.render-content'); - const $rawContent = $segment.find('.raw-content'); - - let comboMarkdownEditor; - - const setupDropzone = async ($dropzone) => { - if ($dropzone.length === 0) return null; - - let disableRemovedfileEvent = false; // when resetting the dropzone (removeAllFiles), disable the "removedfile" event - let fileUuidDict = {}; // to record: if a comment has been saved, then the uploaded files won't be deleted from server when clicking the Remove in the dropzone - const dz = await createDropzone($dropzone[0], { - url: $dropzone.attr('data-upload-url'), - headers: {'X-Csrf-Token': csrfToken}, - maxFiles: $dropzone.attr('data-max-file'), - maxFilesize: $dropzone.attr('data-max-size'), - acceptedFiles: (['*/*', ''].includes($dropzone.attr('data-accepts'))) ? null : $dropzone.attr('data-accepts'), - addRemoveLinks: true, - dictDefaultMessage: $dropzone.attr('data-default-message'), - dictInvalidFileType: $dropzone.attr('data-invalid-input-type'), - dictFileTooBig: $dropzone.attr('data-file-too-big'), - dictRemoveFile: $dropzone.attr('data-remove-file'), - timeout: 0, - thumbnailMethod: 'contain', - thumbnailWidth: 480, - thumbnailHeight: 480, - init() { - this.on('success', (file, data) => { - file.uuid = data.uuid; - fileUuidDict[file.uuid] = {submitted: false}; - const input = $(``).val(data.uuid); - $dropzone.find('.files').append(input); - }); - this.on('removedfile', (file) => { - if (disableRemovedfileEvent) return; - $(`#${file.uuid}`).remove(); - if ($dropzone.attr('data-remove-url') && !fileUuidDict[file.uuid].submitted) { - $.post($dropzone.attr('data-remove-url'), { - file: file.uuid, - _csrf: csrfToken, - }); - } - }); - this.on('submit', () => { - $.each(fileUuidDict, (fileUuid) => { - fileUuidDict[fileUuid].submitted = true; - }); - }); - this.on('reload', () => { - $.getJSON($editContentZone.attr('data-attachment-url'), (data) => { - // do not trigger the "removedfile" event, otherwise the attachments would be deleted from server - disableRemovedfileEvent = true; - dz.removeAllFiles(true); - $dropzone.find('.files').empty(); - fileUuidDict = {}; - disableRemovedfileEvent = false; - - for (const attachment of data) { - const imgSrc = `${$dropzone.attr('data-link-url')}/${attachment.uuid}`; - dz.emit('addedfile', attachment); - dz.emit('thumbnail', attachment, imgSrc); - dz.emit('complete', attachment); - dz.files.push(attachment); - fileUuidDict[attachment.uuid] = {submitted: true}; - $dropzone.find(`img[src='${imgSrc}']`).css('max-width', '100%'); - const input = $(``).val(attachment.uuid); - $dropzone.find('.files').append(input); - } - }); - }); - }, - }); - dz.emit('reload'); - return dz; - }; - - const cancelAndReset = (dz) => { - showElem($renderContent); - hideElem($editContentZone); - if (dz) { - dz.emit('reload'); - } - }; - - const saveAndRefresh = (dz, $dropzone) => { - showElem($renderContent); - hideElem($editContentZone); - const $attachments = $dropzone.find('.files').find('[name=files]').map(function () { - return $(this).val(); - }).get(); - $.post($editContentZone.attr('data-update-url'), { - _csrf: csrfToken, - content: comboMarkdownEditor.value(), - context: $editContentZone.attr('data-context'), - files: $attachments, - }, (data) => { - if (!data.content) { - $renderContent.html($('#no-content').html()); - $rawContent.text(''); - } else { - $renderContent.html(data.content); - $rawContent.text(comboMarkdownEditor.value()); - - const refIssues = $renderContent.find('p .ref-issue'); - attachRefIssueContextPopup(refIssues); - } - const $content = $segment; - if (!$content.find('.dropzone-attachments').length) { - if (data.attachments !== '') { - $content.append(`
`); - $content.find('.dropzone-attachments').replaceWith(data.attachments); - } - } else if (data.attachments === '') { - $content.find('.dropzone-attachments').remove(); - } else { - $content.find('.dropzone-attachments').replaceWith(data.attachments); - } - if (dz) { - dz.emit('submit'); - dz.emit('reload'); - } - initMarkupContent(); - initCommentContent(); - }); - }; - - if (!$editContentZone.html()) { - $editContentZone.html($('#issue-comment-editor-template').html()); - comboMarkdownEditor = await initComboMarkdownEditor($editContentZone.find('.combo-markdown-editor')); - - const $dropzone = $editContentZone.find('.dropzone'); - const dz = await setupDropzone($dropzone); - $editContentZone.find('.cancel.button').on('click', (e) => { - e.preventDefault(); - cancelAndReset(dz); - }); - $editContentZone.find('.save.button').on('click', (e) => { - e.preventDefault(); - saveAndRefresh(dz, $dropzone); - }); - } else { - comboMarkdownEditor = getComboMarkdownEditor($editContentZone.find('.combo-markdown-editor')); - } - - // Show write/preview tab and copy raw content as needed - showElem($editContentZone); - hideElem($renderContent); - if (!comboMarkdownEditor.value()) { - comboMarkdownEditor.value($rawContent.text()); - } - comboMarkdownEditor.focus(); -} - -export function initRepository() { - if ($('.page-content.repository').length === 0) { - return; - } - - initRepoBranchTagSelector('.js-branch-tag-selector'); - - // Options - if ($('.repository.settings.options').length > 0) { - // Enable or select internal/external wiki system and issue tracker. - $('.enable-system').on('change', function () { - if (this.checked) { - $($(this).data('target')).removeClass('disabled'); - if (!$(this).data('context')) $($(this).data('context')).addClass('disabled'); - } else { - $($(this).data('target')).addClass('disabled'); - if (!$(this).data('context')) $($(this).data('context')).removeClass('disabled'); - } - }); - $('.enable-system-radio').on('change', function () { - if (this.value === 'false') { - $($(this).data('target')).addClass('disabled'); - if ($(this).data('context') !== undefined) $($(this).data('context')).removeClass('disabled'); - } else if (this.value === 'true') { - $($(this).data('target')).removeClass('disabled'); - if ($(this).data('context') !== undefined) $($(this).data('context')).addClass('disabled'); - } - }); - const $trackerIssueStyleRadios = $('.js-tracker-issue-style'); - $trackerIssueStyleRadios.on('change input', () => { - const checkedVal = $trackerIssueStyleRadios.filter(':checked').val(); - $('#tracker-issue-style-regex-box').toggleClass('disabled', checkedVal !== 'regexp'); - }); - } - - // Labels - initCompLabelEdit('.repository.labels'); - - // Milestones - if ($('.repository.new.milestone').length > 0) { - $('#clear-date').on('click', () => { - $('#deadline').val(''); - return false; - }); - } - - // Repo Creation - if ($('.repository.new.repo').length > 0) { - $('input[name="gitignores"], input[name="license"]').on('change', () => { - const gitignores = $('input[name="gitignores"]').val(); - const license = $('input[name="license"]').val(); - if (gitignores || license) { - $('input[name="auto_init"]').prop('checked', true); - } - }); - } - - // Compare or pull request - const $repoDiff = $('.repository.diff'); - if ($repoDiff.length) { - initRepoCommonBranchOrTagDropdown('.choose.branch .dropdown'); - initRepoCommonFilterSearchDropdown('.choose.branch .dropdown'); - } - - initRepoCloneLink(); - initCitationFileCopyContent(); - initRepoCommonLanguageStats(); - initRepoSettingBranches(); - - // Issues - if ($('.repository.view.issue').length > 0) { - initRepoIssueCommentEdit(); - - initRepoIssueBranchSelect(); - initRepoIssueTitleEdit(); - initRepoIssueWipToggle(); - initRepoIssueComments(); - - initRepoDiffConversationNav(); - initRepoIssueReferenceIssue(); - - - initRepoIssueCommentDelete(); - initRepoIssueDependencyDelete(); - initRepoIssueCodeCommentCancel(); - initRepoPullRequestUpdate(); - initCompReactionSelector($(document)); - - initRepoPullRequestMergeForm(); - } - - // Pull request - const $repoComparePull = $('.repository.compare.pull'); - if ($repoComparePull.length > 0) { - // show pull request form - $repoComparePull.find('button.show-form').on('click', function (e) { - e.preventDefault(); - hideElem($(this).parent()); - - const $form = $repoComparePull.find('.pullrequest-form'); - showElem($form); - }); - } - - initUnicodeEscapeButton(); -} - -function initRepoIssueCommentEdit() { - // Edit issue or comment content - $(document).on('click', '.edit-content', onEditContent); - - // Quote reply - $(document).on('click', '.quote-reply', async function (event) { - event.preventDefault(); - const target = $(this).data('target'); - const quote = $(`#${target}`).text().replace(/\n/g, '\n> '); - const content = `> ${quote}\n\n`; - let editor; - if ($(this).hasClass('quote-reply-diff')) { - const $replyBtn = $(this).closest('.comment-code-cloud').find('button.comment-form-reply'); - editor = await handleReply($replyBtn); - } else { - // for normal issue/comment page - editor = getComboMarkdownEditor($('#comment-form .combo-markdown-editor')); - } - if (editor) { - if (editor.value()) { - editor.value(`${editor.value()}\n\n${content}`); - } else { - editor.value(content); - } - editor.focus(); - editor.moveCursorToEnd(); - } - }); -} diff --git a/web_src/js/features/repo-legacy.ts b/web_src/js/features/repo-legacy.ts index 5844037770cf3..e3fa54739523b 100644 --- a/web_src/js/features/repo-legacy.ts +++ b/web_src/js/features/repo-legacy.ts @@ -243,7 +243,8 @@ export function initRepoCommentForm() { initListSubmits('select-label', 'labels'); initListSubmits('select-assignees', 'assignees'); initListSubmits('select-assignees-modify', 'assignees'); - initListSubmits('select-reviewers-modify', 'assignees'); + initListSubmits('select-reviewers', 'reviewers'); + initListSubmits('select-reviewers-modify', 'reviewers'); function selectItem(select_id, input_id) { const $menu = $(`${select_id} .menu`); @@ -275,6 +276,8 @@ export function initRepoCommentForm() { icon = svg('octicon-project', 18, 'tw-mr-2'); } else if (input_id === '#assignee_id') { icon = `avatar`; + } else if (input_id === '#reviewer_id') { + icon = `avatar`; } $list.find('.selected').html(` @@ -314,6 +317,7 @@ export function initRepoCommentForm() { selectItem('.select-project', '#project_id'); selectItem('.select-milestone', '#milestone_id'); selectItem('.select-assignee', '#assignee_id'); + selectItem('.select-reviewer', '#reviewer_id'); } export function initRepository() { From 800232b0d422d4a3634c763a99d4da78e270ffac Mon Sep 17 00:00:00 2001 From: bb-ben <70356237+bboerben@users.noreply.github.com> Date: Sun, 3 Nov 2024 15:25:10 +0800 Subject: [PATCH 16/26] more updates on templates Some gt-* classes were replaced by the crossponding tw-* classes. --- templates/repo/issue/new_form.tmpl | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index fe856807a5733..e5283d3e3c373 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -53,11 +53,13 @@
{{ctx.Locale.Tr "repo.issues.review.reviewers"}} - {{svg "octicon-gear" 16 "gt-ml-2"}} + {{if .HasIssuesOrPullsWritePermission}} + {{svg "octicon-gear" 16 "tw-ml-1"}} + {{end}}
- {{svg "octicon-search" 16}} + {{svg "octicon-search" 16}}
{{ctx.Locale.Tr "repo.issues.new.clear_reviewers"}}
@@ -75,9 +77,9 @@ {{range .Reviewers}} {{if .Team}} - {{svg "octicon-check" 16}} + {{svg "octicon-check" 16}} - {{svg "octicon-people" 16 "gt-ml-4 gt-mr-2"}}{{$.Repository.OwnerName}}/{{.Team.Name}} + {{svg "octicon-people" 16 "tw-ml-4 tw-mr-1"}}{{$.Repository.OwnerName}}/{{.Team.Name}} {{end}} @@ -96,7 +98,7 @@ {{else if .Team}} - {{ctx.AvatarUtils.Avatar .Team 28 "tw-mr-2 tw-align-middle"}}{{$.Repository.OwnerName}}/{{.Team.Name}} + {{svg "octicon-people" 20 "tw-mr-2"}}{{$.Repository.OwnerName}}/{{.Team.Name}} {{end}} {{end}} From d8d66b5c91de2b5b6fd840718162b267edb98339 Mon Sep 17 00:00:00 2001 From: bb-ben <70356237+bboerben@users.noreply.github.com> Date: Sun, 3 Nov 2024 15:26:39 +0800 Subject: [PATCH 17/26] simplify a condition statement --- routers/web/repo/issue.go | 38 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 9633ab9890086..544e07a5be22b 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1261,30 +1261,28 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull // Check reviewers var reviewerIDs []int64 - if isPull { - if len(form.ReviewerIDs) > 0 { - reviewerIDs, err = base.StringsToInt64s(strings.Split(form.ReviewerIDs, ",")) - if err != nil { - return nil, nil, nil, 0, 0 - } - - // Check if the passed reviewers (user/team) actually exist - for _, rID := range reviewerIDs { - // negative reviewIDs represent team requests - if rID < 0 { - _, err := organization.GetTeamByID(ctx, -rID) - if err != nil { - ctx.ServerError("GetTeamByID", err) - return nil, nil, nil, 0, 0 - } - continue - } + if isPull && len(form.ReviewerIDs) > 0 { + reviewerIDs, err = base.StringsToInt64s(strings.Split(form.ReviewerIDs, ",")) + if err != nil { + return nil, nil, nil, 0, 0 + } - _, err := user_model.GetUserByID(ctx, rID) + // Check if the passed reviewers (user/team) actually exist + for _, rID := range reviewerIDs { + // negative reviewIDs represent team requests + if rID < 0 { + _, err := organization.GetTeamByID(ctx, -rID) if err != nil { - ctx.ServerError("GetUserByID", err) + ctx.ServerError("GetTeamByID", err) return nil, nil, nil, 0, 0 } + continue + } + + _, err := user_model.GetUserByID(ctx, rID) + if err != nil { + ctx.ServerError("GetUserByID", err) + return nil, nil, nil, 0, 0 } } } From 25ebf691a82ff673ab6d04c23cc1523b631ac117 Mon Sep 17 00:00:00 2001 From: bb-ben <70356237+bboerben@users.noreply.github.com> Date: Sun, 3 Nov 2024 16:46:12 +0800 Subject: [PATCH 18/26] fix an internal error Team has negative ID while Users has positive. We need to use TeamReviewRequest for teams and ReviewRequest for a individual user. Also, we have to validate the permissions before send out the notification --- services/pull/pull.go | 57 +++++++++++++++++-------------------------- 1 file changed, 23 insertions(+), 34 deletions(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index 7e7006d3055b3..3a970c24b5c7c 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -117,38 +117,6 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *iss assigneeCommentMap[assigneeID] = comment } - for _, reviewerID := range reviewerIDs { - // negative reviewIDs represent team requests - if reviewerID < 0 { - team, err := organization.GetTeamByID(ctx, -reviewerID) - if err != nil { - return err - } - err = issue_service.IsValidTeamReviewRequest(ctx, team, issue.Poster, true, issue) - if err != nil { - return err - } - _, err = issue_service.TeamReviewRequest(ctx, issue, issue.Poster, team, true) - if err != nil { - return err - } - continue - } - - reviewer, err := user_model.GetUserByID(ctx, reviewerID) - if err != nil { - return err - } - permDoer, err := access_model.GetUserRepoPermission(ctx, issue.Repo, issue.Poster) - if err != nil { - return err - } - err = issue_service.IsValidReviewRequest(ctx, reviewer, issue.Poster, true, issue, &permDoer) - if err != nil { - return err - } - } - pr.Issue = issue issue.PullRequest = pr @@ -232,11 +200,32 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *iss } for _, reviewerID := range reviewerIDs { + // negative reviewIDs represent team requests + if reviewerID < 0 { + team, err := organization.GetTeamByID(ctx, -reviewerID) + if err != nil { + return err + } + err = issue_service.IsValidTeamReviewRequest(ctx, team, issue.Poster, true, issue) + if err != nil { + return err + } + _, err = issue_service.TeamReviewRequest(ctx, issue, issue.Poster, team, true) + if err != nil { + return err + } + continue + } + reviewer, err := user_model.GetUserByID(ctx, reviewerID) if err != nil { - return ErrDependenciesLeft + return err + } + permDoer, err := access_model.GetUserRepoPermission(ctx, issue.Repo, issue.Poster) + if err != nil { + return err } - _, err = issue_service.ReviewRequest(ctx, issue, issue.Poster, reviewer, true) + err = issue_service.IsValidReviewRequest(ctx, reviewer, issue.Poster, true, issue, &permDoer) if err != nil { return err } From 941f7fdf806c706af3bcc5e3d4960cb4a0856a1f Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 3 Nov 2024 19:08:36 +0800 Subject: [PATCH 19/26] fix conflicts --- web_src/js/features/repo-issue-sidebar.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/web_src/js/features/repo-issue-sidebar.ts b/web_src/js/features/repo-issue-sidebar.ts index f33e192f291a4..4148ba26c42ca 100644 --- a/web_src/js/features/repo-issue-sidebar.ts +++ b/web_src/js/features/repo-issue-sidebar.ts @@ -223,6 +223,8 @@ function selectItem(select_id, input_id) { icon = svg('octicon-project', 18, 'tw-mr-2'); } else if (input_id === '#assignee_id') { icon = `avatar`; + } else if (input_id === '#reviewer_id') { + icon = `avatar`; } $list.find('.selected').html(` @@ -265,10 +267,12 @@ export function initRepoIssueSidebar() { initListSubmits('select-label', 'labels'); initListSubmits('select-assignees', 'assignees'); initListSubmits('select-assignees-modify', 'assignees'); + initListSubmits('select-reviewers', 'reviewers'); initListSubmits('select-reviewers-modify', 'assignees'); // Milestone, Assignee, Project selectItem('.select-project', '#project_id'); selectItem('.select-milestone', '#milestone_id'); selectItem('.select-assignee', '#assignee_id'); + selectItem('.select-reviewer', '#reviewer_id'); } From 8de6e400a346aa656df6ef191029754d5e158df6 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 8 Nov 2024 20:31:53 +0800 Subject: [PATCH 20/26] make code easier to maintain --- routers/api/v1/repo/pull.go | 10 +++++- routers/web/repo/issue.go | 39 +++++++++++++---------- routers/web/repo/pull.go | 16 ++++++++-- services/agit/agit.go | 8 +++-- services/pull/pull.go | 13 +++++++- tests/integration/actions_trigger_test.go | 6 ++-- tests/integration/pull_merge_test.go | 3 +- tests/integration/pull_update_test.go | 3 +- 8 files changed, 71 insertions(+), 27 deletions(-) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index f81e9cff24a14..0f00b33248477 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -593,7 +593,15 @@ func CreatePullRequest(ctx *context.APIContext) { } } - if err := pull_service.NewPullRequest(ctx, repo, prIssue, labelIDs, []string{}, pr, assigneeIDs, reviewerIDs); err != nil { + prOpts := &pull_service.NewPullRequestOptions{ + Repo: repo, + Issue: prIssue, + LabelIDs: labelIDs, + PullRequest: pr, + AssigneeIDs: assigneeIDs, + ReviewerIDs: reviewerIDs, + } + if err := pull_service.NewPullRequest(ctx, prOpts); err != nil { if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) { ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err) } else if errors.Is(err, user_model.ErrBlockedUser) { diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index fa20c11effc3a..44c1f62c6a7c6 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1159,7 +1159,11 @@ func DeleteIssue(ctx *context.Context) { } // ValidateRepoMetas check and returns repository's meta information -func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull bool) ([]int64, []int64, []int64, int64, int64) { +func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull bool) (ret struct { + LabelIDs, AssigneeIDs, ReviewerIDs []int64 + MilestoneID, ProjectID int64 +}, +) { var ( repo = ctx.Repo.Repository err error @@ -1167,7 +1171,7 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull labels := RetrieveRepoMetas(ctx, ctx.Repo.Repository, isPull) if ctx.Written() { - return nil, nil, nil, 0, 0 + return ret } var labelIDs []int64 @@ -1176,7 +1180,7 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull if len(form.LabelIDs) > 0 { labelIDs, err = base.StringsToInt64s(strings.Split(form.LabelIDs, ",")) if err != nil { - return nil, nil, nil, 0, 0 + return ret } labelIDMark := make(container.Set[int64]) labelIDMark.AddMultiple(labelIDs...) @@ -1199,11 +1203,11 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull milestone, err := issues_model.GetMilestoneByRepoID(ctx, ctx.Repo.Repository.ID, milestoneID) if err != nil { ctx.ServerError("GetMilestoneByID", err) - return nil, nil, nil, 0, 0 + return ret } if milestone.RepoID != repo.ID { ctx.ServerError("GetMilestoneByID", err) - return nil, nil, nil, 0, 0 + return ret } ctx.Data["Milestone"] = milestone ctx.Data["milestone_id"] = milestoneID @@ -1213,11 +1217,11 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull p, err := project_model.GetProjectByID(ctx, form.ProjectID) if err != nil { ctx.ServerError("GetProjectByID", err) - return nil, nil, nil, 0, 0 + return ret } if p.RepoID != ctx.Repo.Repository.ID && p.OwnerID != ctx.Repo.Repository.OwnerID { ctx.NotFound("", nil) - return nil, nil, nil, 0, 0 + return ret } ctx.Data["Project"] = p @@ -1229,7 +1233,7 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull if len(form.AssigneeIDs) > 0 { assigneeIDs, err = base.StringsToInt64s(strings.Split(form.AssigneeIDs, ",")) if err != nil { - return nil, nil, nil, 0, 0 + return ret } // Check if the passed assignees actually exists and is assignable @@ -1237,18 +1241,18 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull assignee, err := user_model.GetUserByID(ctx, aID) if err != nil { ctx.ServerError("GetUserByID", err) - return nil, nil, nil, 0, 0 + return ret } valid, err := access_model.CanBeAssigned(ctx, assignee, repo, isPull) if err != nil { ctx.ServerError("CanBeAssigned", err) - return nil, nil, nil, 0, 0 + return ret } if !valid { ctx.ServerError("canBeAssigned", repo_model.ErrUserDoesNotHaveAccessToRepo{UserID: aID, RepoName: repo.Name}) - return nil, nil, nil, 0, 0 + return ret } } } @@ -1263,7 +1267,7 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull if isPull && len(form.ReviewerIDs) > 0 { reviewerIDs, err = base.StringsToInt64s(strings.Split(form.ReviewerIDs, ",")) if err != nil { - return nil, nil, nil, 0, 0 + return ret } // Check if the passed reviewers (user/team) actually exist @@ -1273,7 +1277,7 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull _, err := organization.GetTeamByID(ctx, -rID) if err != nil { ctx.ServerError("GetTeamByID", err) - return nil, nil, nil, 0, 0 + return ret } continue } @@ -1281,12 +1285,13 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull _, err := user_model.GetUserByID(ctx, rID) if err != nil { ctx.ServerError("GetUserByID", err) - return nil, nil, nil, 0, 0 + return ret } } } - return labelIDs, assigneeIDs, reviewerIDs, milestoneID, form.ProjectID + ret.LabelIDs, ret.AssigneeIDs, ret.ReviewerIDs, ret.MilestoneID, ret.ProjectID = labelIDs, assigneeIDs, reviewerIDs, milestoneID, form.ProjectID + return ret } // NewIssuePost response for creating new issue @@ -1304,11 +1309,13 @@ func NewIssuePost(ctx *context.Context) { attachments []string ) - labelIDs, assigneeIDs, _, milestoneID, projectID := ValidateRepoMetas(ctx, *form, false) + validateRet := ValidateRepoMetas(ctx, *form, false) if ctx.Written() { return } + labelIDs, assigneeIDs, milestoneID, projectID := validateRet.LabelIDs, validateRet.AssigneeIDs, validateRet.MilestoneID, validateRet.ProjectID + if projectID > 0 { if !ctx.Repo.CanRead(unit.TypeProjects) { // User must also be able to see the project. diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 97abfd25351ac..9e69a5595d667 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1269,11 +1269,13 @@ func CompareAndPullRequestPost(ctx *context.Context) { return } - labelIDs, assigneeIDs, reviewerIDs, milestoneID, projectID := ValidateRepoMetas(ctx, *form, true) + validateRet := ValidateRepoMetas(ctx, *form, true) if ctx.Written() { return } + labelIDs, assigneeIDs, reviewerIDs, milestoneID, projectID := validateRet.LabelIDs, validateRet.AssigneeIDs, validateRet.ReviewerIDs, validateRet.MilestoneID, validateRet.ProjectID + if setting.Attachment.Enabled { attachments = form.Files } @@ -1318,8 +1320,16 @@ func CompareAndPullRequestPost(ctx *context.Context) { } // FIXME: check error in the case two people send pull request at almost same time, give nice error prompt // instead of 500. - - if err := pull_service.NewPullRequest(ctx, repo, pullIssue, labelIDs, attachments, pullRequest, assigneeIDs, reviewerIDs); err != nil { + prOpts := &pull_service.NewPullRequestOptions{ + Repo: repo, + Issue: pullIssue, + LabelIDs: labelIDs, + AttachmentUUIDs: attachments, + PullRequest: pullRequest, + AssigneeIDs: assigneeIDs, + ReviewerIDs: reviewerIDs, + } + if err := pull_service.NewPullRequest(ctx, prOpts); err != nil { switch { case repo_model.IsErrUserDoesNotHaveAccessToRepo(err): ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error()) diff --git a/services/agit/agit.go b/services/agit/agit.go index b4ebb2eb18296..83b12dfcdb8fd 100644 --- a/services/agit/agit.go +++ b/services/agit/agit.go @@ -137,8 +137,12 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git. Type: issues_model.PullRequestGitea, Flow: issues_model.PullRequestFlowAGit, } - - if err := pull_service.NewPullRequest(ctx, repo, prIssue, []int64{}, []string{}, pr, []int64{}, []int64{}); err != nil { + prOpts := &pull_service.NewPullRequestOptions{ + Repo: repo, + Issue: prIssue, + PullRequest: pr, + } + if err := pull_service.NewPullRequest(ctx, prOpts); err != nil { return nil, err } diff --git a/services/pull/pull.go b/services/pull/pull.go index 3a970c24b5c7c..dd0629d06c77d 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -42,8 +42,19 @@ func getPullWorkingLockKey(prID int64) string { return fmt.Sprintf("pull_working_%d", prID) } +type NewPullRequestOptions struct { + Repo *repo_model.Repository + Issue *issues_model.Issue + LabelIDs []int64 + AttachmentUUIDs []string + PullRequest *issues_model.PullRequest + AssigneeIDs []int64 + ReviewerIDs []int64 +} + // NewPullRequest creates new pull request with labels for repository. -func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *issues_model.Issue, labelIDs []int64, uuids []string, pr *issues_model.PullRequest, assigneeIDs, reviewerIDs []int64) error { +func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error { + repo, issue, labelIDs, uuids, pr, assigneeIDs, reviewerIDs := opts.Repo, opts.Issue, opts.LabelIDs, opts.AttachmentUUIDs, opts.PullRequest, opts.AssigneeIDs, opts.ReviewerIDs if err := issue.LoadPoster(ctx); err != nil { return err } diff --git a/tests/integration/actions_trigger_test.go b/tests/integration/actions_trigger_test.go index 4a542884fca2b..4718aa7e7352c 100644 --- a/tests/integration/actions_trigger_test.go +++ b/tests/integration/actions_trigger_test.go @@ -145,7 +145,8 @@ func TestPullRequestTargetEvent(t *testing.T) { BaseRepo: baseRepo, Type: issues_model.PullRequestGitea, } - err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil, nil) + prOpts := &pull_service.NewPullRequestOptions{Repo: baseRepo, Issue: pullIssue, PullRequest: pullRequest} + err = pull_service.NewPullRequest(git.DefaultContext, prOpts) assert.NoError(t, err) // load and compare ActionRun @@ -199,7 +200,8 @@ func TestPullRequestTargetEvent(t *testing.T) { BaseRepo: baseRepo, Type: issues_model.PullRequestGitea, } - err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil, nil) + prOpts = &pull_service.NewPullRequestOptions{Repo: baseRepo, Issue: pullIssue, PullRequest: pullRequest} + err = pull_service.NewPullRequest(git.DefaultContext, prOpts) assert.NoError(t, err) // the new pull request cannot trigger actions, so there is still only 1 record diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index 729d22facfb33..2351e169bc520 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -520,7 +520,8 @@ func TestConflictChecking(t *testing.T) { BaseRepo: baseRepo, Type: issues_model.PullRequestGitea, } - err = pull.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil, nil) + prOpts := &pull.NewPullRequestOptions{Repo: baseRepo, Issue: pullIssue, PullRequest: pullRequest} + err = pull.NewPullRequest(git.DefaultContext, prOpts) assert.NoError(t, err) issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "PR with conflict!"}) diff --git a/tests/integration/pull_update_test.go b/tests/integration/pull_update_test.go index c283488563a8a..dfe47c1053123 100644 --- a/tests/integration/pull_update_test.go +++ b/tests/integration/pull_update_test.go @@ -173,7 +173,8 @@ func createOutdatedPR(t *testing.T, actor, forkOrg *user_model.User) *issues_mod BaseRepo: baseRepo, Type: issues_model.PullRequestGitea, } - err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil, nil) + prOpts := &pull_service.NewPullRequestOptions{Repo: baseRepo, Issue: pullIssue, PullRequest: pullRequest} + err = pull_service.NewPullRequest(git.DefaultContext, prOpts) assert.NoError(t, err) issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "Test Pull -to-update-"}) From 6fb9583b0fd20edcbba07294fe0d4071c6eec71e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 8 Nov 2024 21:26:05 +0800 Subject: [PATCH 21/26] fix duplicate code --- routers/api/v1/repo/pull.go | 46 ++------- routers/api/v1/repo/pull_review.go | 108 +++++++++++----------- routers/web/repo/issue.go | 38 ++++---- routers/web/repo/pull.go | 5 +- services/issue/assignee.go | 25 +++-- services/mailer/mailer.go | 2 +- services/pull/pull.go | 39 ++------ tests/integration/api_pull_review_test.go | 5 +- 8 files changed, 110 insertions(+), 158 deletions(-) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 0f00b33248477..42262e08a0f4e 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -16,7 +16,6 @@ import ( activities_model "code.gitea.io/gitea/models/activities" git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" - "code.gitea.io/gitea/models/organization" access_model "code.gitea.io/gitea/models/perm/access" pull_model "code.gitea.io/gitea/models/pull" repo_model "code.gitea.io/gitea/models/repo" @@ -554,44 +553,6 @@ func CreatePullRequest(ctx *context.APIContext) { return } } - // handle reviewers - var reviewerIDs []int64 - - for _, r := range form.Reviewers { - var reviewer *user_model.User - if strings.Contains(r, "@") { - reviewer, err = user_model.GetUserByEmail(ctx, r) - } else { - reviewer, err = user_model.GetUserByName(ctx, r) - } - - if err != nil { - if user_model.IsErrUserNotExist(err) { - ctx.NotFound("UserNotExist", fmt.Sprintf("User with id '%s' not exist", r)) - return - } - ctx.Error(http.StatusInternalServerError, "GetUser", err) - return - } - reviewerIDs = append(reviewerIDs, reviewer.ID) - } - - // handle teams as reviewers - if ctx.Repo.Repository.Owner.IsOrganization() && len(form.TeamReviewers) > 0 { - for _, t := range form.TeamReviewers { - var teamReviewer *organization.Team - teamReviewer, err = organization.GetTeam(ctx, ctx.Repo.Owner.ID, t) - if err != nil { - if organization.IsErrTeamNotExist(err) { - ctx.NotFound("TeamNotExist", fmt.Sprintf("Team '%s' not exist", t)) - return - } - ctx.Error(http.StatusInternalServerError, "ReviewRequest", err) - return - } - reviewerIDs = append(reviewerIDs, -teamReviewer.ID) - } - } prOpts := &pull_service.NewPullRequestOptions{ Repo: repo, @@ -599,8 +560,13 @@ func CreatePullRequest(ctx *context.APIContext) { LabelIDs: labelIDs, PullRequest: pr, AssigneeIDs: assigneeIDs, - ReviewerIDs: reviewerIDs, } + + prOpts.Reviewers, prOpts.TeamReviewers = parseReviewersByNames(ctx, form.Reviewers, form.TeamReviewers) + if ctx.Written() { + return + } + if err := pull_service.NewPullRequest(ctx, prOpts); err != nil { if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) { ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err) diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go index 34bbaf560008f..def860eee8fc7 100644 --- a/routers/api/v1/repo/pull_review.go +++ b/routers/api/v1/repo/pull_review.go @@ -656,6 +656,47 @@ func DeleteReviewRequests(ctx *context.APIContext) { apiReviewRequest(ctx, *opts, false) } +func parseReviewersByNames(ctx *context.APIContext, reviewerNames, teamReviewerNames []string) (reviewers []*user_model.User, teamReviewers []*organization.Team) { + var err error + for _, r := range reviewerNames { + var reviewer *user_model.User + if strings.Contains(r, "@") { + reviewer, err = user_model.GetUserByEmail(ctx, r) + } else { + reviewer, err = user_model.GetUserByName(ctx, r) + } + + if err != nil { + if user_model.IsErrUserNotExist(err) { + ctx.NotFound("UserNotExist", fmt.Sprintf("User '%s' not exist", r)) + return nil, nil + } + ctx.Error(http.StatusInternalServerError, "GetUser", err) + return nil, nil + } + + reviewers = append(reviewers, reviewer) + } + + if ctx.Repo.Repository.Owner.IsOrganization() && len(teamReviewerNames) > 0 { + for _, t := range teamReviewerNames { + var teamReviewer *organization.Team + teamReviewer, err = organization.GetTeam(ctx, ctx.Repo.Owner.ID, t) + if err != nil { + if organization.IsErrTeamNotExist(err) { + ctx.NotFound("TeamNotExist", fmt.Sprintf("Team '%s' not exist", t)) + return nil, nil + } + ctx.Error(http.StatusInternalServerError, "ReviewRequest", err) + return nil, nil + } + + teamReviewers = append(teamReviewers, teamReviewer) + } + } + return reviewers, teamReviewers +} + func apiReviewRequest(ctx *context.APIContext, opts api.PullReviewRequestOptions, isAdd bool) { pr, err := issues_model.GetPullRequestByIndex(ctx, ctx.Repo.Repository.ID, ctx.PathParamInt64(":index")) if err != nil { @@ -672,42 +713,15 @@ func apiReviewRequest(ctx *context.APIContext, opts api.PullReviewRequestOptions return } - reviewers := make([]*user_model.User, 0, len(opts.Reviewers)) - permDoer, err := access_model.GetUserRepoPermission(ctx, pr.Issue.Repo, ctx.Doer) if err != nil { ctx.Error(http.StatusInternalServerError, "GetUserRepoPermission", err) return } - for _, r := range opts.Reviewers { - var reviewer *user_model.User - if strings.Contains(r, "@") { - reviewer, err = user_model.GetUserByEmail(ctx, r) - } else { - reviewer, err = user_model.GetUserByName(ctx, r) - } - - if err != nil { - if user_model.IsErrUserNotExist(err) { - ctx.NotFound("UserNotExist", fmt.Sprintf("User '%s' not exist", r)) - return - } - ctx.Error(http.StatusInternalServerError, "GetUser", err) - return - } - - err = issue_service.IsValidReviewRequest(ctx, reviewer, ctx.Doer, isAdd, pr.Issue, &permDoer) - if err != nil { - if issues_model.IsErrNotValidReviewRequest(err) { - ctx.Error(http.StatusUnprocessableEntity, "NotValidReviewRequest", err) - return - } - ctx.Error(http.StatusInternalServerError, "IsValidReviewRequest", err) - return - } - - reviewers = append(reviewers, reviewer) + reviewers, teamReviewers := parseReviewersByNames(ctx, opts.Reviewers, opts.TeamReviewers) + if ctx.Written() { + return } var reviews []*issues_model.Review @@ -716,12 +730,16 @@ func apiReviewRequest(ctx *context.APIContext, opts api.PullReviewRequestOptions } for _, reviewer := range reviewers { - comment, err := issue_service.ReviewRequest(ctx, pr.Issue, ctx.Doer, reviewer, isAdd) + comment, err := issue_service.ReviewRequest(ctx, pr.Issue, ctx.Doer, &permDoer, reviewer, isAdd) if err != nil { if issues_model.IsErrReviewRequestOnClosedPR(err) { ctx.Error(http.StatusForbidden, "", err) return } + if issues_model.IsErrNotValidReviewRequest(err) { + ctx.Error(http.StatusUnprocessableEntity, "", err) + return + } ctx.Error(http.StatusInternalServerError, "ReviewRequest", err) return } @@ -736,35 +754,17 @@ func apiReviewRequest(ctx *context.APIContext, opts api.PullReviewRequestOptions } if ctx.Repo.Repository.Owner.IsOrganization() && len(opts.TeamReviewers) > 0 { - teamReviewers := make([]*organization.Team, 0, len(opts.TeamReviewers)) - for _, t := range opts.TeamReviewers { - var teamReviewer *organization.Team - teamReviewer, err = organization.GetTeam(ctx, ctx.Repo.Owner.ID, t) + for _, teamReviewer := range teamReviewers { + comment, err := issue_service.TeamReviewRequest(ctx, pr.Issue, ctx.Doer, teamReviewer, isAdd) if err != nil { - if organization.IsErrTeamNotExist(err) { - ctx.NotFound("TeamNotExist", fmt.Sprintf("Team '%s' not exist", t)) + if issues_model.IsErrReviewRequestOnClosedPR(err) { + ctx.Error(http.StatusForbidden, "", err) return } - ctx.Error(http.StatusInternalServerError, "ReviewRequest", err) - return - } - - err = issue_service.IsValidTeamReviewRequest(ctx, teamReviewer, ctx.Doer, isAdd, pr.Issue) - if err != nil { if issues_model.IsErrNotValidReviewRequest(err) { - ctx.Error(http.StatusUnprocessableEntity, "NotValidReviewRequest", err) + ctx.Error(http.StatusUnprocessableEntity, "", err) return } - ctx.Error(http.StatusInternalServerError, "IsValidTeamReviewRequest", err) - return - } - - teamReviewers = append(teamReviewers, teamReviewer) - } - - for _, teamReviewer := range teamReviewers { - comment, err := issue_service.TeamReviewRequest(ctx, pr.Issue, ctx.Doer, teamReviewer, isAdd) - if err != nil { ctx.ServerError("TeamReviewRequest", err) return } diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 44c1f62c6a7c6..7a434ce887622 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1160,8 +1160,11 @@ func DeleteIssue(ctx *context.Context) { // ValidateRepoMetas check and returns repository's meta information func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull bool) (ret struct { - LabelIDs, AssigneeIDs, ReviewerIDs []int64 - MilestoneID, ProjectID int64 + LabelIDs, AssigneeIDs []int64 + MilestoneID, ProjectID int64 + + Reviewers []*user_model.User + TeamReviewers []*organization.Team }, ) { var ( @@ -1263,34 +1266,37 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull } // Check reviewers - var reviewerIDs []int64 + var reviewers []*user_model.User + var teamReviewers []*organization.Team if isPull && len(form.ReviewerIDs) > 0 { - reviewerIDs, err = base.StringsToInt64s(strings.Split(form.ReviewerIDs, ",")) + reviewerIDs, err := base.StringsToInt64s(strings.Split(form.ReviewerIDs, ",")) if err != nil { return ret } - // Check if the passed reviewers (user/team) actually exist for _, rID := range reviewerIDs { // negative reviewIDs represent team requests if rID < 0 { - _, err := organization.GetTeamByID(ctx, -rID) + teamReviewer, err := organization.GetTeamByID(ctx, -rID) if err != nil { ctx.ServerError("GetTeamByID", err) return ret } + teamReviewers = append(teamReviewers, teamReviewer) continue } - _, err := user_model.GetUserByID(ctx, rID) + reviewer, err := user_model.GetUserByID(ctx, rID) if err != nil { ctx.ServerError("GetUserByID", err) return ret } + reviewers = append(reviewers, reviewer) } } - ret.LabelIDs, ret.AssigneeIDs, ret.ReviewerIDs, ret.MilestoneID, ret.ProjectID = labelIDs, assigneeIDs, reviewerIDs, milestoneID, form.ProjectID + ret.LabelIDs, ret.AssigneeIDs, ret.MilestoneID, ret.ProjectID = labelIDs, assigneeIDs, milestoneID, form.ProjectID + ret.Reviewers, ret.TeamReviewers = reviewers, teamReviewers return ret } @@ -2556,7 +2562,7 @@ func UpdatePullReviewRequest(ctx *context.Context) { return } - err = issue_service.IsValidTeamReviewRequest(ctx, team, ctx.Doer, action == "attach", issue) + _, err = issue_service.TeamReviewRequest(ctx, issue, ctx.Doer, team, action == "attach") if err != nil { if issues_model.IsErrNotValidReviewRequest(err) { log.Warn( @@ -2567,12 +2573,6 @@ func UpdatePullReviewRequest(ctx *context.Context) { ctx.Status(http.StatusForbidden) return } - ctx.ServerError("IsValidTeamReviewRequest", err) - return - } - - _, err = issue_service.TeamReviewRequest(ctx, issue, ctx.Doer, team, action == "attach") - if err != nil { ctx.ServerError("TeamReviewRequest", err) return } @@ -2594,7 +2594,7 @@ func UpdatePullReviewRequest(ctx *context.Context) { return } - err = issue_service.IsValidReviewRequest(ctx, reviewer, ctx.Doer, action == "attach", issue, nil) + _, err = issue_service.ReviewRequest(ctx, issue, ctx.Doer, &ctx.Repo.Permission, reviewer, action == "attach") if err != nil { if issues_model.IsErrNotValidReviewRequest(err) { log.Warn( @@ -2605,12 +2605,6 @@ func UpdatePullReviewRequest(ctx *context.Context) { ctx.Status(http.StatusForbidden) return } - ctx.ServerError("isValidReviewRequest", err) - return - } - - _, err = issue_service.ReviewRequest(ctx, issue, ctx.Doer, reviewer, action == "attach") - if err != nil { if issues_model.IsErrReviewRequestOnClosedPR(err) { ctx.Status(http.StatusForbidden) return diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 9e69a5595d667..dd9671efbe8ad 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1274,7 +1274,7 @@ func CompareAndPullRequestPost(ctx *context.Context) { return } - labelIDs, assigneeIDs, reviewerIDs, milestoneID, projectID := validateRet.LabelIDs, validateRet.AssigneeIDs, validateRet.ReviewerIDs, validateRet.MilestoneID, validateRet.ProjectID + labelIDs, assigneeIDs, milestoneID, projectID := validateRet.LabelIDs, validateRet.AssigneeIDs, validateRet.MilestoneID, validateRet.ProjectID if setting.Attachment.Enabled { attachments = form.Files @@ -1327,7 +1327,8 @@ func CompareAndPullRequestPost(ctx *context.Context) { AttachmentUUIDs: attachments, PullRequest: pullRequest, AssigneeIDs: assigneeIDs, - ReviewerIDs: reviewerIDs, + Reviewers: validateRet.Reviewers, + TeamReviewers: validateRet.TeamReviewers, } if err := pull_service.NewPullRequest(ctx, prOpts); err != nil { switch { diff --git a/services/issue/assignee.go b/services/issue/assignee.go index a0aa5a339b1d3..6992b9c815356 100644 --- a/services/issue/assignee.go +++ b/services/issue/assignee.go @@ -61,7 +61,12 @@ func ToggleAssigneeWithNotify(ctx context.Context, issue *issues_model.Issue, do } // ReviewRequest add or remove a review request from a user for this PR, and make comment for it. -func ReviewRequest(ctx context.Context, issue *issues_model.Issue, doer, reviewer *user_model.User, isAdd bool) (comment *issues_model.Comment, err error) { +func ReviewRequest(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, permDoer *access_model.Permission, reviewer *user_model.User, isAdd bool) (comment *issues_model.Comment, err error) { + err = isValidReviewRequest(ctx, reviewer, doer, isAdd, issue, permDoer) + if err != nil { + return nil, err + } + if isAdd { comment, err = issues_model.AddReviewRequest(ctx, issue, reviewer, doer) } else { @@ -79,8 +84,8 @@ func ReviewRequest(ctx context.Context, issue *issues_model.Issue, doer, reviewe return comment, err } -// IsValidReviewRequest Check permission for ReviewRequest -func IsValidReviewRequest(ctx context.Context, reviewer, doer *user_model.User, isAdd bool, issue *issues_model.Issue, permDoer *access_model.Permission) error { +// isValidReviewRequest Check permission for ReviewRequest +func isValidReviewRequest(ctx context.Context, reviewer, doer *user_model.User, isAdd bool, issue *issues_model.Issue, permDoer *access_model.Permission) error { if reviewer.IsOrganization() { return issues_model.ErrNotValidReviewRequest{ Reason: "Organization can't be added as reviewer", @@ -109,7 +114,7 @@ func IsValidReviewRequest(ctx context.Context, reviewer, doer *user_model.User, } } - lastreview, err := issues_model.GetReviewByIssueIDAndUserID(ctx, issue.ID, reviewer.ID) + lastReview, err := issues_model.GetReviewByIssueIDAndUserID(ctx, issue.ID, reviewer.ID) if err != nil && !issues_model.IsErrReviewNotExist(err) { return err } @@ -137,7 +142,7 @@ func IsValidReviewRequest(ctx context.Context, reviewer, doer *user_model.User, return nil } - if doer.ID == issue.PosterID && issue.OriginalAuthorID == 0 && lastreview != nil && lastreview.Type != issues_model.ReviewTypeRequest { + if doer.ID == issue.PosterID && issue.OriginalAuthorID == 0 && lastReview != nil && lastReview.Type != issues_model.ReviewTypeRequest { return nil } @@ -152,7 +157,7 @@ func IsValidReviewRequest(ctx context.Context, reviewer, doer *user_model.User, return nil } - if lastreview != nil && lastreview.Type == issues_model.ReviewTypeRequest && lastreview.ReviewerID == doer.ID { + if lastReview != nil && lastReview.Type == issues_model.ReviewTypeRequest && lastReview.ReviewerID == doer.ID { return nil } @@ -163,8 +168,8 @@ func IsValidReviewRequest(ctx context.Context, reviewer, doer *user_model.User, } } -// IsValidTeamReviewRequest Check permission for ReviewRequest Team -func IsValidTeamReviewRequest(ctx context.Context, reviewer *organization.Team, doer *user_model.User, isAdd bool, issue *issues_model.Issue) error { +// isValidTeamReviewRequest Check permission for ReviewRequest Team +func isValidTeamReviewRequest(ctx context.Context, reviewer *organization.Team, doer *user_model.User, isAdd bool, issue *issues_model.Issue) error { if doer.IsOrganization() { return issues_model.ErrNotValidReviewRequest{ Reason: "Organization can't be doer to add reviewer", @@ -212,6 +217,10 @@ func IsValidTeamReviewRequest(ctx context.Context, reviewer *organization.Team, // TeamReviewRequest add or remove a review request from a team for this PR, and make comment for it. func TeamReviewRequest(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, reviewer *organization.Team, isAdd bool) (comment *issues_model.Comment, err error) { + err = isValidTeamReviewRequest(ctx, reviewer, doer, isAdd, issue) + if err != nil { + return nil, err + } if isAdd { comment, err = issues_model.AddTeamReviewRequest(ctx, issue, reviewer, doer) } else { diff --git a/services/mailer/mailer.go b/services/mailer/mailer.go index c5846e6104d5b..5cb6d035213e3 100644 --- a/services/mailer/mailer.go +++ b/services/mailer/mailer.go @@ -382,7 +382,7 @@ func (s *dummySender) Send(from string, to []string, msg io.WriterTo) error { if _, err := msg.WriteTo(&buf); err != nil { return err } - log.Info("Mail From: %s To: %v Body: %s", from, to, buf.String()) + log.Debug("Mail From: %s To: %v Body: %s", from, to, buf.String()) return nil } diff --git a/services/pull/pull.go b/services/pull/pull.go index dd0629d06c77d..3362cb97ff70f 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -49,12 +49,13 @@ type NewPullRequestOptions struct { AttachmentUUIDs []string PullRequest *issues_model.PullRequest AssigneeIDs []int64 - ReviewerIDs []int64 + Reviewers []*user_model.User + TeamReviewers []*organization.Team } // NewPullRequest creates new pull request with labels for repository. func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error { - repo, issue, labelIDs, uuids, pr, assigneeIDs, reviewerIDs := opts.Repo, opts.Issue, opts.LabelIDs, opts.AttachmentUUIDs, opts.PullRequest, opts.AssigneeIDs, opts.ReviewerIDs + repo, issue, labelIDs, uuids, pr, assigneeIDs := opts.Repo, opts.Issue, opts.LabelIDs, opts.AttachmentUUIDs, opts.PullRequest, opts.AssigneeIDs if err := issue.LoadPoster(ctx); err != nil { return err } @@ -209,39 +210,17 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error { } notify_service.IssueChangeAssignee(ctx, issue.Poster, issue, assignee, false, assigneeCommentMap[assigneeID]) } - - for _, reviewerID := range reviewerIDs { - // negative reviewIDs represent team requests - if reviewerID < 0 { - team, err := organization.GetTeamByID(ctx, -reviewerID) - if err != nil { - return err - } - err = issue_service.IsValidTeamReviewRequest(ctx, team, issue.Poster, true, issue) - if err != nil { - return err - } - _, err = issue_service.TeamReviewRequest(ctx, issue, issue.Poster, team, true) - if err != nil { - return err - } - continue - } - - reviewer, err := user_model.GetUserByID(ctx, reviewerID) - if err != nil { - return err - } - permDoer, err := access_model.GetUserRepoPermission(ctx, issue.Repo, issue.Poster) - if err != nil { + permDoer, err := access_model.GetUserRepoPermission(ctx, repo, issue.Poster) + for _, reviewer := range opts.Reviewers { + if _, err = issue_service.ReviewRequest(ctx, pr.Issue, issue.Poster, &permDoer, reviewer, true); err != nil { return err } - err = issue_service.IsValidReviewRequest(ctx, reviewer, issue.Poster, true, issue, &permDoer) - if err != nil { + } + for _, teamReviewer := range opts.TeamReviewers { + if _, err = issue_service.TeamReviewRequest(ctx, pr.Issue, issue.Poster, teamReviewer, true); err != nil { return err } } - return nil } diff --git a/tests/integration/api_pull_review_test.go b/tests/integration/api_pull_review_test.go index cadb0765c341e..ba6b62d0d7013 100644 --- a/tests/integration/api_pull_review_test.go +++ b/tests/integration/api_pull_review_test.go @@ -11,6 +11,7 @@ import ( auth_model "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/models/db" issues_model "code.gitea.io/gitea/models/issues" + access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" @@ -422,7 +423,9 @@ func TestAPIPullReviewStayDismissed(t *testing.T) { pullIssue.ID, user8.ID, 1, 1, 2, false) // user8 dismiss review - _, err = issue_service.ReviewRequest(db.DefaultContext, pullIssue, user8, user8, false) + permUser8, err := access_model.GetUserRepoPermission(db.DefaultContext, pullIssue.Repo, user8) + assert.NoError(t, err) + _, err = issue_service.ReviewRequest(db.DefaultContext, pullIssue, user8, &permUser8, user8, false) assert.NoError(t, err) reviewsCountCheck(t, From e4e2de3badac12371b3d7097bc14cd19320feaa9 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 9 Nov 2024 01:45:47 +0800 Subject: [PATCH 22/26] refactor reviewer list --- routers/web/repo/compare.go | 1 + routers/web/repo/issue.go | 149 +++++++-------- services/issue/assignee.go | 3 + templates/repo/issue/new_form.tmpl | 58 +----- .../repo/issue/sidebar/reviewer_list.tmpl | 172 +++++++++--------- templates/repo/issue/sidebar/wip_switch.tmpl | 2 +- .../repo/issue/view_content/sidebar.tmpl | 2 +- web_src/css/base.css | 1 + web_src/css/repo.css | 8 + .../features/repo-issue-sidebar-combolist.ts | 89 +++++++++ web_src/js/features/repo-issue-sidebar.ts | 43 ++--- web_src/js/features/repo-issue.ts | 11 -- 12 files changed, 277 insertions(+), 262 deletions(-) create mode 100644 web_src/js/features/repo-issue-sidebar-combolist.ts diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 39279728375f0..77cbd4adfba3c 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -789,6 +789,7 @@ func CompareDiff(ctx *context.Context) { if !nothingToCompare { // Setup information for new form. RetrieveRepoMetas(ctx, ctx.Repo.Repository, true) + RetrieveRepoReviewers(ctx, ctx.Repo.Repository, nil, true) if ctx.Written() { return } diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 7a434ce887622..9c7a82fb0ccb4 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -557,7 +557,7 @@ func renderMilestones(ctx *context.Context) { } // RetrieveRepoMilestonesAndAssignees find all the milestones, assignees, and reviewers of a repository -func RetrieveRepoMilestonesAssigneesAndReviewers(ctx *context.Context, repo *repo_model.Repository) { +func RetrieveRepoMilestonesAndAssignees(ctx *context.Context, repo *repo_model.Repository) { var err error ctx.Data["OpenMilestones"], err = db.Find[issues_model.Milestone](ctx, issues_model.FindMilestoneOptions{ RepoID: repo.ID, @@ -584,8 +584,6 @@ func RetrieveRepoMilestonesAssigneesAndReviewers(ctx *context.Context, repo *rep ctx.Data["Assignees"] = shared_user.MakeSelfOnTop(ctx.Doer, assigneeUsers) handleTeamMentions(ctx) - - retrieveReviewers(ctx) } func retrieveProjects(ctx *context.Context, repo *repo_model.Repository) { @@ -656,34 +654,66 @@ func retrieveProjects(ctx *context.Context, repo *repo_model.Repository) { // repoReviewerSelection items to bee shown type repoReviewerSelection struct { - IsTeam bool - Team *organization.Team - User *user_model.User - Review *issues_model.Review - CanChange bool - Checked bool - ItemID int64 + IsTeam bool + Team *organization.Team + User *user_model.User + Review *issues_model.Review + CanBeDismissed bool + CanChange bool + Requested bool + ItemID int64 +} + +type issueSidebarReviewersData struct { + Repository *repo_model.Repository + RepoOwnerName string + RepoLink string + IssueID int64 + CanChooseReviewer bool + OriginalReviews issues_model.ReviewList + TeamReviewers []*repoReviewerSelection + Reviewers []*repoReviewerSelection + CurrentPullReviewers []*repoReviewerSelection } // RetrieveRepoReviewers find all reviewers of a repository func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, issue *issues_model.Issue, canChooseReviewer bool) { - ctx.Data["CanChooseReviewer"] = canChooseReviewer + data := &issueSidebarReviewersData{} + data.RepoLink = ctx.Repo.RepoLink + data.Repository = repo + data.RepoOwnerName = repo.OwnerName + data.CanChooseReviewer = canChooseReviewer - originalAuthorReviews, err := issues_model.GetReviewersFromOriginalAuthorsByIssueID(ctx, issue.ID) - if err != nil { - ctx.ServerError("GetReviewersFromOriginalAuthorsByIssueID", err) - return - } - ctx.Data["OriginalReviews"] = originalAuthorReviews + var posterID int64 + var isClosed bool + var reviews issues_model.ReviewList - reviews, err := issues_model.GetReviewsByIssueID(ctx, issue.ID) - if err != nil { - ctx.ServerError("GetReviewersByIssueID", err) - return - } + if issue == nil { + posterID = ctx.Doer.ID + } else { + posterID = issue.PosterID + if issue.OriginalAuthorID > 0 { + posterID = 0 // for migrated PRs, no poster ID + } - if len(reviews) == 0 && !canChooseReviewer { - return + data.IssueID = issue.ID + isClosed = issue.IsClosed || issue.PullRequest.HasMerged + + originalAuthorReviews, err := issues_model.GetReviewersFromOriginalAuthorsByIssueID(ctx, issue.ID) + if err != nil { + ctx.ServerError("GetReviewersFromOriginalAuthorsByIssueID", err) + return + } + data.OriginalReviews = originalAuthorReviews + + reviews, err = issues_model.GetReviewsByIssueID(ctx, issue.ID) + if err != nil { + ctx.ServerError("GetReviewersByIssueID", err) + return + } + if len(reviews) == 0 && !canChooseReviewer { + return + } } var ( @@ -695,11 +725,7 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is ) if canChooseReviewer { - posterID := issue.PosterID - if issue.OriginalAuthorID > 0 { - posterID = 0 - } - + var err error reviewers, err = repo_model.GetReviewers(ctx, repo, ctx.Doer.ID, posterID) if err != nil { ctx.ServerError("GetReviewers", err) @@ -725,9 +751,9 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is for _, review := range reviews { tmp := &repoReviewerSelection{ - Checked: review.Type == issues_model.ReviewTypeRequest, - Review: review, - ItemID: review.ReviewerID, + Requested: review.Type == issues_model.ReviewTypeRequest, + Review: review, + ItemID: review.ReviewerID, } if review.ReviewerTeamID > 0 { tmp.IsTeam = true @@ -758,7 +784,7 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is currentPullReviewers := make([]*repoReviewerSelection, 0, len(pullReviews)) for _, item := range pullReviews { if item.Review.ReviewerID > 0 { - if err = item.Review.LoadReviewer(ctx); err != nil { + if err := item.Review.LoadReviewer(ctx); err != nil { if user_model.IsErrUserNotExist(err) { continue } @@ -767,7 +793,7 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is } item.User = item.Review.Reviewer } else if item.Review.ReviewerTeamID > 0 { - if err = item.Review.LoadReviewerTeam(ctx); err != nil { + if err := item.Review.LoadReviewerTeam(ctx); err != nil { if organization.IsErrTeamNotExist(err) { continue } @@ -778,10 +804,11 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is } else { continue } - + item.CanBeDismissed = ctx.Repo.Permission.IsAdmin() && !isClosed && + (item.Review.Type == issues_model.ReviewTypeApprove || item.Review.Type == issues_model.ReviewTypeReject) currentPullReviewers = append(currentPullReviewers, item) } - ctx.Data["PullReviewers"] = currentPullReviewers + data.CurrentPullReviewers = currentPullReviewers } if canChooseReviewer && reviewersResult != nil { @@ -809,7 +836,7 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is }) } - ctx.Data["Reviewers"] = reviewersResult + data.Reviewers = reviewersResult } if canChooseReviewer && teamReviewersResult != nil { @@ -837,8 +864,10 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is }) } - ctx.Data["TeamReviewers"] = teamReviewersResult + data.TeamReviewers = teamReviewersResult } + + ctx.Data["IssueSidebarReviewersData"] = data } // RetrieveRepoMetas find all the meta information of a repository @@ -863,7 +892,7 @@ func RetrieveRepoMetas(ctx *context.Context, repo *repo_model.Repository, isPull labels = append(labels, orgLabels...) } - RetrieveRepoMilestonesAssigneesAndReviewers(ctx, repo) + RetrieveRepoMilestonesAndAssignees(ctx, repo) if ctx.Written() { return nil } @@ -884,46 +913,6 @@ func RetrieveRepoMetas(ctx *context.Context, repo *repo_model.Repository, isPull return labels } -func retrieveReviewers(ctx *context.Context) { - // Get reviewer info for pr - var ( - reviewers []*user_model.User - teamReviewers []*organization.Team - reviewersResult []*repoReviewerSelection - ) - reviewers, err := repo_model.GetReviewers(ctx, ctx.Repo.Repository, ctx.Doer.ID, ctx.Doer.ID) - if err != nil { - ctx.ServerError("GetReviewers", err) - return - } - - teamReviewers, err = repo_service.GetReviewerTeams(ctx, ctx.Repo.Repository) - if err != nil { - ctx.ServerError("GetReviewerTeams", err) - return - } - - for _, user := range reviewers { - reviewersResult = append(reviewersResult, &repoReviewerSelection{ - IsTeam: false, - CanChange: true, - User: user, - ItemID: user.ID, - }) - } - - // negative reviewIDs represent team requests - for _, team := range teamReviewers { - reviewersResult = append(reviewersResult, &repoReviewerSelection{ - IsTeam: true, - CanChange: true, - Team: team, - ItemID: -team.ID, - }) - } - ctx.Data["Reviewers"] = reviewersResult -} - // Tries to load and set an issue template. The first return value indicates if a template was loaded. func setTemplateIfExists(ctx *context.Context, ctxDataKey string, possibleFiles []string) (bool, map[string]error) { commit, err := ctx.Repo.GitRepo.GetBranchCommit(ctx.Repo.Repository.DefaultBranch) @@ -1625,7 +1614,7 @@ func ViewIssue(ctx *context.Context) { // Check milestone and assignee. if ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) { - RetrieveRepoMilestonesAssigneesAndReviewers(ctx, repo) + RetrieveRepoMilestonesAndAssignees(ctx, repo) retrieveProjects(ctx, repo) if ctx.Written() { diff --git a/services/issue/assignee.go b/services/issue/assignee.go index 6992b9c815356..52ee9f2b22cbf 100644 --- a/services/issue/assignee.go +++ b/services/issue/assignee.go @@ -277,6 +277,9 @@ func teamReviewRequestNotify(ctx context.Context, issue *issues_model.Issue, doe // CanDoerChangeReviewRequests returns if the doer can add/remove review requests of a PR func CanDoerChangeReviewRequests(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue) bool { + if repo.IsArchived { + return false + } // The poster of the PR can change the reviewers if doer.ID == issue.PosterID { return true diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index 920b5b835eceb..190d52cf4772d 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -47,63 +47,9 @@
- {{template "repo/issue/branch_selector_field" .}} + {{template "repo/issue/branch_selector_field" $}} {{if .PageIsComparePull}} - -
- - {{ctx.Locale.Tr "repo.issues.review.reviewers"}} - {{if .HasIssuesOrPullsWritePermission}} - {{svg "octicon-gear" 16 "tw-ml-1"}} - {{end}} - -
-
- {{svg "octicon-search" 16}} - -
-
{{ctx.Locale.Tr "repo.issues.new.clear_reviewers"}}
- {{range .Reviewers}} - {{if .User}} - - {{svg "octicon-check"}} - - {{ctx.AvatarUtils.Avatar .User 28 "tw-mr-2"}}{{template "repo/search_name" .User}} - - - {{end}} - {{end}} -
- {{range .Reviewers}} - {{if .Team}} - - {{svg "octicon-check" 16}} - - {{svg "octicon-people" 16 "tw-ml-4 tw-mr-1"}}{{$.Repository.OwnerName}}/{{.Team.Name}} - - - {{end}} - {{end}} -
-
-
- - {{ctx.Locale.Tr "repo.issues.new.no_reviewers"}} - -
- {{range .Reviewers}} - {{if .User}} - - {{ctx.AvatarUtils.Avatar .User 28 "tw-mr-2 tw-align-middle"}}{{.User.GetDisplayName}} - - {{else if .Team}} - - {{svg "octicon-people" 20 "tw-mr-2"}}{{$.Repository.OwnerName}}/{{.Team.Name}} - - {{end}} - {{end}} -
-
+ {{template "repo/issue/sidebar/reviewer_list" dict "IssueSidebarReviewersData" $.IssueSidebarReviewersData}}
{{end}} diff --git a/templates/repo/issue/sidebar/reviewer_list.tmpl b/templates/repo/issue/sidebar/reviewer_list.tmpl index cf4d067c0f851..c0842983da223 100644 --- a/templates/repo/issue/sidebar/reviewer_list.tmpl +++ b/templates/repo/issue/sidebar/reviewer_list.tmpl @@ -1,95 +1,79 @@ - -
- - {{ctx.Locale.Tr "repo.issues.review.reviewers"}} - {{if and .CanChooseReviewer (not .Repository.IsArchived)}} - {{svg "octicon-gear" 16 "tw-ml-1"}} - {{end}} - -
- {{if .Reviewers}} -
- {{svg "octicon-search" 16}} - -
- {{end}} - {{if .Reviewers}} - {{range .Reviewers}} +{{$data := .IssueSidebarReviewersData}} +{{$hasCandidates := or $data.Reviewers $data.TeamReviewers}} +
+ {{/* match CreateIssueForm */}} +
+ + {{ctx.Locale.Tr "repo.issues.review.reviewers"}} {{if and $data.CanChooseReviewer}}{{svg "octicon-gear"}}{{end}} + +
+ {{if $hasCandidates}} +
+ {{svg "octicon-search"}} + +
+ {{end}} + {{range $data.Reviewers}} {{if .User}} - - {{svg "octicon-check"}} - - {{ctx.AvatarUtils.Avatar .User 28 "tw-mr-2"}}{{template "repo/search_name" .User}} - + + {{svg "octicon-check"}} {{ctx.AvatarUtils.Avatar .User 20}} {{template "repo/search_name" .User}} {{end}} {{end}} - {{end}} - {{if .TeamReviewers}} - {{if .Reviewers}} -
- {{end}} - {{range .TeamReviewers}} - {{if .Team}} - - {{svg "octicon-check" 16}} - - {{svg "octicon-people" 16 "tw-ml-4 tw-mr-1"}}{{$.Issue.Repo.OwnerName}}/{{.Team.Name}} - - + {{if $data.TeamReviewers}} + {{if $data.Reviewers}}
{{end}} + {{range $data.TeamReviewers}} + {{if .Team}} + + {{svg "octicon-check"}} {{svg "octicon-people" 20}} {{$data.RepoOwnerName}}/{{.Team.Name}} + + {{end}} {{end}} {{end}} - {{end}} +
-
-
- {{ctx.Locale.Tr "repo.issues.new.no_reviewers"}} -
- {{range .PullReviewers}} -
-
+
+ + {{ctx.Locale.Tr "repo.issues.new.no_reviewers"}} + + {{range $data.CurrentPullReviewers}} +
+
{{if .User}} - {{ctx.AvatarUtils.Avatar .User 20 "tw-mr-2"}}{{.User.GetDisplayName}} + {{ctx.AvatarUtils.Avatar .User 20}} {{.User.GetDisplayName}} {{else if .Team}} - {{svg "octicon-people" 20 "tw-mr-2"}}{{$.Issue.Repo.OwnerName}}/{{.Team.Name}} + {{svg "octicon-people" 20}} {{$data.RepoOwnerName}}/{{.Team.Name}} {{end}}
-
- {{if (and $.Permission.IsAdmin (or (eq .Review.Type 1) (eq .Review.Type 3)) (not $.Issue.IsClosed) (not $.Issue.PullRequest.HasMerged))}} - +
+ {{if .CanBeDismissed}} + {{svg "octicon-x" 20}} - {{end}} {{if .Review.Stale}} - - {{svg "octicon-hourglass" 16}} - + {{svg "octicon-hourglass" 16}} {{end}} - {{if and .CanChange (or .Checked (and (not $.Issue.IsClosed) (not $.Issue.PullRequest.HasMerged)))}} - {{svg (Iif .Checked "octicon-trash" "octicon-sync")}} + {{if and .CanChange $data.CanChooseReviewer}} + {{if .Requested}} + + {{svg "octicon-trash"}} + + {{else}} + + {{svg "octicon-sync"}} + + {{end}} {{end}} {{svg (printf "octicon-%s" .Review.Type.Icon) 16 (printf "text %s" (.Review.HTMLTypeColorName))}} @@ -98,14 +82,15 @@
{{end}} {{range .OriginalReviews}} -
-
- - {{svg (MigrationIcon $.Repository.GetOriginalURLHostname) 20 "tw-mr-2"}} - {{.OriginalAuthor}} +
+
+ {{$originalURLHostname := $data.Repository.GetOriginalURLHostname}} + {{$originalURL := $data.Repository.OriginalURL}} + + {{svg (MigrationIcon $originalURLHostname) 20}} {{.OriginalAuthor}}
-
+
{{svg (printf "octicon-%s" .Type.Icon) 16 (printf "text %s" (.HTMLTypeColorName))}} @@ -113,4 +98,29 @@
{{end}}
+ + {{if $data.CurrentPullReviewers}} + + {{end}}
diff --git a/templates/repo/issue/sidebar/wip_switch.tmpl b/templates/repo/issue/sidebar/wip_switch.tmpl index 2f8994673e99d..06a3be0d8f52b 100644 --- a/templates/repo/issue/sidebar/wip_switch.tmpl +++ b/templates/repo/issue/sidebar/wip_switch.tmpl @@ -1,5 +1,5 @@ {{if and (or .HasIssuesOrPullsWritePermission .IsIssuePoster) (not .HasMerged) (not .Issue.IsClosed) (not .IsPullWorkInProgress)}} -
+
{{ctx.Locale.Tr "repo.pulls.still_in_progress"}} {{ctx.Locale.Tr "repo.pulls.add_prefix" (index .PullRequestWorkInProgressPrefixes 0)}} diff --git a/templates/repo/issue/view_content/sidebar.tmpl b/templates/repo/issue/view_content/sidebar.tmpl index 7afb76968a254..7a4027475978f 100644 --- a/templates/repo/issue/view_content/sidebar.tmpl +++ b/templates/repo/issue/view_content/sidebar.tmpl @@ -2,7 +2,7 @@ {{template "repo/issue/branch_selector_field" $}} {{if .Issue.IsPull}} - {{template "repo/issue/sidebar/reviewer_list" $}} + {{template "repo/issue/sidebar/reviewer_list" dict "IssueSidebarReviewersData" $.IssueSidebarReviewersData}} {{template "repo/issue/sidebar/wip_switch" $}}
{{end}} diff --git a/web_src/css/base.css b/web_src/css/base.css index 8d9f810ef8fae..b5a39c7af6fbb 100644 --- a/web_src/css/base.css +++ b/web_src/css/base.css @@ -1381,6 +1381,7 @@ table th[data-sortt-desc] .svg { align-items: stretch; } +.ui.list.flex-items-block > .item, .flex-items-block > .item, .flex-text-block { display: flex; diff --git a/web_src/css/repo.css b/web_src/css/repo.css index 85f33f858e2ec..3f7cec1a1190c 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -50,6 +50,14 @@ width: 300px; } +.issue-sidebar-combo .ui.dropdown .item:not(.checked) svg.octicon-check { + visibility: hidden; +} +.issue-sidebar-combo .ui.dropdown .menu > .item > img, +.issue-sidebar-combo .ui.dropdown .menu > .item > svg { + margin: 0; +} + .issue-content-right .dropdown > .menu { max-width: 270px; min-width: 0; diff --git a/web_src/js/features/repo-issue-sidebar-combolist.ts b/web_src/js/features/repo-issue-sidebar-combolist.ts new file mode 100644 index 0000000000000..834ae1ad6a589 --- /dev/null +++ b/web_src/js/features/repo-issue-sidebar-combolist.ts @@ -0,0 +1,89 @@ +import {fomanticQuery} from '../modules/fomantic/base.ts'; +import {POST} from '../modules/fetch.ts'; +import {queryElemChildren, toggleElem} from '../utils/dom.ts'; + +// if there are draft comments, confirm before reloading, to avoid losing comments +export function issueSidebarReloadConfirmDraftComment() { + 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(); +} + +function collectCheckedValues(elDropdown: HTMLElement) { + return Array.from(elDropdown.querySelectorAll('.menu > .item.checked'), (el) => el.getAttribute('data-value')); +} + +export function initIssueSidebarComboList(container: HTMLElement) { + if (!container) return; + + const updateUrl = container.getAttribute('data-update-url'); + const elDropdown = container.querySelector(':scope > .ui.dropdown'); + const elList = container.querySelector(':scope > .ui.list'); + const elComboValue = container.querySelector(':scope > .combo-value'); + const initialValues = collectCheckedValues(elDropdown); + + elDropdown.addEventListener('click', (e) => { + const elItem = (e.target as HTMLElement).closest('.item'); + if (!elItem) return; + e.preventDefault(); + if (elItem.getAttribute('data-can-change') !== 'true') return; + elItem.classList.toggle('checked'); + elComboValue.value = collectCheckedValues(elDropdown).join(','); + }); + + const updateToBackend = async (changedValues) => { + let changed = false; + for (const value of initialValues) { + if (!changedValues.includes(value)) { + await POST(updateUrl, {data: new URLSearchParams({action: 'detach', id: value})}); + changed = true; + } + } + for (const value of changedValues) { + if (!initialValues.includes(value)) { + await POST(updateUrl, {data: new URLSearchParams({action: 'attach', id: value})}); + changed = true; + } + } + if (changed) issueSidebarReloadConfirmDraftComment(); + }; + + const syncList = (changedValues) => { + const elEmptyTip = elList.querySelector('.item.empty-list'); + queryElemChildren(elList, '.item:not(.empty-list)', (el) => el.remove()); + for (const value of changedValues) { + const el = elDropdown.querySelector(`.menu > .item[data-value="${value}"]`); + const listItem = el.cloneNode(true) as HTMLElement; + listItem.querySelector('svg.octicon-check')?.remove(); + elList.append(listItem); + } + const hasItems = Boolean(elList.querySelector('.item:not(.empty-list)')); + toggleElem(elEmptyTip, !hasItems); + }; + + fomanticQuery(elDropdown).dropdown({ + action: 'nothing', // do not hide the menu if user presses Enter + fullTextSearch: 'exact', + async onHide() { + const changedValues = collectCheckedValues(elDropdown); + if (updateUrl) { + await updateToBackend(changedValues); + } else { + syncList(changedValues); + } + }, + }); +} diff --git a/web_src/js/features/repo-issue-sidebar.ts b/web_src/js/features/repo-issue-sidebar.ts index 9bd94ffe3937a..d654aad0df899 100644 --- a/web_src/js/features/repo-issue-sidebar.ts +++ b/web_src/js/features/repo-issue-sidebar.ts @@ -4,26 +4,7 @@ import {updateIssuesMeta} from './repo-common.ts'; import {svg} from '../svg.ts'; import {htmlEscape} from 'escape-goat'; import {toggleElem} from '../utils/dom.ts'; - -// 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(); -} +import {initIssueSidebarComboList, issueSidebarReloadConfirmDraftComment} from './repo-issue-sidebar-combolist.ts'; function initBranchSelector() { const elSelectBranch = document.querySelector('.ui.dropdown.select-branch'); @@ -78,7 +59,7 @@ function initListSubmits(selector, outerSelector) { ); } if (itemEntries.length) { - reloadConfirmDraftComment(); + issueSidebarReloadConfirmDraftComment(); } } }, @@ -142,7 +123,7 @@ function initListSubmits(selector, outerSelector) { // TODO: Which thing should be done for choosing review requests // to make chosen items be shown on time here? - if (selector === 'select-reviewers-modify' || selector === 'select-assignees-modify') { + if (selector === 'select-assignees-modify') { return false; } @@ -173,7 +154,7 @@ function initListSubmits(selector, outerSelector) { $listMenu.data('issue-id'), '', ); - reloadConfirmDraftComment(); + issueSidebarReloadConfirmDraftComment(); })(); } @@ -182,7 +163,7 @@ function initListSubmits(selector, outerSelector) { $(this).find('.octicon-check').addClass('tw-invisible'); }); - if (selector === 'select-reviewers-modify' || selector === 'select-assignees-modify') { + if (selector === 'select-assignees-modify') { return false; } @@ -213,7 +194,7 @@ function selectItem(select_id, input_id) { $menu.data('issue-id'), $(this).data('id'), ); - reloadConfirmDraftComment(); + issueSidebarReloadConfirmDraftComment(); })(); } @@ -251,7 +232,7 @@ function selectItem(select_id, input_id) { $menu.data('issue-id'), $(this).data('id'), ); - reloadConfirmDraftComment(); + issueSidebarReloadConfirmDraftComment(); })(); } @@ -278,16 +259,14 @@ export function initRepoIssueSidebar() { initBranchSelector(); initRepoIssueDue(); - // Init labels and assignees + // TODO: refactor the legacy initListSubmits&selectItem to initIssueSidebarComboList initListSubmits('select-label', 'labels'); initListSubmits('select-assignees', 'assignees'); initListSubmits('select-assignees-modify', 'assignees'); - initListSubmits('select-reviewers', 'reviewers'); - initListSubmits('select-reviewers-modify', 'assignees'); - - // Milestone, Assignee, Project selectItem('.select-project', '#project_id'); selectItem('.select-milestone', '#milestone_id'); selectItem('.select-assignee', '#assignee_id'); - selectItem('.select-reviewer', '#reviewer_id'); + + // init the combo list: a dropdown for selecting reviewers, and a list for showing selected reviewers and related actions + initIssueSidebarComboList(document.querySelector('.issue-sidebar-combo[data-sidebar-combo-for="reviewers"]')); } diff --git a/web_src/js/features/repo-issue.ts b/web_src/js/features/repo-issue.ts index 721a746aa2c4d..65818a68b0f9b 100644 --- a/web_src/js/features/repo-issue.ts +++ b/web_src/js/features/repo-issue.ts @@ -326,17 +326,6 @@ export function initRepoIssueWipTitle() { export function initRepoIssueComments() { if (!$('.repository.view.issue .timeline').length) return; - $('.re-request-review').on('click', async function (e) { - e.preventDefault(); - const url = this.getAttribute('data-update-url'); - const issueId = this.getAttribute('data-issue-id'); - const id = this.getAttribute('data-id'); - const isChecked = this.classList.contains('checked'); - - await updateIssuesMeta(url, isChecked ? 'detach' : 'attach', issueId, id); - window.location.reload(); - }); - document.addEventListener('click', (e) => { const urlTarget = document.querySelector(':target'); if (!urlTarget) return; From 637e4fbfe9ae8275ac67fe7ee285cad0fda69b08 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 9 Nov 2024 01:48:29 +0800 Subject: [PATCH 23/26] fix lint --- web_src/js/features/repo-issue.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/web_src/js/features/repo-issue.ts b/web_src/js/features/repo-issue.ts index 65818a68b0f9b..92916ec8d72bc 100644 --- a/web_src/js/features/repo-issue.ts +++ b/web_src/js/features/repo-issue.ts @@ -8,7 +8,6 @@ import {parseIssuePageInfo, toAbsoluteUrl} from '../utils.ts'; import {GET, POST} from '../modules/fetch.ts'; import {showErrorToast} from '../modules/toast.ts'; import {initRepoIssueSidebar} from './repo-issue-sidebar.ts'; -import {updateIssuesMeta} from './repo-common.ts'; const {appSubUrl} = window.config; From 9ccb69c51a33bdaa6dbf58fdf203e77837132bb1 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 9 Nov 2024 02:08:56 +0800 Subject: [PATCH 24/26] fix lint --- templates/repo/issue/sidebar/reviewer_list.tmpl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/templates/repo/issue/sidebar/reviewer_list.tmpl b/templates/repo/issue/sidebar/reviewer_list.tmpl index c0842983da223..da8345bb1be63 100644 --- a/templates/repo/issue/sidebar/reviewer_list.tmpl +++ b/templates/repo/issue/sidebar/reviewer_list.tmpl @@ -1,7 +1,7 @@ {{$data := .IssueSidebarReviewersData}} {{$hasCandidates := or $data.Reviewers $data.TeamReviewers}}
{{/* match CreateIssueForm */}}
@@ -53,7 +53,7 @@
{{if .CanBeDismissed}} + data-modal="#issue-sidebar-dismiss-review-modal" data-modal-reviewer-id="{{.Review.ID}}"> {{svg "octicon-x" 20}} {{end}} @@ -63,14 +63,14 @@ {{if and .CanChange $data.CanChooseReviewer}} {{if .Requested}} + data-tooltip-content="{{ctx.Locale.Tr "repo.issues.remove_request_review"}}" + data-url="{{$data.RepoLink}}/issues/request_review?action=detach&issue_ids={{$data.IssueID}}&id={{.ItemID}}"> {{svg "octicon-trash"}} {{else}} + data-tooltip-content="{{ctx.Locale.Tr "repo.issues.re_request_review"}}" + data-url="{{$data.RepoLink}}/issues/request_review?action=attach&issue_ids={{$data.IssueID}}&id={{.ItemID}}"> {{svg "octicon-sync"}} {{end}} From 1759ea413c419b87ee86fa894149b7782b4008cd Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 9 Nov 2024 02:14:55 +0800 Subject: [PATCH 25/26] fine tune --- options/locale/locale_en-US.ini | 1 - routers/api/v1/repo/pull.go | 1 - web_src/js/features/repo-issue-sidebar.ts | 2 -- 3 files changed, 4 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index db82cb0e79fa6..c3639fb72e2f3 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1466,7 +1466,6 @@ issues.new.no_reviewers = No Reviewers issues.new.blocked_user = Cannot create issue because you are blocked by the repository owner. 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.edit.blocked_user = Cannot edit content because you are blocked by the poster or repository owner. -issues.new.clear_reviewers = Clear reviewers issues.choose.get_started = Get Started issues.choose.open_external_link = Open issues.choose.blank = Default diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 42262e08a0f4e..28d7379f07b8a 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -561,7 +561,6 @@ func CreatePullRequest(ctx *context.APIContext) { PullRequest: pr, AssigneeIDs: assigneeIDs, } - prOpts.Reviewers, prOpts.TeamReviewers = parseReviewersByNames(ctx, form.Reviewers, form.TeamReviewers) if ctx.Written() { return diff --git a/web_src/js/features/repo-issue-sidebar.ts b/web_src/js/features/repo-issue-sidebar.ts index d654aad0df899..4a1ef02aab727 100644 --- a/web_src/js/features/repo-issue-sidebar.ts +++ b/web_src/js/features/repo-issue-sidebar.ts @@ -205,8 +205,6 @@ function selectItem(select_id, input_id) { icon = svg('octicon-project', 18, 'tw-mr-2'); } else if (input_id === '#assignee_id') { icon = `avatar`; - } else if (input_id === '#reviewer_id') { - icon = `avatar`; } $list.find('.selected').html(` From be102f58841e751acffc2dae3a00c82615cdb5e6 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 9 Nov 2024 10:02:01 +0800 Subject: [PATCH 26/26] fine tune and add comments --- routers/web/repo/compare.go | 3 +++ routers/web/repo/issue.go | 4 ++-- templates/repo/issue/sidebar/reviewer_list.tmpl | 8 ++++---- web_src/css/repo.css | 1 + web_src/js/features/repo-issue-sidebar-combolist.ts | 4 ++-- 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 77cbd4adfba3c..3477ba36e801b 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -789,6 +789,9 @@ func CompareDiff(ctx *context.Context) { if !nothingToCompare { // Setup information for new form. RetrieveRepoMetas(ctx, ctx.Repo.Repository, true) + if ctx.Written() { + return + } RetrieveRepoReviewers(ctx, ctx.Repo.Repository, nil, true) if ctx.Written() { return diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 9c7a82fb0ccb4..7fa8d428d3656 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -556,7 +556,7 @@ func renderMilestones(ctx *context.Context) { ctx.Data["ClosedMilestones"] = closedMilestones } -// RetrieveRepoMilestonesAndAssignees find all the milestones, assignees, and reviewers of a repository +// RetrieveRepoMilestonesAndAssignees find all the milestones and assignees of a repository func RetrieveRepoMilestonesAndAssignees(ctx *context.Context, repo *repo_model.Repository) { var err error ctx.Data["OpenMilestones"], err = db.Find[issues_model.Milestone](ctx, issues_model.FindMilestoneOptions{ @@ -676,7 +676,7 @@ type issueSidebarReviewersData struct { CurrentPullReviewers []*repoReviewerSelection } -// RetrieveRepoReviewers find all reviewers of a repository +// RetrieveRepoReviewers find all reviewers of a repository. If issue is nil, it means the doer is creating a new PR. func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, issue *issues_model.Issue, canChooseReviewer bool) { data := &issueSidebarReviewersData{} data.RepoLink = ctx.Repo.RepoLink diff --git a/templates/repo/issue/sidebar/reviewer_list.tmpl b/templates/repo/issue/sidebar/reviewer_list.tmpl index da8345bb1be63..2d3218e92726f 100644 --- a/templates/repo/issue/sidebar/reviewer_list.tmpl +++ b/templates/repo/issue/sidebar/reviewer_list.tmpl @@ -37,7 +37,7 @@
-
+
{{ctx.Locale.Tr "repo.issues.new.no_reviewers"}} @@ -81,7 +81,7 @@
{{end}} - {{range .OriginalReviews}} + {{range $data.OriginalReviews}}
{{$originalURLHostname := $data.Repository.GetOriginalURLHostname}} @@ -112,8 +112,8 @@ {{ctx.RootData.CsrfTokenHtml}}
- - + +
diff --git a/web_src/css/repo.css b/web_src/css/repo.css index 32418f602165b..185a5f6f558e5 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -53,6 +53,7 @@ .issue-sidebar-combo .ui.dropdown .item:not(.checked) svg.octicon-check { visibility: hidden; } +/* ideally, we should move these styles to ".ui.dropdown .menu.flex-items-menu > .item ...", could be done later */ .issue-sidebar-combo .ui.dropdown .menu > .item > img, .issue-sidebar-combo .ui.dropdown .menu > .item > svg { margin: 0; diff --git a/web_src/js/features/repo-issue-sidebar-combolist.ts b/web_src/js/features/repo-issue-sidebar-combolist.ts index 834ae1ad6a589..d5416159887fa 100644 --- a/web_src/js/features/repo-issue-sidebar-combolist.ts +++ b/web_src/js/features/repo-issue-sidebar-combolist.ts @@ -80,9 +80,9 @@ export function initIssueSidebarComboList(container: HTMLElement) { async onHide() { const changedValues = collectCheckedValues(elDropdown); if (updateUrl) { - await updateToBackend(changedValues); + await updateToBackend(changedValues); // send requests to backend and reload the page } else { - syncList(changedValues); + syncList(changedValues); // only update the list in the sidebar } }, });